Skip to content

Fix stable scores importing with a LegacyOnlineID of 0 #33612

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

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Jun 10, 2025

Closes #33435.

The root cause of the issue is that the user's database contained a whole lot of scores with LegacyOnlineID of 0, which would trip up

if (instance.LegacyOnlineID < 0 || other.LegacyOnlineID < 0)
return false;

as that method would thus consider scores that are not the same as the same because of the zero, which later trips up

if (clonedScore.Equals(Score) || clonedScore.MatchesOnlineID(Score))

which ends up inserting Score into the list several times, which causes the crash.

You might remember that I tried to fix this once before in #24794. What I did not realise, however, is that stable can still produce replays that have an online ID of zero in them, because zero is just default(long) (reference 1, reference 2), and e.g. scores set offline will have that zero inside.

The alternative way of fixing this would be just to change MatchesOnlineID to reject zeroes, but I think this is a saner overall direction.

Closes ppy#33435.

The root cause of the issue is that the user's database contained a
whole lot of scores with `LegacyOnlineID` of 0, which would trip up

	https://github.com/ppy/osu/blob/97e6187f0d7c3dbee896596a623e34627135bf0e/osu.Game/Extensions/ModelExtensions.cs#L128-L129

as that method would thus consider scores that are not the same as the
same because of the zero, which later trips up

	https://github.com/ppy/osu/blob/97e6187f0d7c3dbee896596a623e34627135bf0e/osu.Game/Screens/Ranking/SoloResultsScreen.cs#L79

which ends up inserting `Score` into the list several times, which
causes the crash.

You might remember that I tried to fix this once before in
ppy#24794. What I did not realise, however,
is that stable *can still produce replays* that have an online ID of
zero in them, because zero *is just `default(long)`*:

	https://github.com/peppy/osu-stable-reference/blob/7205341bb70000a87fa1bd54e7642772e2af85d7/osu!/GameplayElements/Scoring/Score.cs#L123
	https://github.com/peppy/osu-stable-reference/blob/7205341bb70000a87fa1bd54e7642772e2af85d7/osu!/GameplayElements/Scoring/Score.cs#L350

The alternative way of fixing this would be just to change
`MatchesOnlineID` to reject zeroes, but I think this is a saner overall
direction.
@bdach bdach self-assigned this Jun 10, 2025
@bdach bdach added realm deals with local realm database area:import dealing with importing of data from stable or file formats labels Jun 10, 2025
@smoogipoo smoogipoo requested a review from peppy June 25, 2025 01:29
@peppy peppy merged commit 4b66584 into ppy:master Jun 25, 2025
9 checks passed
@bdach bdach deleted the stable-scores-with-zero-legacy-id branch June 25, 2025 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:import dealing with importing of data from stable or file formats realm deals with local realm database size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Results Screen sometimes excludes local scores and shows duplicates
3 participants