Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No UndoRedo inspector hook callback on multiple simultanious edits. #91816

Open
OctagonalHexy opened this issue May 10, 2024 · 2 comments
Open

Comments

@OctagonalHexy
Copy link

Tested versions

v4.3.dev5.official [c9c17d6]

System information

Godot v4.3.dev5 - Linux Mint 20.1 (Ulyssa) - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 (nvidia; 535.171.04) - Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 Threads)

Issue description

I am writing A plugin and using 'add_undo_redo_inspector_hook_callback()' to get callbacks on any edit to an objects properties, and when multiple nodes are selected it simply does not callback.
I was expecting this callback to run once for each property edited that way my plugin could react to each property change.

simplescreenrecorder-2024-05-09_22.57.50.mp4

Steps to reproduce

1: Create an EditorPlugin and put 'add_undo_redo_inspector_hook_callback()' to the _enter_tree method hooking to A function for printing its outputs.(Script provided in MRP)
2: edit any property in the inspector, to see the expected result printed in the output tab.
3: Now highlight more than one node holding control and clicking them, and try editing A property they share in the inspector.
4: No callback, no print function in output.

Minimal reproduction project (MRP)

plugin.gd

@tool
extends EditorPlugin

func _enter_tree() -> void:
	var callback = Callable(self, "callback")
	add_undo_redo_inspector_hook_callback(callback)
	pass


func _exit_tree() -> void:
	remove_undo_redo_inspector_hook_callback(callback)
	pass


func callback(undo_redo:Object,modifyed_object:Object,property:String,new_value:Variant):
	print(undo_redo.to_string())
	print(modifyed_object.to_string())
	print(property)
	print(new_value)
@OctagonalHexy
Copy link
Author

After looking at the ways Godot uses add_undo_redo_inspector_hook_callback() internally, I can confirm that none of the internal uses of this function should be effected by this fix given that multi node editing currently does not apply to Resources like Material3D, and TileSet. #91843

@OctagonalHexy
Copy link
Author

After thinking on this further, I think making this return A callback for each edit in A multi node edit would make this callback fall out of sync with how UndoRedo is assigned when multi editing nodes which feels wrong based on its description in the docs.

Having MultiNodeEdit itself return A single callback with info that can be used to query the changes made would be both more in line with the single do\undo added to UndoRedoManager by MultiNodeEdit when editing multiple nodes, and would require specific handling in plugins to be able to react to MultiNodeEdits theoretically ensuring that no uses of add_undo_redo_inspector_hook_callback() are effected by these changes at all.

I think this is the least impactful way to fix the inability to react to editing multiple nodes in the inspector the way, "Hooks a callback into the undo/redo action creation when a property is modified in the inspector." implies in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant