Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix held gameplay keys stuck after pausing and resuming #28954

Merged
merged 5 commits into from
Jul 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ public bool OnPressed(KeyBindingPressEvent<OsuAction> e)

scaleTransitionContainer.ScaleTo(2, TRANSITION_TIME, Easing.OutQuint);

ResumeRequested?.Invoke();
// 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;
}

Expand Down
267 changes: 267 additions & 0 deletions osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. 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<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<OsuAction> 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<OsuResumeOverlay.OsuClickToResumeCursor>().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<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<ManiaAction> 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<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<OsuAction> actionTrigger && actionTrigger.Action == OsuAction.LeftButton));
AddStep("get key counter X", () => counterX = this.ChildrenOfType<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<OsuAction> 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<OsuResumeOverlay.OsuClickToResumeCursor>().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<OsuResumeOverlay.OsuClickToResumeCursor>().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<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<ManiaAction> 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<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<OsuAction> actionTrigger && actionTrigger.Action == OsuAction.LeftButton));
AddStep("get key counter X", () => counterX = this.ChildrenOfType<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<OsuAction> 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<OsuResumeOverlay.OsuClickToResumeCursor>().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<OsuResumeOverlay.OsuClickToResumeCursor>().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<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<ManiaAction> 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<Ruleset> 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<SkinComponentsContainer>().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<KeyCounter> 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)
{
}
}
}
}
2 changes: 0 additions & 2 deletions osu.Game/Screens/Play/GameplayMenuOverlay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 5 additions & 3 deletions osu.Game/Screens/Play/HUD/KeyCounter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ public abstract partial class KeyCounter : Container
/// <summary>
/// Whether this <see cref="KeyCounter"/> is currently in the "activated" state because the associated key is currently pressed.
/// </summary>
protected readonly Bindable<bool> IsActive = new BindableBool();
public IBindable<bool> IsActive => isActive;

private readonly Bindable<bool> isActive = new BindableBool();

protected KeyCounter(InputTrigger trigger)
{
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions osu.Game/Screens/Play/Player.cs
Original file line number Diff line number Diff line change
Expand Up @@ -987,14 +987,14 @@ private void onFailComplete()
/// <summary>
/// The amount of gameplay time after which a second pause is allowed.
/// </summary>
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;

/// <summary>
/// A set of conditionals which defines whether the current game state and configuration allows for
Expand Down
Loading