Broken collision with invisible CSGCombiners

[hoontee]

Godot version:
3.2.4.beta1 - master

OS/device including version:
Windows 10 2004

Issue description:
Invisible CSGCombiners with use_collision = true will not be collidable.

Minimal reproduction project:
Invisible CSGShape Collision.zip

[Janders1800]

Yep, happens to me as well, they also trow null on _on_body_entered / exited.

[hoontee]

@madmiraal This issue must have been caused by 920f8c8, but it's definitely fixed in #40814.

[madmiraal]

I think this is the desired behaviour i.e. it was fixed with #40919. If any of the components of a CSG shape are made invisible, they are no longer part of the shape. A CSGCombiner groups components, so if it's made invisible the expectation is that it's equivalent to making all of its components invisible.

To demonstrate this, I've slightly modified the MRP, where instead of making the CSGCombiner invisible its CSGBox component is made invisible. The behaviour is the same in 3.2.3.stable and 3.2.4.beta1 and now, with 3.2.4.beta1, the same as making the CSGCombiner (or the only shape of a CSGCombiner) invisible.
43251.zip

[hoontee]

@madmiraal

I think this is the desired behaviour

No, all CSGShapes alone will have collision if made invisible. Additionally, collision in StaticBodies, RigidBodies and KinematicBodies will work if invisible.

The reason making the child CSGBox invisible in your example makes sense is because the CSGCombiner must skip it in its final mesh calculation.

so if it's made invisible the expectation is that it's equivalent to making all of its components invisible

This is the bug, and #40814 intends to fix it.

[hoontee]

Also, an error when invisible:

[Janders1800]

I dont know if that is the desired behaviour but, having invisible CSGShapes collisions is the only way to get around the KinematicBody edge collision bugs; #34436, if you go for a modular level design approach.

[madmiraal]

No, all CSGShapes alone will have collision if made invisible. Additionally, collision in StaticBodies, RigidBodies and KinematicBodies will work if invisible.

The reason making the child CSGBox invisible in your example makes sense is because the CSGCombiner must skip it in its final mesh calculation.

It should be consistent. If making a CSG component invisible should only prevent it from been drawn (the documented behaviour), but not affect the CollisionShape, then the example I provided above shouldn't happen either.

However, currently, visibility does not prevent the CSG component from being drawn. Visibility is used to control which CSG components are included. It affects the mesh directly; resulting in faces being both added and removed depending on the Operation. Since a CSG object only has one mesh for both the VisualInstance and the CollisionShape both are affected.

The way I see it, we have two options:

1. Keep one mesh, collisions only happen with visible CSG components i.e. the bug is that invisible individual CSG Shapes are collidable when they shouldn't be.
2. Have separate meshes for the VisualInstance and the CollisionShape. The parameters visible and use_collision could be used to control which CSG components are visible and which are collidable.

[hoontee]

2. Have separate meshes for the VisualInstance and the CollisionShape. The parameters visible and use_collision could be used to control which CSG components are visible and which are collidable.

I would vastly prefer this over the former option considering my current use cases, but it would break compatibility. I will make a new PR for this.

However, for 3.2's sake, are you favorable of merging #40814 or would you prefer the aforementioned PR proposal?

[hoontee]

Still an issue despite multiple PRs addressing it. Can we decide on whether #40814 or #43381 gets merged?