From 92ab92114e9b669eb182044a12389766e5858277 Mon Sep 17 00:00:00 2001 From: kobewi Date: Wed, 13 Mar 2024 13:22:42 +0100 Subject: [PATCH] Don't duplicate internal nodes --- doc/classes/Node.xml | 4 ++-- editor/scene_tree_dock.cpp | 12 ++++++------ scene/2d/tile_map_layer.cpp | 1 - scene/gui/color_picker.cpp | 5 ----- scene/gui/line_edit.cpp | 1 - scene/gui/rich_text_label.cpp | 1 - scene/gui/tab_container.cpp | 1 - scene/gui/text_edit.cpp | 1 - scene/main/node.cpp | 35 ++++++++--------------------------- scene/main/node.h | 5 +---- 10 files changed, 17 insertions(+), 49 deletions(-) diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml index 29081de38c6..15852097fb5 100644 --- a/doc/classes/Node.xml +++ b/doc/classes/Node.xml @@ -142,7 +142,7 @@ 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 @@ - 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]. diff --git a/editor/scene_tree_dock.cpp b/editor/scene_tree_dock.cpp index 267d35f9945..6e81c84b17e 100644 --- a/editor/scene_tree_dock.cpp +++ b/editor/scene_tree_dock.cpp @@ -424,7 +424,7 @@ void SceneTreeDock::_perform_create_audio_stream_players(const Vector &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 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 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)); } } diff --git a/scene/2d/tile_map_layer.cpp b/scene/2d/tile_map_layer.cpp index 2bb190b1260..5c7526d99e0 100644 --- a/scene/2d/tile_map_layer.cpp +++ b/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(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; diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index aab7c37bbb5..bef3f6450c5 100644 --- a/scene/gui/color_picker.cpp +++ b/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)); diff --git a/scene/gui/line_edit.cpp b/scene/gui/line_edit.cpp index f4d66d5a281..2566c8514bd 100644 --- a/scene/gui/line_edit.cpp +++ b/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); diff --git a/scene/gui/rich_text_label.cpp b/scene/gui/rich_text_label.cpp index 91beafa4c17..26096439503 100644 --- a/scene/gui/rich_text_label.cpp +++ b/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); diff --git a/scene/gui/tab_container.cpp b/scene/gui/tab_container.cpp index c43d0c5d55c..8dd49277d1e 100644 --- a/scene/gui/tab_container.cpp +++ b/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 { diff --git a/scene/gui/text_edit.cpp b/scene/gui/text_edit.cpp index 69da6df7fd2..488dd010854 100644 --- a/scene/gui/text_edit.cpp +++ b/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); diff --git a/scene/main/node.cpp b/scene/main/node.cpp index c3951f48f2e..7c3701cf9e9 100644 --- a/scene/main/node.cpp +++ b/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 *r_duplimap) c instance_roots.push_back(this); for (List::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 *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; diff --git a/scene/main/node.h b/scene/main/node.h index 2d6fa157506..f55c894a6fd 100644 --- a/scene/main/node.h +++ b/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;