Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't duplicate internal nodes #89442

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+15 −31
Open

Don't duplicate internal nodes #89442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/classes/Node.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,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 from 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]
Expand Down
12 changes: 6 additions & 6 deletions editor/scene_tree_dock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,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_valid()) {
Expand Down Expand Up @@ -785,7 +785,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) {
Expand Down Expand Up @@ -964,7 +964,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

Expand Down Expand Up @@ -2263,7 +2263,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--;
Expand Down Expand Up @@ -2668,7 +2668,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.");
}
}

Expand Down Expand Up @@ -3067,7 +3067,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));
}
}
Expand Down
1 change: 0 additions & 1 deletion scene/2d/tile_map_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,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;
Expand Down
26 changes: 7 additions & 19 deletions scene/main/node.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,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();
Expand Down Expand Up @@ -1576,8 +1572,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"));
Expand Down Expand Up @@ -2714,7 +2708,7 @@ Node *Node::_duplicate(int p_flags, HashMap<const Node *, Node *> *r_duplimap) c
for (int i = 0; i < N->get()->get_child_count(); ++i) {
Node *descendant = N->get()->get_child(i);

if (!descendant->get_owner()) {
if (descendant->is_internal() || !descendant->get_owner()) {
continue; // Internal nodes or nodes added by scripts.
}

Expand Down Expand Up @@ -2761,7 +2755,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) {
if (get_child(i)->is_internal()) {
continue;
}
if (instantiated && get_child(i)->data.owner == this) {
Expand Down Expand Up @@ -2949,10 +2943,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);
}
}

Expand Down Expand Up @@ -3068,8 +3062,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);
Expand Down Expand Up @@ -3347,10 +3341,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;
Expand Down Expand Up @@ -3857,8 +3847,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;
Expand Down
5 changes: 1 addition & 4 deletions scene/main/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,6 @@ class Node : public Object {
// 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;
Expand Down Expand Up @@ -473,6 +471,7 @@ class Node : public Object {
}

_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;
Expand Down Expand Up @@ -692,8 +691,6 @@ class Node : public Object {
//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;

Expand Down
Loading