Fix wrong collision reported on move_and_collide

[fabriceci]

fix #59422
fix #59139

When a recover occurs, collisions are reported to move_and_collide, this is a useful change introduced to stabilise the movement in move_and_slide, but it's not something desired for move_and_collide.

As discussed with @reduz, there are 2 ways to solve this problem, the first is to remove the condition that report a collision when recovery occurs but this required to modify test_body_motion which is very sensitive and will remove the improvement mentioned above.

This PR is the safest approach, it checks if collision_safe_fraction < 1 (which means there were no collisions in motion) and only modifies move_and_collide.

Before:

After:

I tried the examples provided by @KoBeWi , the ones provided in the documentation, and custom ones.

[rburing]

I confirm this PR restores the behavior of the move_and_collide method so that it doesn't generate collisions which come only from recovery. Moreover, it uses the same approach that is already implemented in test_move. I have some refactoring ideas but those can wait; this is an improvement, restoring the expected behavior.

[fabriceci]

@reduz :I first suggested to add a parameter MotionParameters but I ended up with this simple condition. @rburing pointed out to me that by using a new parameter in MotionParameters, the code inside the recovered condition (in test_body_motion) will be skipped (in the price of adding/exposing this new parameter).

We can add his suggestion on this PR or on a follow up PR, and possibly only in 4.0. Let me know what's do you think about that.

[Hiiamwilliam]

Oh, so move_and_collide was meant to work the same way as test_move? As in: "return a positive result only if the motion stopped, ignoring touches along the way"? Hmm, that's good to know.

But if that's the case, I suppose the documentation should be updated? test_move explicitly says:

Returns true if a collision would stop the body from moving along the whole path.

Wouldn't move_and_collide need something similar?

[rburing]

In the linked issues we see cases (mainly in user code) where recovery-only should not be reported as a collision, and I would guess this is the most common case. On the other hand there are cases where we do want recovery-only to be reported as a collision. One of those use cases (mainly in engine code) is improving floor detection when there is no snapping; this is already handled in this PR by not changing the internal move_and_collide.

However, another use case where you would want recovery-only to be reported as a collision is calling move_and_collide(Vector2.ZERO) purely to get unstuck and find out how you were unstuck. This would be an argument in favor of adding more configurability, e.g. by adding a parameter in MotionParameters and an extra argument in move_and_collide.

If you use move_and_collide(Vector2.ZERO) only when you spawn the body then you probably don't care how you were unstuck, so you don't need the recovery to be reported as a collision. So maybe "purely getting unstuck and finding out how" is not a practical use case. If that is so then I approve this PR because it fixes real issues.

[akien-mga]

Thanks!