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
Comments
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 |
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;
}
}
|
This behavior does not make sense mathematically or programmatically. Regardless of how XNA handled it there are two ways of looking at this:
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. |
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).
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. |
Id be in favor of a |
I'm not a super fan of |
Perhaps not marking old method as Deprecated and thus having both options available to be used as "right tool for the right task"? There definitely outta be times when the logic isn't being called as often and being able to just guard clause a TryNormalize would be very much easier to do. But on performance "called thousands of times per second" code, the old method can be used in "I know what I am doing" logic to improve fps. Thus we get the best of both worlds in a non breaking change. |
None of the purposed code for
The general argument fails at runtime, unless every possible scenario is validated up front, hence the default. |
Intent
So if you will use the
.Normalize()
function onVector2
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
andY
are 0 orEpsilon
would solve this issue. It would then just don't change the vector in place or return the input vector as it was.Motivation
The motivation is, that i had a bug with my movement vector. It turns out, normalizing it without doing this check:
It would break my code.
The text was updated successfully, but these errors were encountered: