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

Method to normalize zero vectors #2269

Open
Yu266426 opened this issue Jun 22, 2023 · 19 comments · May be fixed by #3157
Open

Method to normalize zero vectors #2269

Yu266426 opened this issue Jun 22, 2023 · 19 comments · May be fixed by #3157

Comments

@Yu266426
Copy link

Description
When normalizing a vector, it is necessary to first check it is not of length zero, otherwise the program will crash.

The game engine bevy has a method for its Vecs that is normalize_or_zero() that allows you to normalize a vector that is potentially of length zero, where it returns a zero vector if that is the case, otherwise normalizing it.

I think this would be useful to have in pygame, as it saves having to type an extra line pretty much every time you want to normalize a vector.

@bilhox
Copy link
Contributor

bilhox commented Jun 22, 2023

Mathematically speaking, it's impossible to normalize a zero vector, so returning a zero vector would be weird. That's why, I don't agree with you.

@Yu266426
Copy link
Author

Well, I don't agree in the sense that just because something doesn't "make sense mathematically" means that a more convenient method shouldn't exist.

But also, that's why the method is called normalize_OR_ZERO, which I think helps illustrate that it isn't just normalizing it.

@bigwhoopgames
Copy link

I would be in support of having this method added. In nearly ever instance of me using normalize() I use a zero vector if I can't normalize.

@Matiiss
Copy link
Member

Matiiss commented Jun 25, 2023

I like the idea of having a way to conveniently get the zero vector back when normalizing, the proposed method's name is quite clear in what it would do and thus I don't see how it wouldn't make sense mathematically either, you either can normalize or you return a zero vector.

An alternative idea would be to add an optional argument to normalize, like zero that would achieve the same thing: vec.normalize(zero=True) (perhaps, it could be a positional-only argument for performance: vec.normalize(True) albeit that makes it less readable, but maybe using keyword args doesn't make it all that slow, so that should be tested)

@DaNubCoding
Copy link

Perhaps xnormalize is also an option, meaning "exclusive normalize", where it will normalize everything excluding zero vector in which case it is handled differently. This is easier to type and doesn't clutter code with long method names. Stuff like FRect uses a similar method of naming.

@Mega-JC
Copy link
Member

Mega-JC commented Jun 28, 2023

I'm not sure if pygame needs this to be honest, especially if there is already a super simple solution to this problem of getting a potential ValueError: Simply checking the vector's thuthiness using an if statement, or short-circuiting with and by writing the expression vector and vector.normalize(). In the latter case, if vector is a zero vector, the expression will simply return vector. If vector is not a zero vector, vector.normalize() will be returned instead. This seems to be the exact behavior desired here.

Another example:

>>> from pygame import Vector2
>>> 
>>> vec_a = Vector2(5, 0)
>>> vec_a.normalize()
Vector2(1, 0)
>>> 
>>> vec_b = Vector2(0, 0)
>>> vec_b.normalize()
Traceback (most recent call last):
  File "<pyshell#21>", line 1, in <module>
    vec_b.normalize()
ValueError: Can't normalize Vector of length Zero
>>>
>>> vec_a_norm_or_zero = vec_a and vec_a.normalize()
>>> vec_a_norm_or_zero
Vector2(1, 0)
>>>
>>> vec_b_norm_or_zero = vec_b and vec_b.normalize()
>>> vec_b_norm_or_zero
Vector2(0, 0)

@bilhox
Copy link
Contributor

bilhox commented Jun 28, 2023

I think however we should have prebuilt Vectors (Like in Unity I think), like Vector2.zero (0 , 0) for example. So it would more easy to compare and check if a vector in null. (or a method that returns true if it's zero ?)

@itzpr3d4t0r
Copy link
Member

I think however we should have prebuilt Vectors (Like in Unity I think), like Vector2.zero (0 , 0) for example. So it would more easy to compare and check if a vector in null. (or a method that returns true if it's zero ?)

There's no need for such method, you can already do if not vector:, vectors support bool checks.

@Matiiss
Copy link
Member

Matiiss commented Jun 30, 2023

I think however we should have prebuilt Vectors (Like in Unity I think), like Vector2.zero (0 , 0) for example. So it would more easy to compare and check if a vector in null. (or a method that returns true if it's zero ?)

Aside from vectors supporting boolean checks, you can get a zero vector by simply instantiating a vector: pygame.Vector2()

@DaNubCoding
Copy link

Simply checking the vector's thuthiness using an if statement, or short-circuiting with and by writing the expression vector and vector.normalize().

These methods work, however they are either cumbersome to write when normalize is used frequently (if statement solution) or simply unclear to the reader as to what they are doing (and solution).

@Mega-JC
Copy link
Member

Mega-JC commented Jul 6, 2023

or simply unclear to the reader as to what they are doing (and solution).

To clarify, vector and vector.normalize() (short-circuting with and) is the same as vector.normalize() if vector else vector (ternary expression), for those who prefer the longer, more explicit syntax. The former just seemed to be the shortest and the easiest to substitute a bare .normalize() method call with in a long sequence of vector operations (Yu266426 implied wanting a concise solution).

@Mega-JC Mega-JC added the math pygame.math label Jul 6, 2023
@DaNubCoding
Copy link

To clarify, vector and vector.normalize() (short-circuting with and) is the same as vector.normalize() if vector else vector (ternary expression), for those who prefer the longer, more explicit syntax. The former just seemed to be the shortest and the easiest to substitute a bare .normalize() method call with in a long sequence of vector operations (Yu266426 implied wanting a concise solution).

Yes the short-circuiting solution is a good solution as of now, but I think it can be made clearer with the implementation of a normalize_or_zero(). It is indeed a more concise version of the ternary solution, but it loses some clarity in comparison.

@JiffyRob
Copy link
Contributor

Just want to point out, if we did implement this, would we also want to do something similar with scale_to_length()? If we don't it's a simple normalize(zero=True) * length but the whole point of this issue is conciseness. I would be in favor of the normalize method just because I use it so often and end up erring out and having to write boilerplate for it constantly. You're right to say that it's pretty simple just to short circuit, but when it is needed so dang often the trade-off makes less sense. Also many pygame users are just learning python and don't know how short circuiting works. At the very least I would put a note in the docs.

@bilhox
Copy link
Contributor

bilhox commented Oct 6, 2024

At the beginning, I was against this idea because it's scientifically wrong. So I checked how it was done on other game engine like Unity and godot, and I found that they both return a zero vector, which joins the idea of the original message.

So if anyone is looking at this issue and wants to implement this feature, this is the way to go imo.

@Doublestuf
Copy link

I'd love to take this issue :). It'd be my first contribution to pygame-ce (been wanting to contribute to it for a while) and second contribution overall. Might take a while, though.

@bilhox
Copy link
Contributor

bilhox commented Oct 8, 2024

Hi @Doublestuf ,

Sure you can do it ! The code you need to modify should be in src_c/math.c in vector_normalize_ip. You'll also have to write a new test in test/math_test.py, and update the documentation in docs/reST/ref/math.rst in the method paragraph.

@Doublestuf Doublestuf linked a pull request Oct 8, 2024 that will close this issue
@oddbookworm oddbookworm linked a pull request Oct 9, 2024 that will close this issue
@oddbookworm
Copy link
Member

I'm still not convinced that this is necessary. It may be what bona-fide game engines do, but pygame-ce is not a game engine. There are ways I can think of that being able to "normalize" the zero vector could go pretty wrong because you're expecting the result to be nonzero in your calculations. IMO implementing this just moves the problem from normalizing to wherever else you use the resulting normalized vector. You still need to be cognizant of the case you have the zero vector, so I don't see any benefit to this in general

@aatle
Copy link
Contributor

aatle commented Oct 9, 2024

Note that the behavior of .normalize_ip() is currently equivalent to .scale_to_length(1), so the latter might also need changes.

@Yu266426
Copy link
Author

Yu266426 commented Oct 9, 2024

I still think a separate method is the way to go here. That way the original functionality is unchanged, while providing a more convenient option for situations like normalising input, or maybe getting an offset direction for AI might end up in the player.

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

Successfully merging a pull request may close this issue.