search

godot-rust / gdnative

Rust bindings for Godot 3
https://godot-rust.github.io
MIT License
3.62k stars 210 forks source link

`Vector2::ZERO.normalized()` returns `Vector2(NaN, NaN)` #1080

Open Shou opened 3 months ago

Shou commented 3 months ago

In Godot, Vector2.ZERO.normalized() returns Vector2.ZERO but the Rust equivalent uses glam which seems to be the cause for returning NaN (but I haven't verified). If that's the case, we should probably use normalize_or and return zero as the fallback instead.

This affects any function that uses normalized() as well, like direction_to().

Bromeon commented 3 months ago

gdnative is currently in maintenance mode (if at all), meaning we're quite hesitant to apply any changes that can be potentially breaking. This behavior has existed for a long time, and it's possible that some code relies on it. It's also documented as such (even if not explicitly, but that expression is NaN as well):

Vector2 normalized

May I ask in which context you encountered this?

Bromeon commented 3 months ago

Note that in gdext (Godot 4 bindings), we indeed have multiple overloads with very explicit docs:

image

Shou commented 3 months ago

May I ask in which context you encountered this?

I have methods for pulling/pushing bodies towards/away from a specified position, and when that position is equal direction_to() ends up making the vector NaN. I wouldn't mind making a PR, but if it's unlikely to be accepted I'm fine either way. I mostly wanted to document this behavior as I tried searching for it here first.

Bromeon commented 3 months ago

We could maybe add a normalized_or_zero method just like in gdext, but I'd rather not change the semantics of the existing one right now 🙂

Documenting would be a good idea, too.

Bromeon commented 2 months ago

@Shou are you interested in a PR that adds normalized_or_zero?