Expose Node::force_parent_owned() to allow avoid duplicate nodes for plugin development

[Daylily-Zeleen]

Describe the project you are working on

Creating a DragonBone plugin by using GDExtension.

Describe the problem or limitation you are having in your project

I need to create an internal tree struct like this:

• DragonBones (Node2D)
  |- DragonBonesArmature (Node2D)

DragonBonesArmature is created by DragonBones automatically, and need to be an abstract class to avoid created by users.

But duplicate a node will duplicate its children, and duplicated a DragonBonesArmature node will failed because of it is an abstract class and can't be instantiated by ClassDB.

In other words, I can't duplicate a DragonBones node in editor.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

If Node::force_parent_owned() is exposed, I can use it to avoid duplicate internal node, like TileMapLayer::set_as_tile_map_internal_node().

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Just bind it in _bind_methods().

If this enhancement will not be used often, can it be worked around with a few lines of script?

No, it is impossible to change the duplicate logic in script.

Is there a reason why this should be core and not an add-on in the asset library?

Node duplication is core function.

[AThousandShips]

This is specifically for child nodes added outside of the constructor, to be clear, you can work around this by adding child nodes in the constructor, i.e. in _init

Edit: This seems to be impossible right now at least, as the _init method isn't called early enough, that however seems like a bug in the code, rather than a missing feature, or at least it should be the first step, and the need for this feature should be evaluated with the main functionality available

Exposing this could be dangerous, it could break and crash scripts by changing dependent data in built in nodes etc.

[Mickeon]

This proposal is describing a fair issue. But the proposed solution does not feel right:

If Node::force_parent_owned() is exposed, I can use it to avoid duplicate internal node

So, exposing a method explicitly designed to be used internally... so that the issue can be worked around, instead of addressing the original issue in the first place. Whatever its purpose is (as the name is fairly vague) it sounds like trouble to me.

[Daylily-Zeleen]

This is specifically for child nodes added outside of the constructor, to be clear, you can work around this by adding child nodes in the constructor, i.e. in _init

Edit: This seems to be impossible right now at least, as the _init method isn't called early enough, that however seems like a bug in the code, rather than a missing feature, or at least it should be the first step, and the need for this feature should be evaluated with the main functionality available

Not all internal nodes can be determined in constructor, TileMapLayer is an example.
A way to avoid duplicate internal node which outside constructor still required.

Exposing this could be dangerous, it could break and crash scripts by changing dependent data in built in nodes etc.

I'm not sure how this change led to the dangerous behavior you mentioned, it seem that data.parent_owned only be used in Node::_duplicate() to skip child node duplication, and the children nodes should be duplicated or not is decided by users.

Edit: data.parent_owned will effect the replace logic by Node::is_owned_by_parent(), but replace node logic use it to distinguish a child node is internal or not, too.

I think the better way is give it a better name to expose it like @KoBeWi comment.

If expose the controllability of data.parent_owned is actually dangerous, we can add a new field like bool can_duplicated_by_parent_duplication;( may be there have a better name) to Node::Data to control duplicate logic,
then bind a new property to Node.

[KoBeWi]

Currently the logic for determining "internal" nodes is rather inconsistent. When dealing with instances, owner decides what is internal or not (related PRs: godotengine/godot#61966, godotengine/godot#84824).
For nodes within the same scene, there is no user-exposed mechanism for that. The force_parent_owned() is a hack (see its implementation
https://github.com/godotengine/godot/blob/61282068f4d59cb48f35ad95391728c58d9008ab/scene/main/node.h#L638), but apparently it's the only way to make nodes "internal" (not to be confused with actual internal nodes that are simply meant to be hidden from the user, see add_child() method arguments).

tbh this could use some re-thinking. We could maybe remove the parent owned hack and make the internal nodes more useful, that is, make them not duplicated when the parent is duplicated. I originally was opposed to that (godotengine/godot#84824 (comment)), but I didn't consider non-instanced nodes, where using owner makes no sense.

[JekSun97]

For me this function is also quite important, since my plugin also has a problem with duplication

[Daylily-Zeleen]

For me this function is also quite important, since my plugin also has a problem with duplication

I think this proposal can't be approved by core members.

I don't know the true case in your plugin, here I just provide my solution to avoid this issue.

For the solution in my Godot-DragonBones to solve this duplicate issue, I create DragonBoneArmature as an internal node (add in constructor) of the DragonBones to avoid duplicate it in editor (you can add internal node after #91018 and #1446). And track this internal node's state and reinitialize/reuse it instead of delete it and add a new one.

[Mickeon]

I think this proposal can't be approved by core members.

Note that even though in the past some of us expressed this is not a good idea, alternatives to solve the same issue are very much welcome and worth looking into.