Merge pull request #14406 from hoelzl/pr-instance-node-order

Fix some issues with instanced scenes

scene/main/node.cpp

@@ -2110,6 +2110,7 @@ Node *Node::_duplicate(int p_flags, Map<const Node *, Node *> *r_duplimap) const
 
 	StringName script_property_name = CoreStringNames::get_singleton()->_script;
 
+	List<const Node *> hidden_roots;
 	List<const Node *> node_tree;
 	node_tree.push_front(this);
 
@@ -2120,11 +2121,16 @@ Node *Node::_duplicate(int p_flags, Map<const Node *, Node *> *r_duplimap) const
 		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);
 				// Skip nodes not really belonging to the instanced hierarchy; they'll be processed normally later
-				if (N->get()->get_child(i)->data.owner != this)
+				// but remember non-instanced nodes that are hidden below instanced ones
+				if (descendant->data.owner != this) {
+					if (descendant->get_parent() && descendant->get_parent() != this && descendant->get_parent()->data.owner == this)
+						hidden_roots.push_back(descendant);
 					continue;
+				}
 
-				node_tree.push_back(N->get()->get_child(i));
+				node_tree.push_back(descendant);
 			}
 		}
 	}
@@ -2201,6 +2207,34 @@ Node *Node::_duplicate(int p_flags, Map<const Node *, Node *> *r_duplimap) const
 		}
 
 		node->add_child(dup);
+		if (i < node->get_child_count() - 1) {
+			node->move_child(dup, i);
+		}
+	}
+
+	for (List<const Node *>::Element *E = hidden_roots.front(); E; E = E->next()) {
+
+		Node *parent = node->get_node(get_path_to(E->get()->data.parent));
+		if (!parent) {
+
+			memdelete(node);
+			return NULL;
+		}
+
+		Node *dup = E->get()->_duplicate(p_flags, r_duplimap);
+		if (!dup) {
+
+			memdelete(node);
+			return NULL;
+		}
+
+		parent->add_child(dup);
+		int pos = E->get()->get_position_in_parent();
+
+		if (pos < parent->get_child_count() - 1) {
+
+			parent->move_child(dup, pos);
+		}
 	}
 
 	return node;

scene/resources/packed_scene.cpp

@@ -270,6 +270,8 @@ Node *SceneState::instance(GenEditState p_edit_state) const {
 				if (i > 0) {
 					if (parent) {
 						parent->_add_child_nocheck(node, snames[n.name]);
+						if (n.index >= 0 && n.index < parent->get_child_count() - 1)
+							parent->move_child(node, n.index);
 					} else {
 						//it may be possible that an instanced scene has changed
 						//and the node has nowhere to go anymore
@@ -386,6 +388,7 @@ Error SceneState::_parse_node(Node *p_owner, Node *p_node, int p_parent_idx, Map
 
 	nd.name = _nm_get_string(p_node->get_name(), name_map);
 	nd.instance = -1; //not instanced by default
+	nd.index = p_node->get_index();
 
 	// if this node is part of an instanced scene or sub-instanced scene
 	// we need to get the corresponding instance states.
@@ -1116,6 +1119,7 @@ void SceneState::set_bundled_scene(const Dictionary &p_dictionary) {
 			nd.type = r[idx++];
 			nd.name = r[idx++];
 			nd.instance = r[idx++];
+			nd.index = r[idx++];
 			nd.properties.resize(r[idx++]);
 			for (int j = 0; j < nd.properties.size(); j++) {
 
@@ -1208,6 +1212,7 @@ Dictionary SceneState::get_bundled_scene() const {
 		rnodes.push_back(nd.type);
 		rnodes.push_back(nd.name);
 		rnodes.push_back(nd.instance);
+		rnodes.push_back(nd.index);
 		rnodes.push_back(nd.properties.size());
 		for (int j = 0; j < nd.properties.size(); j++) {
 
@@ -1284,6 +1289,11 @@ StringName SceneState::get_node_name(int p_idx) const {
 	return names[nodes[p_idx].name];
 }
 
+int SceneState::get_node_index(int p_idx) const {
+	ERR_FAIL_INDEX_V(p_idx, nodes.size(), -1);
+	return nodes[p_idx].index;
+}
+
 bool SceneState::is_node_instance_placeholder(int p_idx) const {
 
 	ERR_FAIL_INDEX_V(p_idx, nodes.size(), false);
@@ -1524,7 +1534,7 @@ int SceneState::add_node_path(const NodePath &p_path) {
 	node_paths.push_back(p_path);
 	return (node_paths.size() - 1) | FLAG_ID_IS_PATH;
 }
-int SceneState::add_node(int p_parent, int p_owner, int p_type, int p_name, int p_instance) {
+int SceneState::add_node(int p_parent, int p_owner, int p_type, int p_name, int p_instance, int p_index) {
 
 	NodeData nd;
 	nd.parent = p_parent;
@@ -1532,6 +1542,7 @@ int SceneState::add_node(int p_parent, int p_owner, int p_type, int p_name, int
 	nd.type = p_type;
 	nd.name = p_name;
 	nd.instance = p_instance;
+	nd.index = p_index;
 
 	nodes.push_back(nd);
 
@@ -1605,6 +1616,7 @@ void SceneState::_bind_methods() {
 	ClassDB::bind_method(D_METHOD("get_node_instance_placeholder", "idx"), &SceneState::get_node_instance_placeholder);
 	ClassDB::bind_method(D_METHOD("get_node_instance", "idx"), &SceneState::get_node_instance);
 	ClassDB::bind_method(D_METHOD("get_node_groups", "idx"), &SceneState::_get_node_groups);
+	ClassDB::bind_method(D_METHOD("get_node_index", "idx"), &SceneState::get_node_index);
 	ClassDB::bind_method(D_METHOD("get_node_property_count", "idx"), &SceneState::get_node_property_count);
 	ClassDB::bind_method(D_METHOD("get_node_property_name", "idx", "prop_idx"), &SceneState::get_node_property_name);
 	ClassDB::bind_method(D_METHOD("get_node_property_value", "idx", "prop_idx"), &SceneState::get_node_property_value);

scene/resources/packed_scene.h

@@ -57,6 +57,7 @@ class SceneState : public Reference {
 		int type;
 		int name;
 		int instance;
+		int index;
 
 		struct Property {
 
@@ -151,6 +152,7 @@ public:
 	String get_node_instance_placeholder(int p_idx) const;
 	bool is_node_instance_placeholder(int p_idx) const;
 	Vector<StringName> get_node_groups(int p_idx) const;
+	int get_node_index(int p_idx) const;
 
 	int get_node_property_count(int p_idx) const;
 	StringName get_node_property_name(int p_idx, int p_prop) const;
@@ -174,7 +176,7 @@ public:
 	int find_name(const StringName &p_name) const;
 	int add_value(const Variant &p_value);
 	int add_node_path(const NodePath &p_path);
-	int add_node(int p_parent, int p_owner, int p_type, int p_name, int p_instance);
+	int add_node(int p_parent, int p_owner, int p_type, int p_name, int p_instance, int p_index);
 	void add_node_property(int p_node, int p_name, int p_value);
 	void add_node_group(int p_node, int p_group);
 	void set_base_scene(int p_idx);

scene/resources/scene_format_text.cpp

@@ -198,6 +198,7 @@ Ref<PackedScene> ResourceInteractiveLoaderText::_parse_node_tag(VariantParser::R
 			int type = -1;
 			int name = -1;
 			int instance = -1;
+			int index = -1;
 			//int base_scene=-1;
 
 			if (next_tag.fields.has("name")) {
@@ -249,7 +250,11 @@ Ref<PackedScene> ResourceInteractiveLoaderText::_parse_node_tag(VariantParser::R
 					owner = 0; //if no owner, owner is root
 			}
 
-			int node_id = packed_scene->get_state()->add_node(parent, owner, type, name, instance);
+			if (next_tag.fields.has("index")) {
+				index = next_tag.fields["index"];
+			}
+
+			int node_id = packed_scene->get_state()->add_node(parent, owner, type, name, instance, index);
 
 			if (next_tag.fields.has("groups")) {
 
@@ -1609,6 +1614,7 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 
 			StringName type = state->get_node_type(i);
 			StringName name = state->get_node_name(i);
+			int index = state->get_node_index(i);
 			NodePath path = state->get_node_path(i, true);
 			NodePath owner = state->get_node_owner_path(i);
 			Ref<PackedScene> instance = state->get_node_instance(i);
@@ -1626,6 +1632,9 @@ Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_r
 			if (owner != NodePath() && owner != NodePath(".")) {
 				header += " owner=\"" + String(owner.simplified()) + "\"";
 			}
+			if (index >= 0) {
+				header += " index=\"" + itos(index) + "\"";
+			}
 
 			if (groups.size()) {
 				String sgroups = " groups=[\n";