ppy / osu

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

Fix break generation not accounting for concurrent hitobjects correctly #28630

Closed peppy closed 6 days ago

peppy commented 6 days ago

Closes https://github.com/ppy/osu/issues/28622.

frenzibyte commented 6 days ago

This part should also be changed to get the maximum end time (i.e. Beatmap.GetLastObjectTime()) instead of the last object's end time, right?

CleanShot 2024-06-27 at 08 06 56

I cannot easily imagine a test case that warrants that to be applied though (I don't understand the logic quite well).

bdach commented 6 days ago

I don't understand the logic quite well

The check is supposed to remove manually-adjusted breaks which are placed after all objects so that they don't hang around.

This part should also be changed to get the maximum end time (i.e. Beatmap.GetLastObjectTime()) instead of the last object's end time, right?

In theory yes, in practice it probably doesn't matter because of two circumstances:

Due to the above I'm pretty sure it's not possible to manufacture a test case that exercises this. I'll add one for posterity at least but yeah.