Debug "Visible Collision Shapes" checkbox creating "MeshInstance"

[shafnaz]

Godot version:
3.3

OS/device including version:
Windows 64 bit

Issue description:

MeshInstance is created for each collision shape when checking the "Visible Collision Shapes" in Debug Menu

Steps to reproduce:
Just import the zip .tscn or recreate the scene with the nodes as shown in the images above. Run it! and check the "Remote" view tree.

Minimal reproduction project:
issue.zip

[Zireael07]

Not a bug - in order to draw the debug shape, it needs to create a mesh instance.

[shafnaz]

But this is new in 3.3. Now, when I have to do a for loop looking for nodes, it will give errors because it's passing by that node too.

[trollodel]

This was introduced by #46397.
Before, the MeshInstance used to show the shape was a CollisionShape child. But I move them at the CollisionObject level (the StaticBody in your case).

There are several solutions to this issue:

• Do nothing (my favourite 😀) and let the user handle it (you can use get_tree().is_debugging_collisions_hint())
• Document this behavior in the CollisionObject description
• Move all the MeshInstances to an intermediate Spatial. Still, you must handle that extra node
• Move the MeshInstance back to the CollisionShape (introducing a special case for it)

[pouleyKetchoupp]

It would probably make sense to avoid adding nodes used for debug to the tree, actually.

Maybe we could have something like:

• Instead of creating a mesh instance node, CollisionObject creates an instance from the visual server directly (the same way it is done for contact points in Viewport)
• Expose get_debug_mesh() from Shape resource to provide a proper way for scripts to access debug shapes for custom rendering (I know I need it at least for 3d physics tests and I'm doing it by extracting the information from the mesh instance currently)

[trollodel]

Instead of creating a mesh instance node, CollisionObject creates an instance from the visual server directly (the same way it is done for contact points in Viewport)

Sounds good, and I'm wiling to do a PR for this (I never use VisualServer APIs before, so I can take a while to do it).

Expose get_debug_mesh() from Shape resource to provide a proper way for scripts to access debug shapes for custom rendering (I know I need it at least for 3d physics tests and I'm doing it by extracting the information from the mesh instance currently)

IMO this is another issue because it'd make #46397 obsolete.

[pouleyKetchoupp]

@trollodel That would be great if you're up for making the PR! It shouldn't be too complicated to do. You would just work with a RID instead of a mesh instance. Creation would be similar to this (using the mesh from the shape instead of the multimesh):

godot/scene/main/viewport.cpp

Lines 458 to 460 in c7b97f0

	contact_3d_debug_instance = RenderingServer::get_singleton()->instance_create();
	RenderingServer::get_singleton()->instance_set_base(contact_3d_debug_instance, contact_3d_debug_multimesh);
	RenderingServer::get_singleton()->instance_set_scenario(contact_3d_debug_instance, find_world_3d()->get_scenario());

And destroying would be like this:

godot/scene/main/viewport.cpp

Line 515 in c7b97f0

RenderingServer::get_singleton()->free(contact_3d_debug_instance);

You can test with the VisualServer equivalent on 3.x first (contact points are currently not working on master, so it's just to be sure you're not stuck because of a similar bug in the new rendering system).

Also my fix from #47848 using NOTIFICATION_PREDELETE wouldn't be needed anymore, since CollisionObject would manage its debug shapes without interacting with the tree.

IMO this is another issue because it'd make #46397 obsolete.

Yeah, it's fine. This can be changed separately.

[trollodel]

@pouleyKetchoupp Thanks for the guidance!

[Xrayez]

Yeah, if it's possible to use servers API directly, that's the first thing to try when coding in C++. Control nodes also suffer from this "extra nodes" issue unfortunately. @KoBeWi previously proposed to implement "internal nodes" feature to handle cases like this, see PR #30391.

[OlexiyKravchuk]

I have the same problem.
As a temporary solution, it is better to make the mesh instances the children of the collision shape nodes rather than their siblings.

[OlexiyKravchuk]

By the way, there is probably a reason for another error report, but my collision shape changes dynamically, but after changing the collision shape, the rendering mesh serving just remains a dead weight without changes and completely useless confusing.

[pouleyKetchoupp]

By the way, there is probably a reason for another error report, but my collision shape changes dynamically, but after changing the collision shape, the rendering mesh serving just remains a dead weight without changes and completely useless confusing.

@OlexiyKravchuk Yes, please open an issue with a minimal project and repro steps so it can be investigated.

[OlexiyKravchuk]

Yes, please open an issue with a minimal project and repro steps so it can be investigated.

@pouleyKetchoupp ready

#48281