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

Add ways to configure EasingFunction::Steps via new StepConfig #17752

Merged
merged 13 commits into from
Feb 11, 2025

Conversation

RobWalt
Copy link
Contributor

@RobWalt RobWalt commented Feb 9, 2025

Objective

  • In Fix rounding in steps easing function #17743, attention was raised to the fact that we supported an unusual kind of step easing function. The author of the fix kindly provided some links to standards used in CSS. It would be desirable to support generally agreed upon standards so this PR here tries to implement an extra configuration option of the step easing function
  • Resolve Support more "step" easing modes #17744

Solution

  • Introduce StepConfig
  • StepConfig can configure both the number of steps and the jumping behavior of the function
  • StepConfig replaces the raw usize parameter of the EasingFunction::Steps(usize) construct.
  • StepConfigs default jumping behavior is end, so in that way it follows Fix rounding in steps easing function #17743

Testing

Migration Guide

  • EasingFunction::Steps now uses a StepConfig instead of a raw usize. You can replicate the previous behavior by replaceing EasingFunction::Steps(10) with EasingFunction::Steps(StepConfig::new(10)).

@RobWalt RobWalt changed the title Feat/step curve config Add ways to configure EasingFunction::Steps via new StepConfig Feb 9, 2025
I modified them slightly so it should fall under fair use!
@RobWalt RobWalt marked this pull request as ready for review February 9, 2025 07:19
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Animation Make things move and change over time A-Math Fundamental domain-agnostic mathematical operations D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Feb 9, 2025
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 the StepConfig is a bit convoluted; can you swap to EaseFunctionSteps(usize, JumpAt) to make this a bit nicer to use? I think that's a good compromise between the initial design and the one proposed in #17777.

@rparrett
Copy link
Contributor

rparrett commented Feb 10, 2025

One other thing I appreciated about the other PR was the consistency with regard to the SVGs, generating them with the same tool that generated the others.

The SVGs in this PR are more informative, but they are also very large, and the style doesn't seem to fit quite as well.

Although in 17777, the svgs did not actually depict the separate step modes properly.

Expand StepConfig docs image
Expand 17777 Step_ docs image

@@ -903,4 +985,71 @@ mod tests {
);
});
}

#[test]
fn step_config_start() {
Copy link
Member

Choose a reason for hiding this comment

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

not sure those tests are really needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it was more of an sanity check while developing the functions. They run in no time though (literally 0.0s on my machine xD) so it's not really a burden imo

@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Feb 11, 2025
@rparrett rparrett 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 Feb 11, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 11, 2025
Merged via the queue into bevyengine:main with commit aa8793f Feb 11, 2025
29 checks passed
@RobWalt RobWalt deleted the feat/step-curve-config branch February 12, 2025 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-Math Fundamental domain-agnostic mathematical operations C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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.

Support more "step" easing modes
5 participants