-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Add methods to Vector2i/3i for consistency with Vector2/3 #2297
Comments
Thanks for evaluating everything. I haven't reviewed the list in depth yet but on general principles it seems sensible to me. |
If we go for consistency, I'd go for more complete one, when possible. Integer operations can be used for more robust computations by scaling coordinates by some value (like 1000000), which can alleviate some of the issues with floating point error accumulation in some cases. For instance, see Robust geometric computation:
That said, methods like:
can actually make sense when used with the scaling technique I mentioned above. More on rotate, perhaps it would be worth to add |
@Xrayez That is called fixed precision, but it won't actually help in most real-world situations. If you use integer coordinates at a resolution of 1 µm (1000000x scale), then you will still have inaccurate calculations, it will just round to a multiple of 1 µm. Fixed precision also has the downside that there is a sharp distance cutoff beyond which nothing can exist, and there is a sharp precision cutoff at small scales that would prevent you from making a microcosm. Compare this with floats where it still has to round to the nearest multiple of something, the difference is that the precision level is dynamic and depends on the magnitude of the number. For 32-bit floats, for numbers between 1 and 2, it rounds to the nearest multiple of 2^-23, for numbers between 2 and 4 is it rounds to the nearest multiple of 2^-22, for numbers between 4 and 8 is it rounds to the nearest multiple of 2^-21, etc. For 64-bit floats, it's the same except between 1 and 2 the step is 2^-52 etc. With floats, there is no sharp cutoff, you can just keep going farther and things get less accurate, or you can zoom in towards zero and things get more accurate, so you can make games at any scale and they should work nearly identically. In conclusion, I don't think this is a good use case for |
Well, that's the point of computational robustness, I'm not talking about precision per se. Integer-based operations perform much faster than using floating point types as well, and since performance is paramount for a lot of games, this definitely has real-world applications. Some games also rely on deterministic outcomes, which floating point types cannot provide most of the time. At this point, we're delving into whether we'd like to have fixed point precision datatypes to be implemented in Godot. I think this won't happen at all in Godot, but I just wanted to share my 2 cents on the topic. 🙂 |
See discussion in godotengine/godot#18136. |
I'm currently working on this. |
@likeich For Vector3i note that I have a branch here: https://github.com/aaronfranke/godot/tree/signed-angle-vec3i The reason I haven't submitted the above branch as a pull request yet is because this proposal is waiting on approval from the core developers. I would recommend not working on this further until we get approval + until my branch has been merged. However, once that happens, you are absolutely welcome to work on this! (you can leave your PR open since you've already made it). |
Sounds good. What exactly does an approved proposal look like though? |
For the record, for now we decided to implement For the rest, and especially the angle methods and dot/cross products, the use case in integer math doesn't seem clearly established so we need more evidence of it being actually useful/needed before adding such methods only for API consistency. |
Note that snapped were added in godotengine/godot#68386 (might be helpful to strike off entries that have been implemented) |
|
Describe the project you are working on
Godot 4.0+
Describe the problem or limitation you are having in your project
There are many methods in
Vector2
andVector3
that don't currently exist inVector2i
andVector3i
in the current master branch. Many of them would make sense to have inVector2i
andVector3i
.Describe the feature / enhancement and how it helps to overcome the problem or limitation
Should likely be added:EDIT: But needs justification in terms of actual use cases.cross
: Makes sense in an integer context, and is a very general method for vectors. Would return an int forVector2i
, and would return aVector3i
forVector3i
. It could also be implemented in 4D.dot
: Makes sense in an integer context, and is a very general method for vectors. Would return an int.length
: Makes sense in an integer context, and is a very general method for vectors. Would return a float.length_squared
: Makes sense in an integer context, and is a very general method for vectors. Would return an int.distance_to
: Makes sense in an integer context, and is a very general method for vectors. Would return a float.distance_squared_to
: Makes sense in an integer context, and is a very general method for vectors. Would return an int.posmod
(v): Makes sense in an integer context, and has many use cases with integers. Would return a vector.Maybe:
snap
,snapped
: Makes sense and would likely be useful.angle_to
,signed_angle_to
: Makes sense and would likely be useful. It could also be implemented in 4D.angle
andangle_to_point
: Makes sense and would likely be useful.orthogonal
: Makes sense and would likely be useful.Maybe not:
rotate
(d): Does not often make sense in an integer context. Would only work properly for rotations that are multiples of 90 degrees, or it would work for very large vectors as @Xrayez pointed out.project
: Likely not very useful in an integer context and does not work in all cases.plane_project
: Unsure if this would make sense forVector2i
, and it's not exposed to GDScript anyway.slide
,bounce
,reflect
: Likely not very useful in an integer context, these methods depend on the normal argument being normalized, so they would only work in cases where a vector is being slid/bounced/reflected off of a trivial XY/XZ/YZ plane.Should not be added:
normalize
(d): Does not make sense in an integer context. Would only work if at most one component was non-zero.inverse
: Does not make sense and would be completely useless in an integer context.lerp
,cubic_interpolate
,move_toward
: Does not make sense in an integer context. Would only work properly for a few edge cases, and would not work for most inputs.floor
,ceil
,round
: Does not make sense and would be completely useless in an integer context.outer
,to_diagonal_matrix
: The end result is aBasis
which uses floats, so if users wanted to use these withVector3i
, it would make sense to just require converting their vector toVector3
first. Also, maybe these methods should be moved toBasis
? I don't see whyVector3
has to depend onBasis
.direction_to
: Depends on the output being normalized. It could work if we made it haveVector3i
inputs and aVector3
output, but then again we could also just require users to convert their inputs toVector3
like above.is_equal_approx
: Does not make sense and would be completely useless in an integer context.limit_length
: Does not make sense for integers.Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
It would work by implementing these methods in Vector2i and Vector3i.
If this enhancement will not be used often, can it be worked around with a few lines of script?
Used often is uncertain and debatable. Yes, this can be worked around with a few lines of script, but this kind of API is something that should be done to Vector2i and Vector3i, since it's core for Vector2 and Vector3 to already have these methods.
Is there a reason why this should be core and not an add-on in the asset library?
Vector2 and Vector3 already have these methods in core, so it would be consistent for Vector2i and Vector3i to have them in core too, rather than having these methods be add-ons in the asset library.
The text was updated successfully, but these errors were encountered: