-
Notifications
You must be signed in to change notification settings - Fork 627
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 vector types with integer components. #908
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, just a few changes.
Resolved the requested changes, let me know if there's anything else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No objections here. Ready to go @varon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor points for discussion prior to merge.
@@ -0,0 +1,544 @@ | |||
// | |||
// Vector3i.cs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor, but why are we repeating the file name in the comments? That seems really pointless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, that should be Vector2i. As a big picture discussion point, not a clue. Jax is the one to talk to about that.
/// <param name="a">Left operand.</param> | ||
/// <param name="b">Right operand.</param> | ||
/// <returns>Result of operation.</returns> | ||
public static Vector2i Add(Vector2i a, Vector2i b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we annotate by-value methods with the Pure attribute?
This is something to look at for the rest of the vector/math library.
Well done and thanks for this very cool contribution, @Vassalware ! |
The discussion in the discord led to the conclusion that
Vector
types with integer components should be added toOpenToolkit.Mathematics
.