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!taiko slider velocity being written incorrectly to .osu file on export #25689

Merged
merged 11 commits into from Dec 7, 2023

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Dec 6, 2023

Apart from fixing the writing to the file, I've also made what I'd consider a huge code quality improvement:

  • The legacy constant has been moved into a local taiko context (and is no longer considered legacy.. which I think makes a lot more sense).
  • The weird subclassing of a realm type which somehow worked is now gone.

Does this need test coverage? I'd argue not as now there is no special logic for taiko decode/encode when SliderMultiplier is concerned. Testing it would just look weird.

@bdach
Copy link
Collaborator

bdach commented Dec 6, 2023

Does this need test coverage?

Judging by the amount of times we've had to touch this accursed thing and subsequently broke it in some accursed way? 100%. No matter how weird it looks.

@peppy
Copy link
Sponsor Member Author

peppy commented Dec 6, 2023

Hmm, it's basically going to be opening the editor, exporting to an .osz file and testing the file contents' SliderMultiplier matches the bindable... unless you have something else in mind?

@bdach
Copy link
Collaborator

bdach commented Dec 6, 2023

Sure. If that covers the incorrect encoding then that works.

@peppy
Copy link
Sponsor Member Author

peppy commented Dec 6, 2023

Tests added!

Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
@bdach
Copy link
Collaborator

bdach commented Dec 7, 2023

!diffcalc
RULESET=taiko

Copy link

github-actions bot commented Dec 7, 2023

@bdach bdach merged commit 07da9d9 into ppy:master Dec 7, 2023
17 checks passed
@peppy peppy deleted the taiko-multiplier-fix branch December 11, 2023 14:02
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.

by default, Taiko base SV is not 1.40
2 participants