Implement objective-undo for Animation editing #56770
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ator-dev
changed the title
ator-dev
force-pushed
the
undo-redo-cumulative-animation
branch
2 times, most recently
from
d457392 to
af6b8a9
Compare
ator-dev
force-pushed
the
undo-redo-cumulative-animation
branch
from
af6b8a9 to
f055161
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tests out my UndoRedo abstraction layer ('objective' cumulative UndoRedo) from #56621 and godotengine/godot-proposals#3714; I'm PR'ing this to make it easier for contributors to see how my proposed changes work in practice.
Several open issues and other problems I've noticed are fixed 'automatically', yet in most cases the code has been considerably simplified; as is the main intention of my proposal, contributors would no longer have to mess around with fiddly undo-related functions that have to be called in exactly the 'right' order all the time.
For example, everything visually refreshes at the correct time such as the track playback/easing settings; without adding refresh functions to the undo stack, contributors would be encouraged to integrate this into the main subroutines where necessary, meaning they'll always work correctly instead of in specific situations. I'm aware this may not always be desired though, and there are other good ways of implementing it if needed.
By far the best improvement though is the reliability. Using the old UndoRedo system, keys moved and manipulated would pretty often end up in a different state after being undone/redone, with duplicated/missing keys and even crashes being very easy to come by. Yes this could have been fixed by 'just' changing the subroutines added (or their order) on the undo stack, but some edge cases are so complex I wouldn't even know where to start; and anyway, editing would likely only work for a few more months before being broken by an arbitrary modification. Using this system, all state is guaranteed to end up where it started, and edge cases are practically eliminated.
Bonus: I implemented undo/redo for two very complex operations, Cleanup and Optimize Anim, simply by adding 2 lines to each (
create/commit_action_cumulative) (;EDIT: CI is currently failing because I naively included a header file where it shouldn't have been included. This will be replaced by a technique appropriate to Godot once I have the time, hopefully in a few days or less.