Fix multiple issues with one-way collisions

[madmiraal]

This is a salvage of #36280, because, despite the original author's decision to close #36280 in favour of #38471, #38471 does not solve all the issues that were solved by #36280.

This PR does a few things:

• For RigidBodies, uses the collision normal determined by relative motion to determine whether or not a one-way collision has occurred.
• For KinematicBodies, performs additional checks to ensure a one-way collision has occurred, and averages the recovery step over all collision shapes.

I have made some slight changes to the original PR to minimise the necessary changes and make this PR easy to review. I have also made the original author a co-author of the commit.

I have tested and confirmed that this PR fixes the following issues:
Fixes #25732
Fixes #25967
Fixes #28794
Fixes #34242
Fixes #35776
Fixes #40006
Fixes #42175
Fixes #43266

Note: #36280 claimed to fix other issues too. However, I found that several of these issues were already fixed (so I have updated them accordingly), or are actually not fixed by this PR.

Edit: Updated to remove the change that caused the bug reported by @Rhathe.

Edit: Updated to fix the bugs identified by @Rhathe here and here.

Edit: Updated to fix #43266, which is the same bug identified by @AttackButton here.

[madmiraal]

As suggested by @capnm here, updated to ensure a zero vector is not normalized.

[bemyak]

#38471 does not solve all the issues that were solved by #36280

@madadam Did you test it with or without the patch from this comment ?

[RandomShaper]

I've checked the changes carefully. I don't see anything fishy and I understand the rationale of most of them.

I've also tested the 3.2 version of this PR on my own game, which doesn't use one-way much, but also as a way to spot potential regressions or relevant behavior changes in the general behavior of body collisions. Everything seems alright.

So I'm already giving my approval, but I'd like to see a project with heavy use of one-way collisions running with this changes, to confirm for good that the changes work as expected.

[Rhathe]

Last time I tested these changes I noticed it introduced a bug in the 3.2 branch as I noted here: #36280 (comment)

This is the original:

This is with the changes:

Is that bug still here?

test_move_one_way.zip

[capnm]

I think this is a bug in your project, if you reset the excessive one_way_collision_margin
it works as expected:

(You are probably trying to abuse this margin for »side_way« collisions.)

[Rhathe]

I think this is a bug in your project, if you reset the excessive one_way_collision_margin
it works as expected:

(You are probably trying to abuse this margin for »side_way« collisions.)

The excessive one_way_collision_margin is only to make sure this effect shows up consistently and thus a more obvious bug to fix. When set with just a value of 1, the bug still shows up, you may just need to run it a couple of times to see it occur.

Even if the bug only showed up with an "excessive" value, it is still a bug either way that does not show up before this change.

[capnm]

The excessive one_way_collision_margin is only to make sure this effect shows up consistently and thus a more obvious bug to fix.

Please fill then an issue for that what you think is »obvious bug to fix«. I can't reproduce it.
(The one_way_collision_margin should be probably removed, as the one way collision now works even by high speed without it.)
Thanks.

[Rhathe]

The excessive one_way_collision_margin is only to make sure this effect shows up consistently and thus a more obvious bug to fix.

Please fill then an issue for that what you think is »obvious bug to fix«. I can't reproduce it.
(The one_way_collision_margin should be probably removed, as the one way collision now works even by high speed without it.)
Thanks.

I'm not sure an issue should be created for a bug introduced in a pull request that has not been merged yet. I've added a new version of the test project so that it more easily shows up. The only changes are that the KinematicBody2D moves with a vertical velocity of 3000 instead of 1000, and the one_way_collision_margin is set to 0. The bug now should show up all the time.

test_move_one_way_2.zip

[capnm]

the KinematicBody2D moves with a vertical velocity of 3000 instead of 1000

I don't think this is a reasonable speed, indeed now the kinematic body jumps at the one way collision
shape.

[madmiraal]

Updated, so a perpendicular collision is detected as a valid collision. Now both the RigidBody and the KinematicBody behave the same way:

[capnm]

I think that isn't right, it should be one way collision...
I suspect the rigid body collided at a small angle before?

pr42574-move-horizontal2.zip

3.2.3.stable:

[madmiraal]

@capnm The RigidBody is not moving perfectly horizontally. It's actually rocking slightly, which is why you see the contact points flickering. This will result in slightly different collisions due to slightly different starting conditions. With the original example project, the collision normal is <-1, 0.000295927> i.e. pointing slightly upwards; so it's stopped. In your example project, the collision normal is <-1, -0.000422864> i.e. pointing slightly downwards; so it's allowed through. Setting the RigidBody to Character mode, prevents rotations i.e. the rocking and the collision normal is <-1, 0>, which is stopped as with the KinematicBody

Note: RigidBody one-way collisions are only checked at the time of collision. The RigidBody is then either allowed or disallowed to pass through until it exits the one-way collision body. This results in the differences seen around the edge case of a perpendicular collision.

[pouleyKetchoupp]

For comparison, here are the results on the same tests for KinematicBody2D on other versions of the code.

3.2.3 stable

Body angle 0, Platform size 64:
Test FAILED: size=64.0, angle=195.0, body angle=0.0
Test FAILED: size=64.0, angle=210.0, body angle=0.0
Test FAILED: size=64.0, angle=225.0, body angle=0.0
Test FAILED: size=64.0, angle=315.0, body angle=0.0
Test FAILED: size=64.0, angle=330.0, body angle=0.0
Test FAILED: size=64.0, angle=345.0, body angle=0.0

Body angle 45, Platform size 64:
Test FAILED: size=64.0, angle=195.0, body angle=45.0
Test FAILED: size=64.0, angle=345.0, body angle=45.0

Body angle 45, Platform size 32:
Test FAILED: size=32.0, angle=195.0, body angle=45.0
Test FAILED: size=32.0, angle=210.0, body angle=45.0
Test FAILED: size=32.0, angle=225.0, body angle=45.0
Test FAILED: size=32.0, angle=330.0, body angle=45.0
Test FAILED: size=32.0, angle=345.0, body angle=45.0

This PR without the threshold change on normal checks

Body angle 0, Platform size 64:
Test FAILED: size=64.0, angle=180.0, body angle=0.0

Body angle 45, Platform size 64:
Test FAILED: size=64.0, angle=180.0, body angle=45.0
Test FAILED: size=64.0, angle=195.0, body angle=45.0
Test FAILED: size=64.0, angle=345.0, body angle=45.0

Body angle 45, Platform size 32:
Test FAILED: size=32.0, angle=15.0, body angle=45.0
Test FAILED: size=32.0, angle=30.0, body angle=45.0
Test FAILED: size=32.0, angle=150.0, body angle=45.0
Test FAILED: size=32.0, angle=165.0, body angle=45.0
Test FAILED: size=32.0, angle=180.0, body angle=45.0
Test FAILED: size=32.0, angle=195.0, body angle=45.0
Test FAILED: size=32.0, angle=210.0, body angle=45.0
Test FAILED: size=32.0, angle=225.0, body angle=45.0
Test FAILED: size=32.0, angle=330.0, body angle=45.0
Test FAILED: size=32.0, angle=345.0, body angle=45.0

[madmiraal]

Updated to include:

1. Tolerances for dot product direction calculations. Note: I've used CMP_EPSILON to avoid creating another hard-coded constant.
2. Applied the division by cbk.amount for recovery motion to test_body_ray_separation and to both in Godot 3D too for consistency.

[pouleyKetchoupp]

1. Note: I've used `CMP_EPSILON` to avoid creating another hard-coded constant.

That works! It's a lower tolerance value, but the same tests pass so it seems fine.

Given this PR is now fully tested and doesn't produce any obvious regression, I think it's finally ready to go :)

[akien-mga]

Thanks a ton to everyone involved!

[pouleyKetchoupp]

@Rhathe Please test your use cases again to see if some of them are still a problem. If it's the case, I would encourage you to open new issues, clearly stating what your use case is and what the expected behavior is.

#42574 (comment)
https://github.com/godotengine/godot/files/5561017/test_move_one_way_rotated.zip
This case now goes through like in 3.2.3 stable, which is the expected behavior.

#42574 (comment)
https://github.com/godotengine/godot/files/5612930/test_move_one_way_diamond.zip
This case now collides, but it seems like the expected behavior since the body is hitting the top right corner of a one-way collision. This is one of the cases this PR fixes.

[chucklepie]

Will this fix all jittery issues (fluctuating velocity.y) with using move_and_slide on tilemaps (one way and not one way), i.e. with this patch can we safely stop having to put multiple raycasts on our nodes because is_on_floor() fails to return the correct value every other frame?

Thanks in anticipation of good news :)

[pouleyKetchoupp]

@chucklepie This PR fixes some visible jittering issues, but it's difficult to guarantee it will fix any case of is_on_floor() being inconsistent. Feel free to point out a specific issue or open one with your use case and we'll check if it's already fixed.