Fix empty and invisible CSG shapes.

[madmiraal]

Currently, there is inconsistency in how CSG shapes create collision shapes when they or their components are invisible or change their visibility:

#40919 significantly improved this, but some inconsistency remains:

This PR makes the collision shapes consistent with the CSG shape:

Fixes #43251.

For those that are interested, the project used to create the above tests is: 43251.zip. It consists of 20 tests: every combination of changing the visibility of a platform or part thereof and a RigidBody ball above it to test the CollisionShape.

There are two layers. The top layer creates the shapes with the toggled components visible. The bottom layer creates the shapes with the toggled components invisible. Then the visibility of the CSG shape, or a part of it, is toggled every two seconds.

Each layer has three rows. The front row of each layer has three shapes that consist of a single shape. The second row of each layer has three shapes that consist of double shapes created from two shapes without a CSGCombiner. The back row of each layer has four shapes that consist of double shapes created from two shapes inside a CSGCombiner.

In the front row, the left two shapes are contained within a CSGCombiner; the right one is on its own. The left shape (Test1 and Test11) toggles the visibility of the CSGCombiner. The middle shape (Test2 and Test12) toggles the visibility of the shape inside the CSGCombiner. The right shape (Test3 and Test13) toggles the visibility of the shape.

In the middle row, the left two shapes toggle the visibility of the parent shape; the right shape toggles the visibility of the child shape. In the left shape (Test4 and Test14) the ball is above the parent shape. In the middle shape (Test5 and Test15) the ball is above the child shape. In the right shape (Test 6 and Test16) the ball is also above the child shape.

In the back row, the left two shapes toggle the visibility of the CSGCombiner; the right two shapes toggle the visibility of a child shape. In the left most shape (Test7 and Test17) the ball is above the first child. In the second to left most shape (Test8 and Test18) the ball is above the second child. In the second to right most shape (Test9 and Test19) the ball is above the first child. In the right most shape (Test10 and Test20) the ball is above the second child.

Finally, there was a discussion in #43251 about what the right solution was. Two options were floated:

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.

This PR implements option 1. However, @hoontee preferred option 2 and created #43322 to implement that approach. I think option 1 is the better solution, because it is more intuitive. Furthermore, if a user has a need for separating the VisualInstance and the CollisionShape, they can already do this: they literally create two separate meshes and, with one, enable Use Collision and remove it from the VisualInstance layer. To demonstrate this I've recreated the project presented in #43322: 43251a.zip. This approach has the added advantage of not constraining the differences to the components of the CSG shape.

[hoontee]

✓ You will need to update the docs to reflect these changes:

[hoontee]

The difference between our approaches boils down to keeping or discarding collision with invisible shapes.

The reason I don't agree with this approach is because it's inconsistent with the behavior of all other collisions in the engine. StaticBodies, RigidBodies, KinematicBodies, Areas, RayCasts... they all work if visible = false. This PR would make it so the only way to achieve this behavior with CSGShapes is by changing the VisualInstance layer. It works, but it's inconsistent.

[Janders1800]

Changing layers is not an option on GLES2. I don't think this is a good fix, I much prefer the #43322 approach.

EDIT: Nvm I've mistaken visual layers for light layers.

[madmiraal]

Updated the documentation to drop the caveat.

[madmiraal]

The reason I don't agree with this approach is because it's inconsistent with the behavior of all other collisions in the engine. StaticBodies, RigidBodies, KinematicBodies, Areas, RayCasts... they all work if visible = false.

Perhaps they shouldn't, but the point is: CSG shapes have a useful feature that allows us to create a CollisionShape from the VisualInstance by simply checking a tick-box. A CollisionShape that can be manipulated as easily as the CSG shape. The alternative is to change the behaviour to create a separate CollisionShape the way MeshInstance does. This would provide the same behaviour, but then the CollisionShape would no longer be easily manipulable.

However, the root of the problem is, no matter how it is implemented, there will always be inconsistent behaviour. Visibility cannot be used to simply prevent a CSG component from being drawn, because depending on the Operation, faces will be both added and removed. Therefore, it may as well be used to control how the mesh is created. A mesh that, if used to create a CollisionShape, is consistent with the VisualInstance.

[hoontee]

OK, but what about toggling the root shape's visibility? Currently (3.2.3, not 3.2.4 beta 1) it's possible to hide it with visible = false and enable collision with use_collision = true as intended from the documentation. Do you think it's a good idea to remove this exclusion?

This is what #40814 intended to do; fix the problems without sacrificing function. The root shape deserves the ability to be hidden without disabling collision as no changes to the shape are taking place.

[hoontee]

Also, and I should clarify why this I'm contesting this PR; for my project I'm toggling visibility of Spatials which contain mixed collision types including CSG.

With this PR, my RigidBody props will fall through the floor if using visible, and it's cumbersome to change the VisualInstance layer of any descendant CSG shapes.

I really hope you may review #40814 and give feedback or make changes, for it's what I believe to be the best of all worlds here.

[hoontee]

Testing your demo project with #40814 yields the following results:

For the sake of the demonstration, I've added balls to the other halves of the back two rows.
43251 4.0.zip

Only toggling a child shape's visibility should affect collision. Thus, the ones highlighted in pink should have their collision updated, which are as follows:

• Test2: Toggles the visibility of the shape inside the CSGCombiner.
• Test6: Toggles the visibility of the child shape.
• Test9: Toggles the visibility of the first child shape.
• Test10: Toggles the visibility of the second child shape.

The most important reason why I believe #40814 to be preferable over this PR is consistency over collision with invisible objects. No other nodes lose collision if you hide them.

Secondly, and almost as important, #40814 aims to mitigate stuttering caused by unnecessary regeneration of shapes when toggling the root shape's visibility. This requires that root shapes never recalculate when their visibility is toggled.

It's been many months now without an approved solution for those issues, and the only thing preventing any sort of fix is our disagreement on the approach.