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 ManiaModInvert permanently destroying the beatmap #28689

Merged
merged 9 commits into from
Jul 2, 2024

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Jul 1, 2024

Fixes #28687

Steps to reproduce:

  • Select a converted beatmap (osu! -> osu!mania).
  • Select the invert mod
  • Deselect the invert mod
  • Select the constant speed mod.

Expected: Star rating should be equal to that of NM.
Actual: Star rating differs.

@peppy We may want a hotfix release for this (though I believe it's been broken since the mod's been introduced...).

@smoogipoo smoogipoo added the next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! label Jul 1, 2024
@peppy peppy self-requested a review July 1, 2024 07:55
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jul 2, 2024
@peppy
Copy link
Member

peppy commented Jul 2, 2024

@smoogipoo please take a look at 005af28. In addition to your changes, I've moved the bindable list portion to EditorBeatmap because having a bindable list replaced post-construction doesn't fill be with a lot of confidence.

@smoogipoo
Copy link
Contributor Author

Any reason for not making it an IReadOnlyList, matching HitObjects?

@peppy
Copy link
Member

peppy commented Jul 2, 2024

Any reason for not making it an IReadOnlyList, matching HitObjects?

I originally did, but I believe I ran into an issue. Let me check..

@smoogipoo
Copy link
Contributor Author

Reverted the last two commits because they're too ugly to exist, requiring the beatmap class to be reworked in general.

@smoogipoo
Copy link
Contributor Author

lgtm.

@bdach bdach self-requested a review July 2, 2024 05:40
@bdach bdach merged commit 3bf3e91 into ppy:master Jul 2, 2024
14 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:mods next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! ruleset/osu!mania size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select osu!mania difficulty increasing mods lowering stars (osu!lazer)
3 participants