search

ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.13k stars 2.25k forks source link

Fix crash on calculating playlist duration when rate-changing mods are present #28572

Closed bdach closed 3 months ago

bdach commented 3 months ago

Regressed in https://github.com/ppy/osu/pull/28399.

To reproduce, enter a playlist that has an item with a rate-changing mod (rather than create it yourself).

This is happening because APIRuleset has CreateInstance() unimplemented:

https://github.com/ppy/osu/blob/b4cefe0cc2fda0ab4b5af6138ee158bd32262f9a/osu.Game/Online/API/Requests/Responses/APIBeatmap.cs#L159

and only triggers when the playlist items in question originate from web.

This is why it is bad to have interface implementations throw outside of maybe mock implementations for tests. CreateInstance() is a scourge elsewhere in general, we need way less of it in the codebase (because while convenient, it's also problematic to implement in online contexts, and also expensive because reflection).

ppy-sentryintegration[bot] commented 3 months ago

Sentry issue: OSU-1CD8