Fix for "Save Branch as Scene"

[revilo]

Fixes #42611.

Hello Godoters! This PR is for discussion on my solution for #42611. While the PR solves the problems described in the original ticket, I'm new to the Godot engine and therefore appreciate your feedback on this solution.

Result of my analysis:
The original "Save Branch as Scene" function of the SceneTreeDock uses the duplicate_and_reown function of the Node class to create a clone of the selected node, saving it to a file and then replacing the subtree by the scene instance. My analysis showed that duplicate_and_reown has the following flaws:

1. Editable flag of instanced scenes gets lost
2. Recursive _duplicate_and_reown skips children of instanced scenes (owner check at the beginning of the method)

While [1] was rather easy to fix, [2] was not easily possible, as there would have been a lot of special cases to handle, which the duplicate_and_reown method currently isn't aware of (e.g. changed properties of editable children, nodes added to editable children).

I found out the the Node class has three "duplicate" methods:

• duplicate
• duplicate_from_editor (used in SceneTreeDock's "Duplicate" function)
• duplicate_and_reown (used in SceneTreeDock's "Save Branch as Scene" function, looked a bit outdated)

I've checked the duplicate_from_editor and found out that it is aware of creating duplicates with (editable) instanced scenes. Using the "Duplicate" function in Godot v4 editor together with the MRP from #42611 showed good results. Only issue was that the "editable" flag got lost as well (but can be fixed easily).

Summary of changes in this PR:

1. Modified SceneTreeDock's "Save Branch as Scene" to use duplicate_from_editor instead of duplicate_and_reown.
2. Removed duplicate_and_reown as it was only used in "Save Branch as Scene" (and not exposed to scripting)
3. Added Node::reown method to recursively change owners of a tree (was originally part of duplicate_and_reown) - use it in the modified "Save Branch as Scene" function to update the owner in the duplicated subtree
4. Fixed Node::duplicate_from_editor to copy the "editable" flag of instanced children

I'm looking forward to your feedback on these changes. :-)

PS: During development I've recognized that when saving a .tscn, Godot 4.0 writes out a script = null for each node without a script. I'm not sure whether this is a bug or a feature, but I just want to point out that this has nothing to do with the changes in this PR in case you wonder. It happens with vanilla Godot 4.0 from master branch when saving a scene.

Edit: Updated formatting + added minor clarifications

Edit 2: Updated list of changes (removed reown function after code review)

[KoBeWi]

What is the purpose of reown_map if it ever has just one element? Wouldn't it be better to check if the owner is null?

[revilo]

What is the purpose of reown_map if it ever has just one element? Wouldn't it be better to check if the owner is null?

The reown function is based on the original _duplicate_and_reown which did it in the same way. I thought, I stick to that approach as it is more generalized. But you are right, in this particular case a check for null would be sufficient.

[KoBeWi]

Sooo the reown map should be removed. Other than that it looks ok.

[revilo]

Sooo the reown map should be removed. Other than that it looks ok.

Agreed, will be done!

[revilo]

Dear @KoBeWi, thank you for your code review and comments! I've incorporated the requested change by replacing the Node::reown method by SceneTreeDock::_set_node_owner_recursive (without the use of any map). Please let me know if there are any other things that should be changed.

[akien-mga]

Thanks!