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

Illegal recursion allowed through PackedScene #84037

Open
warent opened this issue Oct 27, 2023 · 4 comments
Open

Illegal recursion allowed through PackedScene #84037

warent opened this issue Oct 27, 2023 · 4 comments

Comments

@warent
Copy link

warent commented Oct 27, 2023

Godot version

v4.1.2.stable.official [399c9dc]

System information

Godot v4.1.2.stable - Windows 10.0.22621 - Vulkan (Mobile) - dedicated NVIDIA GeForce RTX 3090 Ti (NVIDIA; 30.0.15.1216) - AMD Ryzen 9 5950X 16-Core Processor (32 Threads)

Issue description

When you create a Resource that @export a variable, it will not allow a circular reference through other Resources.

However, you can circumvent this and break Godot by creating the circular reference through a PackedScene such that the structure looks like the following:

ExampleResource:
@export my_scene: PackedScene

MyScene
@export my_resource: ExampleResource

i.e.
ExampleResource -> MyScene -> ExampleResource

You can save and run the project, but "MyScene" thinks that my_resource is null even if it is set in the editor. When you exit the project and try to re-open it, you get the following error:

  scene/resources/resource_format_text.cpp:283 - res://MainScene.tscn:8 - Parse Error: 
  Failed loading resource: res://MainScene.tscn. Make sure resources have been imported by opening the project in the editor at least once.
  editor/editor_data.cpp:626 - Index p_idx = 1 is out of bounds (edited_scene.size() = 1).

From this point, you can no longer open MyScene or ExampleResource.

Steps to reproduce

  1. Create a script that extends Resource (e.g. MyResource)
  2. Have that script export a variable of type PackedScene
  3. Create a Scene (e.g. named MyScene)
  4. Attach a script to the scene that exports a variable of the same type as the resource you created above
  5. Create a new instance of the resource (e.g. MyResourceInstance)
  6. In MyResourceInstance, attach MyScene to the exported variable field
  7. in MyScene, attach MyResourceInstance to the exported variable field

Minimal reproduction project

ResourceRecursion.zip

@warent
Copy link
Author

warent commented Oct 27, 2023

note for #80877

@warent
Copy link
Author

warent commented Oct 27, 2023

Note that I would actually like this to be allowed. My use-case is for example defining "Item" resources. These Items can appear in the shop, and when the user purchases them they instantiate the referenced PackedScene. I want the PackedScene to easily know which Item it belonged to, therefore I want the script associated with it to also reference back to the same resource.

In other words, the shop has a list of ItemResources which reference PackedScenes. These PackedScenes (which are ingame representations of the purchased item) would like to reference back to the same ItemResource to know what data they are supposed to represent.

@Caellian
Copy link

Caellian commented Jun 30, 2024

I have a simpler case where I want to link doors/portals in two scenes by selecting map resource they're supposed to load. They're bidirectional and shown in the inspector which causes the same issue.

So having just a normal Node2D which @export a Resource in two scenes causes the same recursion issue.

The editor knows the resource isn't broken and if the first scene is already loaded I can connect them, so I'm guessing fixing this likely requires resources to be loaded without recursively resolving them by first loading all linked resources and then connecting the graph as a subsequent step.

As a workaround I'm using file names and formatting those into a cached load() call. This isn't ideal because renaming the files doesn't update these references.

This makes me also think that maybe some WeakResource type could be a viable workaround.

@dxdesjardins
Copy link

dxdesjardins commented Oct 11, 2024

There is a proposal discussing introducing an Exported WeakReference that would theoretically solve this issue. Unfortunately it doesn't seem to be gaining much traction. I've posted a temporary non-ideal solution on that thread.

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

No branches or pull requests

4 participants