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

Add new combo and combo skip to HitObject #113

Merged
merged 16 commits into from
Sep 5, 2023
Merged

Conversation

OliBomby
Copy link
Contributor

@OliBomby OliBomby commented Sep 2, 2023

Seemed like this was missing. I need new combos for my project.

Spinners dont parse the combo skip value because this mimics osu! stable behaviour.
Hold notes also dont parse the combo information.

Closes #112

@tybug
Copy link
Collaborator

tybug commented Sep 3, 2023

I've made some changes here. Before, this pull included breaking changes: from slider import Circle; c = Circle(...) is part of the public api and any new parameters should have a default value (unless we want to bump the major version, which I don't particularly want to right now).

I've also gone ahead and parsed the combo skip for every hitobject. Even if combo skip values are unused for certain hitobjs by osu stable, as long as they are present in the type information of the beatmap I think it is more correct for us to parse them according to spec and provide them. A consumer can then decide how to use the combo skip number in the context of different hitobjs.

I haven't tested this besides making sure code tests pass, so probably worth a double check to ensure I didn't break anything.

slider/beatmap.py Outdated Show resolved Hide resolved
slider/beatmap.py Outdated Show resolved Hide resolved
slider/beatmap.py Outdated Show resolved Hide resolved
@tybug
Copy link
Collaborator

tybug commented Sep 5, 2023

thanks for the HoldNote fix 👍

this looks good to me

@tybug tybug merged commit 1835627 into llllllllll:master Sep 5, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New Combos not parsed
3 participants