From ced8dda1a29da0697bf5e47c7ab0734f473b6892 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Nov 2024 17:30:50 +0900 Subject: [PATCH 1/2] Clear previous `LastLocalUserScore` when returning to song select This seems like the lowest friction way of fixing https://github.com/ppy/osu/issues/30885. We could also only null this on application, but this feels worse because - It would require local handling (potentially complex) in `BeatmapOffsetControl` if we want to continue displaying the graph and button after clicking it. - It would make the session static very specific in usage and potentially make future usage not possible due to being nulled in only a very specific scenario. One might argue that it would be nice to have this non-null until the next play, but if such a usage comes up I'd propose we rename this session static and add a new one with that purpose. --- osu.Game/Configuration/SessionStatics.cs | 4 +++- osu.Game/Screens/Play/PlayerLoader.cs | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/osu.Game/Configuration/SessionStatics.cs b/osu.Game/Configuration/SessionStatics.cs index 225f209380..18631f5d00 100644 --- a/osu.Game/Configuration/SessionStatics.cs +++ b/osu.Game/Configuration/SessionStatics.cs @@ -10,6 +10,7 @@ using osu.Game.Online.API.Requests.Responses; using osu.Game.Overlays; using osu.Game.Overlays.Mods; using osu.Game.Scoring; +using osu.Game.Screens.Play; namespace osu.Game.Configuration { @@ -77,7 +78,8 @@ namespace osu.Game.Configuration TouchInputActive, /// - /// Stores the local user's last score (can be completed or aborted). + /// Contains the local user's last score (can be completed or aborted) after exiting . + /// Will be cleared to null when leaving . /// LastLocalUserScore, diff --git a/osu.Game/Screens/Play/PlayerLoader.cs b/osu.Game/Screens/Play/PlayerLoader.cs index 3e36c630db..0db96b71ad 100644 --- a/osu.Game/Screens/Play/PlayerLoader.cs +++ b/osu.Game/Screens/Play/PlayerLoader.cs @@ -28,6 +28,7 @@ using osu.Game.Localisation; using osu.Game.Overlays; using osu.Game.Overlays.Notifications; using osu.Game.Performance; +using osu.Game.Scoring; using osu.Game.Screens.Menu; using osu.Game.Screens.Play.PlayerSettings; using osu.Game.Skinning; @@ -78,6 +79,8 @@ namespace osu.Game.Screens.Play private FillFlowContainer disclaimers = null!; private OsuScrollContainer settingsScroll = null!; + private Bindable lastScore = null!; + private Bindable showStoryboards = null!; private bool backgroundBrightnessReduction; @@ -179,6 +182,8 @@ namespace osu.Game.Screens.Play { muteWarningShownOnce = sessionStatics.GetBindable(Static.MutedAudioNotificationShownOnce); batteryWarningShownOnce = sessionStatics.GetBindable(Static.LowBatteryNotificationShownOnce); + lastScore = sessionStatics.GetBindable(Static.LastLocalUserScore); + showStoryboards = config.GetBindable(OsuSetting.ShowStoryboard); const float padding = 25; @@ -347,6 +352,8 @@ namespace osu.Game.Screens.Play highPerformanceSession?.Dispose(); highPerformanceSession = null; + lastScore.Value = null; + return base.OnExiting(e); } From c26c84ba4519ade44fb3196a0e8187dde35605ab Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 28 Nov 2024 18:03:19 +0900 Subject: [PATCH 2/2] Add test coverage governing new behaviour --- .../Navigation/TestSceneScreenNavigation.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs index eda7ce925a..5646649d33 100644 --- a/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs +++ b/osu.Game.Tests/Visual/Navigation/TestSceneScreenNavigation.cs @@ -354,6 +354,23 @@ namespace osu.Game.Tests.Visual.Navigation AddAssert("retry count is 1", () => player.RestartCount == 1); } + [Test] + public void TestLastScoreNullAfterExitingPlayer() + { + AddUntilStep("wait for last play null", getLastPlay, () => Is.Null); + + var getOriginalPlayer = playToCompletion(); + + AddStep("attempt to retry", () => getOriginalPlayer().ChildrenOfType().First().Action()); + AddUntilStep("wait for last play matches player", getLastPlay, () => Is.EqualTo(getOriginalPlayer().Score.ScoreInfo)); + + AddUntilStep("wait for player", () => Game.ScreenStack.CurrentScreen != getOriginalPlayer() && Game.ScreenStack.CurrentScreen is Player); + AddStep("exit player", () => (Game.ScreenStack.CurrentScreen as Player)?.Exit()); + AddUntilStep("wait for last play null", getLastPlay, () => Is.Null); + + ScoreInfo getLastPlay() => Game.Dependencies.Get().Get(Static.LastLocalUserScore); + } + [Test] public void TestRetryImmediatelyAfterCompletion() {