Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector2.Normalize should do a zero division check if the input vector is (0, 0) #8125

Open
SelfDevTV opened this issue Jan 19, 2024 · 4 comments

Comments

@SelfDevTV
Copy link

Intent

So if you will use the .Normalize() function on Vector2 and the input vector is of format (X: 0, Y: 0) it will do a zero division and the new vector now is (X: NaN, Y: NaN).

A little check against if both X and Y are 0 or Epsilon would solve this issue. It would then just don't change the vector in place or return the input vector as it was.

...
if (value.X == 0 && value.Y == 0) { return value  };
...

Motivation

The motivation is, that i had a bug with my movement vector. It turns out, normalizing it without doing this check:

if(moveDir != Vector2.Zero)
{
    moveDir.Normalize();
}

It would break my code.

@SelfDevTV
Copy link
Author

If you could assign me, I will try to implement these features if wanted. I will also look at all the other vectors that use this Normalize function in any shape or form.

@mrhelmut
Copy link
Contributor

For the time being, this is by design. MonoGame has been designed to replicate the behavior of XNA, which was setting the vector to NaN if there was a division by 0 (which is "mathematically correct").

So breaking how Normalize work is not desirable. However, we can eventually discuss the addition of non-breaking changes, like a new method doing that or an optional parameter, example:

        public void Normalize(bool defaultToZero = false)
        {
            if (defaultToZero && X == 0.0f && Y == 0.0f)
                return;
            float val = 1.0f / MathF.Sqrt((X * X) + (Y * Y));
            X *= val;
            Y *= val;
        }

Alternatively, you could do your own extension method:

static class Vector2Extensions
{
    public static Vector2 NormalizeZero(this Vector2 v)
    {
        if (v == Vector2.Zero) return v;
        v.Normalize();
        return v;
    }
}

moveDir = moveDir.NormalizeZero(); (this is what we do on our games).

@sconosciute
Copy link

This behavior does not make sense mathematically or programmatically. Regardless of how XNA handled it there are two ways of looking at this:

  1. You can not normalize the zero vector as it's length can not be made one, so you should return the zero vector.
  2. Client code has asked you to perform an impossible task, which is an exceptional state so you should throw an exception noting that the Vector2 can not be normalized, thus resulting in client code refactoring as you suggest above.

Returning NaN is not behavior that is noted anywhere in the docs nor is it reasonable for someone to expect that behavior from a math function like this. It should not break anyone's code to take the first approach and return the zero vector, worst case you have someone performing a now redundant check and new people using the library have clear behavior that aligns to expectations.

It is 2024, XNA is long dead and new users want to use Monogame to make new games, what was essentially a bug or at least bad behavior in the original library should really just be fixed at this point.

@mrhelmut
Copy link
Contributor

I think I need to be more specific on what we're meaning with "for the time being". Sorry for the lack of context.

You're right, though there is one unintuitive aspect of MonoGame we should be aware of (and that I know is a source of frustration for newcomers but we're working toward resolving that).

It is 2024, XNA is long dead and new users want to use Monogame to make new game

That's on paper, and only the visible face. The surprising part is that there is still commercial games using it (yes), and there is a number of studios who still have code based on it that do not wish things to break. Those studios are among the most silent users of MonoGame but are doing the games that reach the largest number of players. And as harmless as the proposed change sounds, with all the different .NET runtimes and code around, we can never be that sure that something as trivial will not cascade into issues.

Yes, that doesn't make sense to newcomers and it makes things look backward and unintuitive, as well as raising the "old-timers vs newcomers" debate, but we're working toward a common ground. The current goal is to make the current API future-proof for old-timers, seal it in its own "XNA-compatible" branch, and move MonoGame forward with breaking changes in a "present & future" branch (and definitely make Normalize to not return NaN).

It's just that we're not quite there yet, so "for the being" there is still going to be some oddities, but you can expect changes in that regard.

Which is also why I didn't close the issue. It's relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants