From ae472865d0978e79b6e2993f76771d82daa431bf Mon Sep 17 00:00:00 2001 From: ajreckof Date: Thu, 28 Mar 2024 20:26:44 +0100 Subject: [PATCH] fix node duplication in update after external changes. --- doc/classes/Node.xml | 3 +- editor/editor_data.cpp | 9 +-- editor/editor_node.cpp | 16 +++--- .../4.2-stable.expected | 7 +++ scene/main/node.compat.inc | 41 +++++++++++++ scene/main/node.cpp | 57 ++++++++++--------- scene/main/node.h | 7 ++- 7 files changed, 95 insertions(+), 45 deletions(-) create mode 100644 scene/main/node.compat.inc diff --git a/doc/classes/Node.xml b/doc/classes/Node.xml index e6fdd229bf1..ae6cd9596c4 100644 --- a/doc/classes/Node.xml +++ b/doc/classes/Node.xml @@ -795,8 +795,9 @@ + - Replaces this node by the given [param node]. All children of this node are moved to [param node]. + Replaces this node by the given [param node]. If [param keep_children] is [code]true[/code] all children of this node are moved to [param node]. If [param keep_groups] is [code]true[/code], the [param node] is added to the same groups that the replaced node is in (see [method add_to_group]). [b]Warning:[/b] The replaced node is removed from the tree, but it is [b]not[/b] deleted. To prevent memory leaks, store a reference to the node in a variable, or use [method Object.free]. diff --git a/editor/editor_data.cpp b/editor/editor_data.cpp index 4b68a21cb98..6ab29c18509 100644 --- a/editor/editor_data.cpp +++ b/editor/editor_data.cpp @@ -722,15 +722,8 @@ bool EditorData::check_and_update_scene(int p_idx) { new_scene->set_scene_file_path(edited_scene[p_idx].root->get_scene_file_path()); Node *old_root = edited_scene[p_idx].root; - for (int i = 0; i < old_root->get_child_count(); i++) { - memdelete(old_root->get_child(i)); - } - old_root->replace_by(new_scene); + old_root->replace_by(new_scene, false, false); memdelete(old_root); - edited_scene.write[p_idx].root = new_scene; - if (!new_scene->get_scene_file_path().is_empty()) { - edited_scene.write[p_idx].path = new_scene->get_scene_file_path(); - } edited_scene.write[p_idx].selection = new_selection; return true; diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index edb6fee75c9..db4d46cd182 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -3818,6 +3818,14 @@ void EditorNode::_set_current_scene_nocheck(int p_idx) { } } + if (editor_data.check_and_update_scene(p_idx)) { + if (!editor_data.get_scene_path(p_idx).is_empty()) { + editor_folding.load_scene_folding(editor_data.get_edited_scene_root(p_idx), editor_data.get_scene_path(p_idx)); + } + + EditorUndoRedoManager::get_singleton()->clear_history(false, editor_data.get_scene_history_id(p_idx)); + } + Dictionary state = editor_data.restore_edited_scene_state(editor_selection, &editor_history); _edit_current(true); @@ -3827,14 +3835,6 @@ void EditorNode::_set_current_scene_nocheck(int p_idx) { if (tabs_to_close.is_empty()) { callable_mp(this, &EditorNode::_set_main_scene_state).call_deferred(state, get_edited_scene()); // Do after everything else is done setting up. } - - if (editor_data.check_and_update_scene(p_idx)) { - if (!editor_data.get_scene_path(p_idx).is_empty()) { - editor_folding.load_scene_folding(editor_data.get_edited_scene_root(p_idx), editor_data.get_scene_path(p_idx)); - } - - EditorUndoRedoManager::get_singleton()->clear_history(false, editor_data.get_scene_history_id(p_idx)); - } } void EditorNode::setup_color_picker(ColorPicker *p_picker) { diff --git a/misc/extension_api_validation/4.2-stable.expected b/misc/extension_api_validation/4.2-stable.expected index 8623e8eb254..c9eed86c1ac 100644 --- a/misc/extension_api_validation/4.2-stable.expected +++ b/misc/extension_api_validation/4.2-stable.expected @@ -248,3 +248,10 @@ Validate extension JSON: Error: Field 'classes/AcceptDialog/methods/register_tex Validate extension JSON: Error: Field 'classes/AcceptDialog/methods/remove_button/arguments/0': type changed value in new API, from "Control" to "Button". Changed argument type to the more specific one actually expected by the method. Compatibility method registered. + + +GH-89992 +-------- +Validate extension JSON: Error: Field 'classes/Node/methods/replace_by/arguments': size changed value in new API, from 2 to 3. + +Added optional argument to prevent children to be reparented during replace_by. Compatibility method registered. diff --git a/scene/main/node.compat.inc b/scene/main/node.compat.inc new file mode 100644 index 00000000000..69ece1a40d2 --- /dev/null +++ b/scene/main/node.compat.inc @@ -0,0 +1,41 @@ +/**************************************************************************/ +/* node.compat.inc */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#ifndef DISABLE_DEPRECATED + +void Node::_replace_by_bind_compat_89992(Node *p_node, bool p_keep_data) { + replace_by(p_node, p_keep_data, true); +} + +void Node::_bind_compatibility_methods() { + ClassDB::bind_compatibility_method(D_METHOD("replace_by", "node", "keep_groups"), &Node::_replace_by_bind_compat_89992, DEFVAL(false)); +} + +#endif diff --git a/scene/main/node.cpp b/scene/main/node.cpp index d4e2d6eb16d..eb8cee97861 100644 --- a/scene/main/node.cpp +++ b/scene/main/node.cpp @@ -29,6 +29,7 @@ /**************************************************************************/ #include "node.h" +#include "node.compat.inc" #include "core/config/project_settings.h" #include "core/core_string_names.h" @@ -3007,7 +3008,7 @@ static void find_owned_by(Node *p_by, Node *p_node, List *p_owned) { } } -void Node::replace_by(Node *p_node, bool p_keep_groups) { +void Node::replace_by(Node *p_node, bool p_keep_groups, bool p_keep_children) { ERR_THREAD_GUARD ERR_FAIL_NULL(p_node); ERR_FAIL_COND(p_node->data.parent); @@ -3028,13 +3029,13 @@ void Node::replace_by(Node *p_node, bool p_keep_groups) { _replace_connections_target(p_node); if (data.owner) { - for (int i = 0; i < get_child_count(); i++) { - find_owned_by(data.owner, get_child(i), &owned_by_owner); + if (p_keep_children) { + for (int i = 0; i < get_child_count(); i++) { + find_owned_by(data.owner, get_child(i), &owned_by_owner); + } } - _clean_up_owner(); } - Node *parent = data.parent; int index_in_parent = get_index(false); @@ -3046,31 +3047,33 @@ void Node::replace_by(Node *p_node, bool p_keep_groups) { emit_signal(SNAME("replacing_by"), p_node); - 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 - Node *child_owner = child->get_owner() == this ? p_node : child->get_owner(); - child->set_owner(nullptr); - p_node->add_child(child); - child->set_owner(child_owner); + if (p_keep_children) { + 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 + Node *child_owner = child->get_owner() == this ? p_node : child->get_owner(); + child->set_owner(nullptr); + p_node->add_child(child); + child->set_owner(child_owner); + } + } + + for (Node *E : owned) { + if (E->data.owner != p_node) { + E->set_owner(p_node); + } + } + + for (Node *E : owned_by_owner) { + if (E->data.owner != owner) { + E->set_owner(owner); + } } } p_node->set_owner(owner); - for (Node *E : owned) { - if (E->data.owner != p_node) { - E->set_owner(p_node); - } - } - - for (Node *E : owned_by_owner) { - if (E->data.owner != owner) { - E->set_owner(owner); - } - } - p_node->set_scene_file_path(get_scene_file_path()); } @@ -3597,7 +3600,7 @@ void Node::_bind_methods() { ClassDB::bind_method(D_METHOD("create_tween"), &Node::create_tween); ClassDB::bind_method(D_METHOD("duplicate", "flags"), &Node::duplicate, DEFVAL(DUPLICATE_USE_INSTANTIATION | DUPLICATE_SIGNALS | DUPLICATE_GROUPS | DUPLICATE_SCRIPTS)); - ClassDB::bind_method(D_METHOD("replace_by", "node", "keep_groups"), &Node::replace_by, DEFVAL(false)); + ClassDB::bind_method(D_METHOD("replace_by", "node", "keep_groups", "keep_children"), &Node::replace_by, DEFVAL(false), DEFVAL(true)); ClassDB::bind_method(D_METHOD("set_scene_instance_load_placeholder", "load_placeholder"), &Node::set_scene_instance_load_placeholder); ClassDB::bind_method(D_METHOD("get_scene_instance_load_placeholder"), &Node::get_scene_instance_load_placeholder); diff --git a/scene/main/node.h b/scene/main/node.h index f49eeec9cd7..99def103380 100644 --- a/scene/main/node.h +++ b/scene/main/node.h @@ -310,6 +310,11 @@ private: Variant _call_thread_safe_bind(const Variant **p_args, int p_argcount, Callable::CallError &r_error); protected: +#ifndef DISABLE_DEPRECATED + void _replace_by_bind_compat_89992(Node *p_node, bool p_keep_data = false); + static void _bind_compatibility_methods(); +#endif // DISABLE_DEPRECATED + void _block() { data.blocked++; } void _unblock() { data.blocked--; } @@ -629,7 +634,7 @@ public: return binds; } - void replace_by(Node *p_node, bool p_keep_data = false); + void replace_by(Node *p_node, bool p_keep_groups = false, bool p_keep_children = true); void set_process_mode(ProcessMode p_mode); ProcessMode get_process_mode() const;