Potential bug in `PhysicsBody2D.gd`

[Daniel-Cortez]

(I'm not really sure if this is actually a bug or not (hence "potential"), but still reporting here, as such mistakes can be hard to find otherwise.)

I was lurking through the code in PhysicsBody2D.gd and accidentally found what seems to be a pretty sneaky mistake.

SonicWorldsNext/Scripts/Objects/PhysicsObject.gd

Line 265 in 99d4ff5

if move_and_collide(rayHitVec-(normHitVec*($HitBox.shape.size.y/2))-Vector2(0,yGroundDiff).rotated(rotation),true,true,true):

According to the documentation for move_and_collide(), the 3'rd argument (safe_margin) is supposed to be float, not bool.

Now what makes this mistake "sneaky" is that GDScript automatically (implicitly) converts that value from bool to float (in this case, from true to 1.0, while the default value of that argument is 0.08), never warning about a type mismatch, which is why the abovementioned line essentially becomes

if move_and_collide(rayHitVec-(normHitVec*($HitBox.shape.size.y/2))-Vector2(0,yGroundDiff).rotated(rotation),true,1.0,true):

I'm not very well acquainted with the physics code, so I'm asking more experienced users/devs: is argument safe_margin here really supposed to be 1.0? If not, what would be the best way to fix it?

[Daniel-Cortez]

Tried to dig for the source of that mistake, and here's what I found:

Before Godot 4.0, PhysicsObject used to be inherited from KinematicBody2D, which had the following definition for move_and_collide():

KinematicCollision2D move_and_collide (
    Vector2 rel_vec,
    bool infinite_inertia=true,
    bool exclude_raycast_shapes=true,
    bool test_only=false )

But after transitioning onto Godot 4, PhysicsObject was changed, now it inherits from CharacterBody2D, which inherits from PhysicsBody2D that has a different definition for move_and_collide():

KinematicCollision2D move_and_collide(
    motion: Vector2,
    test_only: bool = false,
    safe_margin: float = 0.08,
    recovery_as_collision: bool = false)

Arguments 2, 3 and 4 don't match the previous definition, which seems to be the source of that mistake: when updating to Godot 4, PhysicsObject was transitioned from KinematicBody2D to CharacterBody2D as a base, but the arguments in that move_and_collide() call were never updated.

I'm still not sure what consequences this mistake may cause. Would be nice if someone with a better understanding of physics could look into it.

[DimensionWarped]

I'm surprised that didn't just spit errors. I guess it's like you said and the bool was getting cast to float. I'm not immediately sure what the implications are, but I'll try and have a look.

[kites222]

Sorry if you dont like AI, i dont think the ai is correct, it said: The code was written for Godot 3’s KinematicBody2D.move_and_collide(), which used this signature:

move_and_collide(rel_vec, infinite_inertia=true, exclude_raycast_shapes=true, test_only=false)

Then you migrated the project to Godot 4, where CharacterBody2D/PhysicsBody2D.move_and_collide() is:

move_and_collide(
motion: Vector2,
test_only: bool = false,
safe_margin: float = 0.08,
recovery_as_collision: bool = false
)

So the original code:
move_and_collide(vec, true, true, true)

was meant to be:
infinite_inertia = true
exclude_raycast_shapes = true
test_only = true

But now, in Godot 4, this gets misinterpreted as:
test_only = true
safe_margin = 1.0 (because true → 1.0 float)
recovery_as_collision = true

That safe_margin = 1.0 is absolutely not intended.