Fix `ManiaModInvert` permanently destroying the beatmap

ppy / osu

rhythm is just a *click* away!

https://osu.ppy.sh

MIT License

15.13k stars 2.25k forks source link

Fix `ManiaModInvert` permanently destroying the beatmap #28689

Closed smoogipoo closed 3 months ago

smoogipoo commented 3 months ago

Fixes https://github.com/ppy/osu/issues/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...).

peppy commented 3 months ago

@smoogipoo please take a look at 005af280f2ee8c05adb0bc0b5609b52eb40900fc. 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 commented 3 months ago

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

peppy commented 3 months ago

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 commented 3 months ago

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

smoogipoo commented 3 months ago

lgtm.