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

Change Ellipse representation and improve helpers #11435

Merged

Conversation

Jondolf
Copy link
Contributor

@Jondolf Jondolf commented Jan 20, 2024

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.

@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
@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Jan 20, 2024
Copy link
Contributor

@NiseVoid NiseVoid left a comment

Choose a reason for hiding this comment

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

The improved consistency is nice, more helpers is good too

@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 b592a72 Jan 20, 2024
25 checks passed
@Jondolf Jondolf deleted the ellipse-representation-and-helpers branch January 20, 2024 18:26
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-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants