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 osu! and base HP processor break time simulation #25420

Merged
merged 4 commits into from Nov 24, 2023

Conversation

smoogipoo
Copy link
Contributor

@smoogipoo smoogipoo commented Nov 13, 2023

Prereqs:

The osu! implementation (from the PR above) carried forward a bug from osu! stable that made the simulated break time calculation unable to deal with overlapping breaks. One map where this occurs is https://osu.ppy.sh/beatmapsets/201#osu/614 . I'm unsure of any other maps with the same issue.

The base implementation also exhibited a bug, but in this case it is relating to the length of break time. In osu!stable (v8+) and osu!lazer, drain time stops for the full duration between two hitobjects that are separated by a break. The simulation instead assumed that drain starts again once the break ends, which is not the case.
Plus, the existing implementation was hard to follow with the way it used indices.

Please note that this makes drain rate no longer match osu!stable in these particular cases.

@smoogipoo smoogipoo changed the title Fix osu! and base HP processor break time implementation Fix osu! and base HP processor break time simulation Nov 13, 2023
@bdach
Copy link
Collaborator

bdach commented Nov 23, 2023

@smoogipoo do you mind double-checking my merge conflict resoluton on this? Also this is still valid slash good to go, right?

@smoogipoo
Copy link
Contributor Author

Looks fine, and yes this is still good to go.

@peppy peppy merged commit 2f3c051 into ppy:master Nov 24, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants