Undo/Redo and tool scripts

jonSS thanks ( you knew how to do it after all )

Why would you think otherwise? Your question (most of them in fact) was really unclear because you tend to jump to conclusions and operate under strange (often wrong) presuppositions. So I had to decipher through your noise what's really bothering you. Please work on clarity of your communication. Also try to take one problem at a time. You cram all this half-broken stuff into your project and then get confused when multiple things at the same time won't work as you think they should. It's a mess that requires extra effort to untangle. Be aware of this when asking questions. Always try to fully isolate an issue, best into a fresh unencumbered project. It facilitates clarity and understanding. Help us help you

add_undo_method() and add_do_method() as well as add_undo_property() and add_do_property() don't actually do anything noticeable when called. They just register callbacks or values respectively. So the order in which you call them makes no difference. The only important thing is that they get called between create_action() and commit_action() which form a begin/end block for the rest of the calls that define action's behavior.

Only when you finally call commit_action(), the action gets inserted into scene's undo queue. At this point the method(s) registered via add_do_method() will be called and the value(s) specified by add_do_property() will be assigned. From that moment on, do/undo values and do/undo methods you supplied to the action will be set and/or called whenever it's your action's turn to be undone or redone.

This has nothing to do with undo. The acquirement of the EditorUndoRedoManager in my example is a bit hacky (as I pointed out above) just to keep the example self contained, or should I say it; minimal The code is NOT a generalized boilerplate to copypaste into your projects.

In a real project you'd obtain the manager from the actual plugin node, so no abstract classes would need to be instantiated.

jonSS As a quick hack you can pluck the manager from one of the built in plugins currently present in the editor tree. This too shouldn't be used in a real project, only when playing with the stuff to learn how it works. This now shouldn't confuse the runtime compiler:

if Engine.is_editor_hint():
	undoredo = get_tree().get_root().find_children("*", "EditorPlugin", true, false)[0].get_undo_redo()

jonSS Yes, the most convenient way is to inject the manager reference into the edited object in EditorPlugin::_handles(). Or declare a static reference in the edited object class and assign it once from EditorPlugin::_enter_tree()

Alternatively you can name the plugin node and then each edited node can get the manager from it using something similar to the "hack" I posted above.

jonSS Editor undo queue is scene specific. Each edited scene has its own queue. That's why undo/redo commands are in the Scene menu . Read the docs for EditorUndoRedoManager class.

If you want a node specific or otherwise independent system, use UndoRedo object. However this will not be tied with the editor undo queue and you'll have to add your own gui for issuing undo commands. This is likely not something you actually need here.

Btw it's re-do, not re-ndo.

jonSS Again, it has nothing to do with undo system per se. This thread is meant as a tutorial on how it actually works.
In your case your whole data architecture is likely not properly done. That's why you have problems conforming the whole thing to engine's undo system. So if you want/need to use that system, reorganize your plugin to play nice with it.

From what I've seen so far in your code, you're not fully understanding how arrays/dictionaries work. Best to first entrench your knowledge about that in a simpler project. This is fundamental, so cursory understanding is not enough if you wish to make something more ambitious like a plugin.

Btw just posting random excerpts of code form your project is again not a very good communication of the problem. For umpteenth time, isolate the problem into a fresh project, make a new thread about it.

jonSS The thing with undo/redo for a complex object/action is that you need to have a function that does the operation and a function that undoes the operation, both changing the state of the object. How exactly to do this is closely knit with your data architecture. You typically need to store only necessary parameters to (un)perform the action. That's precisely what Godot's UndoRedo class helps you with. This is how undo is implemented in almost every app, it's not a Godot specific thing.

To conclude, here's a bit more elaborate example that implements undo in a plugin that draws grided circles on a canvas item node. It's somewhat similar to your situation @jonSS only simpler, for clarity. the action is implemented in a proper way so the undo works between any number of drawable nodes in the scene. I won't go into any explanations as I think the code is self explanatory enough. If not, just study it harder

@tool
class_name Drawable extends CanvasItem

@export var color = Color.YELLOW
var draw_list = {}

func _draw():
	for pos in draw_list.keys():
		draw_circle(pos, 32, color, true)
		
func _get_property_list():
	return [{"name": "draw_list", "type" : TYPE_DICTIONARY, "usage": PROPERTY_USAGE_STORAGE}] 

@tool
extends EditorPlugin

var drawable = null # current selected node we draw on
var stroke_draw_list = {} # current stroke draw list, used for undo

func _handles(object):
	drawable = object if object is Drawable else null
	return object is Drawable

func _forward_canvas_gui_input(e):
	if not drawable or e is InputEventKey: 
		return false
	if e is InputEventMouseMotion and e.button_mask == MOUSE_BUTTON_MASK_LEFT:
		add_to_current_stroke()
	if e is InputEventMouseButton and e.button_index == MOUSE_BUTTON_LEFT and not e.is_pressed():
		end_stroke()
	return true
		
func add_to_current_stroke():
	var position = snapped(Vector2i(drawable.get_local_mouse_position()), Vector2i(64, 64))
	if not drawable.draw_list.has(position):
		drawable.draw_list[position] = 1 # some dummy value, when drawing textures can be a texture reference
		stroke_draw_list[position] = 1
		drawable.queue_redraw()

func end_stroke():
	remove_from_draw_list(drawable, stroke_draw_list)
	var undoredo = get_undo_redo()
	undoredo.create_action("draw")
	undoredo.add_do_method(self, "add_to_draw_list", drawable, stroke_draw_list.duplicate())
	undoredo.add_undo_method(self, "remove_from_draw_list", drawable, stroke_draw_list.duplicate())
	undoredo.commit_action()
	stroke_draw_list.clear()
	
func add_to_draw_list(object, stroke):
	object.draw_list.merge(stroke)
	object.queue_redraw()
	
func remove_from_draw_list(object, stroke):
	for pos in stroke.keys():
		object.draw_list.erase(pos)
	object.queue_redraw()

jonSS That's all the code. I never said the class will appear on the list of nodes. It won't because it inherits an abstract class. You can just attach the Drawable script to any existing canvas item node and it will work. But that all is beside the point. You can inherit any other canvas item class. Makes no difference for what the example is about.

The color is for the whole node. It's just to differentiate between the nodes so we can see how undo properly functions with multiple nodes. Not meant to work as some kind of brush color and it's not included in the undoable action. Although that'd be easy to implement. I'm leaving it to you as a homework.

I'm here in the business of demonstrating the basic key concepts in the most compact form, not finetuning the user experience