-
Notifications
You must be signed in to change notification settings - Fork 16
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
enables Beatmaps to be written back to .osu files #101
Conversation
However, during the test of the generated |
Before I take a look at this, can you fix the inspection errors? (you might want to lint locally with flake8 instead of pushing to test; from experience, fixing one inspection often causes another to pop up). Github made me manually approve the workflows to run on this pr since you're a first time contributor, which is why it didn't run when you first opened the PR. |
Inspection errors are all
|
This comment was marked as resolved.
This comment was marked as resolved.
Now I learn the problem can be solved by setting the |
…it objects lines in generated .osu files will be the same as in original .osu files
I don't think |
I had written |
please don't force push if you can absolutely avoid it. It messes up the history of anyone who has checked out this pr (ie, me). |
generally I prefer accessing known apis (ie |
I've cleaned up the testing code, slightly opinionated-ly. The assert (tp1.parent is not None) == (tp2.parent is not None) instead of if timing_point1.parent is None:
assert timing_point2.parent is None which only catches the case of "tp1 parent none and tp2 parent not none", and not "tp1 parent not none and tp2 parent none". Let me know if you hugely disagree with any of my changes. I'll do another pass on this pr soon, but the testing code looks good to me now. |
Good insight. |
A few things:
The largest remaining thing to do is combo colors, which as you've noted above we currently discard. I think a good path forward would be changing how we store the I think this PR is good to go now. I'll wait to make sure you don't have any problems with my recent changes, and then merge. |
I think it may be better to replace the nested function |
Custom separators can now be used, by passing |
still feels like an overengineered solution (re: separators), but I'm not going to fuss too much. That may also just be my bias against this functional style of programming showing. Let's get this in as-is. Thanks for the pr! |
pack
function is added forHitObject
andCurve
as well. Written.osu
has been tested and can be recognized and played by osu! client.Colours
section in.osu
file is omitted since slider does not parse this section. Support for parsing colours may be added in the future, so the generated.osu
can include this section. Default values are chosen to be the same as those used by the Beatmap editor which is embeded in osu! client.