godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
86.16k stars 19.16k forks source link

No warning on recursion of mesh in array mesh #93486

Open AskSkivdal opened 6 days ago

AskSkivdal commented 6 days ago

Tested versions

System information

Godot v4.3.beta2 - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 (NVIDIA; 32.0.15.5599) - AMD Ryzen 7 3700X 8-Core Processor (16 Threads)

Issue description

Godot lets you use an array mesh as its own shadow mesh, therby allowing recurtion. When opening the inspector on that perticular mesh instance 3d the editor hangs and eventualy closes without any explenation in the logs.

Steps to reproduce

  1. Create a project
  2. Make a scene and add a MeshInstance3D node to it
  3. Set the mesh to be an array mesh
  4. In a toolscript set the shadow_mesh to be the mesh
    $MeshInstance3D.mesh.shadow_mesh = $MeshInstance3D.mesh
  5. Run the script
  6. Open the MeshInstance3D in the inspector
  7. Crash

Minimal reproduction project (MRP)

crashoncyclicmesh.zip

ChristopheClaustre commented 4 days ago

What do we want here ? Should it be possible to set as shadow mesh the mesh itself ?

If yes, is there such case in another class and how is it handled ?

AskSkivdal commented 4 days ago

I feel like the bare minimum would be feedback in the logs about why a crash occured.

AThousandShips commented 4 days ago

It's not always possible, an infinite recursion is really a hard case, but if you're running a debug version you should get crash logs if you run from console

ChristopheClaustre commented 4 days ago

image it crash in an infinite call on Resource::hash_edited_version_for_preview()

ChristopheClaustre commented 4 days ago

This first crash case can be workedaround I think but hard to tell how much other cases it hides.

ChristopheClaustre commented 4 days ago

After a quick (and probably dirty) workaround on first crash, I get this more problematic crash (still stack overflow): image

If I understand well, we fetch all properties recursively when we want to show an object in inspector. I don't think it is specific to MeshInstance, it could happen anytime there is a cycle in property like it is the case here. @clayjohn I think that topic is not rendering but editor.

We can detect a cycle in property at the cost of memory when fetching properties for inspector but now we have to determine how we want the interface to behave when it happens ?