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_slide_with_snap cause artifact #32297

Closed
wants to merge 1 commit into from

Conversation

Raphael2048
Copy link
Contributor

close #31097
Screen Shot 2019-09-24 at 2 03 19 PM

When near the slope, applying snap may cause object intersect with the slope, so just cancel it in this case.

@Chaosus Chaosus added this to the 3.2 milestone Sep 24, 2019
@akien-mga akien-mga requested a review from reduz November 7, 2019 11:59
@akien-mga
Copy link
Member

Conflicts with changes I just merged from #30588.

@Demiu Maybe you can help review this PR as you also modified this code recently?

@akien-mga
Copy link
Member

I rebased locally and confirmed that it fixes #31097.

I hoped it might fix #24874 too, but it seems not.

@Demiu
Copy link
Contributor

Demiu commented Nov 15, 2019

This definitely seems to not do what it should

if (p_stop_on_slope && col.travel.cross(p_floor_direction) == Vector3(0, 0, 0))

col.travel is the result of a move_and_collide along the snap vector. In 99% of use cases this == will almost always be true, as most people will use snap vector that's parallel to the p_floor_direction, so a cross will almost always return a zero vector, and if it does snapping doesn't occur. Or very close to it, due to move_and_collides pre un-stucking code, which is when we would NOT want to apply that motion as it would move us sideways. Thus this basically disables snapping for the most popular use case of it. Unless I missed something I think is completely wrong and just breaks snapping. This is from a mathematical point of view, programmatically this == will almost always be false no matter what because floats and this is an == instead of is_equal_approx. This applies both to 2d and 3d.

It seems the issue with #31097 isn't the application of col.travel itself (because for 2d the snap movement was already limited to be perpendicular to floor plane:
col.travel = p_floor_direction * p_floor_direction.dot(col.travel);
I might be wrong about this but it seems the real problem is that what KinematicBody2D::move_and_collide considers to be "snapped to the floor" doesn't align with what the KinematicBody2D::separate_raycast_shapes considers to be "not intersecting with anything". So move_and_collide moves the body "down" and then separate_raycast_shapes separates it from the ground by moving it perpendicular to the floor (not the floor vector) which is a little up and to the side for a slope. Rinse, repeat and you're slowly moving down a slope. At least I think. This hypothesis works for both 2D and 3D

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Jan 23, 2020
@aaronfranke
Copy link
Member

@raphael10241024 Is this still desired? If so, it needs to be rebased on the latest master branch.

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

Successfully merging this pull request may close these issues.

2D KinematicBody slides down slope with move_and_slide_with_snap
5 participants