body_test_motion not reporting initial contact

[godotproposer]

Godot version

4.0.alpha3.official.256069eaf

System information

Manjaro Linux

Issue description

If a body is intersected, PhysicsServer2D.body_test_motion may report a different collider that the body isn't intersecting. I suspect this ensures more consistent sliding behavior within CharacterBody2D.move_and_slide. But I think it's unfit for the scope of this function, making it unreliable for tests.

Steps to reproduce

1. Position a body where it overlaps Collider A.
2. Position Collider B where it would overlap the body after it was ejected from Collider A.
3. Write a script identifying which collider is reported by body_test_motion.
4. Observe as Collider B is reported first, although it did not initially overlap the body, whereas Collider A did. Note that subsequent reports are inconsistent. (Or, open the minimal reproduction project, inspect the scene, then run & observe the console output.)

Minimal reproduction project

minimalrepro.zip

[Zireael07]

Saw the same in 3D. Had to rely on Raycasts instead

[Chaosus]

Does it correctly reporting on 3.x branch?

[Zireael07]

(Not OP, but: seeing this on 3.4 release build)

[godotproposer]

(Not OP, but: seeing this on 3.4 release build)

Same, I recreated the minimal repro in 3.4 with the same results

[Hiiamwilliam]

body_test_motion (on both 3.X and master) ends up calling an internal method test_body_motion.

I don't have the courage to actually understand what's going on there, but if we are to trust the code comments, there are two that look important for this issue:

//STEP 1, FREE BODY IF STUCK

then later

// STEP 2 ATTEMPT MOTION

So maybe the fact that it's not reporting the initially intersected body could be intended? Who knows. However, if that's the case, it should be documented.

[godotproposer]

So maybe the fact that it's not reporting the initially intersected body could be intended? Who knows. However, if that's the case, it should be documented.

I read the source and came to the same conclusion, yet I am convinced it is a design flaw. Currently, the method is unsuitable for tests, which is designated as its use case by name.

A more glaring flaw with the design is that the automation removes options from the user. If "step 1" of the method was removed, it would be trivial to replicate the current functionality by calling it a second time in the event of a collision. With "step 1" present, one cannot replicate the functionality where it isn't.

This is made worse because the lower-level physics code is not exposed to scripting. Thus, one cannot script a variation of the method which lacks "step 1"; one would need to modify and build the engine from source. If those lower-level physics methods won't be exposed, then the exposed functions shouldn't assume what the user wants in this manner.

[AttackButton]

Yeah, got this issue very easily today trying to use collide_separation_ray param property. From long distance it confirms the test collision correctly. But up close these problems start to happen.

[AttackButton]

Actually, it is a pretty neat code, just needs a tweak or two.

edit: Probably backup the first intersection_query_results colliders when enter in the "stuck" part and compare it with other queries of intersection_query_results.