Wrong collision response of test_move() when the CharacterBody2D is inside a One Way Collision

[MewPurPur]

Godot version

4.0.alpha7

System information

Ubuntu 20.04.3 LTS

Issue description

Expected behavior: Collision detection with one-way CollisionShapes using test_move() only happens when the body is moving against the CollisionShape

What happens: test_move() detects one-way collisions even when moving along a vector with no y-component. The CharacterBody2D itself doesn't seem to detect a collision.

For example, if the one-way collision margin is the default of 1 and the safe margin of the CharacterBody is 0.01, the vertical space where this misfired collision can occur seems to be ~0.43px. This gets wider with bigger collision margins of the one-way collisions or with smaller safe margins used in test_move().

Steps to reproduce

1. Create a CharacterBody2D and a StaticBody2D with one-way collision.
2. Give the CharacterBody2D a script and run test_move() on the CharacterBody2D with its global_transform, a horizontal distance vector, and a small safe margin. The smaller it is, the easier to get a misfired collision.
3. Place the CharacterBody2D inside the very top of the StaticBody2D, while still being inside it. The one-way collision margin of the StaticBody2D can be increased, or the safe margin inside test_move reduced, to see the effect more clearly.

With the minimal reproduction project, just run it, read the script comments, and notice how for some time, the output logs print "true".

Minimal reproduction project

test_move bug.zip

[MewPurPur]

I could replicate this on 4.0.alpha1

Edit: Also on 3.4.3.stable, where test_move uses the KinematicBody2D's safe margin

[rburing]

The following seems to be what happens in the engine in test_body_motion:

When the body is "deep" inside the one-way collision shape, i.e. in the recovery step each pair of contact points has a depth greater than cbk.valid_depth (i.e. greater than 1 here), the counter cbk.invalid_by_dir is incremented, and this causes the pair of shapes (the shape of the body and the one-way collision shape) to be added to excluded_shape_pairs. No recover_motion is calculated. Then, in the motion step there are no collision tests at all and the motion succeeds with safe = 1, which implies no collision is reported.

When the body is "near the surface" inside the one-way collision shape, i.e. in the recovery step each pair of contact points has a depth less than cbk.valid_depth (i.e less than 1 here), then the counter cbk.invalid_by_dir is not incremented, and the pair of shapes is not added to excluded_shape_pairs. Some recover_motion is calculated based purely on the contact points, which ignores the one-way collision direction (in this case, the recovery motion is directed upwards). Then, in the motion step there is a collision test (because shapes were not excluded). Namely there is an initial test, consisting of two steps, to decide if the body is stuck. Firstly it is checked whether there is any overlap, which is true because the recovery motion did not depenetrate the body completely. Secondly it is checked (if applicable, which it is here) whether the motion_normal points in the direction of the one-way direction. Now we note that motion_normal is computed as motion / motion.length(), which is Vector2(nan, nan) if motion = Vector2(0, 0). Hence the second test fails, and the body is detected to be stuck, which cancels the further kinematic solving and which sets safe = 0, which causes a collision to be reported.

[MewPurPur]

@rburing While in the test project, I used Vector2(0, 0), this behavior also occurred with horizontal vectors, such as Vector2(4, 0) which has a normal that isn't nan.

[rburing]

The "stuck" check I mentioned at the end is here: https://github.com/godotengine/godot/blob/563690347a6b9da4882bca15ebd9f3d79f30d3e6/servers/physics_2d/godot_space_2d.cpp#L788-L799

Indeed, it also decides that it's stuck for horizontal motion vectors, in which case motion_normal.dot(direction) == 0.0.

I'm not sure where to begin to fix the issue. cc @fabriceci

[MewPurPur]

Updated the MRP. Still valid in Godot4 beta4

[MewPurPur]

The issue is valid in 4.2.dev4

[elvisish]

Would this bug affect testing if there's a one-way collision anywhere Vector2.DOWN? I've been trying to detect if there are one-ways using test_move below enemies, but it can't find them, but it works fine with horizontal tests.

EDIT: confirmed, it detects correctly if I change the tile to a non-oneway.

If you want to test_move by starting horizontally along and then check downwards, this works with solid tiles, but doesn't with one-way tiles:

var transposed_transform_drop = global_transform
transposed_transform_drop.origin.x += collision_node.shape.size.x + 12 * sign(dir_to_player.x)
var distance_down_to_check = 4500 * delta
if test_move(transposed_transform_drop, Vector2(0.0,distance_down_to_check)):
    jumping = true

This is the convoluted version that works with one-way tiles:

var transposed_transform_drop = global_transform
transposed_transform_drop.origin.x += collision_node.shape.size.x + 12 * sign(dir_to_player.x)
var distance_down_to_check = 4500 * delta
query.set_collision_mask(collision_mask)
query.exclude = [self]
query.shape = collision_node.shape
query.transform = transposed_transform_drop
query.motion = Vector2(0.0,distance_down_to_check)
var down_check = get_world_2d().direct_space_state.cast_motion(query)
if not (down_check[0] == 1.0 and down_check[1] == 1.0):
    jumping = true