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

Refactor RayShape and rename to SeparationRayShape #51896

Merged
merged 5 commits into from
Aug 27, 2021

Conversation

pouleyKetchoupp
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp commented Aug 19, 2021

Ray shapes were reverted in #51490, but they can be useful in some cases so this PR is putting them back, while refactoring the code to fix some issues and make the code easier to maintain.

Fixes #34098
Fixes #34663
Edit: Closes godotengine/godot-proposals#711

I've split in separate commits for clarity, here are the detailed changes for all of them:

  • Restore RayShape as a regular shape type

Partial revert from previously removing ray shapes completely, added back as a shape type but without the specific character controller code.

  • Fix RayShape collision detection

One-way collision is disabled for both rigid bodies and character bodies.

Kinematic margin is now applied to ray shapes to help getting consistent results in slopes and flat surfaces.

Convex shapes don't return inverted normals when a segment test starts inside (raycasting will be made consistent in a separate patch).

Ray shapes also discard contacts when fully contained inside a shape and when the contact direction is inverted, so the behavior is consistent with all shape types. Now they always separate only when intersecting the top of a shape (for downward rays).

  • Fix CharacterBody motion with RayShape

Make separation ray shapes work properly in move_and_slide, wihtout the specific code in CharacterBody like before.

Now most of the logic is handled inside the physics server. The only thing that's needed is to use ray shapes only for recovery and ignore them when performing the motion itself (unless we're snapping or slips on slope is on).

Also ported some recent CharacterBody fixes from 2D to 3D to make things consistent and fix certain cases with ray shapes that failed.
Edit: This has been ported separately in #51904.

  • Rename slips_on_slope to slide_on_slope

Also added some precision to the documentation.

  • Rename RayShape to SeparationRayShape

Makes it clearer that it's used for special cases when picking a collision shape.

@Xrayez
Copy link
Contributor

Xrayez commented Aug 19, 2021

Closes godotengine/godot-proposals#711. 🙂

scene/3d/physics_body_3d.cpp Outdated Show resolved Hide resolved
GDCLASS(SeparationRayShape2D, Shape2D);

real_t length = 20.0;
bool slide_on_slope = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true for all slope (wall, etc.) or it's like in CharacterBody2D floor_stop_on_slope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit different from stop_on_slope. There's no notion of floor or walls, so yes it does affect all slopes. What slide_on_slope does is that when it's enabled the contact normals are equal to the slope normals, so the body is pushed back from the slope and slides along. When it's not enabled, ray shapes always use their own normal as contact normals, so the body is just pushed up and doesn't slide in slopes.

In practice, slide_on_slope doesn't affect CharacterBody much (except for the resulting normals) because this node has its own logic for sliding, but in the case of RigidBody with gravity applied it causes the body to slide down on slopes.

servers/physics_2d/collision_solver_2d_sw.cpp Outdated Show resolved Hide resolved
@@ -726,6 +727,11 @@
<description>
</description>
</method>
<method name="separation_ray_shape_create">
<return type="RID" />
<description>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not better to add a description? (same for PhysicsServer3D.xml)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! It's needed for all the other shape create functions as well. I'm keeping a note of this, but I don't want to pollute the current PR with changing all of them.


Vector2 from = p_transform_A.get_origin();
Vector2 to = from + p_transform_A[1] * (ray->get_length() + p_margin);
if (p_motion_A != Vector2()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you use use_approx_equals in this method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, I think it's more about handling p_motion_A if provided as an argument, versus the default value Vector2() when the argument is not used. The math will be ok even if close to zero, so there's no need for the extra checks involved in is_approx_equal.

Partial revert from previously removing ray shapes completely, added
back as a shape type but without the specific character controller code.
One-way collision is disabled for both rigid bodies and character
bodies.

Kinematic margin is now applied to ray shapes to help getting consistent
results in slopes and flat surfaces.

Convex shapes don't return inverted normals when a segment test starts
inside (raycasting will be made consistent in a separate patch).

Ray shapes also discard contacts when fully contained inside a shape
and when the contact direction is inverted, so the behavior is
consistent with all shape types. Now they always separate only when
intersecting the top of a shape (for downward rays).
Make separation ray shapes work properly in move_and_slide, wihtout the
specific code in CharacterBody like before.

Now most of the logic is handled inside the physics server. The only
thing that's needed is to use ray shapes only for recovery and ignore
them when performing the motion itself (unless we're snapping or slips
on slope is on).
Also added some precision to the documentation.
Makes it clearer that it's used for special cases when picking a
collision shape.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants