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 taiko and mania using the incorrect slider length when converting beatmaps #25595

Merged
merged 4 commits into from
Nov 30, 2023

Conversation

smoogipoo
Copy link
Contributor

Fixes #20902

Basically, in osu!stable, the distance for these conversion functions is extracted so early that the rest of the slider code doesn't actually get run. It uses the value from the beatmap file directly. Compare that to lazer, which was using the post-processed distance.

I've only tested this with the given beatmap, but I will run a diffcalc on this PR and check additional cases (there should hopefully be some differences I can check).

Comment on lines 190 to 191
else
distance = distanceData.Distance;
Copy link
Member

Choose a reason for hiding this comment

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

Does this fallback case realistically get hit under some scenario?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I can tell, no. I only added it for sanity/safety.

@peppy peppy self-requested a review November 29, 2023 08:24
@peppy peppy added the blocked Don't merge this. label Nov 29, 2023
@smoogipoo
Copy link
Contributor Author

!diffcalc
RULESET=taiko
SCORE_PROCESSOR_A=ppy/osu-queue-score-statistics#174
SCORE_PROCESSOR_B=ppy/osu-queue-score-statistics#174

Copy link

github-actions bot commented Nov 29, 2023

@smoogipoo
Copy link
Contributor Author

smoogipoo commented Nov 29, 2023

Taiko spreadsheet above looks good to me. Checked https://osu.ppy.sh/beatmapsets/217667#osu/1151178 and https://osu.ppy.sh/beatmapsets/884669#taiko/1935058 and confirmed the new conversions match osu-stable.

Just going to run mania too for sanity's sake...

!diffcalc
RULESET=mania
SCORE_PROCESSOR_A=ppy/osu-queue-score-statistics#174
SCORE_PROCESSOR_B=ppy/osu-queue-score-statistics#174

Copy link

github-actions bot commented Nov 29, 2023

@smoogipoo
Copy link
Contributor Author

Mania also looks good. Tested https://osu.ppy.sh/beatmapsets/1749133#mania/3578111 and, although it is now impossible to play and looks weird, it matches osu!stable.

This PR's good to go from me now.

@smoogipoo smoogipoo removed the blocked Don't merge this. label Nov 30, 2023
@peppy peppy merged commit b2e1a63 into ppy:master Nov 30, 2023
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.

Taiko drum roll lengths don't always match stable
2 participants