Contacts and PhysicsDirectBodyState3D

[tgift]

trafficstars

Two changes to PhysicsDirectBodyState3D and some associated classes. Both related to the way contact information is stored and passed to the scripting language.

The first modification makes it possible to compute the impact velocity at the time of contact. For that both the velocity of the state object as well as the collider at the point and time of contact are needed. Velocities can be obtained in "on_body_entered()", but these are after collision has been resolved and don't reflect the velocities at the moment of impact.

The original "get_contact_collider_velocity_at_position()" method was returning not the velocity of the collider, but the velocity of the state body. This method was changed to return the collider's velocity, and a new method "get_contact_velocity_at_position()" was added to return the velocity of the state object. Changes were made to the contact structure to store the new velocity. With these changes the impact velocity (or speed in this case) can be computed as:

var state = PhysicsServer3D.body_get_direct_state(self)
...
var vel1 = state.get_contact_velocity_at_position(i)
var vel2 = state.get_contact_collider_velocity_at_position(i)
vat normal = state.get_contact_local_normal()
var contact_speed = normal.dot(vel1 - vel2)

Anyone relying on the previous behavior of "get_contact_collider_velocity_at_position()" would have to switch to the new method.

The second changes the values returned by "get_contact_local_position()" and "get_contact_collider_local_position()". The physics engine resolves collisions in world space relative to the A body in a GodotBodyPair3D pair. Body B is oriented in world space but translated relative to body A. Contact information was stored in this A relative space and body_B's contact position was always in this space. The state objects contacts are mirror's (almost) of each other, so if your object was body B in the pair, then your contact position would be in the wrong space. If you were body A in a pair, then the collider's position would be in the wrong space.

# Works fine if state body is body_A in the body pair, but returns an incorrect value if this contact was body_B
var pos = state.get_contact_local_position(i)

# Same here, works half the time.
var pos = state.get_contact_collision_position(i)

This change amends the contacts to always store and return positions relative to their respective objects.

Bugsquad edit: This closes https://github.com/godotengine/godot-proposals/issues/4184.

[Calinou]

Thanks for opening a pull request :slightly_smiling_face:

Feature pull requests should be associated to a feature proposal to justify the need for implementing the feature in Godot core. Please open a proposal on the Godot proposals repository (and link to this pull request in the proposal's body).

[rburing]

Could you please make a separate PR with only the bugfix for get_contact_local_position and get_contact_collider_position in Godot Physics? In general we prefer each PR to do one thing, and bugfixes are easier to get merged than feature proposals. (When the bugfix PR is merged, the feature proposal PR can be rebased on top of it.)

[chabi211]

I dont know much about this stuff myself. I am struggling with get_contact_local_postion() and a google search lead me here. From what I understand you fixed the function. What Id like to know is how can I impliment the fix. On that topic, I did some reading on whatever you wrote and I was looking at the code there by [Contact collider pos now relative to it's origin]. I see on line 363 and 367 (the new). It says: "if (B->can_report_contacts())" on both lines. I would imagine one of those lines should be "if (A...". Idk, just something I noticed. Sorry for back etiquette. I dont actually know anything about github. just created account to ask this.

[tgift]

@rburing I’m out of town for until the end of the week, but could look at a new PR then. After perusing the docs more carefully, I think the implementation of “ get_contact_collider_velocity_at_position()” is actually a bug and so this change does not break compatibility. The only new feature being propose is the new function “ get_contact_velocity_at_position()”

@chabi211 I’ll take a look at the code you mentioned when I get back. A separate PR with the just the local position fixes should be easier for you to merge in.

[evanrinehart]

Was using contact reporting in 3.5 and am probably experiencing this mirrored 50% of the time bug. Same object already in the scene reporting opposite collision points vs that object instanced. Could be they spawn as body B in the pair instead of A.

[evanrinehart]

Important comment here for this PR. So I tried to take the changes and apply them to 3.5 without the added velocity feature, and I found a bug in the proposed reporting fix.

In godot_body_pair_3d.cpp on line 363 in the updated code, it should test A if it can report contacts. Instead it tests B twice and A never reports. So when I tried the changes as is it fixed 1/2 of the problem. Changing line 363 to if (A->can_report_contacts()) { accomplishes the fix.

I'm using Godot 3.5 and now my stuff is working. I hope you guys get this resolved!

[tgift]

@evanrinehart Thanks for catching that

[D2klaas]

This issues seems to be fixed but is awaiting review?

Just wanted to add the following:

When a collision occurs two contacts will be reported, one for each collider. The first contact gets reported correct, the second one seems to get a copy of that contact but does not translate the position into its own location. The contact normal does work correctly. So, exact half of all contacts get the wrong positions. bang.zip

Would love to see this fixed but im not the c++ guy and i dont know if i can help with the needed review.

[tgift]

@D2klass, I need to address the can_report_contacts() error first and we'll see after that. Rburing suggested I split the PR up into bug fixes vs. the new feature. Probably what I should have done to start with, but I just don't have time to work on this at the moment. If I get time I'll fix the contacts and re-post but unfortunately godot is on the backburner at the moment.

[tgift]

Updated to the latest version of the main branch and corrected issues related to that. Also fixed the error found by evanrinehart.

[evanrinehart]

Still using a hacked version of 3.5 with the fix for this bug, which as I understand it still exists in godot 4. I hope it doesn't get forgotten now that it's over a year later and godot 4 is getting a lot of work.

UPDATE: actually I see work on the place where the mirroring bug used to be. And someone told me they can't reproduce the effect in godot 4. So maybe that part of the PR is no longer needed.

[rburing]

@evanrinehart From a brief test it looks like this PR still fixes a bug.

I think the method get_contact_velocity_at_position should be renamed get_contact_local_velocity_at_position, so that we have all the get_contact_local_* methods with the same prefix, similar to get_contact_collider_*.

(IMHO the _at_position suffix is a bit verbose, but to preserve compatibility we can keep it for now.)

I think it's okay to put the new method and the bugfix in this same PR. @tgift could you please rebase?

[tgift]

I'll try to bring the PR up to date this weekend and I'll look at the possible name change. Will have to refresh my memory on all of this.

[tgift]

Rebased and conflicts resolved. @rburing I looked at the function names and you are correct. I renamed "get_contact_velocity_at_position" to "get_contact_local_velocity_at_position" to be more consistent.

[rburing]

Thanks @tgift, the changes look great now. Could you just amend the commit message (using git commit --amend) to be a bit more concise? Only information about the final version of the commit is relevant. After that it will be ready to merge.

Edit: Also the documentation of the new method was lost in the rebase. The docs you wrote earlier can be viewed at https://github.com/godotengine/godot/commit/3ece13de9367ae063cb5589aab77719f94bc2cf4. It would be great if you could add those docs back as well.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution :tada:

[Zireael07]

Yay! No more hacks needed for damage on collision <3