Skip to content

Convert gameplay leaderboard to skinnable component #32939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Apr 24, 2025

Makes the leaderboard a full skinnable component (placed in the ruleset-specific layer).

Users with skins that have custom HUD layouts need to add the gameplay leaderboard back themselves in their desired position.

This change also allows the leaderboard to stop overlapping the playfield in taiko.

argon triangles classic
osu! Screenshot 2025-04-24 at 13 02 58 Screenshot 2025-04-24 at 13 10 56 Screenshot 2025-04-24 at 13 03 41
taiko Screenshot 2025-04-24 at 13 03 06 Screenshot 2025-04-24 at 13 09 03 Screenshot 2025-04-24 at 13 03 47
catch Screenshot 2025-04-24 at 13 03 11 Screenshot 2025-04-24 at 13 10 59 Screenshot 2025-04-24 at 13 03 53
mania Screenshot 2025-04-24 at 13 03 15 Screenshot 2025-04-24 at 13 11 03 Screenshot 2025-04-24 at 13 03 58

This PR converts the leaderboard into a full-fledged skinnable component that can be manipulated by users at will.

Notably, this finally allows #20422 to be fixed - although it's a very mixed bag, for several reasons:

  • Because of taiko players' refusal to see reason insistence on keeping stable behaviours related to aspect ratio treatment, I have to assume the worst case scenario, which means than on typical resolutions like 16:9 (or even worse, 4:3), the leaderboard will likely not occupy as much vertical space as it probably could.
  • Additionally, there's the problem of where to put the spectator list. I settled on putting it to the right of the leaderboard, but that's kind of janky, because the leaderboard sometimes collapses and sometimes fully hides, leading to a very awkward space left behind. If we had the capability to anchor elements to other elements, maybe this could be resolved, but for now, I'm not sure what to do. I think if some users are bothered by it they can move it where they want it. Or delete it.

As with the spectator list, if some users have modified the layout on their skin, they will not have the leaderboard added back, and I'm not trying to add any migrations to try and do it for them.

Recommend reviewing with whitespace off.

bdach added 2 commits April 24, 2025 12:50
…ations

As time goes on, default skin layouts are getting more and more
complicated because of per-ruleset overrides. This was already sort of
apparent in ppy#31527, and I'm about to make
it worse, so before I do, this is a test scene that is supposed to make
it easier to check all possible combinations at a glance.
This PR converts the leaderboard into a full-fledged skinnable component
that can be manipulated by users at will.

Notably, this finally allows ppy#20422 to
be fixed - although it's a very mixed bag, for several reasons:

- Because of taiko players' refusal to see reason^W^W^W^Winsistence on
  keeping stable behaviours related to aspect ratio treatment, I have to
  assume the worst case scenario, which means than on typical
  resolutions like 16:9 (or even worse, 4:3), the leaderboard will
  likely not occupy as much vertical space as it probably could.

- Additionally, there's the problem of where to put the spectator list.
  I settled on putting it to the right of the leaderboard, but that's
  kind of janky, because the leaderboard sometimes collapses and
  sometimes fully hides, leading to a very awkward space left behind. If
  we had the capability to anchor elements to other elements, maybe this
  could be resolved, but for now, I'm not sure what to do. I think if
  some users are bothered by it they can move it where they want it. Or
  delete it.
@bdach bdach self-assigned this Apr 24, 2025
@bdach bdach added notable feature Attach to pull requests which should be highlighted on the changelog. Things users want to read. and removed size/XL labels Apr 24, 2025
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Depending on whether we want to offer a "legacy" variant of the leaderboard matching stable skins, it might be a good idea to rename this component to something like ArgonGameplayLeaderboard before it gets persisted to the JSONs.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with either. We already have a good flow for migrating component names so it's not a huge deal.

@@ -71,10 +70,7 @@ private void load()

ScoreProcessor.ApplyNewJudgementsWhenFailed = true;

LoadComponentAsync(new GameplayChatDisplay(Room)
{
Expanded = { BindTarget = LeaderboardExpandedState },
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This being coupled to leaderboard expansion state is a bit of a spoke in the wheel. I simplified this to just use LocalUserPlaying which is one half of what was steering the expansion state. Maybe fine?

leaderboard.Anchor = Anchor.BottomLeft;
leaderboard.Origin = Anchor.BottomLeft;
leaderboard.Position = pos;
leaderboard.Height = 170;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This height can be improved later when the positioning of taiko combo counter is adjusted to match stable (should be on the drum).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:gameplay area:skinning notable feature Attach to pull requests which should be highlighted on the changelog. Things users want to read. size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants