feat: Dir3 coordinate system constants #21843
Conversation
ickshonpe
left a comment
ickshonpe
left a comment
There was a problem hiding this comment.
I don't like the Dir2 -> Dir3 conversion very much. The const fns for CompassQuadrant seem fine though, I think they should probably be split off into their own PR.
|
|
||
| /// Get the dir2 representing this direction | ||
| pub const fn to_dir2(self) -> Dir2 { | ||
| match self { | ||
| CompassQuadrant::North => Dir2::NORTH, | ||
| CompassQuadrant::East => Dir2::EAST, | ||
| CompassQuadrant::South => Dir2::SOUTH, | ||
| CompassQuadrant::West => Dir2::WEST, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
These seem fine and could be split off into a separate PR as less controversial.
| CompassOctant::West => Dir2::WEST, | ||
| CompassOctant::NorthWest => Dir2::NORTH_WEST, | ||
| } | ||
| } |
| /// from a dir2 representing a compass direction | ||
| /// per bevy space standard, Y is the up direction | ||
| /// so returns (x, 0, y) | ||
| #[inline] | ||
| pub const fn from_compass_dir(compass: Dir2) -> Self { | ||
| let v = compass.as_vec2(); | ||
| Dir3(Vec3::new(v.x, 0.0, v.y)) | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't really like this, I think it's too context dependent for bevy_math. It's very natural to assume thatDir2(Vec2 { x, y }) maps to Dir3(Vec3 { x, y, z: 0. }). These conversions should be left to users to implement themselves imo.
There was a problem hiding this comment.
Bevy has a coordinate system. +Y is up. This is defined and documented. And this is in line with the compass type.
alice-i-cecile
added
C-Usability
| } | ||
| } | ||
|
|
||
| impl<T: Into<Dir2>> From<T> for Dir3 { |
There was a problem hiding this comment.
It's not the explicit from_compass_dir constructor that's the problem I guess, but this blanket From impl. From's behaviour is meant to be obvious, but there are multiple natural interpretations here, and users often won't check the docs for From.
3 participants
Objective
Solution
Testing
Showcase
While a showcase should aim to be brief and digestible, you can use a toggleable section to save space on longer showcases:
Click to view showcase