[3.x] Fix multiple issues with test_body_motion() and test_body_ray_separaton().

[madmiraal]

This is a the 3.2 branch version of PR #35945.

I've created this because #34098, #34436, #34596, #35713 and #35780 are regression issues from 3.1 and since master has changed so much, #35945 is no longer cherry-pickable.

Update:
Now includes additional fixes added to #35945 for #17893, #18433, #26550, #28717 and #32182.

[madmiraal]

As with #35945, updated with fixes to #18433, #26550 and #32182 and improvement to #31981.

[athuyaoo]

Tested it with his build, it works apparently

[ghost]

Hi, travisryte shared his compiled version and I've been testing it with my project. With no code changes to my character controller, the bumps which happen when crossing from one collider to another on the floor are gone. However, the body occasionally gets stuck if I'm touching both the floor and the wall at the same time. is_on_floor() becomes unstable then.

[Zireael07]

IIRC is_on_floor being unstable when you touch two things is a different preexisting problem?

[ghost]

Here's a video of what I mean.
Link

I'm simply doing this in my code:

velocity = move_and_slide_with_snap(velocity, Vector3.DOWN, Vector3.UP)

[ghost]

IIRC is_on_floor being unstable when you touch two things is a different preexisting problem?

No, it's new with this fix.

[madmiraal]

@MBZSoftProds, please share your project; so I can investigate.

[ghost]

Here you go.

Link

Hope this works. It's rather large.

Edit:
The player script is called PC_main.gd.
Player scene is called PC_inst.tscn

[jknightdoeswork]

Can confirm that this is a 3.2 regression.

In my game, the golf ball constantly bounces if placed on tile seams.

In 3.2, if the ball comes to rest on a seam, it resolves the collision slightly into the air. Then the ball is no longer colliding with anything, and drops back down to the ground. It usually spends 2 frames in the air, before returning to the ground.

I just tested in 3.1 and sure enough, this bug is not there.

Here's hoping that this gets merged and that the code in general gets cleaned up. It's pretty complicated, and critical code. I'm pretty sure that there are a whole host of problems with test_body_motion.

There is another bug I've noticed with move_and_collide.

Using a BoxShape for the ground causes move_and_collide to produce unpredictable and inexact normals around seams. Even on top of the middle of the BoxShape, you get normals like this: (-0.000001, 1, -0.000002) from move_and_collide.

Using ConvexPolygonShape also has the issue.

Using ConcavePolygonShape produces precise normals using move_and_collide.

I wrote about it in this issue:
See my comment on #28032

I will test this as soon as I can and report if the seam depenetration issue I'm seeing is resolved. I imagine it will be.

I will also report if the imprecise normal issue is resolved or not.

Thank you so much @madmiraal

[madmiraal]

Added a fix for the sticking identified by @MBZSoftProds.

Note: There is still some stickiness, and some on_floor() instability, but I agree with @Zireael07 this was there before, appears to be due to something else, and may be related to #29392.

[realkotob]

Would be really nice to have this merged for the upcoming release, let me know if there is anything I can do to help with that

[DenisBelmondo]

Hi @madmiraal, great fix so far. I've noticed some issues with regards to KinematicBody sticking in this branch. Here are some of my observations:

Using the stock move_and_slide and move_and_slide_with_snap (both are affected by this issue, ruling out the possibility that it is _with_snap's mechanisms), a KinematicBody occasionally sticks to surfaces. This includes floors and walls. With floors, you can eliminate the issue entirely by doing a floor check with test_move(relVec=Vector3.DOWN) or move_and_collide(test_only=true) and setting the y velocity to 0, but the same approach cannot be used for walls.

Below is a gif of the player (using a BoxShape, but still reproducible with a CapsuleShape in my experience) attempting to slide across a continuous wall (not made of GridMap tiles or multiple meshes) but getting stuck along the way.

When you remove the floor check however, you can see that the character gets stuck in it intermittently:

In my experience, setting the CollisionShape margin within reasonable bounds (0.04 to 0.2) did not have an effect. Anything beyond those margin values seemed to either worsen it or have no effect at all.

stucktest.zip

Here is a minimal reproduction project. This was created with the fix-34596-3.2 branch, compiled with Mono. Apologies if this is an inconvenience, as my project is being written in C#.

To test just the walls, insert

if (TestFloor())
{
    Vel.y = 0f;
}

at the beginning of _PhysicsProcess.

[ghost]

Tried the fix-34596-3.2 branch too. It works much better than before. It fixes the floor issues still and wall sticking is almost gone. It only happens rarely and to a much lesser extent than before.

However, multiple moving KinematicBody entities, i.e. AI controlled entities, together with the player introduces regular slow downs and stutter for around a second ...

[jknightdoeswork]

Tested this in my golf game. When the golf ball is on the ground and gravity is present, move_and_collide will no longer apply horizontal forces. Seems like the gravity causes the golf ball to instantly collide and instantly stop, the perpendicular velocity vector is not applied to the transform at all. Is this intended behaviour?

[madmiraal]

@DenisBelmondo This is the residual stickiness (and floor instability) I'm referring to. As @jknightdoeswork pointed out, and beautifully illustrated by @Zylann here, this appears to be due to the effective rounding of box shapes created by the collision margin and the non-perpendicular collision normals returned when colliding with the edge of a BoxShape.

This non-perpendicular normal will push the KinematicBody back (or to the side as in #29392) when using move_and_slide() or move_and_slide_with_snap(). This happens even when colliding with two aligned BoxShapes, because the collision with each CollisionShape is calculated independently and then collated. If the motion towards two aligned BoxShapes is greater than the motion along them, the KinematicBody gets stuck, and if the collated normal is more than 45° off the vertical, the on_floor() returns false. This is why setting the y velocity to zero works: there is no motion towards the boxes.

In short, this is a separate, pre-existing issue, and addressing it is not part of this PR.

[madmiraal]

@MBZSoftProds I think the stuttering you're seeing may be related to #37118.

[madmiraal]

@jknightdoeswork I'm confused by what you're asking. KinematicBodies don't have forces applied to them and move_and_collide() doesn't return an updated linear_velocity vector.

If you're referring to the sliding seen in #18433, this was a bug, which has been resolved with this PR.

[jknightdoeswork]

@madmiraal

When a sphere is on a box, calling move_and_collide on the sphere with a velocity of (-10.0, -1.0, 0) will not move the sphere. If you disable gravity and apply (-10.0, 0, 0). The sphere moves as expected.

I'm not certain whether this new behavior is "more correct" than before the patch, since it makes sense that move_and_collide would stop the ball very quickly, since it is on the ground, and gravity is pushing it into the ground, but this behavior is very different than before the patch. Perhaps there is a way to use the travel or remainder values to have the behavior I am attempting to achieve.

In my golf game, I am integrating forces myself and using move_and_collide in order to move the ball. I do this so I can have very custom control over bounces and friction, and so I can fully predict the path of the ball before shooting.

Before the patch, when the ball was on the ground, it would behave much differently.
After this patch, when the ball is on the ground it cannot be moved horizontally.

I can make a reproduction project if needed.

[jknightdoeswork]

@DenisBelmondo You can try to get exact normals by using Concave shape for your walls/ground. You can do this by creating a cube mesh, then clicking Mesh -> Create Trimesh Static Body (above the viewport), and then using that collision shape.

[realkotob]

I am working on an identical project as @jknightdoeswork -- a minigolf game where I use move_and_collide to handle bouncing and movement, with collisions happening between a sphere and Trimesh Static Bodies.

I can try to provide a stripped down project if it would be useful for comparison, but I haven't tried this patch yet with that project.

@jknightdoeswork We should get in touch and share our experiences :b

[jknightdoeswork]

@asheraryam Definetly should. I'm 00jknight in the godot discord.

@madmiraal This code block causes the patch to behave similarly to before the patch:

    var collision = move_and_collide(v, true, true, false)
    if collision != null:
        var projected_remainder = collision.remainder.project(collision.normal)
        translation += collision.remainder - projected_remainder

Note that this is moving the object along the remaining velocity vector. You could theoretically move it using move_and_collide again in a loop, until the velocity vector has been consumed.

Again, I'm not sure if this is better or worse than before the patch. At first glance, it seems like a regression, but upon reflection you can see that the patch is now causing move_and_collide to behave more like how it claims to behave in it's documentation ie: instantly stopping on a collision.

Note that this branch resolves the "seam bouncing" issue that I was previously seeing. So great job on that!

[vesk4000]

Great commit, can it be included in stable?

[vesk4000]

Great commit, very important changes that need to be included in Godot as soon as possible.

[madmiraal]

Rebased following merge of #42575.

[Two-Tone]

Is there any chance of this being merged soon?

[vesk4000]

Is there any chance of this being merged soon?

To be honest, even though I like the PR, I doubt it. On the other hand a lot of the physics issues seem to be getting fixed for 4.0 thanks to @pouleyKetchoupp

[Two-Tone]

It being fixed in 4.0 is great and all, but that is irrelevant for people that expect to be using 3.2 long term.

…

On Sat, Feb 27, 2021 at 7:38 AM Vesk ***@***.***> wrote: Is there any chance of this being merged soon? To be honest, even though I like the PR, I doubt it. On the other hand a lot of the physics issues seem to be getting fixed for 4.0 thanks to @pouleyKetchoupp <https://github.com/pouleyKetchoupp> — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#37498 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBEIS4VFMO3UUUVOSLKW43TBDYVXANCNFSM4LZBYY7A> .

[Riteo]

Is there any chance of this being merged soon?

I highly doubt it. If you look at the 4.0 version of this PR you'll see how this set of changed isn't planned to be merged, but instead is kept open just to be able to salvage whatever is considered good by @pouleyKetchoupp.

In any case though, you can just merge this PR in your own fork and call it a day.

[Two-Tone]

And again, what about all the other people who'll be still using 3.2? Do you really expect thousands of developers to support a custom fork of 3.2 just for fixes like these when they could have just been merged instead of going "It's fixed in 4.0, fuck em"?

The whole "if you don't like it make your own fork" attitude is unnecessarily hostile towards users and gamedevs.

[Riteo]

And again, what about all the other people who'll be still using 3.2? Do you really expect thousands of developers to support a custom fork of 3.2 just for fixes like these when they could have just been merged instead of going "It's fixed in 4.0, fuck em"?

The whole "if you don't like it make your own fork" attitude is unnecessarily hostile towards users and gamedevs.

Oh sorry, I thought you really needed it right now for a project of yours. I liked this PR a lot too but apparently the methods behind it may lead to a few different issues down the line. There has been a pretty extensive discussion about it in #35945.

Also, AFAIK it is planned to fix them properly in this version too, but this simply isn't the right solution.

[Riteo]

@Two-Tone sorry for the double-post, but it seems also that @pouleyKetchoupp has actually ported at least some of the fixes he's made to 3.2. #46149 is an example of this.

[vesk4000]

And again, what about all the other people who'll be still using 3.2? Do you really expect thousands of developers to support a custom fork of 3.2 just for fixes like these when they could have just been merged instead of going "It's fixed in 4.0, fuck em"?

The whole "if you don't like it make your own fork" attitude is unnecessarily hostile towards users and gamedevs.

I mean at the end of the day at some point any team just has to push future updates to a new version of their product. They can't support older versions forever. To be honest I'm glad that they hired @pouleyKetchoupp and these physics issues are getting fixed at all in 4.0 as I wasn't sure that would be happening any time soon. Yeah maybe they should just merge this PR as a kind of quick fix before 4.0 comes out. But I don't really see much of a problem as I just compiled it myself and have been using it while waiting for 4.0. Also I don't know really know why you might want to use 3.2 instead of 4.0 unless it's hard to transition a project (which it hopefully shouldn't be), although I do respect that there might be some reasons to do so, but then again you could just compile this yourself and from my experience it works perfectly well.

[FeralBytes]

@madmiraal Having read this and the related PRs very throughly, I know that this PR will not be merged. But is it still possible that per the discussion in #35945 that your technique could still be put to the test in the tests? So that we might understand if the Unique but still possible cases that this solution will not work, to see if they actually exist? Or have you concluded that the "U" "anisotropy limitation" scenario mentioned by Reduz would indeed not be solvable in a single iteration? Thank you for your time and for fueling fixes faster than they other wise would have landed. Also so that core contributors don't see that statement as being rude. Please know that I am aware we are all human, we can only do so much so fast, especially if we are to do it well. So that is why many hands make the load lighter.

[madmiraal]

@FeralBytes, the 'U' scenario presented in #35945, is a red herring. As presented previously, 'in reality, things either fit or not but not "just fit" (I mean whathever you collide to has to actually reach this scenario from a previous one where it's not colliding, which is likely impossible) so in these cases, users just realize this and shrink the kinematic body a tiny bit to solve the problem. Just think about it, you will realize it's not a realistic use case.'

The 'U' scenario presented is not a realistic use case. No approach and no finite number of iterations will resolve this situation. Furthermore, the solution: jumping to a collision-free zone somewhere outside of the enclosed space is not a desirable outcome either; so I don't think it's a scenario worth discussing.

What can cause a problem is how Godot physics tries to avoid the possibility of unwanted collisions being detected when moving perpendicular to a surface due to floating point numerical impression. Godot physics moves the KinematicBody away from all the collision surfaces by the Safe Margin. This is done by increasing the size of the CollisionShapes by the Safe Margin during the recovery step. An issue arises when the KinematicBody is between two parallel collisions surfaces. When the size is increased, the CollisionShape can go from fitting to not fitting. The current approach provides an iterative solution to better place the KinematicBody between the two surfaces. The approach used in this PR may result in the KinematicBody being closer to the opposite surface. However, this potential issue cannot be resolved by increasing the number of iterations. Instead, as suggested previously, 'users just realize this and shrink the kinematic body a tiny bit to solve the problem.`

Note: This is not a problem in the Bullet physics solution, because, instead of moving the KinematiBody away from the collision surfaces by increasing the size of the KinematicBody by the Safe Margin, the Safe Margin is used as a minimum collision depth during the motion. A second recovery stage is performed after the motion to correct any resulting overlap. This means that the Bullet physics' solution never has a change in state from fitting to not fitting; so there is no issue.

[akien-mga]

Closing as the master PR was rejected, see #35945 (comment) and PR discussion for details.