-
-
Notifications
You must be signed in to change notification settings - Fork 198
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 Mul operator for Quaternion + Vector3. #894
Conversation
Thank you! The CI errors seem unrelated, I'll fix them tomorrow. Could you maybe add a simple Please also squash to 1 commit in the end 🙂 |
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-894 |
Yep can do, though quick question, is there a reason why itests are being used instead of rust's builtin unit test? |
0290b88
to
17d6303
Compare
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.
Yes, the reason is that we need Godot to run, and Rust's #[test]
isn't configurable to change the "host" application.
See also https://godot-rust.github.io/book/contribute/dev-tools.html.
Could you run rustfmt?
Rebased onto master to resolve the unrelated CI issues (#895). |
3dd2012
to
979bfcd
Compare
ran rust format and clarified comment |
979bfcd
to
4a31626
Compare
#[itest] | ||
fn quaternion_mul2() { | ||
use godot::builtin::real; | ||
use std::f32::consts::PI; | ||
|
||
let q = Quaternion::from_axis_angle(-Vector3::UP, PI / 2.0 as real); | ||
let rotated = q * Vector3::new(1.0, 4.2, 2.0); | ||
assert_eq_approx!(rotated.x, -2.0); | ||
assert_eq_approx!(rotated.y, 4.2); | ||
assert_eq_approx!(rotated.z, 1.0); | ||
} |
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.
Could you make one of these 2 tests a non-90° rotation? Maybe run the same in GDScript and check the values. Thanks a lot! 🙂
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.
Added a non-90° and checked back in godot to make sure numbers are correct.
899aa5d
to
a971385
Compare
Thanks a lot, nice work on your first pull request! 😊 |
Fixes #893.