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 changing slider path segment type not being undoable #26423

Merged
merged 2 commits into from
May 24, 2024

Conversation

B3nn1
Copy link
Contributor

@B3nn1 B3nn1 commented Jan 7, 2024

Fixes #26328.

Unfortunately, this creates a new issue where selecting a HitObject that is the first in a new combo can create another, seemingly redundant saveState (I believe due to TernaryState changes from the MenuItem.)
This is possibly related to #10314.
Hence why I decided to post this as a draft for now.

@OliBomby
Copy link
Contributor

I can't recreate the issue you described. I debugged it with a breakpoint in the action and it never triggered when i selected a bunch of hitobjects. From looking at the code it also seems that only clicking the menu item should be able to trigger the action.

@B3nn1
Copy link
Contributor Author

B3nn1 commented Jan 24, 2024

I can't recreate the issue you described.

Unfortunately, I can consistently recreate it with a setup like shown in the video. I tried to but couldn't figure out why LegacyBeatmapEncoder generates a different encode output after just selecting something else and not changing anything in the map itself.

2024-01-24.11-27-30_edit.mp4

@OliBomby
Copy link
Contributor

This time I was able to recreate the issue and I looked further into it. The issue seems to be unrelated to your changes and instead related to #11901. Your issue setup also works on the current master branch.

So I think your changes are good to go.

@B3nn1 B3nn1 marked this pull request as ready for review January 24, 2024 13:16
@bdach bdach changed the title Make changeHandler save changes to the PathType of PathControlPointPieces Fix changing slider path segment type not being undoable May 24, 2024
@bdach bdach merged commit 742108f into ppy:master May 24, 2024
13 of 17 checks passed
@B3nn1 B3nn1 deleted the undo_sliderpoint_curvetype_change branch May 24, 2024 09:46
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.

Changing the type of a node does not count towards undo history
4 participants