Make UndoRedo history independent for each scene

[KoBeWi]

Describe the project you are working on:
A game with lots of scenes to switch between.

Describe the problem or limitation you are having in your project:
Godot's UndoRedo is one of the most unfriendly things in the editor. You can rely on it only if you work on one scene, but since you can open multiple scenes at once, you obviously do that often, because it's convenient. And this is when bad things happen.

Changing scene gets registered in the undo history. This means that when you change a scene and then switch to another scene and change it and THEN you want to undo the first scene, you can't without undoing the latter scene. Worst case, when you make some action that breaks undo history and you can't undo other scenes even if they were completely unrelated to your action. This is absolutely horrible experience, if I didn't use VCS the editor would be 40% less usable for me.

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
Each scene should have it's own undo history. Unfortunately it's not as simple as that, because Godot has Resources which might not be tied to a specific scene and changes in the Resources are kept in the same undo history for everything. Still, this is a matter worth solving, so I have 2 possible solutions in mind. But first and foremost, Resources should also have their own undo stack, independent from all scenes. At least the file resources. Built-in ones are tied to scene, so they might share the same undo history.

Now, as to how to handle it:

• Option 1: make Resource inspector have its own UndoRedo. This means that when you press Ctrl + Z, it would only affect scenes. When you edit resource, but want to undo it, you have to press e.g. Alt + Z or something. This is a bit tricky with nodes and built-in resources, but should be possible to do. We could make it simpler by discarding the Resource change history when the editor resource is removed from the inspector (so you wouldn't be able to keep undo history for multiple resources; scenes are more important anyways).

• Option 2: make a global MasterUndoRedo with references to UndoRedos in each scene/resource and each UndoRedo action would have a timestamp. When you undo something, the MasterUndoRedo would look for another action in queue, but ignoring actions from other scenes. This way the resource history would be intertwined with scene history, which would allow to have one central undo history, at a cost of being a bit more confusing.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
Basically what I described above but in C++.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
That's definitely more than few lines.

Is there a reason why this should be core and not an add-on in the asset library?:
It's about making the editor usable.

(btw related godotengine/godot#6797)

[me2beats]

yes, I also think it would be helpful.
And even more logical imo because a scene should be self-sufficient and minimally dependent on global objects.

Also I wanted to make a couple of plugins related to undo/redo, for example undo history slider like in zbrush, but I missed some API methods, first of all this one: #1745 (#1746 would be useful too).
and yes, my plugin would not be very useful without the ability to do undo/redo for a specific scene.

[me2beats]

Questions:

• How the UndoRedo methods might change?
  https://docs.godotengine.org/ru/stable/classes/class_undoredo.html
  for example a scene argument could be added to some methods signatures

• Should UndoRedo still be one singleton, or should each scene have an UndoRedo object?

• Can there be undo not related to a specific scene?
  Could for example some editor plugin want to have Undo/Redo?

[KoBeWi]

How the UndoRedo methods might change?

They don't need to change, unless we go with the timestamp approach.

Should UndoRedo still be one singleton, or should each scene have an UndoRedo object?

Each opened scene has an associated SceneState object, so we could add an UndoRedo in there.

Can there be undo not related to a specific scene?
Could for example some editor plugin want to have Undo/Redo?

Well, this is already possible now. You can instance UndoRedo like any other class. It's more about handling UndoRedo context or something.

[elvisish]

Had some trouble recently as I like to have a tab for the player scene and a tab for the enemy scene, and if I undo and it changes, I lose undo/redo, since scene changes are part of the undo but not redo-able?

[ator-dev]

@KoBeWi (or anyone else interested) - I have made a proof-of-concept in which undo-redo actions are automatically assigned a context, meaning that any appropriate context - for example, editor settings, project settings, a resource, or a scene - can have its own stack. Pressing Ctrl+Z or Ctrl+Shift+Z/Ctrl+Y applies to the area which is currently focused, and I've given the inspector the ability to have focus. I'm currently finishing off some details of this before making a PR for it.

One problem I'd like feedback on first is the issue of how separate stacks are implemented. My approach is making the UndoRedo class automatically assign different stacks to different objects, but reading this proposal I now realise I could have used the approach of instantiating the same single-stack class multiple times.

Do you think I should throw away my work on making a multi-stack version of the class, or is it worth continuing to develop? I personally think it's a less expensive approach memory-wise and minimises larger API changes, but the contributor community may think differently.

[KoBeWi]

I think that a separate object per context would be better, because it would make them less dependent, but if you are close to finishing, you can submit your current approach and it can be discussed from there. Keep in mind that UndoRedo can be used in user plugins (even separately from editor's), so just make sure this use-case doesn't break.

[ator-dev]

it would make them less dependent

I'm not quite sure what you mean it's dependent on, but I agree that it makes the class more complex which could be seen as an issue - something has to get more complex either way though of course. You're right, I should just finish it and submit for discussion.

UndoRedo can be used in user plugins (even separately from editor's), so just make sure this use-case doesn't break.

Unfortunately some API changes have to be made, because now the history stack to be used must be specified by anything calling UndoRedo functions. An alternative approach would be to create an UndoRedoContext class which the functions can be called from (a different instance can be used per context by a script), but this requires even larger changes. This is a consequence of contextual undo, hopefully that's alright...

[KoBeWi]

I'm not quite sure what you mean it's dependent on

I meant coupled. With single UndoRedo for multiple contexts, they depend on that single object, while they don't need to (because they have separate stacks).

Unfortunately some API changes have to be made, because now the history stack to be used must be specified by anything calling UndoRedo functions.

API changes are fine, but creating a UndoRedo from script and using it for whatever should still be possible, without making it unnecessarily complex.

[ator-dev]

they depend on that single object

Thanks for clarifying - it's true all (main editor) stacks will depend on the same UndoRedo object. I don't personally see this as a problem because they can still easily be added to (happens automatically) and deleted from (could be managed using a memory limit) this same object, but of course you can decide looking at the implementation.