Allow apply_floor_snap to preserve the horizontal position regardless of stop_on_slopes

[yosoyfreeman]

Removes the stop_on_slopes check from apply_floor_snap, allowing the part of the code that preserves the horizontal position to work independently of the value of stop_on_slopes. This should provide a more consistent behavior and prevent the character from sliding along the horizontal axis as a result of what should be a pure vertical snapping.

Fixes godotengine/godot-proposals#9255

It would be technically more correct to set p_cancel_sliding on the parameters too, but it seemed like something for another PR.

EDIT:

@a0kami has made a test scene to make sure it is working as intended.

Pre-PR: The non stop on slopes has non vertical motion on the snapping, more noticeable incrementing safe margin a bit. You can notice how snapping those bodies cause a wrong horizontal motion. Note how the bodies still snap even if the safe margin is large enough to touch the platform, contrary to the stop on slopes bodies that will not try to snap the body if the distance to the floor is less than the safe margin.

pre-pr.mp4

After-pr: The snapping is now consistent between both types of bodies, the non stop on slopes bodies now snap correctly along the snapping vector, and the safe margin is being respected too.

After-pr.mp4

As you can see the different is quite noticeable and we are gaining a lot of precision.

TEST PROJECT BY @a0kami:

test.tar.gz

[yosoyfreeman]

I have been just pointed out that there is an equivalent check in the 2D version. I'm not that familiar with the 2D engine but i will check it out and try to make the equivalent changes there too.

[yosoyfreeman]

PR updated with videos of pre and post PR and a test project.

[rburing]

The change makes sense to me. Indeed floor snapping should only move the body along the up axis. Any motion orthogonal to the up axis is from the "free the body if stuck" phase of body_test_motion. It should be fine to cancel that motion, because the previous (non-floor-snapping) move_and_collide (from the loop in _move_and_slide_grounded) already took care of freeing the body if stuck.

Edit: Something equivalent to the previous sentence should be added to the code comment as well.

Edit 2: I guess the original idea may have been to use the depenetration of the body to intentionally cause some sliding.

[yosoyfreeman]

Thank you! I'll probably have some time tonight or tomorrow!

To be clear, as is my first PR in Godot:

• Should i change the comment in the 3D node to match your new one on the 2D node? for coherence, also yours is clearer.

• I should add a code comment explaining why we can safely cancel the motion, as you explained.

Are those all the changes needed or do i need to tackle something else?

Thank you again for your time!

[rburing]

Should i change the comment in the 3D node to match your new one on the 2D node?

Yes.

I should add a code comment explaining why we can safely cancel the motion, as you explained.

Yes.

Are those all the changes needed or do i need to tackle something else?

Those are all the changes, yes. Then please squash the commits. After that I think we can merge it.

[yosoyfreeman]

Done! Hope i didn't messed up with squashing, kind of confusing, but i think everything is okay.

[rburing]

Looks good. Thanks for working on this!

[yosoyfreeman]

Awesome! Thank you a lot for the assistance!

[Repiteo]

Thanks! Congratulations on your first merged contribution! 🎉