godotengine / godot

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

CollisionObject3D doesn't check for `CSG*` objects with `Use Collision` enabled in their collider warning check. #85027

Open mikest opened 11 months ago

mikest commented 11 months ago

Godot version

4.2.rc1

System information

Godot v4.2.rc1 - Windows 10.0.22621 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4090 (NVIDIA; 31.0.15.3598) - 13th Gen Intel(R) Core(TM) i9-13900K (32 Threads)

Issue description

Minor nit, the warnings for shapes don't check for colliders on CSG objects.

image

Collision is enabled:

image

Steps to reproduce

Create an Area3D with a CSGCylinder3D in it and turn on "Use Collision". The warning persists.

Expectation: the warning should go away.

Minimal reproduction project

N/A

brno32 commented 11 months ago

confirmed this on master

AThousandShips commented 11 months ago

This is not a bug, see the documentation of the opinion:

This will always act like a static body.

You are effectively adding a static body, not a collision shape

mikest commented 11 months ago

No, if the Use Collision is enabled, a collision object is generated, and collisions are performed. This is also in the docs. The warning is in error, as a CSGObject with Use Collision enabled has a collision object.

The warning produces spurious noise in the UX that draws attention to a problem that doesn't exists.

AThousandShips commented 11 months ago

No, it requires a shape not a collision object, it generates a body

The warning is not in error, the CSG shape generates a body

Please see the documentation and here

It generates a collision object, not a shape

mikest commented 11 months ago

Okay, let's try this another way: what should I change to make the warning go away? I only have CSG objects in the Area3D. The code works at run time, my Area3D can perform collisions.

What is a user to do with the warning other than ignore it?

AThousandShips commented 11 months ago

Nothing, you're not setting things up right, you shouldn't do this, you should use collision shapes with Area3D

You are setting things up in an incorrect way, that's why you get a warning, it doesn't work correctly, try for example detecting collisions like body_entered from the Area3D if something intersects with the CSG shape

Please do not valid ignore warnings

mikest commented 11 months ago

The documentation you provided seems to support my point: there is, in fact, a collision shape associated with the Area3D. Which would mean this warning is incorrect:

image
AThousandShips commented 11 months ago

No, it does not, please listen to me and read the documentation again:

This will always act like a static body.

A static body, not a collision shape, as you can see in the code: https://github.com/godotengine/godot/blob/fa1fb2a53e20a3aec1ed1ffcc516f880f74db1a6/modules/csg/csg_shape.cpp#L46-L63

The wording should be improved to avoid confusion

mikest commented 11 months ago

What's holding my character up? He's not falling through the floor. So.... seems like he is "interacting with other objects".

image

Perhaps "interaction" is referring to only the detection signals, not the physics engine collisions? If that's the case, then this warning could stand to use some improvement. A naïve user would rightly expect "Interact" when referring to a collider to be the collision.

I think what you are saying is that the collision behavior has nothing to do with the CSG* shapes being in the Area3D, and that the signal handlers for notifications won't work?

mikest commented 11 months ago

The wording should be improved to avoid confusion

I think we're on the same page :)

To your point, I removed the Area3D and put my CSG objects unparented, into the root of the scene. Everything behaves the same, so I get what you mean by "static body, not collisionshape"

AThousandShips commented 11 months ago

A naïve user would rightly expect "Interact" when referring to a collider to be the collision.

I disagree, they should listen to the documentation and not assume the warning is wrong

I think what you are saying is that the collision behavior has nothing to do with the CSG* shapes being in the Area3D, and that the signal handlers for notifications won't work?

That's literally what I said yes 🙂

mikest commented 11 months ago

Thanks for taking the time to clarify!

AThousandShips commented 11 months ago

You're welcome! I'll look at improving the documentation tomorrow

Been busy and won't be able to focus on this one at the moment

SK1PPR commented 10 months ago

Hello @AThousandShips I would like to contribute for this issue. If possible could you assign it to me and tell me what to do next, as I am new to open source contribution and projects.

AThousandShips commented 10 months ago

See here 🙂

ghildim commented 1 month ago

Hey same boat, new to open source what can I do next?

AThousandShips commented 1 month ago

See the link above

rishikiram commented 1 month ago

Made a pull request to update the documentation of CSGShape3D, use_collision() method. see here #97008