Fix CSGShape3D debug collision shapes being visible in editor

[MajorMcDoom]

CSGShape3D is improperly co-opting the debug collision visualization system to render editor gizmos. As such, the gizmos do not respect any gizmo visibility settings (see #87919 ).

The only way to control the visibility of the collision gizmos currently is to make the CSGShape3D itself invisible (workaround comes from #84174). This is incorrect because an invisible CSGShape3D can still have collision, so the gizmo should still be visible. This also affects runtime debug collision visualization (see #96767).

This PR makes the debug collision geometry only visible in the running game, essentially removing their usage as gizmos.

• Fixes Collision wireframes of CSGBox3D doesn't hide in the scene view when gizmos are hidden #87919
• Fixes CSGShape3D debug collision visualization disappears when node is made invisible #96767
• Represents a small step toward a proper gizmo solution by removing broken bandaid solutions and setting a clean slate.

[MajorMcDoom]

UPDATE:

This issue was previously reported, and was unfortunately misdiagnosed and led to an incorrect solution that introduces another problem that will have to be corrected (perhaps as part of this pull request):
Original issue report: #84132
Merged incorrect fix: #84174

The issue was misdiagnosed as being a gizmo that fails to hide. This is not the case - it is runtime debug collision shape visualization, not an editor gizmo.

The subsequent merge made the debug collision shape visibility dependent on the visibility of the CSG node itself. While this partially masked the problem of the previously unhideable wireframe (it is now hideable by hiding the CSG visuals), it still is not hideable via the gizmo toggles, and also leads to a new incorrect behaviour: As the documentation for the use_collision property states: making a CSG invisible does not disable the collision. This means the debug collision shape visualization should remain visible to accurately reflect the presence of the collision shape, regardless of the visibility of the CSG.

Because this new pull request will completely remove the debug collision shapes from the editor, the visibility dependency introduced by #84174 can be reverted. I will do this in a commit amend tomorrow.

[MajorMcDoom]

Aforementioned changes from #84174 have been reverted.

[fire]

Let us know if #84174 fixes your bug.

[MajorMcDoom]

Let us know if #84174 fixes your bug.

@fire See my two followup comments. That's a merge which actually misdiagnosed the problem and introduced another issue, which this merge would also fix.

[BastiaanOlij]

@MajorMcDoom it's worth adding screenshots to PRs like this to make clear what the change is.

I haven't followed the previous discussions, but this is an interesting one.

Traditionally Godot has always had the visual and physics part of an object separate. Collision shapes are drawn in editor as well. You can see this clearly when you move the collision shape away from the object:

You can hide the collision shapes however if they are in the way, something that doesn't effect physics for all the right reasons, but does catch people off guard.

CSG breaks the mould. It's a single node that handles both the visual and the physics aspect of the object. Showing the collision shape superimposed over the mesh is in a way expected behaviour, you can tell immediately whether you've forgotten to enable collisions or if they are indeed enabled.

However, if you're not interested there is no way to hide them, and especially for complex extruded shapes, it can be a lot of visual clutter not to mention that it slows the IDE down. I use CSGPolygon a lot to create stuff like race tracks, often the track itself will be several CSGPolygon shapes all following the same path but without collisions just to bring in detail, and a separate CSGPolygon shape that is used to create the collision shape. It gets pretty gnarly at times :)

While I find it important to be able to see the collision shape and would not want that removed, I'd love to hide it once I'm happy with the composition of the scene.

[MajorMcDoom]

@BastiaanOlij Thanks for the response!

I'm not sure if there's been a miscommunication somewhere, but I'm aware that CSG is a "hybrid" between visual and collision.

The problem is:

• CSG is currently using debug collision shapes (as in the feature you use to see collision shapes in the running game) as a gizmo.
• Because it isn't using gizmos, it of course breaks all the visibility rules that gizmos have, hence not being hideable in the editor by any means.
• This was previously reported in CSG collision shape editor gizmo does not hide #84132, and was improperly "fixed" by Hide CSGShape's debug_collision_shape when it is invisible #84174 by making the debug collision shapes (that are not supposed to run in the editor in the first place) hidden whenever the CSG itself is hidden.
• Hide CSGShape's debug_collision_shape when it is invisible #84174 introduces an additional issue of CSG invisibility now misleading the user into thinking the collision has been turned off, when it fact it has not. This is explicitly mentioned in the documentation for CSGs.

The debug collision shape is still important to have for CSG at runtime, because the feature should work for all collision shapes. That is why this PR still leaves that intact.

Whether or not a CSG should show collision gizmos is a discussion beyond the scope of this PR. The fact is, however, that they are currently not showing gizmos - they are improperly using debug collision shapes in place of gizmos.

[BastiaanOlij]

Whether or not a CSG should show collision gizmos is a discussion beyond the scope of this PR. The fact is, however, that they are currently not showing gizmos - they are improperly using debug collision shapes in place of gizmos.

Ah, ok, I indeed missed that nuance. Sounds like a shortcut was taken to make something draw. I'd have to take a closer look to give more sensible feedback :)

[BastiaanOlij]

Ok, as best as I can judge there are a few incorrect assumptions here which means I think a little more work needs to be done here.

When we look at normal collision shapes it is correct that the debug collision shape is drawn at runtime when this is enabled to help find issues with the collision setup in ones game.
However the same shape is also drawn in the IDE, as a transparent shape when the node is selected and as a grey wireframe when the node is not selected, to visualise the collision shapes in the scene. However you can turn this off in the view settings as this can be visually cluttering.

When we look at CSG the problem we have is that we have dual functionality here. The main purpose of CSG is the mesh that is generated and the gizmo controls focus on the manipulation of this mesh. The collision shape that can be added is "extra" but we lack control over whether we want to visualise this shape in the IDE.

This PR is born out of an understandable annoyance that this shape is always drawn, and drawn fully visible. It thus relegates this to the original functionality of just showing the shape during runtime if collision shapes are enabled in the debug menu.

There is however value in seeing the collision shape in the IDE at times and taking that away will frustrate users who rely on seeing that shape.
What is needed is similar functionality as exists on the collision shape node itself. E.g. it should be drawn less obvious when the CSG node is not selected, and you should be able to toggle this on or off.
If that can be added to this PR, I think this would be good functionality to have.

[MajorMcDoom]

@BastiaanOlij is right in that there needs to be a proper gizmo solution for CSGShape3D at some point, but I would like to address an inaccuracy in his assessment regarding the preservation of existing use cases:

There is actually currently no use at all for the CSGShape3D collision visualization as a separate gizmo because:

• CSG visual geometry is identical to the collision geometry, so visualization is already possible.
• For the use case of visualizing collision-only CSGs, the aforementioned improper fix (Hide CSGShape's debug_collision_shape when it is invisible #84174) has already made it impossible, by making the collision geometry visualization dependent on CSG visibility.

In other words, the current gizmo solution is entirely useless because it only renders a wireframe over an identical underlying mesh, and only when the underlying mesh is already visible.

As such, this PR:

• Does not break any existing use cases (they are already broken).
• Fixes two UX bugs.
• Represents a small step toward a proper gizmo solution by removing broken bandaid solutions and setting a clean slate.

[BastiaanOlij]

Speaking to @MajorMcDoom on private channels a couple days ago and his further findings reported here, I'm in agreement that the current state of things are as he says and that in order to move forward, fixing one problem at a time is better than endlessly flipping back and forth.

We just have to be mindful that if anyone complains that we don't go back to a more broken situation but make clear that we will be taking steps after this step.

So I'm for merging this and making sure that the current behaviour makes sense, and then in the future following it up with a proper gizmo implementation for CSG collisions.

[akien-mga]

Thanks!

[MajorMcDoom]

For posterity, I've made a follow-up issue here: #96807

[ydeltastar]

Can this be cherry-picked for 4.3? The debug draw noise in a level full of CSG composites is very high and it can't be disabled.