Skip to content

Conversation

@cart
Copy link
Member

@cart cart commented Nov 13, 2024

Objective

We currently use special "floating" constructors for EasingCurve, FunctionCurve, and ConstantCurve (ex: easing_curve). This erases the type being created (and in general "what is happening" structurally), for very minimal ergonomics improvements. With rare exceptions, we prefer normal X::new() constructors over floating x() constructors in Bevy. I don't think this use case merits special casing here.

Solution

Add EasingCurve::new(), use normal constructors everywhere, and remove the floating constructors.

I think this should land in 0.15 in the interest of not breaking people later.

@cart cart added the A-Math Fundamental domain-agnostic mathematical operations label Nov 13, 2024
@cart cart added this to the 0.15 milestone Nov 13, 2024
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.

I think this is clearer, more consistent and easier to discover (since it's automatically linked in the docs in both directions).

@cart cart force-pushed the curve-constructors branch from b71c082 to 001c49a Compare November 13, 2024 03:28
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 13, 2024
@mweatherley
Copy link
Contributor

mweatherley commented Nov 13, 2024

I don't feel particularly strongly about this either way, but some notes here about why these are this way:

  1. When the Curve API was first conceived, most of the curve return types of adaptor methods were erased. I moved away from that later for various reasons, but this is sort of a vestige of that.
  2. The return types of these library functions are not really "worth knowing" in a meaningful way; all of their useful methods just derive from them being curves. That is, the EasingCurve/FunctionCurve/ConstantCurve namespaces are effectively otherwise useless.
  3. It's significantly fewer keystrokes :)

Copy link
Contributor

@IQuick143 IQuick143 left a comment

Choose a reason for hiding this comment

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

I don't see an issue with this, and I like the consistency.
Though I second all that mw said.

Copy link
Contributor

@MiniaczQ MiniaczQ left a comment

Choose a reason for hiding this comment

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

Not controversial

@mweatherley mweatherley added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 13, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Nov 13, 2024
Merged via the queue into bevyengine:main with commit 7477928 Nov 13, 2024
32 checks passed
mockersf pushed a commit that referenced this pull request Nov 16, 2024
#16367)

We currently use special "floating" constructors for `EasingCurve`,
`FunctionCurve`, and `ConstantCurve` (ex: `easing_curve`). This erases
the type being created (and in general "what is happening"
structurally), for very minimal ergonomics improvements. With rare
exceptions, we prefer normal `X::new()` constructors over floating `x()`
constructors in Bevy. I don't think this use case merits special casing
here.

Add `EasingCurve::new()`, use normal constructors everywhere, and remove
the floating constructors.

I think this should land in 0.15 in the interest of not breaking people
later.
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Dec 2, 2024
bevyengine#16367)

# Objective

We currently use special "floating" constructors for `EasingCurve`,
`FunctionCurve`, and `ConstantCurve` (ex: `easing_curve`). This erases
the type being created (and in general "what is happening"
structurally), for very minimal ergonomics improvements. With rare
exceptions, we prefer normal `X::new()` constructors over floating `x()`
constructors in Bevy. I don't think this use case merits special casing
here.

## Solution

Add `EasingCurve::new()`, use normal constructors everywhere, and remove
the floating constructors.

I think this should land in 0.15 in the interest of not breaking people
later.
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
bevyengine#16367)

# Objective

We currently use special "floating" constructors for `EasingCurve`,
`FunctionCurve`, and `ConstantCurve` (ex: `easing_curve`). This erases
the type being created (and in general "what is happening"
structurally), for very minimal ergonomics improvements. With rare
exceptions, we prefer normal `X::new()` constructors over floating `x()`
constructors in Bevy. I don't think this use case merits special casing
here.

## Solution

Add `EasingCurve::new()`, use normal constructors everywhere, and remove
the floating constructors.

I think this should land in 0.15 in the interest of not breaking people
later.
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 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.

7 participants