From 581ae2f679a00e98ea3e773a9c0c59494b9be8e5 Mon Sep 17 00:00:00 2001 From: tsunyoku Date: Mon, 12 Feb 2024 12:51:35 +0000 Subject: [PATCH 1/7] handle key presses when watching legacy relax replays --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 24 +++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 3679425389..55d8b6f55f 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -38,12 +38,17 @@ namespace osu.Game.Rulesets.Osu.Mods private ReplayState state = null!; private double lastStateChangeTime; + private DrawableOsuRuleset ruleset = null!; + private bool hasReplay; + private bool legacyReplay; public void ApplyToDrawableRuleset(DrawableRuleset drawableRuleset) { + ruleset = (DrawableOsuRuleset)drawableRuleset; + // grab the input manager for future use. - osuInputManager = ((DrawableOsuRuleset)drawableRuleset).KeyBindingInputManager; + osuInputManager = ruleset.KeyBindingInputManager; } public void ApplyToPlayer(Player player) @@ -51,6 +56,7 @@ namespace osu.Game.Rulesets.Osu.Mods if (osuInputManager.ReplayInputHandler != null) { hasReplay = true; + legacyReplay = ruleset.ReplayScore.ScoreInfo.IsLegacyScore; return; } @@ -59,7 +65,7 @@ namespace osu.Game.Rulesets.Osu.Mods public void Update(Playfield playfield) { - if (hasReplay) + if (hasReplay && !legacyReplay) return; bool requiresHold = false; @@ -125,6 +131,20 @@ namespace osu.Game.Rulesets.Osu.Mods isDownState = down; lastStateChangeTime = time; + // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. + if (legacyReplay) + { + if (!down) + { + osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); + return; + } + + osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + wasLeft = !wasLeft; + return; + } + state = new ReplayState { PressedActions = new List() From 2a02566283f831b469a2b37a4a645aae2cb07bc3 Mon Sep 17 00:00:00 2001 From: tsunyoku Date: Mon, 12 Feb 2024 17:45:00 +0000 Subject: [PATCH 2/7] refactor `down` and `wasLeft` management into respective `PressHandler` classes --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 67 +++++++++++++++++------ 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 55d8b6f55f..47b7e543d8 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -39,6 +39,7 @@ namespace osu.Game.Rulesets.Osu.Mods private double lastStateChangeTime; private DrawableOsuRuleset ruleset = null!; + private PressHandler pressHandler = null!; private bool hasReplay; private bool legacyReplay; @@ -56,10 +57,16 @@ namespace osu.Game.Rulesets.Osu.Mods if (osuInputManager.ReplayInputHandler != null) { hasReplay = true; + + Debug.Assert(ruleset.ReplayScore != null); legacyReplay = ruleset.ReplayScore.ScoreInfo.IsLegacyScore; + + pressHandler = new LegacyReplayPressHandler(this); + return; } + pressHandler = new PressHandler(this); osuInputManager.AllowGameplayInputs = false; } @@ -131,20 +138,6 @@ namespace osu.Game.Rulesets.Osu.Mods isDownState = down; lastStateChangeTime = time; - // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. - if (legacyReplay) - { - if (!down) - { - osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); - return; - } - - osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); - wasLeft = !wasLeft; - return; - } - state = new ReplayState { PressedActions = new List() @@ -152,11 +145,53 @@ namespace osu.Game.Rulesets.Osu.Mods if (down) { - state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + pressHandler.HandlePress(wasLeft); wasLeft = !wasLeft; } + else + { + pressHandler.HandleRelease(wasLeft); + } + } + } - state.Apply(osuInputManager.CurrentState, osuInputManager); + private class PressHandler + { + protected readonly OsuModRelax Mod; + + public PressHandler(OsuModRelax mod) + { + Mod = mod; + } + + public virtual void HandlePress(bool wasLeft) + { + Mod.state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + } + + public virtual void HandleRelease(bool wasLeft) + { + Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + } + } + + // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. + private class LegacyReplayPressHandler : PressHandler + { + public LegacyReplayPressHandler(OsuModRelax mod) + : base(mod) + { + } + + public override void HandlePress(bool wasLeft) + { + Mod.osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + } + + public override void HandleRelease(bool wasLeft) + { + Mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); } } } From cc733ea809f3eb1d0887bf8c92605581fd21a9b3 Mon Sep 17 00:00:00 2001 From: tsunyoku Date: Mon, 12 Feb 2024 18:00:05 +0000 Subject: [PATCH 3/7] add inline comment for supposedly backwards ternary --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index 47b7e543d8..a5643e5b49 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -191,6 +191,7 @@ namespace osu.Game.Rulesets.Osu.Mods public override void HandleRelease(bool wasLeft) { + // this intentionally releases right when `wasLeft` is true because `wasLeft` is set at point of press and not at point of release Mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); } } From 5101979ac099b7e590e020020bf3b0a7bf134164 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Tue, 13 Feb 2024 00:34:06 +0000 Subject: [PATCH 4/7] only use `LegacyReplayPressHandler` on legacy replays --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index a5643e5b49..d2e4e0c669 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -1,4 +1,4 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. using System; @@ -61,7 +61,7 @@ namespace osu.Game.Rulesets.Osu.Mods Debug.Assert(ruleset.ReplayScore != null); legacyReplay = ruleset.ReplayScore.ScoreInfo.IsLegacyScore; - pressHandler = new LegacyReplayPressHandler(this); + pressHandler = legacyReplay ? new LegacyReplayPressHandler(this) : new PressHandler(this); return; } From 22e9c4a3b59e414a881ceae5abc885389a0af5b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Tue, 13 Feb 2024 10:19:55 +0100 Subject: [PATCH 5/7] Use private interface rather than weird inheritance --- osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs | 38 ++++++++++++++--------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs index d2e4e0c669..31511c01b8 100644 --- a/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs +++ b/osu.Game.Rulesets.Osu/Mods/OsuModRelax.cs @@ -39,7 +39,7 @@ namespace osu.Game.Rulesets.Osu.Mods private double lastStateChangeTime; private DrawableOsuRuleset ruleset = null!; - private PressHandler pressHandler = null!; + private IPressHandler pressHandler = null!; private bool hasReplay; private bool legacyReplay; @@ -155,44 +155,52 @@ namespace osu.Game.Rulesets.Osu.Mods } } - private class PressHandler + private interface IPressHandler { - protected readonly OsuModRelax Mod; + void HandlePress(bool wasLeft); + void HandleRelease(bool wasLeft); + } + + private class PressHandler : IPressHandler + { + private readonly OsuModRelax mod; public PressHandler(OsuModRelax mod) { - Mod = mod; + this.mod = mod; } - public virtual void HandlePress(bool wasLeft) + public void HandlePress(bool wasLeft) { - Mod.state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); - Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + mod.state.PressedActions.Add(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + mod.state.Apply(mod.osuInputManager.CurrentState, mod.osuInputManager); } - public virtual void HandleRelease(bool wasLeft) + public void HandleRelease(bool wasLeft) { - Mod.state.Apply(Mod.osuInputManager.CurrentState, Mod.osuInputManager); + mod.state.Apply(mod.osuInputManager.CurrentState, mod.osuInputManager); } } // legacy replays do not contain key-presses with Relax mod, so they need to be triggered by themselves. - private class LegacyReplayPressHandler : PressHandler + private class LegacyReplayPressHandler : IPressHandler { + private readonly OsuModRelax mod; + public LegacyReplayPressHandler(OsuModRelax mod) - : base(mod) { + this.mod = mod; } - public override void HandlePress(bool wasLeft) + public void HandlePress(bool wasLeft) { - Mod.osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); + mod.osuInputManager.KeyBindingContainer.TriggerPressed(wasLeft ? OsuAction.LeftButton : OsuAction.RightButton); } - public override void HandleRelease(bool wasLeft) + public void HandleRelease(bool wasLeft) { // this intentionally releases right when `wasLeft` is true because `wasLeft` is set at point of press and not at point of release - Mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); + mod.osuInputManager.KeyBindingContainer.TriggerReleased(wasLeft ? OsuAction.RightButton : OsuAction.LeftButton); } } } From 02de9122d4466d2a30a915bbbc19e5f143fec824 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Wed, 14 Feb 2024 07:17:05 +0300 Subject: [PATCH 6/7] Remove behaviour of flipping catcher plate on direction change --- .../TestSceneCatchSkinConfiguration.cs | 111 ------------------ .../Skinning/CatchSkinConfiguration.cs | 13 -- .../Legacy/CatchLegacySkinTransformer.cs | 13 -- osu.Game.Rulesets.Catch/UI/Catcher.cs | 10 +- 4 files changed, 1 insertion(+), 146 deletions(-) delete mode 100644 osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs delete mode 100644 osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs diff --git a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs b/osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs deleted file mode 100644 index e2fc31d869..0000000000 --- a/osu.Game.Rulesets.Catch.Tests/TestSceneCatchSkinConfiguration.cs +++ /dev/null @@ -1,111 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -#nullable disable - -using System.Linq; -using NUnit.Framework; -using osu.Framework.Bindables; -using osu.Framework.Graphics; -using osu.Framework.Graphics.Containers; -using osu.Framework.Testing; -using osu.Framework.Utils; -using osu.Game.Beatmaps; -using osu.Game.Beatmaps.ControlPoints; -using osu.Game.Rulesets.Catch.Judgements; -using osu.Game.Rulesets.Catch.Objects; -using osu.Game.Rulesets.Catch.Objects.Drawables; -using osu.Game.Rulesets.Catch.Skinning; -using osu.Game.Rulesets.Catch.UI; -using osu.Game.Skinning; -using osu.Game.Tests.Visual; -using Direction = osu.Game.Rulesets.Catch.UI.Direction; - -namespace osu.Game.Rulesets.Catch.Tests -{ - public partial class TestSceneCatchSkinConfiguration : OsuTestScene - { - private Catcher catcher; - - private readonly Container container; - - public TestSceneCatchSkinConfiguration() - { - Add(container = new Container { RelativeSizeAxes = Axes.Both }); - } - - [TestCase(false)] - [TestCase(true)] - public void TestCatcherPlateFlipping(bool flip) - { - AddStep("setup catcher", () => - { - var skin = new TestSkin { FlipCatcherPlate = flip }; - container.Child = new SkinProvidingContainer(skin) - { - Child = catcher = new Catcher(new DroppedObjectContainer()) - { - Anchor = Anchor.Centre - } - }; - }); - - Fruit fruit = new Fruit(); - - AddStep("catch fruit", () => catchFruit(fruit, 20)); - - float position = 0; - - AddStep("record fruit position", () => position = getCaughtObjectPosition(fruit)); - - AddStep("face left", () => catcher.VisualDirection = Direction.Left); - - if (flip) - AddAssert("fruit position changed", () => !Precision.AlmostEquals(getCaughtObjectPosition(fruit), position)); - else - AddAssert("fruit position unchanged", () => Precision.AlmostEquals(getCaughtObjectPosition(fruit), position)); - - AddStep("face right", () => catcher.VisualDirection = Direction.Right); - - AddAssert("fruit position restored", () => Precision.AlmostEquals(getCaughtObjectPosition(fruit), position)); - } - - private float getCaughtObjectPosition(Fruit fruit) - { - var caughtObject = catcher.ChildrenOfType().Single(c => c.HitObject == fruit); - return caughtObject.Parent!.ToSpaceOfOtherDrawable(caughtObject.Position, catcher).X; - } - - private void catchFruit(Fruit fruit, float x) - { - fruit.ApplyDefaults(new ControlPointInfo(), new BeatmapDifficulty()); - var drawableFruit = new DrawableFruit(fruit) { X = x }; - var judgement = fruit.CreateJudgement(); - catcher.OnNewResult(drawableFruit, new CatchJudgementResult(fruit, judgement) - { - Type = judgement.MaxResult - }); - } - - private class TestSkin : TrianglesSkin - { - public bool FlipCatcherPlate { get; set; } - - public TestSkin() - : base(null!) - { - } - - public override IBindable GetConfig(TLookup lookup) - { - if (lookup is CatchSkinConfiguration config) - { - if (config == CatchSkinConfiguration.FlipCatcherPlate) - return SkinUtils.As(new Bindable(FlipCatcherPlate)); - } - - return base.GetConfig(lookup); - } - } - } -} diff --git a/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs b/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs deleted file mode 100644 index ea8d742b1a..0000000000 --- a/osu.Game.Rulesets.Catch/Skinning/CatchSkinConfiguration.cs +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. -// See the LICENCE file in the repository root for full licence text. - -namespace osu.Game.Rulesets.Catch.Skinning -{ - public enum CatchSkinConfiguration - { - /// - /// Whether the contents of the catcher plate should be visually flipped when the catcher direction is changed. - /// - FlipCatcherPlate - } -} diff --git a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs index fb8af9bdb6..d1ef47cf17 100644 --- a/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs +++ b/osu.Game.Rulesets.Catch/Skinning/Legacy/CatchLegacySkinTransformer.cs @@ -122,19 +122,6 @@ namespace osu.Game.Rulesets.Catch.Skinning.Legacy result.Value = LegacyColourCompatibility.DisallowZeroAlpha(result.Value); return (IBindable)result; - - case CatchSkinConfiguration config: - switch (config) - { - case CatchSkinConfiguration.FlipCatcherPlate: - // Don't flip catcher plate contents if the catcher is provided by this legacy skin. - if (GetDrawableComponent(new CatchSkinComponentLookup(CatchSkinComponents.Catcher)) != null) - return (IBindable)new Bindable(); - - break; - } - - break; } return base.GetConfig(lookup); diff --git a/osu.Game.Rulesets.Catch/UI/Catcher.cs b/osu.Game.Rulesets.Catch/UI/Catcher.cs index 147850a9b7..dca01fc61a 100644 --- a/osu.Game.Rulesets.Catch/UI/Catcher.cs +++ b/osu.Game.Rulesets.Catch/UI/Catcher.cs @@ -112,11 +112,6 @@ namespace osu.Game.Rulesets.Catch.UI public Vector2 BodyScale => Scale * body.Scale; - /// - /// Whether the contents of the catcher plate should be visually flipped when the catcher direction is changed. - /// - private bool flipCatcherPlate; - /// /// Width of the area that can be used to attempt catches during gameplay. /// @@ -339,8 +334,6 @@ namespace osu.Game.Rulesets.Catch.UI skin.GetConfig(CatchSkinColour.HyperDash)?.Value ?? DEFAULT_HYPER_DASH_COLOUR; - flipCatcherPlate = skin.GetConfig(CatchSkinConfiguration.FlipCatcherPlate)?.Value ?? true; - runHyperDashStateTransition(HyperDashing); } @@ -352,8 +345,7 @@ namespace osu.Game.Rulesets.Catch.UI body.Scale = scaleFromDirection; // Inverse of catcher scale is applied here, as catcher gets scaled by circle size and so do the incoming fruit. - caughtObjectContainer.Scale = (1 / Scale.X) * (flipCatcherPlate ? scaleFromDirection : Vector2.One); - hitExplosionContainer.Scale = flipCatcherPlate ? scaleFromDirection : Vector2.One; + caughtObjectContainer.Scale = new Vector2(1 / Scale.X); // Correct overshooting. if ((hyperDashDirection > 0 && hyperDashTargetPosition < X) || From 2981ebe3d5725e4e26ad2d7b9ec22b9afe87fdf5 Mon Sep 17 00:00:00 2001 From: Dan Balasescu Date: Wed, 14 Feb 2024 17:13:44 +0900 Subject: [PATCH 7/7] Fix inspection --- .../Rulesets/TestSceneRulesetSkinProvidingContainer.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs b/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs index 11f3fe660d..981258e8d1 100644 --- a/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs +++ b/osu.Game.Tests/Rulesets/TestSceneRulesetSkinProvidingContainer.cs @@ -43,13 +43,13 @@ namespace osu.Game.Tests.Rulesets AddStep("setup provider", () => { - var rulesetSkinProvider = new RulesetSkinProvidingContainer(Ruleset.Value.CreateInstance(), Beatmap.Value.Beatmap, Beatmap.Value.Skin); - - rulesetSkinProvider.Add(requester = new SkinRequester()); - + requester = new SkinRequester(); requester.OnLoadAsync += () => textureOnLoad = requester.GetTexture("test-image"); - Child = rulesetSkinProvider; + Child = new RulesetSkinProvidingContainer(Ruleset.Value.CreateInstance(), Beatmap.Value.Beatmap, Beatmap.Value.Skin) + { + requester + }; }); AddAssert("requester got correct initial texture", () => textureOnLoad != null);