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

Vector Refactorings #55

Merged
merged 3 commits into from
Sep 3, 2020
Merged

Vector Refactorings #55

merged 3 commits into from
Sep 3, 2020

Conversation

giawa
Copy link
Owner

@giawa giawa commented Sep 2, 2020

General Notes

This change adds Get to the Vector2 class, removes code duplication, adds CopyTo for vectors without System.Numerics and adds an exception for Get if the index is out of range.

Testing

Limited testing - made sure it compiled and ran a few local tests.

… when system.numerics.vector is unavailable

Always enable extension methods so that we have a single codebase for those methods, and throw an Exception when an index is out of range for the Get methods.
@giawa giawa changed the base branch from master to dotnetcore September 2, 2020 17:48
@giawa giawa requested a review from TheAIBot September 2, 2020 17:48
@giawa giawa changed the title Refactor/vector changes Vector Refactorings Sep 2, 2020
@ghost
Copy link

ghost commented Sep 2, 2020

I think this should be merged. Once it is, I will finish my matrix pull request.

Copy link
Collaborator

@TheAIBot TheAIBot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...adds CopyTo for vectors without System.Numerics..." woops i guess i caused that :)
Pointed out some non important things, other wise it looks good.

OpenGL/Math/Vector2.cs Outdated Show resolved Hide resolved
OpenGL/Math/Vector2.cs Outdated Show resolved Hide resolved
OpenGL/Math/Vector3.cs Outdated Show resolved Hide resolved
OpenGL/Math/Vector4.cs Outdated Show resolved Hide resolved
@giawa
Copy link
Owner Author

giawa commented Sep 3, 2020

Awesome, thanks for taking a look! I implemented the suggestions you made.

@giawa giawa merged commit f20d0e2 into dotnetcore Sep 3, 2020
@ghost ghost mentioned this pull request Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants