Undo/Redo symmetry easily breaks when making edits from inspector

[MajorMcDoom]

Tested versions

Reproducible in 4.1, 4.2, 4.3

System information

Godot v4.3.stable - Windows 10.0.22631 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1660 SUPER (NVIDIA; 32.0.15.6109) - Intel(R) Core(TM) i7-10700F CPU @ 2.90GHz (16 Threads)

Issue description

When making edits, even with only one scene open (without conflicting global edits arising from having multiple scenes open), it is easy to break the symmetry of undo/redo if you make edits touch any global resources (e.g. editing a material from a material slot in the inspector). This leads to very frustrating situations where changes you thought you already wiped out (with undos and new operations) make a reappearance and don't go away. You have to either manually neutralize the unwanted changes, or go back all the way before the unwanted changes were made and recreate your desired changes. That is the best case. In the worst case, these re-emerging changes are undetected and saved, causing mysterious bugs.

Case 1:
When undoing a change scene, it can skip a global change (does not undo it), but not skip it when redoing, leading to an asymmetry. The same thing can happen if you undo a global change that is preceded by a scene change, in which case the scene change can get skipped.

godot_undo.mp4

Case 2:
When performing operations that "reset" the history stack, it only resets either the scene stack, or the global stack depending on what scope the operation is in. Because the other stack is left alone, the history cursor skips ahead of any entries in the other stack, leaving "remnants" of edits that were performed before an intended history reset.

godot_undo_redo_2.mp4

Expected Behaviour
The expected behaviour is that when undo/redo are used when a scene is open, it steps back/forward by exactly one entry in the history panel, in a manner that:

• Accurately retraces the user's operation history
• Is symmetrical in either direction of history traversal
  It is worth noting that this expected behaviour is what happens when navigating the history manually via clicking in the history panel. The issues described only occur when performing undo/redo actions.

Relevant Links
Proposal for separate undo histories
Pull request for separate undo histories

Steps to reproduce

Make sure to have the history panel open for clearer feedback during the repro.

Repro for Case 1:

• Open test.tscn in the attached project
• Select the sphere
• Edit its albedo color from the inspector
• Undo
• Move the sphere
• Undo
• Redo
• Observe that the redo operation alters the color of the sphere, instead of moving the sphere

Repro for Case 2:

• Open test.tscn in the attached project
• Select the sphere
• Move the sphere
• Edit its albedo color from the inspector
• Undo
• Move the sphere again
• Observe that the second move operation skips past the color edit, leaving it in the history stack

Minimal reproduction project (MRP)

bug-report.zip

[KoBeWi]

It's not possible to store external (global) resource changes in a scene.

Consider this:

1. User edits a material
2. In SceneA, they add a Gradient to the material
3. In SceneB, they edit the gradient
4. They go to SceneA, undo and close the scene. The gradient no longer exists at this point
5. In SceneB you can still undo/redo modifications on invalid object?

Closing a scene effectively removes part of the history, making it lose continuity.

Note that Resource changes are properly tracked as part of a scene if the resource is built-in, which makes it belong to the scene. The user is not able to edit resource that belongs to another scene, so the history continuity is preserved.

Both cases you listed work as "expected", it's more a problem of visualization. In first example, when you undo the second Translate, the History skips albedo_color, because it's already undone. The dock just fails to show that. When you redo and then undo again, the albedo_color change is no longer skipped. Undo/redo works based on timestamp of the actions and you can't undo something that you did undo already, hence the discrepancy.
Your second video shows the same thing, but differently.

My fix would be to either:

• Improve visualization of both histories. If you undo action, it should be clear from the graph that the action is undone.
• Make actions appear before the undone global action. If Translate were before albedo change, it would be more intuitive. This won't work with timestamps though, so it requires more changes.

[MajorMcDoom]

It's not possible to store external (global) resource changes in a scene.

Consider this:

1. User edits a material

2. In SceneA, they add a Gradient to the material

3. In SceneB, they edit the gradient

4. They go to SceneA, undo and close the scene. The gradient no longer exists at this point

5. In SceneB you can still undo/redo modifications on invalid object?

If you've undone the creation of a resource from the scene in which you created it, then it could be argued that the operations on it from any other scene would simply be moot (not have any effect and be skipped). But I see how this could be frustrating for users in a different way.

Note that Resource changes are properly tracked as part of a scene if the resource is built-in, which makes it belong to the scene. The user is not able to edit resource that belongs to another scene, so the history continuity is preserved.

I am aware of this, yes.

Both cases you listed work as "expected", it's more a problem of visualization. In first example, when you undo the second Translate, the History skips albedo_color, because it's already undone. The dock just fails to show that. When you redo and then undo again, the albedo_color change is no longer skipped. Undo/redo works based on timestamp of the actions and you can't undo something that you did undo already, hence the discrepancy. Your second video shows the same thing, but differently.

My fix would be to either:

* Improve visualization of both histories. If you undo action, it should be clear from the graph that the action is undone.

* Make actions appear before the undone global action. If Translate were before albedo change, it would be more intuitive. This won't work with timestamps though, so it requires more changes.

Regardless of what the solution is, there is no way that in a session involving only one open scene and two changes that asymmetrical undo/redo could be "expected". Your explanation might make perfect sense from a systemic view, but UX-wise, it's contrived and difficult to understand/justify. I don't think a change in visualization would cut it either, because the history panel isn't even available by default.

The case I am trying to make here is that by the time I perform the first translation, the color change operation should be completely removed from the history. Since, as you say, it is already undone. If I undo an action, and perform a new action, the undone action should never come back to life. It's extremely frustrating when I make a bunch of critical changes, undo/redo just to visualize what I've done, and by doing this simple thing that should be safe, Godot decides to pick up unwanted remnants from the past. Now I am stuck with either having to manually neutralize the unwanted past change while keeping my new changes, or go back all the way and manually recreate my wanted changes again from scratch. And the worst part is that doing this does not prevent it from happening again, because it will just do it again the next time I undo/redo.

[KoBeWi]

And what happens when you have no scene open? You can still edit resources from FileSystem. I assume the changes will be still part of "global history", but once you open a scene, it becomes unavailable. Should it be cleared then?

[MajorMcDoom]

And what happens when you have no scene open? You can still edit resources from FileSystem. I assume the changes will be still part of "global history", but once you open a scene, it becomes unavailable. Should it be cleared then?

Maybe it's transfered, or maybe it's cleared, or maybe it stays where it is because it's all before the scene was opened anyway. I don't know. If my initially proposed solution falls short, it can be expanded on or ignored. I will remove it because it is clearly becoming distracting.

All that being said, when the engine moves an object for an undo then changes color for an immediate subsequent redo, that's a major UX bug. The discussion shouldn't be about whether to fix it or how to show it's actually fine. It should be trying to figure out a better solution.

I want to stress that this isn't always a trivial thing like a color or position change. With a couple undos and redos, you can silently introduce a major bug into your game if you for example made a change to a custom resource with critical values.

[RobProductions]

I have to agree that it feels really awkward as a user for the undo system to be asymmetric like this even if it stems from a technical limitation. The fact that you can make changes from your scene even if you're technically modifying a global resource can create a lot confusion while the undo system still works this way. I would say just having one global undo list is easier to work with than this but I understand people might miss history being tracked per scene.

In absence of storing the resource change in the scene history, I have another approach... if we reference the first video in the post, if a "scene action" comes along and is added to the history list I believe it should wipe out any global actions above our current position in the history. In other words, since the albedo change was undone and new actions happen on top of it, the albedo change is a point in history that we don't care about anymore. Because really in what use case would a user want to hit redo and see a change to a global resource that they made 10 steps back after making changes in their scene? I understand wanting to track both sources, but if new history comes in for the scene, it's safe to assume that we can throw out whatever was above because they opted into creating a "branching point" in the timeline.

I don't think this is a visualization issue either because just repeating the steps @MajorMcDoom showed in the video makes it seem like an unexpected change came out of nowhere. Even if you had perfect clarity on this and knew how it worked, how would a user get to a state where they've restored their Translate action without going through the unwanted albedo change? If they only have access to the redo button, there is no way to simply redo just the Translate despite that being the last action they took.

[bikemurt]

I would suggest Godot take some hints from Blender here, their undo system has some documentation available: https://developer.blender.org/docs/features/core/undo/

Certainly they have similar situations arise where external resources are edited (e.g. texture painting, modifying an image), while other operations are happening to objects within Blender - AFAIK it is one global undo system, where some steps are differential changes and some are absolute "snapshots" of an object's data.

[KoBeWi]

I believe it should wipe out any global actions above our current position in the history.

That sounds like plausible solution. I opened a fix:
#97410
See if the new behavior is more expected.

[MajorMcDoom]

I believe it should wipe out any global actions above our current position in the history.

That sounds like plausible solution. I opened a fix: #97410 See if the new behavior is more expected.

Tested, and it seems to work great, thank you!