From afeb138ea002767d8c9a9d6049b00be21d0edfac Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 21:47:38 +0900 Subject: [PATCH 1/5] Fix occasional flash when quick exiting / retrying from player The gist of the issue is that `fadeOut` was being called *twice* in the quick exit/retry scenarios, causing weirdness with transforms. I've restructured things to ensure it's only called once. --- osu.Game/Screens/Play/Player.cs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 398e8df5c99a..91e984f5bc8c 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -86,6 +86,7 @@ public abstract partial class Player : ScreenWithBeatmapBackground, ISamplePlayb public Action RestartRequested; private bool isRestarting; + private bool noExitTransition; private Bindable mouseWheelDisabled; @@ -297,10 +298,7 @@ private void load(OsuConfigManager config, OsuGameBase game, CancellationToken c { if (!this.IsCurrentScreen()) return; - if (PerformExit(false)) - // The hotkey overlay dims the screen. - // If the operation succeeds, we want to make sure we stay dimmed to keep continuity. - fadeOut(true); + PerformExit(false, true); }, }, }); @@ -318,10 +316,7 @@ private void load(OsuConfigManager config, OsuGameBase game, CancellationToken c { if (!this.IsCurrentScreen()) return; - if (Restart(true)) - // The hotkey overlay dims the screen. - // If the operation succeeds, we want to make sure we stay dimmed to keep continuity. - fadeOut(true); + Restart(true); }, }, }); @@ -600,8 +595,9 @@ private IBeatmap loadPlayableBeatmap(Mod[] gameplayMods, CancellationToken cance /// Whether the pause or fail dialog should be shown before performing an exit. /// If and a dialog is not yet displayed, the exit will be blocked and the relevant dialog will display instead. /// + /// Whether the exit should perform without a transition, because the screen had faded to black already. /// Whether this call resulted in a final exit. - protected bool PerformExit(bool showDialogFirst) + protected bool PerformExit(bool showDialogFirst, bool withoutTransition = false) { bool pauseOrFailDialogVisible = PauseOverlay.State.Value == Visibility.Visible || FailOverlay.State.Value == Visibility.Visible; @@ -639,6 +635,8 @@ protected bool PerformExit(bool showDialogFirst) // Screen may not be current if a restart has been performed. if (this.IsCurrentScreen()) { + noExitTransition = withoutTransition; + // The actual exit is performed if // - the pause / fail dialog was not requested // - the pause / fail dialog was requested but is already displayed (user showing intention to exit). @@ -709,7 +707,7 @@ public bool Restart(bool quickRestart = false) RestartRequested?.Invoke(quickRestart); - return PerformExit(false); + return PerformExit(false, quickRestart); } /// @@ -1254,10 +1252,10 @@ protected virtual Task ImportScore(Score score) ShowUserStatistics = true, }; - private void fadeOut(bool instant = false) + private void fadeOut() { - float fadeOutDuration = instant ? 0 : 250; - this.FadeOut(fadeOutDuration); + if (!noExitTransition) + this.FadeOut(250); if (this.IsCurrentScreen()) { From 3262b6d98931a085cee486cd5cc88142f4d133fa Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 22:10:45 +0900 Subject: [PATCH 2/5] Refactor to avoid dual-boolean mess --- .../Gameplay/TestSceneStoryboardWithOutro.cs | 2 +- .../Multiplayer/MultiplayerPlayer.cs | 2 +- osu.Game/Screens/Play/Player.cs | 48 ++++++++++++------- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs index aff6139c08fa..4f1a63341a47 100644 --- a/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs +++ b/osu.Game.Tests/Visual/Gameplay/TestSceneStoryboardWithOutro.cs @@ -223,7 +223,7 @@ private Storyboard createStoryboard(double duration) protected partial class OutroPlayer : TestPlayer { - public void ExitViaPause() => PerformExit(true); + public void ExitViaPause() => PerformExitWithConfirmation(); public new FailOverlay FailOverlay => base.FailOverlay; diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs index e560c5ca5dd1..a50f3440f58d 100644 --- a/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs +++ b/osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerPlayer.cs @@ -158,7 +158,7 @@ private void failAndBail(string message = null) if (!string.IsNullOrEmpty(message)) Logger.Log(message, LoggingTarget.Runtime, LogLevel.Important); - Schedule(() => PerformExit(false)); + Schedule(() => PerformExit()); } private void onGameplayStarted() => Scheduler.Add(() => diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 91e984f5bc8c..ac49b4c42dbd 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -86,7 +86,7 @@ public abstract partial class Player : ScreenWithBeatmapBackground, ISamplePlayb public Action RestartRequested; private bool isRestarting; - private bool noExitTransition; + private bool skipExitTransition; private Bindable mouseWheelDisabled; @@ -290,7 +290,7 @@ private void load(OsuConfigManager config, OsuGameBase game, CancellationToken c { SaveReplay = async () => await prepareAndImportScoreAsync(true).ConfigureAwait(false), OnRetry = Configuration.AllowUserInteraction ? () => Restart() : null, - OnQuit = () => PerformExit(true), + OnQuit = () => PerformExitWithConfirmation(), }, new HotkeyExitOverlay { @@ -298,7 +298,7 @@ private void load(OsuConfigManager config, OsuGameBase game, CancellationToken c { if (!this.IsCurrentScreen()) return; - PerformExit(false, true); + PerformExit(true); }, }, }); @@ -443,7 +443,7 @@ private Drawable createOverlayComponents(IWorkingBeatmap working) { HoldToQuit = { - Action = () => PerformExit(true), + Action = () => PerformExitWithConfirmation(), IsPaused = { BindTarget = GameplayClockContainer.IsPaused }, ReplayLoaded = { BindTarget = DrawableRuleset.HasReplayLoaded }, }, @@ -480,7 +480,7 @@ private Drawable createOverlayComponents(IWorkingBeatmap working) OnResume = Resume, Retries = RestartCount, OnRetry = () => Restart(), - OnQuit = () => PerformExit(true), + OnQuit = () => PerformExitWithConfirmation(), }, }, }; @@ -583,26 +583,24 @@ private IBeatmap loadPlayableBeatmap(Mod[] gameplayMods, CancellationToken cance } /// - /// Attempts to complete a user request to exit gameplay. + /// Attempts to complete a user request to exit gameplay, with confirmation. /// /// /// /// This should only be called in response to a user interaction. Exiting is not guaranteed. /// This will interrupt any pending progression to the results screen, even if the transition has begun. /// + /// + /// This method will show the pause or fail dialog before performing an exit. + /// If a dialog is not yet displayed, the exit will be blocked and the relevant dialog will display instead. /// - /// - /// Whether the pause or fail dialog should be shown before performing an exit. - /// If and a dialog is not yet displayed, the exit will be blocked and the relevant dialog will display instead. - /// - /// Whether the exit should perform without a transition, because the screen had faded to black already. - /// Whether this call resulted in a final exit. - protected bool PerformExit(bool showDialogFirst, bool withoutTransition = false) + /// + protected bool PerformExitWithConfirmation() { bool pauseOrFailDialogVisible = PauseOverlay.State.Value == Visibility.Visible || FailOverlay.State.Value == Visibility.Visible; - if (showDialogFirst && !pauseOrFailDialogVisible) + if (!pauseOrFailDialogVisible) { // if the fail animation is currently in progress, accelerate it (it will show the pause dialog on completion). if (ValidForResume && GameplayState.HasFailed) @@ -621,6 +619,22 @@ protected bool PerformExit(bool showDialogFirst, bool withoutTransition = false) } } + return PerformExit(); + } + + /// + /// Attempts to complete a user request to exit gameplay. + /// + /// + /// + /// This should only be called in response to a user interaction. Exiting is not guaranteed. + /// This will interrupt any pending progression to the results screen, even if the transition has begun. + /// + /// + /// Whether the exit should perform without a transition, because the screen had faded to black already. + /// Whether this call resulted in a final exit. + protected bool PerformExit(bool skipTransition = false) + { // Matching osu!stable behaviour, if the results screen is pending and the user requests an exit, // show the results instead. if (GameplayState.HasPassed && !isRestarting) @@ -635,7 +649,7 @@ protected bool PerformExit(bool showDialogFirst, bool withoutTransition = false) // Screen may not be current if a restart has been performed. if (this.IsCurrentScreen()) { - noExitTransition = withoutTransition; + skipExitTransition = skipTransition; // The actual exit is performed if // - the pause / fail dialog was not requested @@ -707,7 +721,7 @@ public bool Restart(bool quickRestart = false) RestartRequested?.Invoke(quickRestart); - return PerformExit(false, quickRestart); + return PerformExit(quickRestart); } /// @@ -1254,7 +1268,7 @@ protected virtual Task ImportScore(Score score) private void fadeOut() { - if (!noExitTransition) + if (!skipExitTransition) this.FadeOut(250); if (this.IsCurrentScreen()) From d1b5d31ea66fc3c8aa777675e935fa103564a38f Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 22:23:42 +0900 Subject: [PATCH 3/5] Add explicit parameter in --- osu.Game/Screens/Play/Player.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index ac49b4c42dbd..8d7665a0d92e 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -298,7 +298,7 @@ private void load(OsuConfigManager config, OsuGameBase game, CancellationToken c { if (!this.IsCurrentScreen()) return; - PerformExit(true); + PerformExit(skipTransition: true); }, }, }); @@ -721,7 +721,7 @@ public bool Restart(bool quickRestart = false) RestartRequested?.Invoke(quickRestart); - return PerformExit(quickRestart); + return PerformExit(skipTransition: quickRestart); } /// From 5d3f55fe4d237592e8f1aaad91ffa280e7ecf3ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Thu, 14 Nov 2024 15:05:46 +0100 Subject: [PATCH 4/5] Fill out xmldoc --- osu.Game/Screens/Play/Player.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 8d7665a0d92e..6ab7d81f8322 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -594,7 +594,7 @@ private IBeatmap loadPlayableBeatmap(Mod[] gameplayMods, CancellationToken cance /// This method will show the pause or fail dialog before performing an exit. /// If a dialog is not yet displayed, the exit will be blocked and the relevant dialog will display instead. /// - /// + /// Whether this call resulted in a final exit. protected bool PerformExitWithConfirmation() { bool pauseOrFailDialogVisible = From 1a31e56d4acfecc55a73b0852c08dea06b8bf7b0 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 14 Nov 2024 23:59:55 +0900 Subject: [PATCH 5/5] Fix double restart call still existing --- osu.Game/Screens/Play/Player.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/osu.Game/Screens/Play/Player.cs b/osu.Game/Screens/Play/Player.cs index 6ab7d81f8322..e9722350bd56 100644 --- a/osu.Game/Screens/Play/Player.cs +++ b/osu.Game/Screens/Play/Player.cs @@ -719,9 +719,14 @@ public bool Restart(bool quickRestart = false) // stopping here is to ensure music doesn't become audible after exiting back to PlayerLoader. musicController.Stop(); - RestartRequested?.Invoke(quickRestart); + if (RestartRequested != null) + { + skipExitTransition = quickRestart; + RestartRequested?.Invoke(quickRestart); + return true; + } - return PerformExit(skipTransition: quickRestart); + return PerformExit(quickRestart); } ///