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

Ensure RigidBodies only interact with Bodies with layers in their mask #49166

Closed
wants to merge 1 commit into from

Conversation

madmiraal
Copy link
Contributor

The RigidBody sister PR to #42268 (Areas) and #42641 (KinematicBodys).

Currently, when a RigidBody collides with another CollisionObject the collision_layer->collision_mask combination is checked both ways. This results in unintended and unwanted collisions.

This PR ensures that RigidBodys only collide with CollisionObjects that have one of the RigidBody's collision_layer bits in the other CollisionObject's collision_mask. It fixes both 2D and 3D Godot physics.

Note: Unlike #42268 and #42641, this PR does not apply the same fix to Bullet physics.

The final part of godotengine/godot-proposals#2775.

Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested yet, but I would expect that when both bodies are dynamic and one of them ignores the contact, its mass is considered infinite (like a static/kinematic body) in order to correctly apply impulses on the other one. Otherwise the bodies are going to go through each other.

@@ -403,12 +403,12 @@ bool BodyPair2DSW::pre_solve(real_t p_step) {
c.rA = global_A;
c.rB = global_B - offset_B;

if (A->can_report_contacts()) {
if (A->can_report_contacts() && B->layer_in_mask(A)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a member or at least local variable to avoid repeating the same logic multiple times.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer it this way. It's the same format as the other condition, it's method name makes it easy to understand when reading, and the compiler will inline it anyway.

@madmiraal
Copy link
Contributor Author

I would expect that when both bodies are dynamic and one of them ignores the contact, its mass is considered infinite (like a static/kinematic body) in order to correctly apply impulses on the other one. Otherwise the bodies are going to go through each other.

They should go through each other. If a CollisionObject is not in a layer that is in this CollisionObject's mask, then the first CollisionObject should be ignored.

When I originally created #42641, I used the following test project, created in 3.2 to test a combination of #42642 and #49169 2D and 3D: 15243.zip.

The following videos show the output of those tests
Current 2D i.e. without #42642 and #49169:

15243-current

2D with #42642 and #49169:

15243-fixed

Current 3D i.e. without #42642 and #49169:
Note: The 3D box layout and colours are the same as the 2D so use the labels in the 2D versions above to confirm the expected outcomes.

15243-current-3D

3D with #42642 and #49169:

15243-fixed-3D

@pouleyKetchoupp
Copy link
Contributor

@madmiraal You can see in the 2d version how rigid body 1 slightly goes into rigid body 2, my guess is it's a buggy collision that is due to what I'm talking about and would be fixed with my suggestion.
We can't design a system where some object will half collide, from the user point of view an object should either collide or go through.

@pouleyKetchoupp
Copy link
Contributor

After more investigation on the topic of contact impulses, here's my conclusion:

The visible overlap in 2D I mentioned before is not actually due to the problem I'm pointing out, it's because the sprites are larger than the collision shapes in your test scene, that's why it also occurs in the first gif.

The issue I'm talking about is not visible in your scene because collisions, motions, shapes are very simple, but it has the potential to cause problems in more complex scenarios.

Applied impulses in one solver iteration are not physically correct
I've made some tests in 3D based on your test project to illustrate this issue.

In this scenario, RigidBody1 is falling on top of RigidBody2, and layers are set in this way:
-RigidBody1 is affected by RigidBody2
-RigidBody2 is not affected by RigidBody1

Here's the overlap before the solver kicks in to handle contact impulses:

If the rigid bodies collided normally, here's what you would get in one solver iteration:
(both would be pushed from each other)

And here's the comparative result of this PR after one solver iteration:

This PR Proposed fix
image image

You can see with the current state of this PR, RigidBody1 is not pushed out enough in one solver iteration, because RigidBody2 is expected to move too during impulse calculations, and in the end only RigidBody1 actually receives an impulse.

The fix is not complicated, it just consists in considering RigidBody2 kinematic (infinite mass and infinite inertia) when it comes to interacting with RigidBody1. There's no negative side effect and it can only improve physics for a similar case in more complex scenarios.

@madmiraal
Copy link
Contributor Author

RigidBodies not fully recovering their penetration with each other is an old, pre-existing issue (#2092, #16505, etc.) that this PR is not aiming to address.

@madmiraal
Copy link
Contributor Author

Updated to include equivalent fix from #50685.

@madmiraal madmiraal changed the title Ensure RigidBodies only interact with Bodies with matching mask Ensure RigidBodies only interact with Bodies with layers in their mask Jul 21, 2021
@pouleyKetchoupp
Copy link
Contributor

Superseded by #50625.

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

Successfully merging this pull request may close these issues.

2 participants