Reported via e-mails.
The web-side change that wasn't ported here is
https://github.com/ppy/osu-web/pull/11151. I wanted to port it at the
time but then ran into issues because this logic should *ideally* also
be applied to the beatmap set overlay leaderboards, but those are
hard-glued to `ScoreInfo`, cannot be easily weaned off it to use
`SoloScoreInfo` instead, and I did not want to make `ScoreInfo` even
more of a mess than it already is.
This time I'm just ignoring it and adding a TODO instead because I have
no confidence I will get review eyes on any refactor of the beatmap set
overlay. All I'll say that such refactors would have potentially
beneficial effects on results screens too which also (ab)use
`ScoreInfo`.
Fixes startup sounds from potentially being fetched from the wrong
source if API connection establishment takes longer than the intro
screen takes to load.
Closes https://github.com/ppy/osu/issues/22492.
As in, `https://osu.ppy.sh/multiplayer/rooms/{id}` links, clicked from
the chat overlay, now directly open in the client.
Additionally, `osu://room/{id}` can be used in the same way to open a
room from a third-party application or a browser.
Intends to close https://github.com/ppy/osu/issues/32859.
The difference between this and https://github.com/ppy/osu/pull/32942 is
that this PR takes the approach of completely moving the score sorting
behaviour to `IGameplayLeaderboardProvider` implementations.
This is going to be helpful for further work - to be precise, I am
looking to add a leaderboard position indicator in the bottom right of
multiplayer player to match stable, and having the position in the
provider will make the implementation of that *much* easier.
The rationale for this change is that the return value was mostly
useless, and at worst, misleading.
When using `LeaderboardManager`, it's assumed that a consumer will bind
to the global `Scores` list to ensure they receive updates for things
like local score changes via the internal realm subscription. If one
decides to instead use the return value of the task, it will be a static
snapshot that potentially becomes stale in the future.
I fell into this trap when refactoring the new leaderboard component
(while attempting to assert correctness that the values we are
displaying were in fact from the fetch operation we requested).
In the interest of keeping things simple, removing the return value
seems to be the best path forward.
Kind of a big oversight this. In wanting to get the leaderboard
refactors to move forward I sort of didn't realise the fact that all of
the error handling related to online status and such in
`BeatmapLeaderboard` kind of... can't stay there...
It's also an all-or-nothing business too - moving this stuff can't
really be done only in part.
Not sure whether tests are warranted if it's more or less moving logic
across?
This is a prerequisite for supporting skinning of leaderboards.
- New `IGameplayLeaderboardProvider` and `IGameplayLeaderboardScore`
interfaces are introduced. They are strictly concerned with supplying
leaderboard data.
- Logic of managing display, which was previously jammed into the
inheritance hierarchy of `GameplayLeaderboard`, is now moved into
`IGameplayLeaderboardProvider` implementations. Solo play,
multiplayer, and multiplayer spectator get their own implementation of
the interface.
- The inheritance hierarchy of `GameplayLeaderboard` and per-player
overriding of the implementation of the gameplay leaderboard is gone.
Only one drawable class (renamed to `DrawableGameplayLeaderboard`) is
allowed to display the leaderboards, across all modes of play.
With `ReplayPlayer` now consuming the `LeaderboardManager`'s global
state, flows such as presenting a score need to set the global state up
correctly to avoid accidentally showing a leaderboard from a completely
different score.
This also incidentally closes https://github.com/ppy/osu/issues/27609.
Band-aid fix for https://github.com/ppy/osu/issues/32671.
Removes all `BindableList.BindTo` from `load()` methods (except one
editor one which looks safe and is kinda hard to fix without moving
drawable load to a blocking operation).
The lack of this is currently failing a unit test on
`osu-server-spectator` current master:
https://github.com/ppy/osu-server-spectator/actions/runs/14193158383/job/39762243965#step:4:28
I don't think the failure actually matters because I don't think we're
using json serialisation on spectator server side anywhere (used to for
iOS at least, but I don't think we do anymore?), but probably better to
be safe than sorry.
Resolves one part of
https://github.com/ppy/osu/discussions/32568#discussioncomment-12612928
A few caveats:
- Layout is slightly different than web intentionally. Web does things
that I think will be difficult to reproduce or just plain look bad in
client, such as:
- On web, the metadata info box has 200px min height and 300px max
height. I just hardcoded 300 units.
- On web, user tags and mapper tags are individually scrollable, and
the amount of space taken up by each is calculated in a way that
is - as far as I can tell - indeterminate, and probably influenced
by some flexbox magic. I just made the entire thing scrollable
instead.
- Because song select shares controls with the beatmap set overlay, now
song select says "Mapper Tags" in the header instead of just "Tags"
too. I think this is fine, because people asked for user tags to be
shown in song select too.
- Search query syntax lifted from
https://github.com/ppy/osu-web/pull/12047.
- Using hardcoded English strings for now, will update to the
translations after the next osu-resources localisations update.
Removes storage of `selectedBeatmap` that was referenced through
multiple class-level methods.
To expound a bit, this structure felt better (or otherwise passing
`APIBeatmap` through methods) alongside removal of the `#nullable
disable`, otherwise each method would check `selectedBeatmap != null`.