Instanced Sprite with ViewportTexture produces infinite error spam

[KoBeWi]

Godot version

3.4 / 4.0 092a286

System information

W10

Issue description

When you have a scene with Sprite root and assign a ViewportTexture to it, this happens when you instance it in another scene:

Steps to reproduce

1. Create a Sprite2D with ViewportTexture
2. Save it as scene
3. Instance it into another scene

Minimal reproduction project

ReproductionProject.zip
Open Spam.tscn and move cursor over 2D viewport.

[HubbleCommand]

I'm guessing that you have added a Viewport as a child of the root Sprite2D? Otherwise I don't know how you were able to set a Viewport Texture.

Before I start getting Viewport Texture must be set to use it, I get

Cannot get path of node as it is not in a scene tree.
(Node not found: "Viewport" (relative to "").)
ViewportTexture: Path to node is invalid.

So Viewport Texture must be set ... makes sense and is expected as the Viewport cannot be found. From more debugging this error is caused by a call to get_size internally.

If I look at the instanced node, the Viewport Path is still just Viewport. I'm guessing that Godot is trying to get a Viewport at /Level/Viewport instead of /Level/Sprite/Viewport. If there's a bug in this issue, my guess is that it's here, expecially after reading the following from the documentation about the viewport_path variable : The path to the Viewport node to display. This is relative to the scene root, not to the node which uses the texture.

So technically speaking, this isn't actually a bug as it's the documentation's expected behavior, although I find it weird...

I fixed this with a few lines of code in ready to set the new viewport_path of the instanced node:

tool
extends Sprite

func _ready():
	var vpText = ViewportTexture.new()
	vpText.viewport_path = get_path() + "/Viewport"  #Get the path of the sprite in the scene, add the path to the viewport
	texture = vpText;

Note that get_path() returns an EditorNode with tool (as it runs in the editor), but should work fine in game.

edit: I kept tool to test in the editor. If you always instance the node at the same path while in the editor you can just replace get_path() with "expected path" + name + "/Viewport"

[HubbleCommand]

I recommend removing the bug tag as this is actually the expected behavior, detailed in my previous comment

[KoBeWi]

I don't really get how local to scene works. I assume it uses owner to determine the root node and it behaves differently with instanced root. I'd have to take a closer look, but for now documenting that you can't use ViewportTexture in a root node (unless it's not instanced) should suffice.

[Shadowblitz16]

Please fix this. I ran into this bug and its very annoying.
Just because a it's root doesn't mean it shouldn't be valid

[MarianoGnu]

I think error description should at least be more clear on what is causing the failure, this way it can be settled up propperly

[MarianoGnu]

I don't really get how local to scene works. I assume it uses owner to determine the root node and it behaves differently with instanced root. I'd have to take a closer look, but for now documenting that you can't use ViewportTexture in a root node (unless it's not instanced) should suffice.

local to scene will make a unique duplicate of the resource on _init of the node, this allows in example to have unique instances of materials so you can change their properties on every node without sharing the same material on all of them

[KoBeWi]

No my confusion was about something else (I know about the duplicating behavior). local_to_scene seems to also set an owner for the resource, which is the scene root (owner variable). Root node is a special case, because its owner is either null or the owner of the parent scene. This is why this fails when used in a root node.

[Rindbee]

godot/doc/classes/Resource.xml

Lines 70 to 72 in 736e3b7

	<member name="resource_local_to_scene" type="bool" setter="set_local_to_scene" getter="is_local_to_scene" default="false">
	If [code]true[/code], the resource will be made unique in each instance of its local scene. It can thus be modified in a scene instance without impacting other instances of that same scene.
	</member>

I'm not sure if the above instance refers to the instance of the resource or the instance of the sub-scene.

But in any case, it makes sense to limit the resource_local_to_scene-enabled resources of the sub-scene root node to those owned by the sub-scene, even though the modifications might be recorded in the main scene's tscn file.

If the above instance refers to the instance of the sub-scene, I think #64388 (solve the problem when ViewportTexture is set multiple times), #64487 (solve the problem when copying resources), #64526 (solve the problem when setting resources during initialization) should solve this problem.

godot/scene/resources/packed_scene.cpp

Lines 277 to 306 in dbd1524

	if (value.get_type() == Variant::OBJECT) {
	//handle resources that are local to scene by duplicating them if needed
	Ref<Resource> res = value;
	if (res.is_valid()) {
	if (res->is_local_to_scene()) {
	HashMap<Ref<Resource>, Ref<Resource>>::Iterator E = resources_local_to_scene.find(res);
	if (E) {
	value = E->value;
	} else {
	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;
	} 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;
	}
	}
	//must make a copy, because this res is local to scene
	}
	}
	} else if (p_edit_state == GEN_EDIT_STATE_INSTANCE) {

The code above seems to show that resources with resource_local_to_scene enabled can be the same. This is one of the reasons why I lean towards the previous explanation. Another reason is the property name, resource local to scene.

If not then I may need to modify #64487 and #64526. How about renaming resource_local_to_scene to resource_unique_when_instantiate?

resource_local_to_scene was introduced in this commit a503f8a, I didn't find any further instructions.

[kpkpkpkpkpkpkpkp]

Not sure where this stands, but it seems to me like the problem is that the node path for viewports can't be relative. They always refer to the path via the root node as mentioned;

If I look at the instanced node, the Viewport Path is still just Viewport. I'm guessing that Godot is trying to get a Viewport at /Level/Viewport instead of /Level/Sprite/Viewport. If there's a bug in this issue, my guess is that it's here, expecially after reading the following from the documentation about the viewport_path variable : The path to the Viewport node to display. This is relative to the scene root, not to the node which uses the texture.

Since instancing always changes the root, it's always going to cause the problem. Making the resource local to scene and correcting the path does fix the error, but then it would mean all instanced viewport textures need to be set individually. The tool script does work, but it seems like a baindaid to make the path relative.

If it were relative by default, (like other node path properties, AnimationPlayer's "root" for example) you wouldn't even have to make things local to scene because the texture would still be able to find the viewport when it's instanced. Maybe there's some reason that it has to be relative to the scene root?