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.

Sign up for GitHub

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

Jump to bottom

Improve Rectangle and Cuboid consistency #11434

Merged
merged 3 commits into from
Jan 20, 2024
Merged

Improve Rectangle and Cuboid consistency #11434

merged 3 commits into from
Jan 20, 2024

Conversation

Jondolf
Copy link
Contributor

@Jondolf Jondolf commented Jan 20, 2024

Objective

The Rectangle and Cuboid primitives currently use different representations:

pub struct Rectangle {
    /// The half width of the rectangle
    pub half_width: f32,
    /// The half height of the rectangle
    pub half_height: f32,
}

pub struct Cuboid {
    /// Half of the width, height and depth of the cuboid
    pub half_extents: Vec3,
}

The property names and helpers are also inconsistent. Cuboid has half_extents, but it also has a method called from_size. Most existing code also uses "size" instead of "extents".

Solution

Represent both Rectangle and Cuboid with half_size properties.

@Jondolf Jondolf added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Math Fundamental domain-agnostic mathematical operations labels Jan 20, 2024
@Jondolf Jondolf mentioned this pull request Jan 20, 2024
Merged
alice-i-cecile
alice-i-cecile approved these changes Jan 20, 2024
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks.

@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Jan 20, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 20, 2024
Merged via the queue into bevyengine:main with commit 6337fb3 Jan 20, 2024
22 checks passed
@Jondolf Jondolf deleted the rectangle-and-cuboid-consistency branch January 20, 2024 18:17
github-merge-queue bot pushed a commit that referenced this pull request Jan 20, 2024
@Jondolf
Change Ellipse representation and improve helpers (#11435)
b592a72 
# Objective

Currently, the `Ellipse` primitive is represented by a `half_width` and
`half_height`. To improve consistency (similarly to #11434), it might
make more sense to use a `Vec2` `half_size` instead.

Alternatively, to make the elliptical nature clearer, the properties
could also be called `radius_x` and `radius_y`.

Secondly, `Ellipse::new` currently takes a *full* width and height
instead of two radii. I would expect it to take the half-width and
half-height because ellipses and circles are almost always defined using
radii. I wouldn't expect `Circle::new` to take a diameter (if we had
that method).

## Solution

Change `Ellipse` to store a `half_size` and `new` to take the half-width
and half-height.

I also added a `from_size` method similar to `Rectangle::from_size`, and
added the `semi_minor` and `semi_major` helpers to get the
semi-minor/major radius.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@NiseVoid NiseVoid NiseVoid approved these changes

Assignees
No one assigned
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Milestone
0.13
Development

Successfully merging this pull request may close these issues.
3 participants
@Jondolf @NiseVoid @alice-i-cecile