From 9e962ce314f188c4bf1f17db7510cb4bf62236ee Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 19 Aug 2024 14:14:12 +0900 Subject: [PATCH 1/5] Add failing test case --- .../Gameplay/TestScenePauseInputHandling.cs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs index bc66947ccd..843e924660 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs @@ -47,6 +47,11 @@ public partial class TestScenePauseInputHandling : PlayerTestScene { Position = OsuPlayfield.BASE_SIZE / 2, StartTime = 5000, + }, + new HitCircle + { + Position = OsuPlayfield.BASE_SIZE / 2, + StartTime = 10000, } } }; @@ -281,6 +286,38 @@ public void TestOsuRegisterInputFromPressingOrangeCursorButPressIsBlocked() AddAssert("button is released in kbc", () => !Player.DrawableRuleset.Playfield.FindClosestParent()!.PressedActions.Any()); } + [Test] + public void TestOsuRegisterInputFromPressingOrangeCursorButPressIsBlocked_PauseWhileHolding() + { + KeyCounter counter = null!; + + loadPlayer(() => new OsuRuleset()); + AddStep("get key counter", () => counter = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == OsuAction.LeftButton)); + + AddStep("press Z", () => InputManager.PressKey(Key.Z)); + AddAssert("circle hit", () => Player.ScoreProcessor.HighestCombo.Value, () => Is.EqualTo(1)); + + AddStep("pause", () => Player.Pause()); + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + + AddStep("resume", () => Player.Resume()); + AddStep("go to resume cursor", () => InputManager.MoveMouseTo(this.ChildrenOfType().Single())); + AddStep("press Z to resume", () => InputManager.PressKey(Key.Z)); + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + + checkKey(() => counter, 1, false); + + seekTo(5000); + + AddStep("press Z", () => InputManager.PressKey(Key.Z)); + + checkKey(() => counter, 2, true); + AddAssert("circle hit", () => Player.ScoreProcessor.HighestCombo.Value, () => Is.EqualTo(2)); + + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + checkKey(() => counter, 2, false); + } + private void loadPlayer(Func createRuleset) { AddStep("set ruleset", () => currentRuleset = createRuleset()); @@ -288,12 +325,17 @@ private void loadPlayer(Func createRuleset) AddUntilStep("player loaded", () => Player.IsLoaded && Player.Alpha == 1); AddUntilStep("wait for hud", () => Player.HUDOverlay.ChildrenOfType().All(s => s.ComponentsLoaded)); - AddStep("seek to gameplay", () => Player.GameplayClockContainer.Seek(0)); - AddUntilStep("wait for seek to finish", () => Player.DrawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(0).Within(500)); + seekTo(0); AddAssert("not in break", () => !Player.IsBreakTime.Value); AddStep("move cursor to center", () => InputManager.MoveMouseTo(Player.DrawableRuleset.Playfield)); } + private void seekTo(double time) + { + AddStep($"seek to {time}ms", () => Player.GameplayClockContainer.Seek(time)); + AddUntilStep("wait for seek to finish", () => Player.DrawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(time).Within(500)); + } + private void checkKey(Func counter, int count, bool active) { AddAssert($"key count = {count}", () => counter().CountPresses.Value, () => Is.EqualTo(count)); From 62dec1cd786717eb6c2f575dd80aca9c85be8967 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 19 Aug 2024 14:14:39 +0900 Subject: [PATCH 2/5] Fix oversight in input blocking from osu! gameplay resume --- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index d90d3d26eb..b12895ae52 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -13,6 +13,7 @@ using osu.Game.Rulesets.UI; using osu.Game.Screens.Play; using osuTK.Graphics; +using TagLib.Flac; namespace osu.Game.Rulesets.Osu.UI { @@ -172,13 +173,14 @@ public OsuResumeOverlayInputBlocker() Depth = float.MinValue; } - public bool OnPressed(KeyBindingPressEvent e) + protected override void Update() { - bool block = BlockNextPress; + base.Update(); BlockNextPress = false; - return block; } + public bool OnPressed(KeyBindingPressEvent e) => BlockNextPress; + public void OnReleased(KeyBindingReleaseEvent e) { } From 86d0079dcdadbbf1521dc3d8520616b8bb27a529 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 19 Aug 2024 15:43:57 +0900 Subject: [PATCH 3/5] Rewrite the fix to look less hacky and direct to the point --- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 23 +++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index b12895ae52..8ae08ed021 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -3,6 +3,7 @@ using System; using osu.Framework.Allocation; +using osu.Framework.Extensions.ObjectExtensions; using osu.Framework.Graphics; using osu.Framework.Graphics.Containers; using osu.Framework.Graphics.Cursor; @@ -36,9 +37,11 @@ private void load() { OsuResumeOverlayInputBlocker? inputBlocker = null; - if (drawableRuleset != null) + var drawableOsuRuleset = (DrawableOsuRuleset?)drawableRuleset; + + if (drawableOsuRuleset != null) { - var osuPlayfield = (OsuPlayfield)drawableRuleset.Playfield; + var osuPlayfield = drawableOsuRuleset.Playfield; osuPlayfield.AttachResumeOverlayInputBlocker(inputBlocker = new OsuResumeOverlayInputBlocker()); } @@ -46,13 +49,14 @@ private void load() { Child = clickToResumeCursor = new OsuClickToResumeCursor { - ResumeRequested = () => + ResumeRequested = action => { // since the user had to press a button to tap the resume cursor, // block that press event from potentially reaching a hit circle that's behind the cursor. // we cannot do this from OsuClickToResumeCursor directly since we're in a different input manager tree than the gameplay one, // so we rely on a dedicated input blocking component that's implanted in there to do that for us. - if (inputBlocker != null) + // note this only matters when the user didn't pause while they were holding the same key that they are resuming with. + if (inputBlocker != null && !drawableOsuRuleset.AsNonNull().KeyBindingInputManager.PressedActions.Contains(action)) inputBlocker.BlockNextPress = true; Resume(); @@ -95,7 +99,7 @@ public partial class OsuClickToResumeCursor : OsuCursor, IKeyBindingHandler true; - public Action? ResumeRequested; + public Action? ResumeRequested; private Container scaleTransitionContainer = null!; public OsuClickToResumeCursor() @@ -137,7 +141,7 @@ public bool OnPressed(KeyBindingPressEvent e) return false; scaleTransitionContainer.ScaleTo(2, TRANSITION_TIME, Easing.OutQuint); - ResumeRequested?.Invoke(); + ResumeRequested?.Invoke(e.Action); return true; } @@ -173,14 +177,13 @@ public OsuResumeOverlayInputBlocker() Depth = float.MinValue; } - protected override void Update() + public bool OnPressed(KeyBindingPressEvent e) { - base.Update(); + bool block = BlockNextPress; BlockNextPress = false; + return block; } - public bool OnPressed(KeyBindingPressEvent e) => BlockNextPress; - public void OnReleased(KeyBindingReleaseEvent e) { } From 2a49167aa0051bc491d374de9a0b05c61daf12e5 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Mon, 19 Aug 2024 15:44:17 +0900 Subject: [PATCH 4/5] Remove flac whatever --- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index 8ae08ed021..b045b82960 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -14,7 +14,6 @@ using osu.Game.Rulesets.UI; using osu.Game.Screens.Play; using osuTK.Graphics; -using TagLib.Flac; namespace osu.Game.Rulesets.Osu.UI { From 2ecf5ec939d2eb5eb12a91fe846365392a8af6a7 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Tue, 20 Aug 2024 16:22:25 +0900 Subject: [PATCH 5/5] Add further test coverage --- .../Gameplay/TestScenePauseInputHandling.cs | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs index 843e924660..8a41d8b573 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs @@ -52,6 +52,11 @@ public partial class TestScenePauseInputHandling : PlayerTestScene { Position = OsuPlayfield.BASE_SIZE / 2, StartTime = 10000, + }, + new HitCircle + { + Position = OsuPlayfield.BASE_SIZE / 2, + StartTime = 15000, } } }; @@ -261,7 +266,7 @@ public void TestManiaHeldInputRemainHeldAfterResume() } [Test] - public void TestOsuRegisterInputFromPressingOrangeCursorButPressIsBlocked() + public void TestOsuHitCircleNotReceivingInputOnResume() { KeyCounter counter = null!; @@ -287,7 +292,7 @@ public void TestOsuRegisterInputFromPressingOrangeCursorButPressIsBlocked() } [Test] - public void TestOsuRegisterInputFromPressingOrangeCursorButPressIsBlocked_PauseWhileHolding() + public void TestOsuHitCircleNotReceivingInputOnResume_PauseWhileHoldingSameKey() { KeyCounter counter = null!; @@ -318,6 +323,32 @@ public void TestOsuRegisterInputFromPressingOrangeCursorButPressIsBlocked_PauseW checkKey(() => counter, 2, false); } + [Test] + public void TestOsuHitCircleNotReceivingInputOnResume_PauseWhileHoldingOtherKey() + { + loadPlayer(() => new OsuRuleset()); + + AddStep("press X", () => InputManager.PressKey(Key.X)); + AddAssert("circle hit", () => Player.ScoreProcessor.HighestCombo.Value, () => Is.EqualTo(1)); + + seekTo(5000); + + AddStep("pause", () => Player.Pause()); + AddStep("release X", () => InputManager.ReleaseKey(Key.X)); + + AddStep("resume", () => Player.Resume()); + AddStep("go to resume cursor", () => InputManager.MoveMouseTo(this.ChildrenOfType().Single())); + AddStep("press Z to resume", () => InputManager.PressKey(Key.Z)); + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + + AddAssert("circle not hit", () => Player.ScoreProcessor.HighestCombo.Value, () => Is.EqualTo(1)); + + AddStep("press X", () => InputManager.PressKey(Key.X)); + AddStep("release X", () => InputManager.ReleaseKey(Key.X)); + + AddAssert("circle hit", () => Player.ScoreProcessor.HighestCombo.Value, () => Is.EqualTo(2)); + } + private void loadPlayer(Func createRuleset) { AddStep("set ruleset", () => currentRuleset = createRuleset());