Skip to content

Checking points when creating CubicBezier (#13726)#13805

Draft
Wuketuke wants to merge 3 commits intobevyengine:mainfrom
Wuketuke:issue_13726
Draft

Checking points when creating CubicBezier (#13726)#13805
Wuketuke wants to merge 3 commits intobevyengine:mainfrom
Wuketuke:issue_13726

Conversation

@Wuketuke
Copy link
Copy Markdown
Contributor

@Wuketuke Wuketuke commented Jun 11, 2024

when calling CubicBezier::new, we both check if the points provided are empty,
or if the points are discontinuous.
in both cases, we now return None.

keep in mind that I added PartialEq to the VectorSpace (so i will have to modify the documentation later).
I dont think its the best solution, and its more something of a conversation starter.

Fixes #13726

Wuketuke added 2 commits June 11, 2024 11:25
@Wuketuke
hacked one little solution in
0e533b1
@Wuketuke
checking points when creating CubicBezier (bevyengine#13726)
6fb44eb 
when calling CubicBezier::new, we both check if the points provided are empty,
or if the points are discontinuous.
in both cases, we now return None.
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Math Fundamental domain-agnostic mathematical operations S-Needs-Review Needs reviewer attention (from anyone!) to move forward M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Jun 11, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 11, 2024

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@stevehello166
Copy link
Copy Markdown
Contributor

stevehello166 commented Jul 17, 2024

The reason the CI test is failing currently is this.

   --> examples/animation/cubic_curve.rs:37:43
    |
37  |     let bezier = CubicBezier::new(points).to_curve();
    |                                           ^^^^^^^^ method not found in `Option<CubicBezier<Vec3>>`
    |

I suggest putting an .unwrap, match statement, or .expect() on it

@NthTensor NthTensor mentioned this pull request Jul 27, 2024
Merged
@BenjaminBrienen BenjaminBrienen added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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 D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CubicBezier should not allow constructing discontinuous curves

4 participants

@Wuketuke @stevehello166 @alice-i-cecile @BenjaminBrienen