From d7651ef38728b31a2ae011ea4b428483cdc24cc7 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 19 Jul 2024 16:52:10 +0300 Subject: [PATCH 1/4] Add extensive test cases for correct input handling while paused in osu! & non-osu! --- .../Gameplay/TestScenePauseInputHandling.cs | 267 ++++++++++++++++++ osu.Game/Screens/Play/HUD/KeyCounter.cs | 8 +- osu.Game/Screens/Play/Player.cs | 4 +- 3 files changed, 274 insertions(+), 5 deletions(-) create mode 100644 osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs diff --git a/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs b/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs new file mode 100644 index 000000000000..d778f2e99138 --- /dev/null +++ b/osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs @@ -0,0 +1,267 @@ +// Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. +// See the LICENCE file in the repository root for full licence text. + +using System; +using System.Linq; +using NUnit.Framework; +using osu.Framework.Allocation; +using osu.Framework.Audio; +using osu.Framework.Testing; +using osu.Framework.Timing; +using osu.Game.Beatmaps; +using osu.Game.Configuration; +using osu.Game.Rulesets; +using osu.Game.Rulesets.Mania; +using osu.Game.Rulesets.Osu; +using osu.Game.Rulesets.Osu.UI; +using osu.Game.Screens.Play.HUD; +using osu.Game.Skinning; +using osu.Game.Storyboards; +using osuTK.Input; + +namespace osu.Game.Tests.Visual.Gameplay +{ + public partial class TestScenePauseInputHandling : PlayerTestScene + { + private Ruleset currentRuleset = new OsuRuleset(); + + protected override Ruleset CreatePlayerRuleset() => currentRuleset; + + protected override bool HasCustomSteps => true; + + [Resolved] + private AudioManager audioManager { get; set; } = null!; + + protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard? storyboard = null) => + new ClockBackedTestWorkingBeatmap(beatmap, storyboard, new FramedClock(new ManualClock { Rate = 1 }), audioManager); + + [SetUp] + public void SetUp() => Schedule(() => + { + foreach (var key in InputManager.CurrentState.Keyboard.Keys) + InputManager.ReleaseKey(key); + + InputManager.MoveMouseTo(Content); + LocalConfig.SetValue(OsuSetting.KeyOverlay, true); + }); + + [Test] + public void TestOsuInputNotReceivedWhilePaused() + { + KeyCounter counter = null!; + + loadPlayer(() => new OsuRuleset()); + AddStep("get key counter", () => counter = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == OsuAction.LeftButton)); + checkKey(() => counter, 0, false); + + AddStep("press Z", () => InputManager.PressKey(Key.Z)); + checkKey(() => counter, 1, true); + + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + checkKey(() => counter, 1, false); + + AddStep("pause", () => Player.Pause()); + AddStep("press Z", () => InputManager.PressKey(Key.Z)); + checkKey(() => counter, 1, false); + + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + checkKey(() => counter, 1, false); + + AddStep("resume", () => Player.Resume()); + AddStep("go to resume cursor", () => InputManager.MoveMouseTo(this.ChildrenOfType().Single())); + AddStep("press Z to resume", () => InputManager.PressKey(Key.Z)); + + // Z key was released before pause, resuming should not trigger it + checkKey(() => counter, 1, false); + + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + checkKey(() => counter, 1, false); + + AddStep("press Z", () => InputManager.PressKey(Key.Z)); + checkKey(() => counter, 2, true); + + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + checkKey(() => counter, 2, false); + } + + [Test] + public void TestManiaInputNotReceivedWhilePaused() + { + KeyCounter counter = null!; + + loadPlayer(() => new ManiaRuleset()); + AddStep("get key counter", () => counter = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == ManiaAction.Key1)); + checkKey(() => counter, 0, false); + + AddStep("press D", () => InputManager.PressKey(Key.D)); + checkKey(() => counter, 1, true); + + AddStep("release D", () => InputManager.ReleaseKey(Key.D)); + checkKey(() => counter, 1, false); + + AddStep("pause", () => Player.Pause()); + AddStep("press D", () => InputManager.PressKey(Key.D)); + checkKey(() => counter, 1, false); + + AddStep("release D", () => InputManager.ReleaseKey(Key.D)); + checkKey(() => counter, 1, false); + + AddStep("resume", () => Player.Resume()); + AddUntilStep("wait for resume", () => Player.GameplayClockContainer.IsRunning); + checkKey(() => counter, 1, false); + + AddStep("press D", () => InputManager.PressKey(Key.D)); + checkKey(() => counter, 2, true); + + AddStep("release D", () => InputManager.ReleaseKey(Key.D)); + checkKey(() => counter, 2, false); + } + + [Test] + public void TestOsuPreviouslyHeldInputReleaseOnResume() + { + KeyCounter counterZ = null!; + KeyCounter counterX = null!; + + loadPlayer(() => new OsuRuleset()); + AddStep("get key counter Z", () => counterZ = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == OsuAction.LeftButton)); + AddStep("get key counter X", () => counterX = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == OsuAction.RightButton)); + + AddStep("press Z", () => InputManager.PressKey(Key.Z)); + 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 and release Z", () => InputManager.Key(Key.Z)); + checkKey(() => counterZ, 1, false); + + AddStep("press X", () => InputManager.PressKey(Key.X)); + AddStep("pause", () => Player.Pause()); + AddStep("release X", () => InputManager.ReleaseKey(Key.X)); + checkKey(() => counterX, 1, true); + + AddStep("resume", () => Player.Resume()); + AddStep("go to resume cursor", () => InputManager.MoveMouseTo(this.ChildrenOfType().Single())); + AddStep("press Z to resume", () => InputManager.PressKey(Key.Z)); + checkKey(() => counterZ, 1, false); + checkKey(() => counterX, 1, false); + } + + [Test] + public void TestManiaPreviouslyHeldInputReleaseOnResume() + { + KeyCounter counter = null!; + + loadPlayer(() => new ManiaRuleset()); + AddStep("get key counter", () => counter = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == ManiaAction.Key1)); + + AddStep("press D", () => InputManager.PressKey(Key.D)); + AddStep("pause", () => Player.Pause()); + + AddStep("release D", () => InputManager.ReleaseKey(Key.D)); + checkKey(() => counter, 1, true); + + AddStep("resume", () => Player.Resume()); + AddUntilStep("wait for resume", () => Player.GameplayClockContainer.IsRunning); + checkKey(() => counter, 1, false); + } + + [Test] + public void TestOsuHeldInputRemainHeldAfterResume() + { + KeyCounter counterZ = null!; + KeyCounter counterX = null!; + + loadPlayer(() => new OsuRuleset()); + AddStep("get key counter Z", () => counterZ = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == OsuAction.LeftButton)); + AddStep("get key counter X", () => counterX = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == OsuAction.RightButton)); + + AddStep("press Z", () => InputManager.PressKey(Key.Z)); + 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)); + checkKey(() => counterZ, 1, true); + + AddStep("release Z", () => InputManager.ReleaseKey(Key.Z)); + checkKey(() => counterZ, 1, false); + + AddStep("press X", () => InputManager.PressKey(Key.X)); + checkKey(() => counterX, 1, true); + + AddStep("pause", () => Player.Pause()); + + AddStep("release X", () => InputManager.ReleaseKey(Key.X)); + AddStep("press X", () => InputManager.PressKey(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)); + checkKey(() => counterZ, 1, false); + checkKey(() => counterX, 1, true); + + AddStep("release X", () => InputManager.ReleaseKey(Key.X)); + checkKey(() => counterZ, 1, false); + checkKey(() => counterX, 1, false); + } + + [Test] + public void TestManiaHeldInputRemainHeldAfterResume() + { + KeyCounter counter = null!; + + loadPlayer(() => new ManiaRuleset()); + AddStep("get key counter", () => counter = this.ChildrenOfType().Single(k => k.Trigger is KeyCounterActionTrigger actionTrigger && actionTrigger.Action == ManiaAction.Key1)); + + AddStep("press D", () => InputManager.PressKey(Key.D)); + checkKey(() => counter, 1, true); + + AddStep("pause", () => Player.Pause()); + + AddStep("release D", () => InputManager.ReleaseKey(Key.D)); + AddStep("press D", () => InputManager.PressKey(Key.D)); + + AddStep("resume", () => Player.Resume()); + AddUntilStep("wait for resume", () => Player.GameplayClockContainer.IsRunning); + checkKey(() => counter, 1, true); + + AddStep("release D", () => InputManager.ReleaseKey(Key.D)); + checkKey(() => counter, 1, false); + } + + private void loadPlayer(Func createRuleset) + { + AddStep("set ruleset", () => currentRuleset = createRuleset()); + AddStep("load player", LoadPlayer); + 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(20000)); + AddUntilStep("wait for seek to finish", () => Player.DrawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(20000).Within(500)); + AddAssert("not in break", () => !Player.IsBreakTime.Value); + } + + private void checkKey(Func counter, int count, bool active) + { + AddAssert($"key count = {count}", () => counter().CountPresses.Value, () => Is.EqualTo(count)); + AddAssert($"key active = {active}", () => counter().IsActive.Value, () => Is.EqualTo(active)); + } + + protected override TestPlayer CreatePlayer(Ruleset ruleset) => new PausePlayer(); + + private partial class PausePlayer : TestPlayer + { + protected override double PauseCooldownDuration => 0; + + public PausePlayer() + : base(allowPause: true, showResults: false) + { + } + } + } +} diff --git a/osu.Game/Screens/Play/HUD/KeyCounter.cs b/osu.Game/Screens/Play/HUD/KeyCounter.cs index f12d2166fcea..66f9dfd6f224 100644 --- a/osu.Game/Screens/Play/HUD/KeyCounter.cs +++ b/osu.Game/Screens/Play/HUD/KeyCounter.cs @@ -24,7 +24,9 @@ public abstract partial class KeyCounter : Container /// /// Whether this is currently in the "activated" state because the associated key is currently pressed. /// - protected readonly Bindable IsActive = new BindableBool(); + public IBindable IsActive => isActive; + + private readonly Bindable isActive = new BindableBool(); protected KeyCounter(InputTrigger trigger) { @@ -36,12 +38,12 @@ protected KeyCounter(InputTrigger trigger) protected virtual void Activate(bool forwardPlayback = true) { - IsActive.Value = true; + isActive.Value = true; } protected virtual void Deactivate(bool forwardPlayback = true) { - IsActive.Value = false; + isActive.Value = false; } protected override void Dispose(bool isDisposing) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 3a08d3be249f..4a419e1431d1 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -987,14 +987,14 @@ private void onFailComplete() /// /// The amount of gameplay time after which a second pause is allowed. /// - private const double pause_cooldown = 1000; + protected virtual double PauseCooldownDuration => 1000; protected PauseOverlay PauseOverlay { get; private set; } private double? lastPauseActionTime; protected bool PauseCooldownActive => - lastPauseActionTime.HasValue && GameplayClockContainer.CurrentTime < lastPauseActionTime + pause_cooldown; + lastPauseActionTime.HasValue && GameplayClockContainer.CurrentTime < lastPauseActionTime + PauseCooldownDuration; /// /// A set of conditionals which defines whether the current game state and configuration allows for From 4f6c7fe7c3c6941db20abc28cf321c03fa58646c Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 19 Jul 2024 16:52:48 +0300 Subject: [PATCH 2/4] Schedule resume operation by one frame to ensure the triggered key down event does not cause a gameplay press --- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index a04ea806403e..8a137e666528 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -116,7 +116,7 @@ public bool OnPressed(KeyBindingPressEvent e) scaleTransitionContainer.ScaleTo(2, TRANSITION_TIME, Easing.OutQuint); - ResumeRequested?.Invoke(); + Schedule(() => ResumeRequested?.Invoke()); return true; } From 818b60a3d80aa5e3c702fadf9cd83397e56e2a66 Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 19 Jul 2024 18:18:37 +0300 Subject: [PATCH 3/4] Fix pause overlay hiding input from ruleset input manager If a key is pressed while the pause overlay is visible, the ruleset input manager will not see it, therefore if the user resumes while the key is held then releases the key, the ruleset input manager will not receive the key up event. --- osu.Game/Screens/Play/GameplayMenuOverlay.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/osu.Game/Screens/Play/GameplayMenuOverlay.cs b/osu.Game/Screens/Play/GameplayMenuOverlay.cs index da239d585ece..2b961278d533 100644 --- a/osu.Game/Screens/Play/GameplayMenuOverlay.cs +++ b/osu.Game/Screens/Play/GameplayMenuOverlay.cs @@ -32,8 +32,6 @@ public abstract partial class GameplayMenuOverlay : OverlayContainer, IKeyBindin private const int button_height = 70; private const float background_alpha = 0.75f; - protected override bool BlockNonPositionalInput => true; - protected override bool BlockScrollInput => false; public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => true; From e539670df1d6d73c077b5a3792ea27c3318e464e Mon Sep 17 00:00:00 2001 From: Salman Ahmed Date: Fri, 19 Jul 2024 19:19:36 +0300 Subject: [PATCH 4/4] Add explanatory note --- osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs index 8a137e666528..d809f2b31896 100644 --- a/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs +++ b/osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs @@ -116,6 +116,8 @@ public bool OnPressed(KeyBindingPressEvent e) scaleTransitionContainer.ScaleTo(2, TRANSITION_TIME, Easing.OutQuint); + // When resuming with a button, we do not want the osu! input manager to see this button press and include it in the score. + // To ensure that this works correctly, schedule the resume operation one frame forward, since the resume operation enables the input manager to see input events. Schedule(() => ResumeRequested?.Invoke()); return true; }