Change the fundamental Undo/Redo system for automatic reliability

[ator-dev]

Describe the project you are working on

The Godot editor

Describe the problem or limitation you are having in your project

Responsibility for Undo/Redo currently falls on contributors building changes and new actions for the Godot editor, using a system that logs arbitrary function calls to the stack. This works well enough for redo actions since they're simply the exact calls that are being made to implement the functionality, but for undo actions the calls are often completely different - at the very least different arguments are passed in, but additionally the functions may be called in a different order or be entirely separate. Undo is not linked to redo, and this makes it very difficult to ensure that the same changes are actually being made; the system isn't mathematically consistent. The smallest change to any of these functions or actions can render their handling in the undo action system incorrect.

#2109 already proposes Undo/Redo changes which I support and have been looking into implementing, but in my opinion adding this extra layer of complexity is not viable at the moment since it will just make the system even harder to maintain and less consistent, potentially impacting the development of 4.0. Without a more stable and usable base system it may not be possible to create separate scene/resource undo stacks without adding a lot of problems, especially since it's just another thing for contributors to maintain when they add or change actions.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I think it shouldn't fall on contributors, who may know little about the systems being used or can simply make mistakes anyway, to maintain undo/redo congruency. Even if it is fastidiously updated and issues are addressed constantly, the problems can never be fully fixed; this isn't what 4.0 needs with it already being unstable.
I'm proposing that a more objective system be tried out to make undo/redo work properly automatically, without developers having to carefully put together the function calls that best replicate each other in reverse.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

This has now been implemented in godotengine/godot#56621

• Every pair in action_cumulative_cache contains all data needed to restore an object to its state before an action; the object and its previous members. Each pair corresponds to the action_level for a cumulative action.

When create_action_cumulative is called:

• An additional argument is passed into p_object as the object about to be mutated.
• The large if-statement block relating to merges works the same as in create_action.
• A map of property names to current property values is created and added to action_cumulative_cache.

When commit_action_cumulative is called:

• The first chunk of lines only declares variables for making the following lines more readable and efficient.
• For each property in the current state of the object, property_list_new (i.e. the properties and values after being changed by the action):
  • If the new value is different from the old value, adds a do_property for setting the property to the new value and an undo_property for reverting it to the old value.
  • Erases this property from the property_list_old, as all necessary data about it has been added to the undo stack (or discarded) and is no longer needed.
• For each property remaining in property_list_old (i.e. the properties that no longer exist in the new object state):
  • Add a do_property for setting the property to an empty string (making the object's setter remove the property) and an undo_property for initialising the property with the old value.
• Same as commit_action from this point except for some cleanup. Also, if the current do_ops size is 0, the action is discarded because no properties were changed (fixes multiple annoyances with useless actions in Animation-editing and other places)

If this enhancement will not be used often, can it be worked around with a few lines of script?

No - if merged, this should be used by many core editor features.

Is there a reason why this should be core and not an add-on in the asset library?

This is editor core.

[ator-dev]

Other considerations include editing the scene tree, and the live-editing functionality. I think these can be worked into the system since the tree could be seen as a special-case separate object (as well as things like the project settings and filesystem), and instead of remote-calling methods on live objects properties could be changed directly. Overall the system would be a lot more modular and make separate history stacks much simpler to implement.

[fire]

Can you revise the section on how your proposal will work to describe the one implemented in the pr?

[reduz]

My feelings about this is that, in my experience, most of the time when dealing with undo and redo, you are not really dealing with properties or something that simply changes state. In most cases you are adding or removing things, so its not really possible to do different to the way it is now.

Even in many cases where you modify properties, its not as simple as replacing new value by existing value, as the existing value has to be cached at the beginning of editing (as in when you drag something and move it).

So, even though I understand this can be useful in some cases, it sounds like it would be a minimal amount of them, for which I am not entirely sure it's worth doing this change.

[ator-dev]

Should I close the proposal since groud (in godotengine/godot#56621 (comment)) rejected this approach to an undo-redo rework? I still think the system could do with reliability improvement in certain areas, but I assume someone with a new idea for that could reopen this proposal or make their own.