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 for stopping the Undo History being desynchronized from actual Undo queue #84557

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

TheSofox
Copy link
Contributor

@TheSofox TheSofox commented Nov 7, 2023

Fix for Issue #83945

The Undo History was getting desynched from the actual Undo queue. The reason is that the editor_undo_redo_manager.cpp (which handles History) and undo_redo.cpp (which handles actual Undo functionality) did their own parallel checks for whether the latest action should be merged with the previous one.

My fix eliminates the checks in editor_undo_redo_manager.cpp and simply queries undo_redo as to whether it merged the latest action. If so, it doesn't create a new action in the History list.

This ensures the two don't get desynched from different ideas about merging, and also future proofs it. Since all the rules for merging are in undo_redo.cpp, they can be changed there without having to also change the rules in editor_undo_redo_manager.cpp (DRY principal)

Production edit: Fixes #83945

@TheSofox TheSofox requested a review from a team as a code owner November 7, 2023 00:25
@YuriSizov YuriSizov added this to the 4.x milestone Nov 7, 2023
@TheSofox TheSofox force-pushed the undo-history-sync-fix branch 2 times, most recently from 513dcb9 to 34a28fe Compare November 7, 2023 12:10
@TheSofox TheSofox requested a review from a team as a code owner November 7, 2023 12:10
@TheSofox TheSofox force-pushed the undo-history-sync-fix branch 2 times, most recently from 7f05bb2 to 0fe4b8b Compare November 7, 2023 19:27
core/object/undo_redo.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga changed the title Fix for stopping the Undo History being desynchronised from actual Undo queue. Fix Issue #83945 Fix for stopping the Undo History being desynchronised from actual Undo queue Nov 10, 2023
@TheSofox TheSofox force-pushed the undo-history-sync-fix branch 2 times, most recently from 6d98b47 to d40a3e8 Compare November 10, 2023 09:43
@TheSofox TheSofox force-pushed the undo-history-sync-fix branch from d40a3e8 to 662522a Compare November 10, 2023 13:04
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

I can't believe it was always this simple to handle 🤦‍♂️

@akien-mga akien-mga modified the milestones: 4.x, 4.2 Nov 10, 2023
@akien-mga akien-mga merged commit 96fa86f into godotengine:master Nov 10, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@TheSofox TheSofox deleted the undo-history-sync-fix branch November 10, 2023 21:15
@akien-mga akien-mga changed the title Fix for stopping the Undo History being desynchronised from actual Undo queue Fix for stopping the Undo History being desynchronized from actual Undo queue Nov 29, 2023
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.

Using the Color Picker screws up/desynchronises Undo History
5 participants