Use Dir3 in Transform APIs
#12530
Use Dir3 in Transform APIs
#12530
Conversation
Adjust documentation
There was a problem hiding this comment.
I wonder if shouldn't we use Dir3 directly, instead of impl TryInto<Dir3>, since Dir3 makes it explicit what is going on and what to expect from those parameters.
For the sake of avoiding breaking changes, it could make sense to use impl TryInto<Dir3>, but if we use Dir3 we don't have to worry about fallback values, since Dir3 will always be normalized and always represents an unit direction.
|
@afonsolage This is mentioned in the linked issue:
I'm fairly neutral on this tbh, I implemented the issue as specified but both options make sense to me. |
djeedai
added
C-Usability
| /// Example | ||
| /// ``` | ||
| /// # use bevy_math::{Vec3, Quat}; | ||
| /// # use bevy_math::{Dir3, Vec3, Quat}; | ||
| /// # use bevy_transform::components::Transform; | ||
| /// # let mut t1 = Transform::IDENTITY; | ||
| /// # let mut t2 = Transform::IDENTITY; | ||
| /// t1.align(Vec3::X, Vec3::Y, Vec3::new(1., 1., 0.), Vec3::Z); | ||
| /// let main_axis_image = t1.rotation * Vec3::X; | ||
| /// t1.align(Dir3::X, Dir3::Y, Vec3::new(1., 1., 0.), Dir3::Z); | ||
| /// let main_axis_image = t1.rotation * Dir3::X; | ||
| /// let secondary_axis_image = t1.rotation * Vec3::new(1., 1., 0.); | ||
| /// assert!(main_axis_image.abs_diff_eq(Vec3::Y, 1e-5)); | ||
| /// assert!(secondary_axis_image.abs_diff_eq(Vec3::new(0., 1., 1.), 1e-5)); | ||
| /// | ||
| /// t1.align(Vec3::ZERO, Vec3::Z, Vec3::ZERO, Vec3::X); | ||
| /// t2.align(Vec3::X, Vec3::Z, Vec3::Y, Vec3::X); | ||
| /// t1.align(Vec3::ZERO, Dir3::Z, Vec3::ZERO, Dir3::X); | ||
| /// t2.align(Dir3::X, Dir3::Z, Dir3::Y, Dir3::X); | ||
| /// assert_eq!(t1.rotation, t2.rotation); | ||
| /// | ||
| /// t1.align(Vec3::X, Vec3::Z, Vec3::X, Vec3::Y); | ||
| /// t1.align(Dir3::X, Dir3::Z, Dir3::X, Dir3::Y); | ||
| /// assert_eq!(t1.rotation, Quat::from_rotation_arc(Vec3::X, Vec3::Z)); | ||
| /// ``` |
There was a problem hiding this comment.
Would be nice if the other methods also gotten an example like this, showcasing how you can mix&match vectors and directions to improve discoverability of the fact that both Dir3 and Vec3 are valid arguments, for new users.
Not a necessary change though.
NthTensor
added
the
S-Ready-For-Final-Review
|
TryInto seems like the right choice to me, but I haven't looked this over super closely. Nice work! |
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
Objective
Make
TransformAPIs more ergonomic by allowing users to passDir3as an argument where a direction is needed. Fixes #12481.Solution
Accept
impl TryInto<Dir3>instead ofVec3for direction/axis arguments inTransformAPIsChangelog
The following
Transformmethods now accept animpl TryInto<Dir3>argument where they previously accepted directions asVec3:Transform::{look_to,looking_to}Transform::{look_at,looking_at}Transform::{align,aligned_by}Migration Guide
This is not a breaking change since the arguments were previously
Vec3which already implementsTryInto<Dir3>, and behavior is unchanged.