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.

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

Local to Scene SubResource Ids change after saving #76693

Closed
ChristopherWalde opened this issue May 3, 2023 · 12 comments · Fixed by #65011
Closed

Local to Scene SubResource Ids change after saving #76693

ChristopherWalde opened this issue May 3, 2023 · 12 comments · Fixed by #65011
Milestone

Comments

@ChristopherWalde
Copy link

Godot version

4.0.2 & 4.0.3 RC1

System information

Windows 10, Vulkan API 1.3.224, NVIDIA RTX A2000 Laptop GPU

Issue description

When saving a scene containing another recently saved instatiated scene with one or more resources being local to scene, Godot changes the affected Resource ID. This only happens if the Node containing the resource being local to scene is the root.
This Occurance is espacially problematic when working in a team with any source control solution.

Steps to reproduce

  • Create a new scene containing a MeshInstance3D node and make it the scene root.
  • Assign a mesh and set the property resource_local_to_scene to true
  • Create a second scene and instantiate the scene containing the MeshInstance3D node
  • Without needing to change either scenes, save the MeshInstance3D scene (or any other instantiated child scene) first, then the scene where the MeshInstance3D scene is initiated

Minimal reproduction project

IdChange.zip
In this example saving either child scene before the main scene will cause the error, but only the IDs of the Node labeled "ChanginIDs" will change.

@AThousandShips
Copy link
Member

To clarify this happens every time the scene is saved? Not just the first time?

@AThousandShips
Copy link
Member

Also what "resource Id" are you talking about here, the id tag in the .tscn file?

@ChristopherWalde
Copy link
Author

To clarify this happens every time the scene is saved? Not just the first time?

Exactly, you can reproduce the id change infinite times with the same scenes.

Also what "resource Id" are you talking about here, the id tag in the .tscn file?

Speaking about the example above, the id of the BoxMesh changes in the level.tscn where the MeshInstance3D is instantiated.
In this case line 5 and 12 "gc1du":

carbon

@AThousandShips
Copy link
Member

Indeed, it only happens when the mesh in question is in the root of the other scene, so must be something related to that

@AThousandShips
Copy link
Member

It is probably because it fetches the local ID from the other scene, and then finds it does not exist in the current scene and therefore has to regenerate it

@AThousandShips
Copy link
Member

It appears to be happening here, but will investigate a bit further:

if (res->is_local_to_scene()) {
// 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]);
if (node_res.is_valid()) {
node_res->copy_from(res);
node_res->configure_for_local_scene(node, resources_local_to_scene);
value = node_res;
}
} else {
HashMap<Ref<Resource>, Ref<Resource>>::Iterator E = resources_local_to_scene.find(res);
Node *base = i == 0 ? node : ret_nodes[0];
if (E) {
value = E->value;
} else {
if (p_edit_state == GEN_EDIT_STATE_MAIN) {
//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
}

@ChristopherWalde
Copy link
Author

ChristopherWalde commented May 3, 2023

First of all, thank you for looking into the problem.

What you are saying is very good to know, but isn't the usecase of the property local_to_scene exactly this scenario? For instance I could have a scene where a Mesh follows a Path, like traintracks for example. If I want to create a level consisting of multiple tracks with different Curve resources, that do have a standard size, I would assign a default curve resource and set it to local to scene. Therefore I can change each track without manipulating every other track.

This at least sounds like the right way to aproach this, but I could very well be mistaken and the best practice is to assign the resource after instantiating a scene.

@AThousandShips
Copy link
Member

My bad I was confused in my first comment and that's why I deleted it, this particular behavior should not be breaking and will see if there's some bug here that can be resolved

@ChristopherWalde
Copy link
Author

No worries, thanks for looking into it.

@rakkarage
Copy link
Contributor

rakkarage commented May 17, 2023

in recent custom 4.1 builds it is regening every ext id on every save even if just one simple file

[gd_scene load_steps=2 format=3 uid="uid://bj3w1run711x7"]

[ext_resource type="Texture2D" uid="uid://c082xolc76p8n" path="res://icon.svg" id="1_ftki2"]

[node name="Node2D" type="Node2D"]

[node name="Sprite2D" type="Sprite2D" parent="."]
position = Vector2(351, 294)
texture = ExtResource("1_ftki2")

can use a script like this save all files and regen all ext ids in project

@tool
extends EditorScript

func listFiles(path: String, extensions: Array = []) -> Array:
	var list := []
	for dir in DirAccess.get_directories_at(path):
		if not dir.begins_with("."):
			list.append_array(listFiles(path + dir + "/", extensions))
	for file in DirAccess.get_files_at(path):
		var ext = file.split(".")[-1]
		if not file.begins_with(".") and ext != "import":
			if not extensions.is_empty() and ext not in extensions:
				continue
			list.append(path + file)
	return list

func _run() -> void:
	for file in listFiles("res://", ["tscn", "tres"]):
		ResourceSaver.save(load(file))

in 4.0.2 this script does not regen every ext id in the project each time it is run (just when needed) but in latest 4.1 it does
but just ctrl+s works'nt too

thanks

@AThousandShips
Copy link
Member

Note that while this is also an issue this is a separate problem and should probably be tracked separately as it likely has a separate fix

@Rindbee
Copy link
Contributor

Rindbee commented Jun 20, 2023

The resource of local_to_scene uses a copy when the scene is instantiated, and the copy generated a new id during pack. That's why the id keeps changing. We can make the copy use the previous id when the scene is instantiated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants