thebetioplane / osuReplayEditorV3

A replay editor for the rhythm game osu!. Allows you to modify cursor and metadata.
27 stars 5 forks source link

Loading sky_delta - Grenade causes memory leak #45

Closed AG89 closed 1 year ago

AG89 commented 2 years ago

I know 2B maps aren't supported but I thought this might be something to report either way. Map: https://osu.ppy.sh/beatmapsets/478093#osu/1029976 Replay (any would work): skydelta-_Grenade_Negative_Infinity_Fragmentation_2022-03-01_Osu-1.zip

thebetioplane commented 1 year ago

Thanks for reporting this, I have taken a look at this and fixed it. Sorry it took so long!

BTW in the past I said the replay editor didn't support 2B maps because the editor didn't have the capability to render two sliders or spinners that overlapped in time, but I added support for that in 2022 (issue #35).

Explanation of the bug if you are interested

That beatmap has a several very mischievous sliders in it at 3m 02s.

Here is one definition in the .osu file:

x,y,time,type,hitSound,curveType|curvePoints,slides,length,edgeSounds,edgeSets,hitSample
88,346,182111,2,0,L|88:-2147483648,2,3.08502948675946E+300,12|0,0:0|0:0,0:0:0:0:

This is a linear slider with two control points, one at (88, 346) and the second at (88, -INT_MIN) and length set a value smaller than a 32 bit float can hold, so it goes to -infinity (probably like the beatmap difficulty name??)

Replay editor tries to evaluate every point on it so it is basically an infinite loop and runs out of memory. When I opened this beatmap in the osu! beatmap editor it causes 100% cpu usage on one core when you navigate to it, so imo I'm not sure of the best way to handle it.

What I've gone ahead and done is set the limit for the maximum length a slider can be to 1 million to avoid the crash. No well-behaved slider should be above that -- the stage dimensions are 640x480 -- and if it were then it will get truncated. Even with imposing a limit, it still lagged severely with such a large slider. To combat that I've culled all slider body parts that outside of the playfield by a factor of 3 or more. They won't render even if the user pans to them.

As a bonus after fixing that, I realized this map also has an invalid spinner, where the start time (1033 ms) is past the end time (630 ms) so it appears already in the fade out time in osu!.

x,y,time,type,hitSound,endTime,hitSample
256,192,1033,12,0,630,0:0:0:0:

In the replay editor it fails to construct the hitobject interval tree as the end time being before the start time is an invariant that can't be broken. To correct this, I've mimicked the osu! behavior by treating all spinners with invalid end time as being 1 ms long.

thebetioplane commented 1 year ago

Screenshot of the problematic slider, even osu! has trouble displaying it

image

AG89 commented 1 year ago

You actually updated this you mad lad. 👍