Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowed sensors to collide with one another #432

Merged
merged 2 commits into from
Feb 12, 2023

Conversation

mihe
Copy link
Contributor

@mihe mihe commented Feb 9, 2023

I figured I could ask this question in the form of a PR. :)

I noticed that sensors are unable to detect other sensors overlapping with them. I realize this is probably an unusual thing to want to have, but it is none the less something that Godot offers, so I must try to implement it.

I dug through the code a bit and found the lines that I've removed in this PR. Removing them seems to unlock the ability to receive sensor-to-sensor contact reports.

Is there a particular reason why this should be there, considering that the existing filtering mechanisms, like ObjectLayerPairFilter, have the opportunity to filter out those collisions already?

Feel free to push back on this if it seems too niche of a use-case. It's a small enough change that I'd be happy to maintain it on my own fork, assuming it even makes sense to begin with.

@jrouwe
Copy link
Owner

jrouwe commented Feb 10, 2023

Hello,

So you're not the first person to mention this: #282

Do you know what this is used for in godot?

@mihe
Copy link
Contributor Author

mihe commented Feb 10, 2023

Oh, I figured it was such a niche thing that I didn't even bother searching for an existing issue/discussion/PR. My bad!

The Area3D node, which is what serves as a sensor in Godot, among other things, exposes signals that are pretty much identical to their body counterparts. So you have area_entered, area_exited, area_shape_entered and area_shape_exited. There's no way to opt in/out of these either. You always end up listening for both areas and bodies when the monitoring property is enabled, which it is by default.

While it's certainly possible to just never call those from the physics server implementation, I'd rather try and get as much coverage as I can of the API that Godot expects.

I can't really speak to what people actually use this for in the games/applications that are made with Godot. Grepping for the relevant signal names across GitHub yields a handful of results that probably could have been solved just as well using the "ghost object" solution you mention in #282. You might need to have access to the code search beta to see this, but here's the query if you're curious.

@jrouwe jrouwe merged commit a76f589 into jrouwe:master Feb 12, 2023
@mihe mihe deleted the sensor-vs-sensor branch February 12, 2023 10:10
@jrouwe
Copy link
Owner

jrouwe commented Feb 12, 2023

I did some further testing and I agree with you that it is really easy to disable the collision between sensors if you don't want them to interact. Thanks for the change!

@mihe
Copy link
Contributor Author

mihe commented Feb 12, 2023

There might be some value in adding a simple sensor to the HelloWorld example as well, considering that you have a contact listener there already, just to show people that you can filter out sensor-to-sensor collisions as early as in ObjectVsBroadPhaseLayerFilter. It might stop someone from shooting themselves in the foot in terms of performance.

@jrouwe
Copy link
Owner

jrouwe commented Feb 12, 2023

I ensured that the SensorTest sample and the sensor unit tests all use a separate layer for the sensors, so if people copy paste from there it should be ok. I think the HelloWorld example should be a minimal example, so I don't want to add sensors there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants