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

Rename Bezier to CubicBezier for clarity #9554

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

jnhyatt
Copy link
Contributor

@jnhyatt jnhyatt commented Aug 23, 2023

Objective

A Bezier curve is a curve defined by two or more control points. In the simplest form, it's just a line. The (arguably) most common type of Bezier curve is a cubic Bezier, defined by four control points. These are often used in animation, etc. Bevy has a Bezier curve struct called Bezier. However, this is technically a misnomer as it only represents cubic Bezier curves.

Solution

This PR changes the struct name to CubicBezier to more accurately reflect the struct's usage. Since it's exposed in Bevy's prelude, it can potentially collide with other Bezier implementations. While that might instead be an argument for removing it from the prelude, there's also something to be said for adding a more general Bezier into Bevy, in which case we'd likely want to use the name Bezier. As a final motivator, not only is the struct located in cubic_spines.rs, there are also several other spline-related structs which follow the CubicXxx naming convention where applicable. For example, CubicSegment represents a cubic Bezier curve (with coefficients pre-baked).


Migration Guide

  • Change all Bezier references to CubicBezier

@alice-i-cecile alice-i-cecile added C-Usability A simple quality-of-life change that makes Bevy easier to use A-Math Fundamental domain-agnostic mathematical operations labels Aug 23, 2023
@hymm hymm added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 28, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 28, 2023
Merged via the queue into bevyengine:main with commit 087a345 Aug 28, 2023
24 checks passed
@aevyrie
Copy link
Member

aevyrie commented Oct 25, 2023

@alice-i-cecile Shouldn't this be done for all the cubic splines, not just one of them? This makes the spline naming inconsistent.

https://docs.rs/bevy/0.11.3/bevy/math/cubic_splines/index.html

rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

A Bezier curve is a curve defined by two or more control points. In the
simplest form, it's just a line. The (arguably) most common type of
Bezier curve is a cubic Bezier, defined by four control points. These
are often used in animation, etc. Bevy has a Bezier curve struct called
`Bezier`. However, this is technically a misnomer as it only represents
cubic Bezier curves.

## Solution

This PR changes the struct name to `CubicBezier` to more accurately
reflect the struct's usage. Since it's exposed in Bevy's prelude, it can
potentially collide with other `Bezier` implementations. While that
might instead be an argument for removing it from the prelude, there's
also something to be said for adding a more general `Bezier` into Bevy,
in which case we'd likely want to use the name `Bezier`. As a final
motivator, not only is the struct located in `cubic_spines.rs`, there
are also several other spline-related structs which follow the
`CubicXxx` naming convention where applicable. For example,
`CubicSegment` represents a cubic Bezier curve (with coefficients
pre-baked).

---

## Migration Guide

- Change all `Bezier` references to `CubicBezier`
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 simple quality-of-life change that makes Bevy easier to use S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants