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

Fix move_and_collide causing sliding on slopes #49901

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

pouleyKetchoupp
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp commented Jun 25, 2021

Make sure the direction of the motion is preserved in move_and_collide, unless the depth is higher than the margin, which means the body needs depenetration in any direction.

Also changed move_and_slide to avoid sliding on the first motion, in order to avoid issues with unstable position on ground when jumping.

The physics servers also return more collision information in test_body_motion, which can be useful for other things in the future (and possibly exposed to scripts).

In 2D:
Fixes #31097
Fixes #45004

In 3D:
Helps with #18433
Although it works only with a higher safe margin like 0.01. The default value of 0.001 still causes some issues, probably related to float precision, which will be investigated separately.

Credits to @fabriceci for investigating move and slide issues and helping with extensive testing.


Move and slide test project made by @fabriceci:
https://github.com/fabriceci/move_and_slide
-Select Classic C++ mode to test standard move_and_slide function
-Classic gdscript is the gdscript conversion of move_and_slide (now updated with this PR)
-Other modes are specific tests for other changes being evaluated


Regression tests

2D Physics tests from Godot demos:
https://github.com/godotengine/godot-demo-projects/tree/master/2d/physics_tests
Functional Tests/Character - Slopes: no regression with KinematicBody2D moving on slopes.
Functional Tests/Character - Tilemap: no regression with KinematicBody2D jumping through one-way collision tiles.
Functional Tests/One Way Collision: no regression with one-way collision at different angles (current state: #42574 (review))

2D test with MRP from #45259:
No regression.

2D test with MRP from #46134:
No regression.

2D test with MRP from #21595:
This issue was fixed by #21653 which also introduced the condition collision.travel.length() < 1 for stop_on_slope, now removed in this PR.
No regression when moving on slopes.

3D test with #35945 (comment):
No regression. The issue actually doesn't occur anymore with a safe margin of 0.01, so the change from #40377 to make 0.001 the default value is probably not needed anymore (to be investigated).

3D test with MRP from #33833:
No regression.

@pouleyKetchoupp
Copy link
Contributor Author

Just pushed a change to expose the new depth and fraction for body_test_motion in PhysicsServer2D, because it's useful to implement custom motion in script (and needed for custom move_and_slide in https://github.com/fabriceci/move_and_slide).

Make sure the direction of the motion is preserved, unless the depth is
higher than the margin, which means the body needs depenetration in any
direction.

Also changed move_and_slide to avoid sliding on the first motion, in
order to avoid issues with unstable position on ground when jumping.

Co-authored-by: fabriceci <[email protected]>
@akien-mga
Copy link
Member

Just pushed a change to expose the new depth and fraction for body_test_motion in PhysicsServer2D, because it's useful to implement custom motion in script

Isn't it relevant to expose in 3D too for consistency?

@pouleyKetchoupp
Copy link
Contributor Author

Just pushed a change to expose the new depth and fraction for body_test_motion in PhysicsServer2D, because it's useful to implement custom motion in script

Isn't it relevant to expose in 3D too for consistency?

Yes that makes sense, but at the moment body_test_motion is not exposed at all in the 3D server so it's a bit outside of this PR's scope. I'm planning to expose body_test_motion the same way as in 2D in a separate PR.

@akien-mga akien-mga merged commit bcd1fc8 into godotengine:master Jun 30, 2021
@akien-mga
Copy link
Member

Thanks!

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