Skip to content

Conversation

@bdach
Copy link
Collaborator

@bdach bdach commented Jul 31, 2025

Closes #34445.

The primary issue is that song select is the only one that supports non-score sort mode, and therefore if any other component changes the sort mode in a way opaque to song select to score, then song select will lose the sort mode because it's using the global leaderboard manager's state which will contain scores sorted by total.

Notably, the bug this is fixing requires specific circumstances. For instance, getting the bug to present requires going through the results screen; it is not enough to just start gameplay for the bug to manifest, because starting gameplay causes a working beatmap refetch:

// Forced refetch is important here to guarantee correct invalidation across all difficulties (editor specific).
Beatmap.Value = beatmaps.GetWorkingBeatmap(beatmap, true);

which will trigger a delayed schedule refetch of the scores:

refetchOperation = Scheduler.AddDelayed(() =>
{
var fetchBeatmapInfo = beatmap.Value.BeatmapInfo;
var fetchRuleset = ruleset.Value ?? fetchBeatmapInfo.Ruleset;
var fetchSorting = Scope.Value == BeatmapLeaderboardScope.Local ? Sorting.Value : LeaderboardSortMode.Score;
// For now, we forcefully refresh to keep things simple.
// In the future, removing this requirement may be deemed useful, but will need ample testing of edge case scenarios
// (like returning from gameplay after setting a new score, returning to song select after main menu).
leaderboardManager.FetchWithCriteria(new LeaderboardCriteria(fetchBeatmapInfo, fetchRuleset, Scope.Value, FilterBySelectedMods.Value ? mods.Value.ToArray() : null, fetchSorting), forceRefresh: true);
if (!initialFetchComplete)
{
// only bind this after the first fetch to avoid reading stale scores.
fetchedScores.BindTo(leaderboardManager.Scores);
fetchedScores.BindValueChanged(_ => updateScores(), true);
initialFetchComplete = true;
}
}, initialFetchComplete ? 300 : 0);

and because the refetch is thusly delayed, there's a very high chance it will not run before the screen is resumed because the wedge will not have its scheduler run until that point in time.

This conundrum is also why there is no test coverage for this, because the above makes test coverage setup rather annoying.

…g select

Closes ppy#34445.

The primary issue is that song select is the only one that supports
non-score sort mode, and therefore if any other component changes the
sort mode in a way opaque to song select to score, then song select will
lose the sort mode because it's using the global leaderboard manager's
state which will contain scores sorted by total.

Notably, the bug this is fixing requires specific circumstances. For
instance, it is not enough to just *start gameplay* for the bug to
manifest, because starting gameplay causes a working beatmap refetch:

https://github.com/ppy/osu/blob/4b73afd1957a9161e2956fc4191c8114d9958372/osu.Game/Screens/SelectV2/SongSelect.cs#L456-L457

which will trigger a *delayed schedule refetch* of the scores:

https://github.com/ppy/osu/blob/d2d3d14f1572ff8fc68fd01ea43c2ef68b5882fa/osu.Game/Screens/SelectV2/BeatmapLeaderboardWedge.cs#L235-L253

and because the refetch is thusly delayed, there's a very high chance it
*will not run before the screen is resumed* because the wedge will not
have its scheduler run until that point in time.

This conundrum is also because there is no test coverage for this,
because the above makes test coverage setup rather annoying.
@bdach bdach self-assigned this Jul 31, 2025
@bdach bdach added area:song-select quick fix Tasks which were taken on because they take no time to fix labels Jul 31, 2025
This is borderline pedantic and mostly irrelevant but I think it makes
some sense for a bit of extra safety. It definitely does not fix any
bugs (that I'm aware of).
@bdach bdach force-pushed the fix-song-select-dropping-sort-mode branch from 9c98ea6 to 7a263ef Compare July 31, 2025 08:20

public void Refresh()
{
if (currentContent is BeatmapLeaderboardWedge leaderboardWedge)
Copy link
Member

Choose a reason for hiding this comment

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

I guess this is okay for now. (rationale: we don't need to queue this because it always refreshes on mode change).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:song-select quick fix Tasks which were taken on because they take no time to fix size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Song Select V2: Game loads scores in default sorting ignoring previously set sorting after loading and leaving the replay

2 participants