Replace confusing code in GodotCapsuleShape2D::get_supports
#83655
Conversation
| @@ -373,8 +373,7 @@ void GodotCapsuleShape2D::get_supports(const Vector2 &p_normal, Vector2 *r_suppo | |||
| if (h > 0 && Math::abs(n.x) > segment_is_valid_support_threshold) { | |||
| // make it flat | |||
| n.y = 0.0; | |||
| n.normalize(); | |||
| n *= radius; | |||
| n.x = radius; | |||
There was a problem hiding this comment.
This would change the behavior if n.x used to be negative. Is that a valid case that can happen?
I agree that the old code is bad though.
There was a problem hiding this comment.
You are right, the fix would be
n.x = Math::sign(n.x) * radius;
There was a problem hiding this comment.
Or
n.x = n.x > 0 ? radius : -radius;
There was a problem hiding this comment.
The top makes more sense to me. Also, I think it preserves the behavior when n.x is zero.
There was a problem hiding this comment.
I wrongly assumed there was a Math::sign its a macro which is SIGN still changed it to use it
golfinq
force-pushed
the
small-capsule-shape-fix
branch
from
20fd3e2 to
e03a65f
Compare
There was a problem hiding this comment.
I have not tested running the code, but the behavior should be identical to the old code, but more straightforward and more performant. With the old code, normalizing a vector with Y equal to 0 will result in X being either -1, 0, or 1, which is identical to SIGN.
|
Thanks! |
akien-mga
changed the title
GodotCapsuleShape2D::get_supports
I am doing some bug hunting and found a place where the code is unnecessarily complicated. Its not a big change, but it helps with understanding what this function is doing when reading it.