Skip to content
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

Fix osu!mania scores failing to convert to new standardised score due to cast failure #24219

Merged
merged 4 commits into from
Jul 16, 2023

Conversation

peppy
Copy link
Member

@peppy peppy commented Jul 14, 2023

Regressed in #23917.
Closes #24217.

Haven't tested yet as I don't have a database ready to do so.

Of note, the reporting issue mentions this happened during a gameplay flow, which should definitely not be possible. If anyone has any ideas how this can happen, I'm all ears. We should definitely get to the bottom of that before pushing out the release.

@bdach
Copy link
Collaborator

bdach commented Jul 15, 2023

I've (re-)tested the following:

  • The pathway that led newly-set scores to undergo conversion to new standardised should no longer be reachable (most easily tested by creating a single beatmap with one object on it and then setting a perfect score on it - on master it'll fail to import with the trace given in issue, on this branch it will not)
  • Scores without score V2 mod set on stable are converted to new standardised
  • Scores with score V2 mod set on stable are not converted to new standardised
  • Scores from old lazer (pre-2023) are converted to new standardised
  • Scores from slightly newer lazer (from 2023, which numerically match old standardised) are converted to new standardised
  • Scores from new lazer (after merge of Bump replay version in encoder after Score V2 changes #23848) are not converted to new standardised

Everything but the first point was tested using the following replays: score-test-cases.zip. Maybe I can adapt that to a test case or something...

Looks good to me, but given I've bungled this pretty bad overall, I'm probably not gonna click the button and see if maybe anyone wants to have a go reviewing... Or you can merge at your leisure if you think the above is sufficient testing I guess.

@peppy peppy merged commit cf70f5e into ppy:master Jul 16, 2023
@peppy peppy deleted the fix-mania-everything branch July 17, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mania score cannot be import
2 participants