-
Notifications
You must be signed in to change notification settings - Fork 108
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
Rotation is broken #37
Comments
Ralle, Good catch! I dug into your issue and the problem is both that:
Victor implements three (radian) rotation functions, each of which has a “Deg” (degrees) counterpart:
Let’s ignore the last 3 “Deg" variants and focus on the first three, comparing the web documentation to the actual implementation. In all 3 cases angles are CCW from +X axis. .rotate(angle) : .rotateBy(rotation): .rotateTo(angle): I’d also make the following observations:
CONCLUSIONS
RESOLUTION Therefore I would suggest the following actions:
All of the above follows for the 3 “Deg” variants as well. They have the same issues. IMPLEMENTATION Cheers |
Wow. What a nice answer. You rock. Just to help Max make his decision. My need was to be able to rotate a vector incrementally, so rotating it 0 would not rotate it at all. |
Ralle, Thanks! In the meantime, you'll find that you can use .rotate() instead of .rotateBy(), and it will do what you were originally looking for. |
+1 Just lost about 2 hours on this before finding this issue! |
Can't believe this is still an issue three years on - just wasted half a day narrowing down the issue before eventually deciding it must be a problem in these functions and finding this issue. Great answer epmoyer thanks! Who maintains the docs? |
@epmoyer that's great investigative work! It seems the documentation is currently unmaintained, but I found the source code for the webpage in the I fixed the documentation issue in my fork and it's now online at https://a-robu.github.io/victor/. |
These are all expected to return the same vector back but rotate them 45 degrees instead.
The text was updated successfully, but these errors were encountered: