Skip to content

Conversation

@chris-ehmann
Copy link
Contributor

@chris-ehmann chris-ehmann commented May 29, 2025

Partially addresses #32543, specifically "Ctrl + Z doesn't bring the deleted object back" (since I believe they're just accidentally middle-clicking while scrolling)

When you quick-delete an object in the editor while it's being dragged (either through a middle mouse button click or shift + right click), it causes the entire undo/redo system to break. I believe that this is caused by changeHandler never being able to call EndChange, as isDraggingBlueprint is erroneously set to false by the middle (or right) mouse button click.

We can fix this with a simple check to see if the MouseUpEvent was specifically caused by the left mouse button.

quick edit: im realizing that this isn't strictly related to objects being deleted, and undo/redo will be locked up when any sort of non left-click mouseup event occurs while an object is being dragged, so i'm renaming to be a bit more appropriate.

Video

Before

before.mp4

After

after2.mp4

@chris-ehmann chris-ehmann changed the title Fix inability to undo/redo when object is deleted while being dragged Fix undo/redo being locked up when a MouseUp event occurs while an object is being dragged May 30, 2025
Copy link
Collaborator

@bdach bdach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't managed to find any breakage here but this 200% needs to be covered by tests because editor input handling code is horribly complex and I'm not going to allow guards with non-obvious purpose without documenting what they're for, in the form of inline comments and/or test coverage.

Test should live in osu.Game.Tests.Visual.Editing.TestSceneComposerSelection.

@pull-request-size pull-request-size bot added size/M and removed size/XS labels Jun 6, 2025
@chris-ehmann
Copy link
Contributor Author

I've added a comment explaining the purpose of the guard clause and test coverage covering the process of dragging an object -> delete via middle click -> undo. I assume this should be good since other processes of testing right click instead or other mouse clicks instead would essentially be repeating the same test, unless I'm completely overlooking something.

bdach added 2 commits June 9, 2025 09:16
- Use constraints for better assert messages
- Use `Editor.Undo()` rather than manual input manager synthesizing
  ctrl-z (ctrl-z is not a platform agnostic binding, see macOS)
@pull-request-size pull-request-size bot added size/S and removed size/M labels Jun 9, 2025
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.

3 participants