From a4174a36447fddeeb13c83fa6724520486271c62 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jan 2025 17:39:34 +0900 Subject: [PATCH 1/3] Add failing test coverage showing offset adjust is not limited correctly --- .../Navigation/TestSceneScreenNavigation.cs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 58e780cf16..326f21ff13 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -317,6 +317,82 @@ namespace osu.Game.Tests.Visual.Navigation AddUntilStep("wait for song select", () => songSelect.IsCurrentScreen()); } + [Test] + public void TestOffsetAdjustDuringPause() + { + Player player = null; + + Screens.Select.SongSelect songSelect = null; + PushAndConfirm(() => songSelect = new TestPlaySongSelect()); + AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + + AddStep("import beatmap", () => BeatmapImportHelper.LoadQuickOszIntoOsu(Game).WaitSafely()); + + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("set mods", () => Game.SelectedMods.Value = new Mod[] { new OsuModNoFail() }); + AddStep("press enter", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player", () => + { + DismissAnyNotifications(); + player = Game.ScreenStack.CurrentScreen as Player; + return player?.IsLoaded == true; + }); + + AddUntilStep("wait for track playing", () => Game.Beatmap.Value.Track.IsRunning); + checkOffset(0); + + AddStep("adjust offset via keyboard", () => InputManager.Key(Key.Minus)); + checkOffset(-1); + + AddStep("pause", () => player.ChildrenOfType().First().Stop()); + AddUntilStep("wait for pause", () => player.ChildrenOfType().First().IsPaused.Value, () => Is.True); + AddStep("attempt adjust offset via keyboard", () => InputManager.Key(Key.Minus)); + checkOffset(-1); + + void checkOffset(double offset) => AddUntilStep($"offset is {offset}", () => Game.BeatmapManager.QueryBeatmap(b => b.ID == Game.Beatmap.Value.BeatmapInfo.ID)!.UserSettings.Offset, + () => Is.EqualTo(offset)); + } + + [Test] + public void TestOffsetAdjustDuringGameplay() + { + Player player = null; + + Screens.Select.SongSelect songSelect = null; + PushAndConfirm(() => songSelect = new TestPlaySongSelect()); + AddUntilStep("wait for song select", () => songSelect.BeatmapSetsLoaded); + + AddStep("import beatmap", () => BeatmapImportHelper.LoadOszIntoOsu(Game).WaitSafely()); + + AddUntilStep("wait for selected", () => !Game.Beatmap.IsDefault); + + AddStep("set mods", () => Game.SelectedMods.Value = new Mod[] { new OsuModNoFail() }); + AddStep("press enter", () => InputManager.Key(Key.Enter)); + + AddUntilStep("wait for player", () => + { + DismissAnyNotifications(); + player = Game.ScreenStack.CurrentScreen as Player; + return player?.IsLoaded == true; + }); + + AddUntilStep("wait for track playing", () => Game.Beatmap.Value.Track.IsRunning); + checkOffset(0); + + AddStep("adjust offset via keyboard", () => InputManager.Key(Key.Minus)); + checkOffset(-1); + + AddStep("seek beyond 10 seconds", () => player.ChildrenOfType().First().Seek(10500)); + AddUntilStep("wait for seek", () => player.ChildrenOfType().First().CurrentTime, () => Is.GreaterThan(10600)); + AddStep("attempt adjust offset via keyboard", () => InputManager.Key(Key.Minus)); + checkOffset(-1); + + void checkOffset(double offset) => AddUntilStep($"offset is {offset}", () => Game.BeatmapManager.QueryBeatmap(b => b.ID == Game.Beatmap.Value.BeatmapInfo.ID)!.UserSettings.Offset, + () => Is.EqualTo(offset)); + } + [Test] public void TestRetryCountIncrements() { From 1d240eb4050d1c195e17cb36c0e511a1e834b6c2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 16 Jan 2025 17:23:02 +0900 Subject: [PATCH 2/3] Fix gameplay limitations for adjusting offset not actually being applied --- osu.Game/Screens/Play/Player.cs | 1 + .../PlayerSettings/BeatmapOffsetControl.cs | 46 +++++++++++++------ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 228b77b780..513f4854ad 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -322,6 +322,7 @@ namespace osu.Game.Screens.Play } dependencies.CacheAs(DrawableRuleset.FrameStableClock); + dependencies.CacheAs(DrawableRuleset.FrameStableClock); // add the overlay components as a separate step as they proxy some elements from the above underlay/gameplay components. // also give the overlays the ruleset skin provider to allow rulesets to potentially override HUD elements (used to disable combo counters etc.) diff --git a/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs b/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs index ac224794ea..e988760834 100644 --- a/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs +++ b/osu.Game/Screens/Play/PlayerSettings/BeatmapOffsetControl.cs @@ -274,20 +274,36 @@ namespace osu.Game.Screens.Play.PlayerSettings beatmapOffsetSubscription?.Dispose(); } + protected override void Update() + { + base.Update(); + Current.Disabled = !allowOffsetAdjust; + } + + private bool allowOffsetAdjust + { + get + { + // General limitations to ensure players don't do anything too weird. + // These match stable for now. + if (player is SubmittingPlayer) + { + Debug.Assert(gameplayClock != null); + + // TODO: the blocking conditions should probably display a message. + if (!player.IsBreakTime.Value && gameplayClock.CurrentTime - gameplayClock.StartTime > 10000) + return false; + + if (gameplayClock.IsPaused.Value) + return false; + } + + return true; + } + } + public bool OnPressed(KeyBindingPressEvent e) { - // General limitations to ensure players don't do anything too weird. - // These match stable for now. - if (player is SubmittingPlayer) - { - // TODO: the blocking conditions should probably display a message. - if (player?.IsBreakTime.Value == false && gameplayClock?.CurrentTime - gameplayClock?.StartTime > 10000) - return false; - - if (gameplayClock?.IsPaused.Value == true) - return false; - } - // To match stable, this should adjust by 5 ms, or 1 ms when holding alt. // But that is hard to make work with global actions due to the operating mode. // Let's use the more precise as a default for now. @@ -296,11 +312,13 @@ namespace osu.Game.Screens.Play.PlayerSettings switch (e.Action) { case GlobalAction.IncreaseOffset: - Current.Value += amount; + if (!Current.Disabled) + Current.Value += amount; return true; case GlobalAction.DecreaseOffset: - Current.Value -= amount; + if (!Current.Disabled) + Current.Value -= amount; return true; } From 224f39825f5f452ec6e7341666b2cae6ac700334 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Fri, 17 Jan 2025 14:16:38 +0900 Subject: [PATCH 3/3] Fix test potentially false-negative due to realm write delays --- .../Navigation/TestSceneScreenNavigation.cs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index 326f21ff13..521d097fb9 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -41,6 +41,7 @@ using osu.Game.Screens.OnlinePlay.Match.Components; using osu.Game.Screens.OnlinePlay.Playlists; using osu.Game.Screens.Play; using osu.Game.Screens.Play.HUD; +using osu.Game.Screens.Play.PlayerSettings; using osu.Game.Screens.Ranking; using osu.Game.Screens.Select; using osu.Game.Screens.Select.Carousel; @@ -351,8 +352,13 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("attempt adjust offset via keyboard", () => InputManager.Key(Key.Minus)); checkOffset(-1); - void checkOffset(double offset) => AddUntilStep($"offset is {offset}", () => Game.BeatmapManager.QueryBeatmap(b => b.ID == Game.Beatmap.Value.BeatmapInfo.ID)!.UserSettings.Offset, - () => Is.EqualTo(offset)); + void checkOffset(double offset) + { + AddUntilStep($"control offset is {offset}", () => this.ChildrenOfType().Single().ChildrenOfType().Single().Current.Value, + () => Is.EqualTo(offset)); + AddUntilStep($"database offset is {offset}", () => Game.BeatmapManager.QueryBeatmap(b => b.ID == Game.Beatmap.Value.BeatmapInfo.ID)!.UserSettings.Offset, + () => Is.EqualTo(offset)); + } } [Test] @@ -389,8 +395,13 @@ namespace osu.Game.Tests.Visual.Navigation AddStep("attempt adjust offset via keyboard", () => InputManager.Key(Key.Minus)); checkOffset(-1); - void checkOffset(double offset) => AddUntilStep($"offset is {offset}", () => Game.BeatmapManager.QueryBeatmap(b => b.ID == Game.Beatmap.Value.BeatmapInfo.ID)!.UserSettings.Offset, - () => Is.EqualTo(offset)); + void checkOffset(double offset) + { + AddUntilStep($"control offset is {offset}", () => this.ChildrenOfType().Single().ChildrenOfType().Single().Current.Value, + () => Is.EqualTo(offset)); + AddUntilStep($"database offset is {offset}", () => Game.BeatmapManager.QueryBeatmap(b => b.ID == Game.Beatmap.Value.BeatmapInfo.ID)!.UserSettings.Offset, + () => Is.EqualTo(offset)); + } } [Test]