Fix one way collisions for KinematicBody2D #38471
Conversation
Rhathe
force-pushed
the
fix
branch
2 times, most recently
from
80d3998 to
7d1da34
Compare
Rhathe
force-pushed
the
fix
branch
2 times, most recently
from
fb58358 to
a677095
Compare
Rhathe
force-pushed
the
fix
branch
2 times, most recently
from
8a27b70 to
2aa8650
Compare
I wrote this for the 3.2 branch. Couldn't make 4.0 branch work on my machine so unable to test whether this would work there. Also, removed dependency on some parts of your code now to avoid adding velocities to the valid depth. I noticed it was making the jumping bug on the horizontal one way collision body. |
|
@Rhathe , after taking a closer look, the only changes that #36280 has compared to this are rather small ones: diff --git a/servers/physics_2d/body_pair_2d_sw.cpp b/servers/physics_2d/body_pair_2d_sw.cpp
index 1523d40d1e..dacbc773e7 100644
--- a/servers/physics_2d/body_pair_2d_sw.cpp
+++ b/servers/physics_2d/body_pair_2d_sw.cpp
@@ -295,17 +295,15 @@ bool BodyPair2DSW::setup(real_t p_step) {
if (A->is_shape_set_as_one_way_collision(shape_A)) {
Vector2 direction = xform_A.get_axis(1).normalized();
bool valid = false;
- if (B->get_linear_velocity().dot(direction) >= 0) {
- for (int i = 0; i < contact_count; i++) {
- Contact &c = contacts[i];
- if (!c.reused)
- continue;
- if (c.normal.dot(direction) > 0) //greater (normal inverted)
- continue;
-
- valid = true;
- break;
- }
+ for (int i = 0; i < contact_count; i++) {
+ Contact &c = contacts[i];
+ if (!c.reused)
+ continue;
+ if (c.normal.dot(direction) > 0) //greater (normal inverted)
+ continue;
+
+ valid = true;
+ break;
}
if (!valid) {
@@ -318,17 +316,15 @@ bool BodyPair2DSW::setup(real_t p_step) {
if (B->is_shape_set_as_one_way_collision(shape_B)) {
Vector2 direction = xform_B.get_axis(1).normalized();
bool valid = false;
- if (A->get_linear_velocity().dot(direction) >= 0) {
- for (int i = 0; i < contact_count; i++) {
- Contact &c = contacts[i];
- if (!c.reused)
- continue;
- if (c.normal.dot(direction) < 0) //less (normal ok)
- continue;
-
- valid = true;
- break;
- }
+ for (int i = 0; i < contact_count; i++) {
+ Contact &c = contacts[i];
+ if (!c.reused)
+ continue;
+ if (c.normal.dot(direction) < 0) //less (normal ok)
+ continue;
+
+ valid = true;
+ break;
}
if (!valid) {
collided = false;
diff --git a/servers/physics_2d/space_2d_sw.cpp b/servers/physics_2d/space_2d_sw.cpp
index 5fca3e4666..f622ba2ae3 100644
--- a/servers/physics_2d/space_2d_sw.cpp
+++ b/servers/physics_2d/space_2d_sw.cpp
@@ -875,7 +875,7 @@ bool Space2DSW::test_body_motion(Body2DSW *p_body, const Transform2D &p_from, co
for (int i = 0; i < cbk.amount; i++) {
Vector2 a = sr[i * 2 + 0];
Vector2 b = sr[i * 2 + 1];
- recover_motion += (b - a) * 0.4;
+ recover_motion += (b - a) / cbk.amount;
}
if (recover_motion == Vector2()) {
Maybe you can just apply them, so I could close #36280 and #40645 ? I think this will make it easier to get this reviewed and merged. |
|
I updated engine builds here to include changes from this PR and a patch from my comment above. |
|
Superseded by #42574, which includes a more minimal version of those fixes. |
#42574 does not necessarily fix all issues with one-way collision at least with just KinematicBody2D, as it did not fix the issues I encountered when having to create this pull request, and even introduced a bug as I detailed here: The commit messages detail exactly what the problems this was trying to fix and how it does so, which are not addressed by #42574. |
The first file of the change seems to deal only with RigidBody2D, and this was only concerned with KinematicBody2D, so it could have been merged separately. The second file is relevant, though I'm not sure what effect it would lead to in addition to my changes without testing everything again, as a lot of the issues this particular pull request doesn't fix may be due to not dealing with RigidBody2D interactions at all. |
|
@akien-mga , yes, as @Rhathe said. I closed my original PR because it contained some bugs, which were fixed here in a much more robust way. I highly recommend keeping it open instead of the others. Minimalism is not a good criteria, if the result works poorly. |
For bodies with one way collision any axis that is directly perpindicular to the one way axis should not be considered for best axis when deciding how to resolve the collision. For example, a bottom corner of box A collides with a top corner of box B which has one way collision of (0,1) or pointing down. If A's corner is closer to the the side edge of B, then previously the best axis chosen would be the horizontal axis. Normally without one way collision this would push A to the side of B. However, since B has one way collision, the repositioning of A to the side would be considered invalid in a later check and this would fail to collide. This fixes that scenario by making sure any best axis selected would never be a perpindicular axis.
…r KinematicBody2D The collision solver has the valid direction passed in for one way collisions in order to get the proper best axis (i.e. not a perpindicular axis). The normal is passed in to the callbacks from the collision solver. Previously the intersecting vector was used to determine the normal but these points were derived at a later point from the actual intersection test thus introducing floating point precision errors and drifting away from the true normal of the intersecting edge. Since we already know the actual normal of the edge, that is used instead. There are changes the one way collision check in the callback. The reposition check now allows angles below 45 degrees and above 0 degrees to be counted for one way collision, allowing for steeper angles to still work for one way collision. During scaling of the body's velocity for repositioning, if the collision is a one way collision, another attempt is made to change the scale so that the body lands within the margins.
Using the normals from the collision edges and the motion of the objects, it can be determined whether or not the test object is leaving the colliding object or entering it. This in addition with previous checks for whether or not a collision edge should be considered for one way collision enables proper one way collision without using unsticking logic when leaving an object through a one way axis.
akien-mga
force-pushed
the
3.x
branch
2 times, most recently
from
71cb8d3 to
c58391c
Compare
|
@Rhathe what's the status on this? |
Several fixes detailed in the commit messages, but essentially takes the motion of the body into account when determining whether the body collides with the one way colliding body, as well as making sure any collision axes are not perpendicular to the one way axis.
Updated version of #38345
Fixes #25967
Fixes #28895
Fixes #42914
Fixes #42916
Fixes #43266
Fixes #43346
Fixes #43973
Uses part of the code from #36280