godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.2k stars 19.97k forks source link

Removing a script and then undoing resets all properties without flagging the scene as changed #79430

Open bruce965 opened 1 year ago

bruce965 commented 1 year ago

Godot version

v4.1.stable.official [970459615], v4.2.dev.custom_build [60f3b7967] (master)

System information

Vulkan API 1.3.217 - Forward+ - Using Vulkan Device #0: AMD - AMD Radeon(TM) Graphics

Issue description

Remove a script by clicking on the "revert" button, then undo (CTRL + Z). Properties are cleared, and the scene is not flagged as "changed".

Steps to reproduce

Initial status: image image

Remove a script by clicking on the "revert" button: image image

Undo (CTRL + Z): image image

Properties are cleared, and the scene is not flagged as "changed".

Minimal reproduction project

remove_undo_bug.zip

AThousandShips commented 1 year ago

Unsure how to handle the tracking of those values, could be a feature with the EditorUndoRedo, but the fact that it treats it as unchanged is because that system depends on the undo redo stuff

TheSofox commented 9 months ago

Been looking into this. Interestingly, there's two different codepaths for handling Attaching and Removing scripts. One from editor_properties.cpp/editor_inspector.cpp from handling it from the Inspector panel , the other scene_tree_dock.cpp for doing it from the Scene Tree panel. There is some very basic code in the Scene Tree panel for preserving properties between scripts when an undo/redo happens, but it only works if you go right from one script to another, and only for fields that the two have in common. Fixing this is going to involve coding a system to save the entire set of properties that a script uses to the Undo/Redo history. This should happen when a new script is added to replace an old one, and also when a script is detached/removed. This new system will need to be used by both codepaths.

TheSofox commented 9 months ago

Created pull request for fixing this in Editor Inspector.