Delay back button appearance when performing a quick restart

[frenzibyte]

• Addresses Back Buttom in Fast Restart #30679

Adding tests for the BackButtonState property seems to be too much work for not much benefit, but I can add one if insisted.

Note that I've created another bindable for this rather than turning AllowBackButton itself into a bindable, this is because we still want to allow the user to exit right after performing a quick restart (since exiting is controlled by AllowBackButton, meanwhile the new bindable only affects the visual state of the button itself).

Preview:

CleanShot.2024-11-24.at.05.38.50.mp4

[bdach]

Maybe even the above is too complex and the back button should just not show on the player loader. Dunno. Will need @peppy's opinion to proceed.

[peppy]

The original rationale from me was that if loading takes too long, the player needs an out (even if they don't have a keyboard).

I haven't looked into the code here yet but the premise of delaying it for the same delay as the metadata is sound enough.

[peppy]

Seems pretty good.

[peppy]

Arguably could be made private until there's a reason to have it public:

diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs
index 514209524e..c52755197b 100644
--- a/osu.Game/OsuGame.cs
+++ b/osu.Game/OsuGame.cs
@@ -178,7 +178,7 @@ public partial class OsuGame : OsuGameBase, IKeyBindingHandler<GlobalAction>, IL
         /// <summary>
         /// Whether the back button is currently displayed.
         /// </summary>
-        public readonly IBindable<bool> BackButtonVisibility = new Bindable<bool>();
+        private readonly IBindable<bool> backButtonVisibility = new Bindable<bool>();
 
         IBindable<LocalUserPlayingState> ILocalUserPlayInfo.PlayingState => playingState;
 
@@ -1194,7 +1194,7 @@ protected override void LoadComplete()
                 if (mode.NewValue != OverlayActivation.All) CloseAllOverlays();
             };
 
-            BackButtonVisibility.ValueChanged += visible =>
+            backButtonVisibility.ValueChanged += visible =>
             {
                 if (visible.NewValue)
                     BackButton.Show();
@@ -1594,14 +1594,14 @@ private void screenChanged(IScreen current, IScreen newScreen)
 
             if (current is IOsuScreen currentOsuScreen)
             {
-                BackButtonVisibility.UnbindFrom(currentOsuScreen.BackButtonVisibility);
+                backButtonVisibility.UnbindFrom(currentOsuScreen.BackButtonVisibility);
                 OverlayActivationMode.UnbindFrom(currentOsuScreen.OverlayActivationMode);
                 API.Activity.UnbindFrom(currentOsuScreen.Activity);
             }
 
             if (newScreen is IOsuScreen newOsuScreen)
             {
-                BackButtonVisibility.BindTo(newOsuScreen.BackButtonVisibility);
+                backButtonVisibility.BindTo(newOsuScreen.BackButtonVisibility);
                 OverlayActivationMode.BindTo(newOsuScreen.OverlayActivationMode);
                 API.Activity.BindTo(newOsuScreen.Activity);
 

[peppy]

Checking this behaviour, it looks back-to-front to me. Confused:

osu/osu.Game/OsuGame.cs

Line 1617 in 3e1b4f4

BackButton.Hide();

[frenzibyte]

That comment meant the footer's back button, not our classic back button. I'll update the comment.

[bdach]

Am I the only one weirded out by this sort of thing? Why does editor apparently "not allow user exit"? The user can exit it just fine.

[frenzibyte]

I failed to notice this one. Looks to have been done this way historically so the back button doesn't display, while still allowing the user to exit via Back action by manual handling in Editor.OnPressed.

This can make better sense with this PR by setting the initial back button state to hidden instead of overriding this.

[frenzibyte]

This should hopefully not cause any behavioural change to the editor due to the handling of the exit action moving from Editor.OnPressed all the way back to BackReceptor. I can't think of it breaking so I pushed the change.

[bdach]

Doesn't this sort of setup mean it's possible to put yourself in the situation where the back button is in view but the screen actually isn't "user-exitable" or however you'd put it?

[frenzibyte]

That is true, but for the sake of simplicity, it should be OsuScreen's responsibility to never show the back button if it explicitly declares to not be exitable. Otherwise, I will have to account for AllowUserExit in this bindable and make this bindable always be triggered when a screen is changed, and that just increases the complexity and takes us back to step one which is "why is this being done differently".

At the moment, I've made OsuScreen decide the initial state of the back button appearance based on whether it is declared to be user-exitable or not:

osu/osu.Game/Screens/OsuScreen.cs

Lines 59 to 62 in 3e1b4f4

	/// <summary>
	/// The initial visibility state of the back button when this screen is entered for the first time.
	/// </summary>
	protected virtual bool InitialBackButtonVisibility => AllowUserExit;