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.

Sign up for GitHub

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

Jump to bottom

Fix pause input handling in osu! not working like stable #29250

Merged
merged 8 commits into from
Aug 5, 2024
Merged

Fix pause input handling in osu! not working like stable #29250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c3b2d81
Add failing test case
frenzibyte Aug 2, 2024
eafc0f7
Fix clicking resume cursor not triggering a gameplay press in osu!
frenzibyte Aug 2, 2024
5368a43
Fix clicking resume overlay hitting underlying hit circle
frenzibyte Aug 2, 2024
f5a3eb5
Add comment
frenzibyte Aug 2, 2024
dc9f6a0
Fix inspections
frenzibyte Aug 2, 2024
c27b35a
Update framework
peppy Aug 2, 2024
419d5a7
Merge branch 'master' into fix-pause-in-osu-again
peppy Aug 5, 2024
20b8905
Replace try-finally with return
smoogipoo Aug 5, 2024
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
2 changes: 1 addition & 1 deletion osu.Android.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<EmbedAssembliesIntoApk>true</EmbedAssembliesIntoApk>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="ppy.osu.Framework.Android" Version="2024.720.0" />
<PackageReference Include="ppy.osu.Framework.Android" Version="2024.802.0" />
</ItemGroup>
<PropertyGroup>
<!-- Fody does not handle Android build well, and warns when unchanged.
Expand Down
9 changes: 9 additions & 0 deletions osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,15 @@ private void onNewResult(DrawableHitObject judgedObject, JudgementResult result)

public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => HitObjectContainer.ReceivePositionalInputAt(screenSpacePos);

private OsuResumeOverlay.OsuResumeOverlayInputBlocker? resumeInputBlocker;

public void AttachResumeOverlayInputBlocker(OsuResumeOverlay.OsuResumeOverlayInputBlocker resumeInputBlocker)
{
Debug.Assert(this.resumeInputBlocker == null);
this.resumeInputBlocker = resumeInputBlocker;
AddInternal(resumeInputBlocker);
}

private partial class ProxyContainer : LifetimeManagementContainer
{
public void Add(Drawable proxy) => AddInternal(proxy);
Expand Down
50 changes: 45 additions & 5 deletions osu.Game.Rulesets.Osu/UI/OsuResumeOverlay.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,30 @@ public partial class OsuResumeOverlay : ResumeOverlay
[BackgroundDependencyLoader]
private void load()
{
OsuResumeOverlayInputBlocker? inputBlocker = null;

if (drawableRuleset != null)
{
var osuPlayfield = (OsuPlayfield)drawableRuleset.Playfield;
osuPlayfield.AttachResumeOverlayInputBlocker(inputBlocker = new OsuResumeOverlayInputBlocker());
}

Add(cursorScaleContainer = new Container
{
Child = clickToResumeCursor = new OsuClickToResumeCursor { ResumeRequested = Resume }
Child = clickToResumeCursor = new OsuClickToResumeCursor
{
ResumeRequested = () =>
{
// 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)
inputBlocker.BlockNextPress = true;

Resume();
}
}
});
}

Expand Down Expand Up @@ -115,10 +136,7 @@ public bool OnPressed(KeyBindingPressEvent<OsuAction> e)
return false;

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

Expand All @@ -143,5 +161,27 @@ private void updateColour()
this.FadeColour(IsHovered ? Color4.White : Color4.Orange, 400, Easing.OutQuint);
}
}

public partial class OsuResumeOverlayInputBlocker : Drawable, IKeyBindingHandler<OsuAction>
{
public bool BlockNextPress;

public OsuResumeOverlayInputBlocker()
{
RelativeSizeAxes = Axes.Both;
Depth = float.MinValue;
}

public bool OnPressed(KeyBindingPressEvent<OsuAction> e)
{
bool block = BlockNextPress;
BlockNextPress = false;
return block;
}

public void OnReleased(KeyBindingReleaseEvent<OsuAction> e)
{
}
}
}
}
102 changes: 75 additions & 27 deletions osu.Game.Tests/Visual/Gameplay/TestScenePauseInputHandling.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@
using NUnit.Framework;
using osu.Framework.Allocation;
using osu.Framework.Audio;
using osu.Framework.Graphics;
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.Objects;
using osu.Game.Rulesets.Osu.UI;
using osu.Game.Screens.Play.HUD;
using osu.Game.Skinning;
Expand All @@ -32,6 +34,23 @@ public partial class TestScenePauseInputHandling : PlayerTestScene
[Resolved]
private AudioManager audioManager { get; set; } = null!;

protected override IBeatmap CreateBeatmap(RulesetInfo ruleset) => new Beatmap
{
HitObjects =
{
new HitCircle
{
Position = OsuPlayfield.BASE_SIZE / 2,
StartTime = 0,
},
new HitCircle
{
Position = OsuPlayfield.BASE_SIZE / 2,
StartTime = 5000,
}
}
};

protected override WorkingBeatmap CreateWorkingBeatmap(IBeatmap beatmap, Storyboard? storyboard = null) =>
new ClockBackedTestWorkingBeatmap(beatmap, storyboard, new FramedClock(new ManualClock { Rate = 1 }), audioManager);

Expand Down Expand Up @@ -70,18 +89,16 @@ public void TestOsuInputNotReceivedWhilePaused()
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);
checkKey(() => counter, 2, true);

AddStep("release Z", () => InputManager.ReleaseKey(Key.Z));
checkKey(() => counter, 1, false);
checkKey(() => counter, 2, false);

AddStep("press Z", () => InputManager.PressKey(Key.Z));
checkKey(() => counter, 2, true);
checkKey(() => counter, 3, true);

AddStep("release Z", () => InputManager.ReleaseKey(Key.Z));
checkKey(() => counter, 2, false);
checkKey(() => counter, 3, false);
}

[Test]
Expand All @@ -90,30 +107,29 @@ 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));
AddStep("get key counter", () => counter = this.ChildrenOfType<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<ManiaAction> actionTrigger && actionTrigger.Action == ManiaAction.Special1));
checkKey(() => counter, 0, false);

AddStep("press D", () => InputManager.PressKey(Key.D));
AddStep("press space", () => InputManager.PressKey(Key.Space));
checkKey(() => counter, 1, true);

AddStep("release D", () => InputManager.ReleaseKey(Key.D));
AddStep("release space", () => InputManager.ReleaseKey(Key.Space));
checkKey(() => counter, 1, false);

AddStep("pause", () => Player.Pause());
AddStep("press D", () => InputManager.PressKey(Key.D));
AddStep("press space", () => InputManager.PressKey(Key.Space));
checkKey(() => counter, 1, false);

AddStep("release D", () => InputManager.ReleaseKey(Key.D));
AddStep("release space", () => InputManager.ReleaseKey(Key.Space));
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));
AddStep("press space", () => InputManager.PressKey(Key.Space));
checkKey(() => counter, 2, true);

AddStep("release D", () => InputManager.ReleaseKey(Key.D));
AddStep("release space", () => InputManager.ReleaseKey(Key.Space));
checkKey(() => counter, 2, false);
}

Expand Down Expand Up @@ -145,8 +161,11 @@ public void TestOsuPreviouslyHeldInputReleaseOnResume()
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(() => counterZ, 2, true);
checkKey(() => counterX, 1, false);

AddStep("release Z", () => InputManager.ReleaseKey(Key.Z));
checkKey(() => counterZ, 2, false);
}

[Test]
Expand All @@ -155,12 +174,12 @@ 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("get key counter", () => counter = this.ChildrenOfType<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<ManiaAction> actionTrigger && actionTrigger.Action == ManiaAction.Special1));

AddStep("press D", () => InputManager.PressKey(Key.D));
AddStep("press space", () => InputManager.PressKey(Key.Space));
AddStep("pause", () => Player.Pause());

AddStep("release D", () => InputManager.ReleaseKey(Key.D));
AddStep("release space", () => InputManager.ReleaseKey(Key.Space));
checkKey(() => counter, 1, true);

AddStep("resume", () => Player.Resume());
Expand Down Expand Up @@ -202,12 +221,14 @@ public void TestOsuHeldInputRemainHeldAfterResume()
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(() => counterZ, 2, true);
checkKey(() => counterX, 1, true);

AddStep("release X", () => InputManager.ReleaseKey(Key.X));
checkKey(() => counterZ, 1, false);
checkKey(() => counterX, 1, false);

AddStep("release Z", () => InputManager.ReleaseKey(Key.Z));
checkKey(() => counterZ, 2, false);
}

[Test]
Expand All @@ -216,22 +237,48 @@ 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("get key counter", () => counter = this.ChildrenOfType<KeyCounter>().Single(k => k.Trigger is KeyCounterActionTrigger<ManiaAction> actionTrigger && actionTrigger.Action == ManiaAction.Special1));

AddStep("press D", () => InputManager.PressKey(Key.D));
AddStep("press space", () => InputManager.PressKey(Key.Space));
checkKey(() => counter, 1, true);

AddStep("pause", () => Player.Pause());

AddStep("release D", () => InputManager.ReleaseKey(Key.D));
AddStep("press D", () => InputManager.PressKey(Key.D));
AddStep("release space", () => InputManager.ReleaseKey(Key.Space));
AddStep("press space", () => InputManager.PressKey(Key.Space));

AddStep("resume", () => Player.Resume());
AddUntilStep("wait for resume", () => Player.GameplayClockContainer.IsRunning);
checkKey(() => counter, 1, true);

AddStep("release D", () => InputManager.ReleaseKey(Key.D));
AddStep("release space", () => InputManager.ReleaseKey(Key.Space));
checkKey(() => counter, 1, false);
}

[Test]
public void TestOsuRegisterInputFromPressingOrangeCursorButPressIsBlocked()
{
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));

AddStep("pause", () => Player.Pause());
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));

// ensure the input manager receives the Z button press...
checkKey(() => counter, 1, true);
AddAssert("button is pressed in kbc", () => Player.DrawableRuleset.Playfield.FindClosestParent<OsuInputManager>()!.PressedActions.Single() == OsuAction.LeftButton);

// ...but also ensure the hit circle in front of the cursor isn't hit by checking max combo.
AddAssert("circle not hit", () => Player.ScoreProcessor.HighestCombo.Value, () => Is.EqualTo(0));

AddStep("release Z", () => InputManager.ReleaseKey(Key.Z));

checkKey(() => counter, 1, false);
AddAssert("button is released in kbc", () => !Player.DrawableRuleset.Playfield.FindClosestParent<OsuInputManager>()!.PressedActions.Any());
}

private void loadPlayer(Func<Ruleset> createRuleset)
Expand All @@ -241,9 +288,10 @@ private void loadPlayer(Func<Ruleset> createRuleset)
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));
AddStep("seek to gameplay", () => Player.GameplayClockContainer.Seek(0));
AddUntilStep("wait for seek to finish", () => Player.DrawableRuleset.FrameStableClock.CurrentTime, () => Is.EqualTo(0).Within(500));
AddAssert("not in break", () => !Player.IsBreakTime.Value);
AddStep("move cursor to center", () => InputManager.MoveMouseTo(Player.DrawableRuleset.Playfield));
}

private void checkKey(Func<KeyCounter> counter, int count, bool active)
Expand Down
2 changes: 1 addition & 1 deletion osu.Game/osu.Game.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Realm" Version="11.5.0" />
<PackageReference Include="ppy.osu.Framework" Version="2024.720.0" />
<PackageReference Include="ppy.osu.Framework" Version="2024.802.0" />
<PackageReference Include="ppy.osu.Game.Resources" Version="2024.802.0" />
<PackageReference Include="Sentry" Version="4.3.0" />
<!-- Held back due to 0.34.0 failing AOT compilation on ZstdSharp.dll dependency. -->
Expand Down
2 changes: 1 addition & 1 deletion osu.iOS.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@
<RuntimeIdentifier>iossimulator-x64</RuntimeIdentifier>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="ppy.osu.Framework.iOS" Version="2024.720.0" />
<PackageReference Include="ppy.osu.Framework.iOS" Version="2024.802.0" />
</ItemGroup>
</Project>
Loading