CubicCurve::default().position() causes panic: "attempt to subtract with overflow" - Default should work for this. #11209

laundmo · 2024-01-04T00:13:36Z

What problem does this solve or what need does it fill?

Currently, unintuitively, when spawning for example a Component with Default which contains a CubicCurve, other code relying on this component before it was updated crashes.

quick reproduction:

let c = CubicCurve::default();
let p: Vec2 = c.position(0.1);

What solution would you like?

The common way default works is by using zeros (like on Vec2/3). I believe a custom Default impl on CubicCurve which does the same to prevent this issue makes sense.

What alternative(s) have you considered?

instead of panicking, a lot of methods could return Option<>. But this is annoying to deal with in game code.

Additional context

@mockersf

# Objective - Implementing `Default` for [`CubicCurve`](https://docs.rs/bevy/latest/bevy/math/cubic_splines/struct.CubicCurve.html) does not make sense because it cannot be mutated after creation. - Closes #11209. - Alternative to #11211. ## Solution - Remove `Default` from `CubicCurve`'s derive statement. Based off of @mockersf comment (#11211 (comment)): > CubicCurve can't be updated once created... I would prefer to remove the Default impl as it doesn't make sense --- ## Changelog - Removed the `Default` implementation for `CubicCurve`. ## Migration Guide - Remove `CubicCurve` from any structs that implement `Default`. - Wrap `CubicCurve` in a new type and provide your own default. ```rust #[derive(Deref)] struct MyCubicCurve<P: Point>(pub CubicCurve<P>); impl Default for MyCubicCurve<Vec2> { fn default() -> Self { let points = [[ vec2(-1.0, -20.0), vec2(3.0, 2.0), vec2(5.0, 3.0), vec2(9.0, 8.0), ]]; Self(CubicBezier::new(points).to_curve()) } } ```

laundmo added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Jan 4, 2024

garychia mentioned this issue Jan 4, 2024

Ensure CubicCurves with the default value have at least one CubicSegment #11211

Closed

BD103 mentioned this issue Jan 13, 2024

Remove Default impl for CubicCurve #11335

Merged

mockersf closed this as completed in #11335 Jan 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CubicCurve::default().position() causes panic: "attempt to subtract with overflow" - Default should work for this. #11209

CubicCurve::default().position() causes panic: "attempt to subtract with overflow" - Default should work for this. #11209

laundmo commented Jan 4, 2024

CubicCurve::default().position() causes panic: "attempt to subtract with overflow" - Default should work for this. #11209

CubicCurve::default().position() causes panic: "attempt to subtract with overflow" - Default should work for this. #11209

Comments

laundmo commented Jan 4, 2024

What problem does this solve or what need does it fill?

What solution would you like?

What alternative(s) have you considered?

Additional context