Add a skin-level setting to leaderboard to allow disabling automatic collapsing

[peppy]

Was a 2 minute implementation, so why not?

Addresses #33523, #22040.

[bdach]

no localisations?

[peppy]

You're going to make me turn on roslyn analysers and then forget about it and ragequite at hot reload no longer working 😅

[bdach]

I would hope that CollapseDuringGameplay can replace ForceExpand usages because this conditional is getting a little ridiculous now.

diff --git a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs
index f45e6326d1..d026afcd6d 100644
--- a/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs
+++ b/osu.Game.Tests/Visual/Gameplay/TestSceneGameplayLeaderboard.cs
@@ -36,7 +36,7 @@ public TestSceneGameplayLeaderboard()
             AddStep("toggle expanded", () =>
             {
                 if (leaderboard.IsNotNull())
-                    leaderboard.ForceExpand.Value = !leaderboard.ForceExpand.Value;
+                    leaderboard.CollapseDuringGameplay.Value = !leaderboard.CollapseDuringGameplay.Value;
             });
 
             AddSliderStep("set player score", 50, 5000000, 1222333, v => playerScore.Value = v);
diff --git a/osu.Game.Tests/Visual/Multiplayer/MultiplayerGameplayLeaderboardTestScene.cs b/osu.Game.Tests/Visual/Multiplayer/MultiplayerGameplayLeaderboardTestScene.cs
index 3008edf41f..955737578a 100644
--- a/osu.Game.Tests/Visual/Multiplayer/MultiplayerGameplayLeaderboardTestScene.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/MultiplayerGameplayLeaderboardTestScene.cs
@@ -167,7 +167,7 @@ public virtual void SetUpSteps()
         public void TestScoreUpdates()
         {
             AddRepeatStep("update state", UpdateUserStatesRandomly, 100);
-            AddToggleStep("switch compact mode", expanded => Leaderboard!.ForceExpand.Value = expanded);
+            AddToggleStep("switch compact mode", collapsed => Leaderboard!.CollapseDuringGameplay.Value = collapsed);
         }
 
         [Test]
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs
index 131b644dcb..c39708352e 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiSpectatorLeaderboard.cs
@@ -58,7 +58,7 @@ public override void SetUpSteps()
                     {
                         Anchor = Anchor.Centre,
                         Origin = Anchor.Centre,
-                        ForceExpand = { Value = true }
+                        CollapseDuringGameplay = { Value = false }
                     }
                 });
             });
diff --git a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs
index 40d8650c69..6141820cb7 100644
--- a/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs
+++ b/osu.Game.Tests/Visual/Multiplayer/TestSceneMultiplayerGameplayLeaderboardTeams.cs
@@ -44,14 +44,16 @@ public override void SetUpSteps()
                     Team2Score = { BindTarget = LeaderboardProvider.TeamScores[1] }
                 }, Add);
 
-                LoadComponentAsync(new GameplayMatchScoreDisplay
+                GameplayMatchScoreDisplay matchScoreDisplay;
+                LoadComponentAsync(matchScoreDisplay = new GameplayMatchScoreDisplay
                 {
                     Anchor = Anchor.BottomCentre,
                     Origin = Anchor.BottomCentre,
                     Team1Score = { BindTarget = LeaderboardProvider.TeamScores[0] },
                     Team2Score = { BindTarget = LeaderboardProvider.TeamScores[1] },
-                    Expanded = { BindTarget = Leaderboard!.ForceExpand },
                 }, Add);
+
+                Leaderboard!.CollapseDuringGameplay.BindValueChanged(_ => matchScoreDisplay.Expanded.Value = !Leaderboard.CollapseDuringGameplay.Value);
             });
         }
     }
diff --git a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs
index 06efffbf6e..7ad8bdf454 100644
--- a/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs
+++ b/osu.Game/Screens/OnlinePlay/Multiplayer/Spectate/MultiSpectatorScreen.cs
@@ -154,7 +154,7 @@ private void load()
             });
             leaderboardFlow.Insert(0, Leaderboard = new DrawableGameplayLeaderboard
             {
-                ForceExpand = { Value = true }
+                CollapseDuringGameplay = { Value = false }
             });
 
             LoadComponentAsync(new GameplayChatDisplay(room)
diff --git a/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs b/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs
index 49b46298c9..f8e54efbf2 100644
--- a/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs
+++ b/osu.Game/Screens/Play/HUD/DrawableGameplayLeaderboard.cs
@@ -20,8 +20,6 @@ namespace osu.Game.Screens.Play.HUD
 {
     public partial class DrawableGameplayLeaderboard : CompositeDrawable, ISerialisableDrawable
     {
-        public readonly Bindable<bool> ForceExpand = new Bindable<bool>();
-
         protected readonly FillFlowContainer<DrawableGameplayLeaderboardScore> Flow;
 
         private bool requiresScroll;
@@ -100,7 +98,6 @@ protected override void LoadComplete()
             configVisibility.BindValueChanged(_ => Scheduler.AddOnce(updateState));
             userPlayingState.BindValueChanged(_ => Scheduler.AddOnce(updateState));
             holdingForHUD.BindValueChanged(_ => Scheduler.AddOnce(updateState));
-            ForceExpand.BindValueChanged(_ => Scheduler.AddOnce(updateState));
             CollapseDuringGameplay.BindValueChanged(_ => Scheduler.AddOnce(updateState));
             updateState();
         }
@@ -112,7 +109,7 @@ private void updateState()
                 scroll.ScrollToStart(false);
 
             Flow.FadeTo(player?.Configuration.ShowLeaderboard != false && configVisibility.Value ? 1 : 0, 100, Easing.OutQuint);
-            expanded.Value = !CollapseDuringGameplay.Value || ForceExpand.Value || userPlayingState.Value != LocalUserPlayingState.Playing || holdingForHUD.Value;
+            expanded.Value = !CollapseDuringGameplay.Value || userPlayingState.Value != LocalUserPlayingState.Playing || holdingForHUD.Value;
         }
 
         /// <summary>

(The test usages are broken but I think they're also broken on master because I broke them. Something to do with userPlayingState being NotPlaying because the tests don't cache a GameplayState probably. Not sure whether fixing is in scope here.)

[peppy]

Hmm... this looks like it'll work, but we'd have to tread carefully to ensure it's never used in cases where the user has control over the leaderboard. At first I was sure it would break due to it being used in a scenario like multiplayer gameplay, but I guess that's not the case anymore.

I'm not sure how I feel about this direction though.. the one usage where it's not user placeable uses a user-exposed setting to change its behaviour.

Does this kind of thing concern you at all? If not I think we can go with it and see how things play out, just have to be a bit careful.

[bdach]

the one usage where it's not user placeable uses a user-exposed setting to change its behaviour

I've already recently done something resembling that in fd3514c. So I guess that's a "no" on whether it concerns me.

I guess I can live with the current state of things but it's just a bit stupid to have two of what is invariably the same flag just with negated semantics next to each other.

[peppy]

Let's give it a try then. Worst that can happen is people's skins get mutated and they report that back to us.