Browse Source

Improve PackedScene instantiate

Make `resource_local_to_scene` behave as described in the documentation. (If I understand correctly, the following **instance** refers to **the instance of the sub-scene**.)
https://github.com/godotengine/godot/blob/2e24b76535dceb9cf18ab8ece3304ed92948c1b5/doc/classes/Resource.xml#L70-L72

If the resources of the sub-scene are modified in the main scene, the modified resources will be recorded in the `tscn` file of the main scene. And the root node of the sub-scene will be set twice.
1. In the main scene, when encountering a sub-scene, the sub-scene will be initialized first;
2. Then use the resources in the main scene to reset the root node of the sub-scene.

This may make `resource_local_to_scene` not work as expected. The resources cannot be shared between the sub-scene root node and other ordinary nodes in the sub-scene.

Yes, if the resources have `resource_local_to_scene` enabled, this patch treats the modified resources of the sub-scene root node as resources in the sub-scene, not in the main scene. Although the modifications are recorded in the `tscn` file of the main scene.
Rindbee 3 years ago
parent
commit
d6c0959cb1
1 changed files with 28 additions and 17 deletions
  1. 28 17
      scene/resources/packed_scene.cpp

+ 28 - 17
scene/resources/packed_scene.cpp

@@ -279,25 +279,34 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
 							Ref<Resource> res = value;
 							Ref<Resource> res = value;
 							if (res.is_valid()) {
 							if (res.is_valid()) {
 								if (res->is_local_to_scene()) {
 								if (res->is_local_to_scene()) {
-									HashMap<Ref<Resource>, Ref<Resource>>::Iterator E = resources_local_to_scene.find(res);
-
-									if (E) {
-										value = E->value;
+									// In a situation where a local-to-scene resource is used in a child node of a non-editable instance,
+									// we need to avoid the parent scene from overriding the resource potentially also used in the root
+									// of the instantiated scene. That would to the instance having two different instances of the resource.
+									// Since at this point it's too late to propagate the resource instance in the parent scene to all the relevant
+									// nodes in the instance (and that would require very complex bookkepping), what we do instead is
+									// tampering the resource object already there with the values from the node in the parent scene and
+									// then tell this node to reference that resource.
+									if (n.instance >= 0) {
+										Ref<Resource> node_res = node->get(snames[nprops[j].name]);
+										node_res->copy_from(res);
+										node_res->configure_for_local_scene(node, resources_local_to_scene);
+										value = node_res;
 									} else {
 									} else {
+										HashMap<Ref<Resource>, Ref<Resource>>::Iterator E = resources_local_to_scene.find(res);
 										Node *base = i == 0 ? node : ret_nodes[0];
 										Node *base = i == 0 ? node : ret_nodes[0];
-
-										if (p_edit_state == GEN_EDIT_STATE_MAIN || p_edit_state == GEN_EDIT_STATE_MAIN_INHERITED) {
-											//for the main scene, use the resource as is
-											res->configure_for_local_scene(base, resources_local_to_scene);
-											resources_local_to_scene[res] = res;
-
+										if (E) {
+											value = E->value;
 										} else {
 										} else {
-											//for instances, a copy must be made
-											Node *base2 = i == 0 ? node : ret_nodes[0];
-											Ref<Resource> local_dupe = res->duplicate_for_local_scene(base2, resources_local_to_scene);
-											resources_local_to_scene[res] = local_dupe;
-											res = local_dupe;
-											value = local_dupe;
+											if (p_edit_state == GEN_EDIT_STATE_MAIN || p_edit_state == GEN_EDIT_STATE_MAIN_INHERITED) {
+												//for the main scene, use the resource as is
+												res->configure_for_local_scene(base, resources_local_to_scene);
+												resources_local_to_scene[res] = res;
+											} else {
+												//for instances, a copy must be made
+												Ref<Resource> local_dupe = res->duplicate_for_local_scene(base, resources_local_to_scene);
+												resources_local_to_scene[res] = local_dupe;
+												value = local_dupe;
+											}
 										}
 										}
 									}
 									}
 									//must make a copy, because this res is local to scene
 									//must make a copy, because this res is local to scene
@@ -398,7 +407,9 @@ Node *SceneState::instantiate(GenEditState p_edit_state) const {
 	}
 	}
 
 
 	for (KeyValue<Ref<Resource>, Ref<Resource>> &E : resources_local_to_scene) {
 	for (KeyValue<Ref<Resource>, Ref<Resource>> &E : resources_local_to_scene) {
-		E.value->setup_local_to_scene();
+		if (E.value->get_local_scene() == ret_nodes[0]) {
+			E.value->setup_local_to_scene();
+		}
 	}
 	}
 
 
 	//do connections
 	//do connections