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

Wildly inconsistent documentation on Dir2 <-> Rot2 conversion methods. #14301

Closed
IQuick143 opened this issue Jul 14, 2024 · 1 comment · Fixed by #14307
Closed

Wildly inconsistent documentation on Dir2 <-> Rot2 conversion methods. #14301

IQuick143 opened this issue Jul 14, 2024 · 1 comment · Fixed by #14307
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy
Milestone

Comments

@IQuick143
Copy link
Contributor

IQuick143 commented Jul 14, 2024

How can Bevy's documentation be improved?

/// Get the rotation that rotates the X-axis to this direction.
#[inline]
pub fn rotation_from_x(self) -> Rot2 {
Rot2::from_sin_cos(self.0.y, self.0.x)
}
/// Get the rotation that rotates this direction to the X-axis.
#[inline]
pub fn rotation_to_x(self) -> Rot2 {
// (This is cheap, it just negates one component.)
self.rotation_from_x().inverse()
}
/// Get the rotation that rotates this direction to the Y-axis.
#[inline]
pub fn rotation_from_y(self) -> Rot2 {
// `x <- y`, `y <- -x` correspond to rotating clockwise by pi/2;
// this transforms the Y-axis into the X-axis, maintaining the relative position
// of our direction. Then we just use the same technique as `rotation_from_x`.
Rot2::from_sin_cos(-self.0.x, self.0.y)
}
/// Get the rotation that rotates the Y-axis to this direction.
#[inline]
pub fn rotation_to_y(self) -> Rot2 {
self.rotation_from_y().inverse()
}

    /// Get the rotation that rotates this direction to the Y-axis.
    #[inline]
    pub fn rotation_from_y(self) -> Rot2 {

This docstring is imho completely backwards to what it should say, notably, it is backwards to what the docstring for rotation_from_x says.

Solution

Figure out which way the methods actually go, and update the docstrings appropriately. (Imo the semantics on the x-axis functions are correct, and the doc strings on the y-axis functions are wrong.)

@IQuick143 IQuick143 added C-Docs An addition or correction to our documentation S-Needs-Triage This issue needs to be labelled labels Jul 14, 2024
@mweatherley
Copy link
Contributor

Yeah it looks like I made a copy-paste error here, the docs for the x versions should be correct.

@mweatherley mweatherley added this to the 0.14.1 milestone Jul 14, 2024
@mweatherley mweatherley added D-Trivial Nice and easy! A great choice to get started with Bevy A-Math Fundamental domain-agnostic mathematical operations and removed S-Needs-Triage This issue needs to be labelled labels Jul 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 14, 2024
# Objective

Fixes #14301 

## Solution

Swap them so that they are no longer swapped.
mockersf pushed a commit that referenced this issue Aug 2, 2024
# Objective

Fixes #14301 

## Solution

Swap them so that they are no longer swapped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Docs An addition or correction to our documentation D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants