Don't duplicate internal nodes

doc/classes/Node.xml

@@ -142,7 +142,7 @@
 			<description>
 				Adds a child [param node]. Nodes can have any number of children, but every child must have a unique name. Child nodes are automatically deleted when the parent node is deleted, so an entire scene can be removed by deleting its topmost node.
 				If [param force_readable_name] is [code]true[/code], improves the readability of the added [param node]. If not named, the [param node] is renamed to its type, and if it shares [member name] with a sibling, a number is suffixed more appropriately. This operation is very slow. As such, it is recommended leaving this to [code]false[/code], which assigns a dummy name featuring [code]@[/code] in both situations.
-				If [param internal] is different than [constant INTERNAL_MODE_DISABLED], the child will be added as internal node. These nodes are ignored by methods like [method get_children], unless their parameter [code]include_internal[/code] is [code]true[/code]. The intended usage is to hide the internal nodes from the user, so the user won't accidentally delete or modify them. Used by some GUI nodes, e.g. [ColorPicker]. See [enum InternalMode] for available modes.
+				If [param internal] is different than [constant INTERNAL_MODE_DISABLED], the child will be added as internal node. These nodes are ignored by methods like [method get_children], unless their parameter [code]include_internal[/code] is [code]true[/code]. It also prevents these nodes being duplicated with their parent. The intended usage is to hide the internal nodes from the user, so the user won't accidentally delete or modify them. Used by some GUI nodes, e.g. [ColorPicker]. See [enum InternalMode] for available modes.
 				[b]Note:[/b] If [param node] already has a parent, this method will fail. Use [method remove_child] first to remove [param node] from its current parent. For example:
 				[codeblocks]
 				[gdscript]
@@ -259,7 +259,7 @@
 			<return type="Node" />
 			<param index="0" name="flags" type="int" default="15" />
 			<description>
-				Duplicates the node, returning a new node with all of its properties, signals, groups, and children copied from the original. The behavior can be tweaked through the [param flags] (see [enum DuplicateFlags]).
+				Duplicates the node, returning a new node with all of its properties, signals, groups, and children copied from the original. The behavior can be tweaked through the [param flags] (see [enum DuplicateFlags]). Internal nodes are not duplicated.
 				[b]Note:[/b] For nodes with a [Script] attached, if [method Object._init] has been defined with required parameters, the duplicated node will not have a [Script].
 			</description>
 		</method>

editor/scene_tree_dock.cpp

@@ -424,7 +424,7 @@ void SceneTreeDock::_perform_create_audio_stream_players(const Vector<String> &p
 
 void SceneTreeDock::_replace_with_branch_scene(const String &p_file, Node *base) {
 	// `move_child` + `get_index` doesn't really work for internal nodes.
-	ERR_FAIL_COND_MSG(base->get_internal_mode() != INTERNAL_MODE_DISABLED, "Trying to replace internal node, this is not supported.");
+	ERR_FAIL_COND_MSG(base->is_internal(), "Trying to replace internal node, this is not supported.");
 
 	Ref<PackedScene> sdata = ResourceLoader::load(p_file);
 	if (sdata.is_null()) {
@@ -814,7 +814,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
 			int highest_id = 0;
 			for (Node *E : selection) {
 				// `move_child` + `get_index` doesn't really work for internal nodes.
-				ERR_FAIL_COND_MSG(E->get_internal_mode() != INTERNAL_MODE_DISABLED, "Trying to move internal node, this is not supported.");
+				ERR_FAIL_COND_MSG(E->is_internal(), "Trying to move internal node, this is not supported.");
 				int index = E->get_index(false);
 
 				if (index > highest_id) {
@@ -993,7 +993,7 @@ void SceneTreeDock::_tool_selected(int p_tool, bool p_confirm_override) {
 			}
 
 			// `move_child` + `get_index` doesn't really work for internal nodes.
-			ERR_FAIL_COND_MSG(node->get_internal_mode() != INTERNAL_MODE_DISABLED, "Trying to set internal node as scene root, this is not supported.");
+			ERR_FAIL_COND_MSG(node->is_internal(), "Trying to set internal node as scene root, this is not supported.");
 
 			//check that from node to root, all owners are right
 
@@ -2331,7 +2331,7 @@ void SceneTreeDock::_do_reparent(Node *p_new_parent, int p_position_in_parent, V
 			return; // Attempt to reparent to itself.
 		}
 		// `move_child` + `get_index` doesn't really work for internal nodes.
-		ERR_FAIL_COND_MSG(p_nodes[ni]->get_internal_mode() != INTERNAL_MODE_DISABLED, "Trying to move internal node, this is not supported.");
+		ERR_FAIL_COND_MSG(p_nodes[ni]->is_internal(), "Trying to move internal node, this is not supported.");
 
 		if (p_nodes[ni]->get_index(false) < first_idx) {
 			nodes_before--;
@@ -2744,7 +2744,7 @@ void SceneTreeDock::_delete_confirm(bool p_cut) {
 	if (!entire_scene) {
 		for (const Node *E : remove_list) {
 			// `move_child` + `get_index` doesn't really work for internal nodes.
-			ERR_FAIL_COND_MSG(E->get_internal_mode() != INTERNAL_MODE_DISABLED, "Trying to remove internal node, this is not supported.");
+			ERR_FAIL_COND_MSG(E->is_internal(), "Trying to remove internal node, this is not supported.");
 		}
 	}
 
@@ -3181,7 +3181,7 @@ void SceneTreeDock::_replace_node(Node *p_node, Node *p_by_node, bool p_keep_pro
 
 	List<Node *> to_erase;
 	for (int i = 0; i < oldnode->get_child_count(); i++) {
-		if (oldnode->get_child(i)->get_owner() == nullptr && oldnode->is_owned_by_parent()) {
+		if (oldnode->get_child(i)->get_owner() == nullptr && oldnode->is_internal()) {
 			to_erase.push_back(oldnode->get_child(i));
 		}
 	}

scene/2d/tile_map_layer.cpp

@@ -1950,7 +1950,6 @@ void TileMapLayer::set_as_tile_map_internal_node(int p_index) {
 	ERR_FAIL_NULL(get_parent());
 	tile_map_node = Object::cast_to<TileMap>(get_parent());
 	set_use_parent_material(true);
-	force_parent_owned();
 	if (layer_index_in_tile_map_node != p_index) {
 		layer_index_in_tile_map_node = p_index;
 		dirty.flags[DIRTY_FLAGS_LAYER_INDEX_IN_TILE_MAP_NODE] = true;

scene/gui/color_picker.cpp

@@ -887,7 +887,6 @@ void ColorPicker::_quick_open_palette_file_selected(const String &p_path) {
 	if (!file_dialog) {
 		file_dialog = memnew(FileDialog);
 		add_child(file_dialog, false, INTERNAL_MODE_FRONT);
-		file_dialog->force_parent_owned();
 		file_dialog->connect("file_selected", callable_mp(this, &ColorPicker::_palette_file_selected));
 		file_dialog->set_access(FileDialog::ACCESS_FILESYSTEM);
 		file_dialog->set_current_dir(Engine::get_singleton()->is_editor_hint() ? "res://" : "user://");
@@ -1741,7 +1740,6 @@ void ColorPicker::_pick_button_pressed() {
 		picker_preview_color->add_theme_style_override(SceneStringName(panel), picker_preview_style_box_color);
 
 		add_child(picker_window, false, INTERNAL_MODE_FRONT);
-		picker_window->force_parent_owned();
 	}
 	set_process_internal(true);
 
@@ -1826,7 +1824,6 @@ void ColorPicker::_options_menu_cbk(int p_which) {
 	if (!file_dialog) {
 		file_dialog = memnew(FileDialog);
 		add_child(file_dialog, false, INTERNAL_MODE_FRONT);
-		file_dialog->force_parent_owned();
 		file_dialog->connect("file_selected", callable_mp(this, &ColorPicker::_palette_file_selected));
 		file_dialog->set_access(FileDialog::ACCESS_FILESYSTEM);
 		file_dialog->set_current_dir(Engine::get_singleton()->is_editor_hint() ? "res://" : "user://");
@@ -1890,7 +1887,6 @@ void ColorPicker::_pick_button_pressed_legacy() {
 		picker_window->hide();
 		picker_window->set_transient(true);
 		add_child(picker_window, false, INTERNAL_MODE_FRONT);
-		picker_window->force_parent_owned();
 
 		picker_texture_rect = memnew(TextureRect);
 		picker_texture_rect->set_anchors_preset(Control::PRESET_FULL_RECT);
@@ -2564,7 +2560,6 @@ void ColorPickerButton::_update_picker() {
 		picker->set_anchors_and_offsets_preset(PRESET_FULL_RECT);
 		popup->add_child(picker);
 		add_child(popup, false, INTERNAL_MODE_FRONT);
-		popup->force_parent_owned();
 		picker->connect("color_changed", callable_mp(this, &ColorPickerButton::_color_changed));
 		popup->connect("about_to_popup", callable_mp(this, &ColorPickerButton::_about_to_popup));
 		popup->connect("popup_hide", callable_mp(this, &ColorPickerButton::_modal_closed));

scene/gui/line_edit.cpp

@@ -2707,7 +2707,6 @@ Key LineEdit::_get_menu_action_accelerator(const String &p_action) {
 void LineEdit::_generate_context_menu() {
 	menu = memnew(PopupMenu);
 	add_child(menu, false, INTERNAL_MODE_FRONT);
-	menu->force_parent_owned();
 
 	menu_dir = memnew(PopupMenu);
 	menu_dir->add_radio_check_item(ETR("Same as Layout Direction"), MENU_DIR_INHERITED);

scene/gui/rich_text_label.cpp

@@ -6834,7 +6834,6 @@ Size2 RichTextLabel::get_minimum_size() const {
 void RichTextLabel::_generate_context_menu() {
 	menu = memnew(PopupMenu);
 	add_child(menu, false, INTERNAL_MODE_FRONT);
-	menu->force_parent_owned();
 	menu->connect(SceneStringName(id_pressed), callable_mp(this, &RichTextLabel::menu_option));
 
 	menu->add_item(ETR("Copy"), MENU_COPY);

scene/gui/tab_container.cpp

@@ -761,7 +761,6 @@ void TabContainer::set_all_tabs_in_front(bool p_in_front) {
 
 	remove_child(tab_bar);
 	add_child(tab_bar, false, all_tabs_in_front ? INTERNAL_MODE_FRONT : INTERNAL_MODE_BACK);
-	tab_bar->force_parent_owned();
 }
 
 bool TabContainer::is_all_tabs_in_front() const {

scene/gui/text_edit.cpp

@@ -7421,7 +7421,6 @@ Key TextEdit::_get_menu_action_accelerator(const String &p_action) {
 void TextEdit::_generate_context_menu() {
 	menu = memnew(PopupMenu);
 	add_child(menu, false, INTERNAL_MODE_FRONT);
-	menu->force_parent_owned();
 
 	menu_dir = memnew(PopupMenu);
 	menu_dir->add_radio_check_item(ETR("Same as Layout Direction"), MENU_DIR_INHERITED);

scene/main/node.cpp

@@ -227,10 +227,6 @@ void Node::_notification(int p_notification) {
 			GDVIRTUAL_CALL(_ready);
 		} break;
 
-		case NOTIFICATION_POSTINITIALIZE: {
-			data.in_constructor = false;
-		} break;
-
 		case NOTIFICATION_PREDELETE: {
 			if (data.inside_tree && !Thread::is_main_thread()) {
 				cancel_free();
@@ -1640,8 +1636,6 @@ void Node::_add_child_nocheck(Node *p_child, const StringName &p_name, InternalM
 	}
 
 	/* Notify */
-	//recognize children created in this node constructor
-	p_child->data.parent_owned = data.in_constructor;
 	add_child_notify(p_child);
 	notification(NOTIFICATION_CHILD_ORDER_CHANGED);
 	emit_signal(SNAME("child_order_changed"));
@@ -2795,12 +2789,8 @@ Node *Node::_duplicate(int p_flags, HashMap<const Node *, Node *> *r_duplimap) c
 		instance_roots.push_back(this);
 
 		for (List<const Node *>::Element *N = node_tree.front(); N; N = N->next()) {
-			for (int i = 0; i < N->get()->get_child_count(); ++i) {
-				Node *descendant = N->get()->get_child(i);
-
-				if (!descendant->get_owner()) {
-					continue; // Internal nodes or nodes added by scripts.
-				}
+			for (int i = 0; i < N->get()->get_child_count(false); ++i) {
+				Node *descendant = N->get()->get_child(i, false);
 
 				// Skip nodes not really belonging to the instantiated hierarchy; they'll be processed normally later
 				// but remember non-instantiated nodes that are hidden below instantiated ones
@@ -2844,10 +2834,7 @@ Node *Node::_duplicate(int p_flags, HashMap<const Node *, Node *> *r_duplimap) c
 		}
 	}
 
-	for (int i = 0; i < get_child_count(); i++) {
-		if (get_child(i)->data.parent_owned) {
-			continue;
-		}
+	for (int i = 0; i < get_child_count(false); i++) {
 		if (instantiated && get_child(i)->data.owner == this) {
 			continue; //part of instance
 		}
@@ -3041,10 +3028,10 @@ void Node::_duplicate_properties(const Node *p_root, const Node *p_original, Nod
 		}
 	}
 
-	for (int i = 0; i < p_original->get_child_count(); i++) {
-		Node *copy_child = p_copy->get_child(i);
+	for (int i = 0; i < p_original->get_child_count(false); i++) {
+		Node *copy_child = p_copy->get_child(i, false);
 		ERR_FAIL_NULL_MSG(copy_child, "Child node disappeared while duplicating.");
-		_duplicate_properties(p_root, p_original->get_child(i), copy_child, p_flags);
+		_duplicate_properties(p_root, p_original->get_child(i, false), copy_child, p_flags);
 	}
 }
 
@@ -3161,8 +3148,8 @@ void Node::replace_by(Node *p_node, bool p_keep_groups) {
 	while (get_child_count()) {
 		Node *child = get_child(0);
 		remove_child(child);
-		if (!child->is_owned_by_parent()) {
-			// add the custom children to the p_node
+		if (!child->is_internal()) {
+			// Add the custom children to the p_node.
 			Node *child_owner = child->get_owner() == this ? p_node : child->get_owner();
 			child->set_owner(nullptr);
 			p_node->add_child(child);
@@ -3440,10 +3427,6 @@ void Node::update_configuration_warnings() {
 #endif
 }
 
-bool Node::is_owned_by_parent() const {
-	return data.parent_owned;
-}
-
 void Node::set_display_folded(bool p_folded) {
 	ERR_THREAD_GUARD
 	data.display_folded = p_folded;
@@ -3953,8 +3936,6 @@ Node::Node() {
 	data.physics_interpolated_client_side = false;
 	data.use_identity_transform = false;
 
-	data.parent_owned = false;
-	data.in_constructor = true;
 	data.use_placeholder = false;
 
 	data.display_folded = false;

scene/main/node.h

@@ -239,8 +239,6 @@ private:
 		// RenderingServer, and specify the mesh in world space.
 		bool use_identity_transform : 1;
 
-		bool parent_owned : 1;
-		bool in_constructor : 1;
 		bool use_placeholder : 1;
 
 		bool display_folded : 1;
@@ -487,6 +485,7 @@ public:
 	}
 
 	_FORCE_INLINE_ bool is_inside_tree() const { return data.inside_tree; }
+	bool is_internal() const { return data.internal_mode != INTERNAL_MODE_DISABLED; }
 
 	bool is_ancestor_of(const Node *p_node) const;
 	bool is_greater_than(const Node *p_node) const;
@@ -706,8 +705,6 @@ public:
 	//hacks for speed
 	static void init_node_hrcr();
 
-	void force_parent_owned() { data.parent_owned = true; } //hack to avoid duplicate nodes
-
 	void set_import_path(const NodePath &p_import_path); //path used when imported, used by scene editors to keep tracking
 	NodePath get_import_path() const;