Skip to content

Fix replay/spectator scroll text not toggling with Ctrl+H#37027

Merged
bdach merged 6 commits intoppy:masterfrom
peppy:fix-scrolling-text-under-skin
Apr 1, 2026
Merged

Fix replay/spectator scroll text not toggling with Ctrl+H#37027
bdach merged 6 commits intoppy:masterfrom
peppy:fix-scrolling-text-under-skin

Conversation

@peppy
Copy link
Copy Markdown
Member

@peppy peppy commented Mar 18, 2026

Intended to add toggle but forgot.

This also fixes #37012 via a convoluted refactor of a lot of stuff. The basic overview is:

  • Moved all replay overlay concerns out of HUDOverlay. We can display this above everything confidently (i think).
  • Split out ReplayOverlay and ReplaySettingsOverlay so the base class can handle the visibility, hotkeys and everything that should be shared with all replay overlay components going forward. Ctrl+H is supposed to hide any of these kinds of details, and I'm sure we'll add more in the future.
  • Reorganised some things in Player so the new structure would work. Mainly the overlays which add a black layer during fade out.

@peppy peppy force-pushed the fix-scrolling-text-under-skin branch from 969b694 to 8b5acf0 Compare March 18, 2026 07:45
Copy link
Copy Markdown
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Replay controls are still fully interactable even when they're toggled off.
Screen.Recording.2026-03-18.at.10.34.15.mov
  • In multiplayer spectator completely hiding the new replay overlay means that there is no way to know which player is which anymore.

@bdach
Copy link
Copy Markdown
Collaborator

bdach commented Apr 1, 2026

FYI since I saw you refer to this as already merged in changelog comments it's not already merged due to review objections above. Also needs merge conflict resolution now. And possibly once more with #37164.

@peppy
Copy link
Copy Markdown
Member Author

peppy commented Apr 1, 2026

🤦

@peppy peppy requested a review from bdach April 1, 2026 06:46
{
this.spectatorPlayerClock = spectatorPlayerClock;

ShowSettingsOverlay = false;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still an issue, as it will hide not only the settings overlay but the scrolling text too, which is important because that's the only visual indication as to who's who on the screen.

diff --git a/osu.Game/Screens/Play/SpectatorPlayer.cs b/osu.Game/Screens/Play/SpectatorPlayer.cs
index 910a4aef65..ab01dd9118 100644
--- a/osu.Game/Screens/Play/SpectatorPlayer.cs
+++ b/osu.Game/Screens/Play/SpectatorPlayer.cs
@@ -35,26 +35,31 @@ protected SpectatorPlayer(Score score, PlayerConfiguration? configuration = null
         [BackgroundDependencyLoader]
         private void load()
         {
+            // TODO: This should be customised for `MultiplayerSpectatorPlayer` to be static and only show the player name.
+            // Or maybe we should completely redesign this to show the user avatar and other things if that happens.
+            OsuTextFlowContainer message = new OsuTextFlowContainer(cp => cp.Font = OsuFont.Style.Body) { AutoSizeAxes = Axes.Both };
+            message.AddText("Watching ");
+            message.AddText(Score.ScoreInfo.User.Username, s => s.Font = s.Font.With(weight: FontWeight.SemiBold));
+            message.AddText(" play ");
+            message.AddText(Beatmap.Value.BeatmapInfo.GetDisplayTitleRomanisable(), s => s.Font = s.Font.With(weight: FontWeight.SemiBold));
+            message.AddText(" live", s => s.Font = s.Font.With(weight: FontWeight.Bold));
+            var scrollingMessage = new ScrollingMessage(message)
+            {
+                Y = 96,
+                Anchor = Anchor.TopCentre,
+                Origin = Anchor.TopCentre,
+            };
+
             if (ShowSettingsOverlay)
             {
                 var replayOverlay = new ReplayOverlay();
                 GameplayClockContainer.Add(replayOverlay);
 
-                // TODO: This should be customised for `MultiplayerSpectatorPlayer` to be static and only show the player name.
-                // Or maybe we should completely redesign this to show the user avatar and other things if that happens.
-                OsuTextFlowContainer message = new OsuTextFlowContainer(cp => cp.Font = OsuFont.Style.Body) { AutoSizeAxes = Axes.Both };
-                message.AddText("Watching ");
-                message.AddText(Score.ScoreInfo.User.Username, s => s.Font = s.Font.With(weight: FontWeight.SemiBold));
-                message.AddText(" play ");
-                message.AddText(Beatmap.Value.BeatmapInfo.GetDisplayTitleRomanisable(), s => s.Font = s.Font.With(weight: FontWeight.SemiBold));
-                message.AddText(" live", s => s.Font = s.Font.With(weight: FontWeight.Bold));
-
-                replayOverlay.SetMessage(new ScrollingMessage(message)
-                {
-                    Y = 96,
-                    Anchor = Anchor.TopCentre,
-                    Origin = Anchor.TopCentre,
-                });
+                replayOverlay.SetMessage(scrollingMessage);
+            }
+            else
+            {
+                GameplayClockContainer.Add(scrollingMessage);
             }
         }
 

is a possible alternative.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or it may be easier to incorporate #37164 into this PR since the two will conflict almost instantaneously. This one is removing the virtual method that one is trying to override.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or I guess I merge this and then #37164 immediately after

Yeah, I'd probably be changing that PR to make the text there always visible, not part of this overlay, or something. Will revisit once this is merged, or feel free to do so for me 😅

Can apply your patch above if you want to keep master in a good state.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll just merge as is since it's not going to get lost in merge conflict resolution anyway.

@bdach bdach merged commit 279effe into ppy:master Apr 1, 2026
5 of 9 checks passed
@github-project-automation github-project-automation bot moved this from Inbox to Done in osu! team task tracker Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Skin elements can overlap the new scrolling text in replays

2 participants