Commit graph

116 commits

Author SHA1 Message Date
Aevyrie
61b98ec80f
Rename trigger.entity() to trigger.target() (#16716)
# Objective

- A `Trigger` has multiple associated `Entity`s - the entity observing
the event, and the entity that was targeted by the event.
- The field `entity: Entity` encodes no semantic information about what
the entity is used for, you can already tell that it's an `Entity` by
the type signature!

## Solution

- Rename `trigger.entity()` to `trigger.target()`

---

## Changelog

- `Trigger`s are associated with multiple entities. `Trigger::entity()`
has been renamed to `Trigger::target()` to reflect the semantics of the
entity being returned.

## Migration Guide

- Rename `Trigger::entity()` to `Trigger::target()`.
- Rename `ObserverTrigger::entity` to `ObserverTrigger::target`
2024-12-08 21:55:09 +00:00
homersimpsons
0707c0717b
✏️ Fix typos across bevy (#16702)
# Objective

Fixes typos in bevy project, following suggestion in
https://github.com/bevyengine/bevy-website/pull/1912#pullrequestreview-2483499337

## Solution

I used https://github.com/crate-ci/typos to find them.

I included only the ones that feel undebatable too me, but I am not in
game engine so maybe some terms are expected.

I left out the following typos:
- `reparametrize` => `reparameterize`: There are a lot of occurences, I
believe this was expected
- `semicircles` => `hemicircles`: 2 occurences, may mean something
specific in geometry
- `invertation` => `inversion`: may mean something specific
- `unparented` => `parentless`: may mean something specific
- `metalness` => `metallicity`: may mean something specific

## Testing

- Did you test these changes? If so, how? I did not test the changes,
most changes are related to raw text. I expect the others to be tested
by the CI.
- Are there any parts that need more testing? I do not think
- How can other people (reviewers) test your changes? Is there anything
specific they need to know? To me there is nothing to test
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

(kept in case I include the `reparameterize` change here)

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.

## Questions

- [x] Should I include the above typos? No
(https://github.com/bevyengine/bevy/pull/16702#issuecomment-2525271152)
- [ ] Should I add `typos` to the CI? (I will check how to configure it
properly)

This project looks awesome, I really enjoy reading the progress made,
thanks to everyone involved.
2024-12-08 01:18:39 +00:00
Carter Anderson
af10aa38aa
AnimatedField and Rework Evaluators (#16484)
# Objective

Animating component fields requires too much boilerplate at the moment:

```rust
#[derive(Reflect)]
struct FontSizeProperty;

impl AnimatableProperty for FontSizeProperty {
    type Component = TextFont;

    type Property = f32;

    fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> {
        Some(&mut component.font_size)
    }
}

animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableKeyframeCurve::new(
        [0.0, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0]
            .into_iter()
            .zip([24.0, 80.0, 24.0, 80.0, 24.0, 80.0, 24.0]),
    )
    .map(AnimatableCurve::<FontSizeProperty, _>::from_curve)
    .expect("should be able to build translation curve because we pass in valid samples"),
);
```

## Solution

This adds `AnimatedField` and an `animated_field!` macro, enabling the
following:

```rust
animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableCurve::new(
        animated_field!(TextFont::font_size),
        AnimatableKeyframeCurve::new(
            [0.0, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0]
                .into_iter()
                .zip([24.0, 80.0, 24.0, 80.0, 24.0, 80.0, 24.0]),
        )
        .expect(
            "should be able to build translation curve because we pass in valid samples",
        ),
    ),
);
```

This required reworking the internals a bit, namely stripping out a lot
of the `Reflect` usage, as that implementation was fundamentally
incompatible with the `AnimatedField` pattern. `Reflect` was being used
in this context just to downcast traits. But we can get downcasting
behavior without the `Reflect` requirement by implementing `Downcast`
for `AnimationCurveEvaluator`.

This also reworks "evaluator identity" to support either a (Component /
Field) pair, or a TypeId. This allows properties to reuse evaluators,
even if they have different accessor methods. The "contract" here is
that for a given (Component / Field) pair, the accessor will return the
same value. Fields are identified by their Reflect-ed field index. The
(TypeId, usize) is prehashed and cached to optimize for lookup speed.

This removes the built-in hard-coded TranslationCurve / RotationCurve /
ScaleCurve in favor of AnimatableField.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-11-27 22:19:55 +00:00
François Mockers
a9a4b069b6
Make some examples deterministic (#16488)
# Objective

- Improve reproducibility of examples

## Solution

- Use seeded rng when needed
- Use fixed z-ordering when needed

## Testing

```sh
steps=5;
echo "cpu_draw\nparallel_query\nanimated_fox\ntransparency_2d" > test
cargo run -p example-showcase -- run --stop-frame 250 --screenshot-frame 100 --fixed-frame-time 0.05 --example-list test --in-ci;
mv screenshots base;
for prefix in `seq 0 $steps`;
do
  echo step $prefix;
  cargo run -p example-showcase -- run --stop-frame 250 --screenshot-frame 100 --fixed-frame-time 0.05 --example-list test;
  mv screenshots $prefix-screenshots;
done;
mv base screenshots
for prefix in `seq 0 $steps`;
do
  echo check $prefix
  for file in screenshots/*/*;
  do
    echo $file;
    diff $file $prefix-$file;
  done;
done;
```
2024-11-23 18:28:47 +00:00
Carter Anderson
513be52505
AnimationEvent -> Event and other improvements (#16440)
# Objective

Needing to derive `AnimationEvent` for `Event` is unnecessary, and the
trigger logic coupled to it feels like we're coupling "event producer"
logic with the event itself, which feels wrong. It also comes with a
bunch of complexity, which is again unnecessary. We can have the
flexibility of "custom animation event trigger logic" without this
coupling and complexity.

The current `animation_events` example is also needlessly complicated,
due to it needing to work around system ordering issues. The docs
describing it are also slightly wrong. We can make this all a non-issue
by solving the underlying ordering problem.

Related to this, we use the `bevy_animation::Animation` system set to
solve PostUpdate animation order-of-operations issues. If we move this
to bevy_app as part of our "core schedule", we can cut out needless
`bevy_animation` crate dependencies in these instances.

## Solution

- Remove `AnimationEvent`, the derive, and all other infrastructure
associated with it (such as the `bevy_animation/derive` crate)
- Replace all instances of `AnimationEvent` traits with `Event + Clone`
- Store and use functions for custom animation trigger logic (ex:
`clip.add_event_fn()`). For "normal" cases users dont need to think
about this and should use the simpler `clip.add_event()`
- Run the `Animation` system set _before_ updating text
- Move `bevy_animation::Animation` to `bevy_app::Animation`. Remove
unnecessary `bevy_animation` dependency from `bevy_ui`
- Adjust `animation_events` example to use the simpler `clip.add_event`
API, as the workarounds are no longer necessary

This is polishing work that will land in 0.15, and I think it is simple
enough and valuable enough to land in 0.15 with it, in the interest of
making the feature as compelling as possible.
2024-11-22 00:16:04 +00:00
Carter Anderson
7477928f13
Use normal constructors for EasingCurve, FunctionCurve, ConstantCurve (#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.
2024-11-13 15:30:05 +00:00
Matty
9beb1d96e7
Incorporate all node weights in additive blending (#16279)
# Objective

In the existing implementation, additive blending effectively treats the
node with least index specially by basically forcing its weight to be
`1.0` regardless of what its computed weight would be (based on the
weights in the `AnimationGraph` and `AnimationPlayer`).

Arguably this makes some amount of sense, because the "base" animation
is often one which was not authored to be used additively, meaning that
its sampled values are interpreted absolutely rather than as deltas.
However, this also leads to strange behavior with respect to animation
masks: if the "base" animation is masked out on some target, then the
next node is treated as the "base" animation, despite the fact that it
would normally be interpreted additively, and the weight of that
animation is thrown away as a result.

This is all kind of weird and revolves around special treatment (if the
behavior is even really intentional in the first place). From a
mathematical standpoint, there is nothing special about how the "base"
animation must be treated other than having a weight of 1.0 under an
`Add` node, which is something that the user can do without relying on
some bizarre corner-case behavior of the animation system — this is the
only present situation under which weights are discarded.

This PR changes this behavior so that the weight of every node is
incorporated. In other words, for an animation graph that looks like
this:
```text
┌───────────────┐                                 
│Base clip      ┼──┐                              
│      0.5      │  │                              
└───────────────┘  │                              
┌───────────────┐  │  ┌───────────────┐     ┌────┐
│Additive clip 1┼──┼─►┤Additive blend ┼────►│Root│
│      0.1      │  │  │      1.0      │     └────┘
└───────────────┘  │  └───────────────┘           
┌───────────────┐  │                              
│Additive clip 2┼──┘                              
│      0.2      │                                 
└───────────────┘                                 
```

Previously, the result would have been
```text
base_clip + 0.1 * additive_clip_1 + 0.2 * additive_clip_2
```

whereas now it would be
```text
0.5 * base_clip + 0.1 * additive_clip_1 + 0.2 * additive_clip_2
```

and in the scenario where `base_clip` is masked out:
```text
additive_clip_1 + 0.2 * additive_clip_2
```
vs.
```text
0.1 * additive_clip_1 + 0.2 * additive_clip_2
```

## Solution

For background, the way that the additive blending procedure works is
something like this:
- During graph traversal, the node values and weights of the children
are pushed onto the evaluator `stack`. The traversal order guarantees
that the item with least node index will be on top.
- Once we reach the `Add` node itself, we start popping off the `stack`
and into the evaluator's `blend_register`, which is an accumulator
holding up to one weight-value pair:
- If the `blend_register` is empty, it is filled using data from the top
of the `stack`.
- Otherwise, the `blend_register` is combined with data popped from the
`stack` and updated.

In the example above, the additive blending steps would look like this
(with the pre-existing implementation):
1. The `blend_register` is empty, so we pop `(base_clip, 0.5)` from the
top of the `stack` and put it in. Now the value of the `blend_register`
is `(base_clip, 0.5)`.
2. The `blend_register` is non-empty: we pop `(additive_clip_1, 0.1)`
from the top of the `stack` and combine it additively with the value in
the `blend_register`, forming `(base_clip + 0.1 * additive_clip_1, 0.6)`
in the `blend_register` (the carried weight value goes unused).
3. The `blend_register` is non-empty: we pop `(additive_clip_2, 0.2)`
from the top of the `stack` and combine it additively with the value in
the `blend_register`, forming `(base_clip + 0.1 * additive_clip_1 + 0.2
* additive_clip_2, 0.8)` in the `blend_register`.

The solution in this PR changes step 1: the `base_clip` is multiplied by
its weight as it is added to the `blend_register` in the first place,
yielding `0.5 * base_clip + 0.1 * additive_clip_1 + 0.2 *
additive_clip_2` as the final result.

### Note for reviewers

It might be tempting to look at the code, which contains a segment that
looks like this:
```rust
if additive {
    current_value = A::blend(
        [
            BlendInput {
                weight: 1.0, // <--
                value: current_value,
                additive: true,
            },
            BlendInput {
                weight: weight_to_blend,
                value: value_to_blend,
                additive: true,
            },
        ]
        .into_iter(),
    );
} 
```
and conclude that the explicit value of `1.0` is responsible for
overwriting the weight of the base animation. This is incorrect.

Rather, this additive blend has to be written this way because it is
multiplying the *existing value in the blend register* by 1 (i.e. not
doing anything) before adding the next value to it. Changing this to
another quantity (e.g. the existing weight) would cause the value in the
blend register to be spuriously multiplied down.

## Testing

Tested on `animation_masks` example. Checked `morph_weights` example as
well.

## Migration Guide

I will write a migration guide later if this change is not included in
0.15.
2024-11-07 19:12:08 +00:00
Design_Dream
8b636f32cf
Remove the invalid system ordering in the animation example. (#16173)
# Objective

In the animation example, there is the code `.add_systems(Update,
init_animations.before(animate_targets))`, where `animate_targets` is
added to the `PostUpdate` in the `AnimationPlugin`. Therefore, the
`.before(animate_targets)` here is ineffective and should be removed.
2024-10-30 23:44:19 +00:00
mgi388
c4c1c8ffa1
Undeprecate is_playing_animation (#16121)
# Objective

- Fixes #16098

## Solution

- Undeprecate `is_playing_animation` and copy the docs from
`animation_is_playing` to it.

## Testing

- CI

## Migration


68e9a34e30/release-content/0.15/migration-guides/_guides.toml (L13-L17)
needs to be removed.
2024-10-27 22:38:07 +00:00
Rob Parrett
30d84519a2
Use en-us locale for typos (#16037)
# Objective

Bevy seems to want to standardize on "American English" spellings. Not
sure if this is laid out anywhere in writing, but see also #15947.

While perusing the docs for `typos`, I noticed that it has a `locale`
config option and tried it out.

## Solution

Switch to `en-us` locale in the `typos` config and run `typos -w`

## Migration Guide

The following methods or fields have been renamed from `*dependants*` to
`*dependents*`.

- `ProcessorAssetInfo::dependants`
- `ProcessorAssetInfos::add_dependant`
- `ProcessorAssetInfos::non_existent_dependants`
- `AssetInfo::dependants_waiting_on_load`
- `AssetInfo::dependants_waiting_on_recursive_dep_load`
- `AssetInfos::loader_dependants`
- `AssetInfos::remove_dependants_and_labels`
2024-10-20 18:55:17 +00:00
Rob Parrett
c65f2920bb
Fix animation_masks example's buttons (#15996)
# Objective

Fixes #15995

## Solution

Corrects a mistake made during the example migration in #15591.

`AnimationControl` was meant to be on the parent, not the child. So the
query in `update_ui` was no longer matching.

## Testing

`cargo run --example animation_masks`
2024-10-18 23:53:10 +00:00
Carter Anderson
015f2c69ca
Merge Style properties into Node. Use ComputedNode for computed properties. (#15975)
# Objective

Continue improving the user experience of our UI Node API in the
direction specified by [Bevy's Next Generation Scene / UI
System](https://github.com/bevyengine/bevy/discussions/14437)

## Solution

As specified in the document above, merge `Style` fields into `Node`,
and move "computed Node fields" into `ComputedNode` (I chose this name
over something like `ComputedNodeLayout` because it currently contains
more than just layout info. If we want to break this up / rename these
concepts, lets do that in a separate PR). `Style` has been removed.

This accomplishes a number of goals:

## Ergonomics wins

Specifying both `Node` and `Style` is now no longer required for
non-default styles

Before:
```rust
commands.spawn((
    Node::default(),
    Style {
        width:  Val::Px(100.),
        ..default()
    },
));
```

After:

```rust
commands.spawn(Node {
    width:  Val::Px(100.),
    ..default()
});
```

## Conceptual clarity

`Style` was never a comprehensive "style sheet". It only defined "core"
style properties that all `Nodes` shared. Any "styled property" that
couldn't fit that mold had to be in a separate component. A "real" style
system would style properties _across_ components (`Node`, `Button`,
etc). We have plans to build a true style system (see the doc linked
above).

By moving the `Style` fields to `Node`, we fully embrace `Node` as the
driving concept and remove the "style system" confusion.

## Next Steps

* Consider identifying and splitting out "style properties that aren't
core to Node". This should not happen for Bevy 0.15.

---

## Migration Guide

Move any fields set on `Style` into `Node` and replace all `Style`
component usage with `Node`.

Before:
```rust
commands.spawn((
    Node::default(),
    Style {
        width:  Val::Px(100.),
        ..default()
    },
));
```

After:

```rust
commands.spawn(Node {
    width:  Val::Px(100.),
    ..default()
});
```

For any usage of the "computed node properties" that used to live on
`Node`, use `ComputedNode` instead:

Before:
```rust
fn system(nodes: Query<&Node>) {
    for node in &nodes {
        let computed_size = node.size();
    }
}
```

After:
```rust
fn system(computed_nodes: Query<&ComputedNode>) {
    for computed_node in &computed_nodes {
        let computed_size = computed_node.size();
    }
}
```
2024-10-18 22:25:33 +00:00
VitalyR
eb19a9ea0b
Migrate UI bundles to required components (#15898)
# Objective

- Migrate UI bundles to required components, fixes #15889

## Solution

- deprecate `NodeBundle` in favor of `Node`
- deprecate `ImageBundle` in favor of `UiImage`
- deprecate `ButtonBundle` in favor of `Button`

## Testing

CI.

## Migration Guide

- Replace all uses of `NodeBundle` with `Node`. e.g.
```diff
     commands
-        .spawn(NodeBundle {
-            style: Style {
+        .spawn((
+            Node::default(),
+            Style {
                 width: Val::Percent(100.),
                 align_items: AlignItems::Center,
                 justify_content: JustifyContent::Center,
                 ..default()
             },
-            ..default()
-        })
+        ))
``` 
- Replace all uses of `ButtonBundle` with `Button`. e.g.
```diff
                     .spawn((
-                        ButtonBundle {
-                            style: Style {
-                                width: Val::Px(w),
-                                height: Val::Px(h),
-                                // horizontally center child text
-                                justify_content: JustifyContent::Center,
-                                // vertically center child text
-                                align_items: AlignItems::Center,
-                                margin: UiRect::all(Val::Px(20.0)),
-                                ..default()
-                            },
-                            image: image.clone().into(),
+                        Button,
+                        Style {
+                            width: Val::Px(w),
+                            height: Val::Px(h),
+                            // horizontally center child text
+                            justify_content: JustifyContent::Center,
+                            // vertically center child text
+                            align_items: AlignItems::Center,
+                            margin: UiRect::all(Val::Px(20.0)),
                             ..default()
                         },
+                        UiImage::from(image.clone()),
                         ImageScaleMode::Sliced(slicer.clone()),
                     ))
```
- Replace all uses of `ImageBundle` with `UiImage`. e.g.
```diff
-    commands.spawn(ImageBundle {
-        image: UiImage {
+    commands.spawn((
+        UiImage {
             texture: metering_mask,
             ..default()
         },
-        style: Style {
+        Style {
             width: Val::Percent(100.0),
             height: Val::Percent(100.0),
             ..default()
         },
-        ..default()
-    });
+    ));
 ```

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2024-10-17 21:11:02 +00:00
andristarr
7482a0d26d
aligning public apis of Time,Timer and Stopwatch (#15962)
Fixes #15834

## Migration Guide

The APIs of `Time`, `Timer` and `Stopwatch` have been cleaned up for
consistency with each other and the standard library's `Duration` type.
The following methods have been renamed:

- `Stowatch::paused` -> `Stopwatch::is_paused`
- `Time::elapsed_seconds` -> `Time::elasped_secs` (including `_f64` and
`_wrapped` variants)
2024-10-16 21:09:32 +00:00
MiniaczQ
f602edad09
Text Rework cleanup (#15887)
# Objective

Cleanup naming and docs, add missing migration guide after #15591 

All text root nodes now use `Text` (UI) / `Text2d`.
All text readers/writers use `Text<Type>Reader`/`Text<Type>Writer`
convention.

---

## Migration Guide

Doubles as #15591 migration guide.

Text bundles (`TextBundle` and `Text2dBundle`) were removed in favor of
`Text` and `Text2d`.
Shared configuration fields were replaced with `TextLayout`, `TextFont`
and `TextColor` components.
Just `TextBundle`'s additional field turned into `TextNodeFlags`
component,
while `Text2dBundle`'s additional fields turned into `TextBounds` and
`Anchor` components.

Text sections were removed in favor of hierarchy-based approach.
For root text entities with `Text` or `Text2d` components, child
entities with `TextSpan` will act as additional text sections.
To still access text spans by index, use the new `TextUiReader`,
`Text2dReader` and `TextUiWriter`, `Text2dWriter` system parameters.
2024-10-15 02:32:34 +00:00
NiseVoid
bdd0af6bfb
Deprecate SpatialBundle (#15830)
# Objective

- Required components replace bundles, but `SpatialBundle` is yet to be
deprecated

## Solution

- Deprecate `SpatialBundle`
- Insert `Transform` and `Visibility` instead in examples using it
- In `spawn` or `insert` inserting a default `Transform` or `Visibility`
with component already requiring either, remove those components from
the tuple

## Testing

- Did you test these changes? If so, how?
Yes, I ran the examples I changed and tests
- Are there any parts that need more testing?
The `gamepad_viewer` and and `custom_shader_instancing` examples don't
work as intended due to entirely unrelated code, didn't check main.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Run examples, or just check that all spawned values are identical
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Linux, wayland trough x11 (cause that's the default feature)

---

## Migration Guide

`SpatialBundle` is now deprecated, insert `Transform` and `Visibility`
instead which will automatically insert all other components that were
in the bundle. If you do not specify these values and any other
components in your `spawn`/`insert` call already requires either of
these components you can leave that one out.

before:
```rust
commands.spawn(SpatialBundle::default());
```

after:
```rust
commands.spawn((Transform::default(), Visibility::default());
```
2024-10-13 17:28:22 +00:00
ickshonpe
6f7d0e5725
split up TextStyle (#15857)
# Objective

Currently text is recomputed unnecessarily on any changes to its color,
which is extremely expensive.

## Solution
Split up `TextStyle` into two separate components `TextFont` and
`TextColor`.

## Testing

I added this system to `many_buttons`:
```rust
fn set_text_colors_changed(mut colors: Query<&mut TextColor>) {
    for mut text_color in colors.iter_mut() {
        text_color.set_changed();
    }
}
```

reports ~4fps on main, ~50fps with this PR.

## Migration Guide
`TextStyle` has been renamed to `TextFont` and its `color` field has
been moved to a separate component named `TextColor` which newtypes
`Color`.
2024-10-13 17:06:22 +00:00
Rob Parrett
6ad6eaa873
Fix println in morph_targets example (#15851)
# Objective

This example uses `println` from a system, which we don't advise people
do. It also gives no context for the debug prints, which I assumed to be
stray debug code at first.

## Solution

Use `info!`, and add a small amount of context so the console output
looks deliberate.

## Testing

`cargo run --example morph_targets`
2024-10-11 15:35:22 +00:00
Tim
3da0ef048e
Remove the Component trait implementation from Handle (#15796)
# Objective

- Closes #15716
- Closes #15718

## Solution

- Replace `Handle<MeshletMesh>` with a new `MeshletMesh3d` component
- As expected there were some random things that needed fixing:
- A couple tests were storing handles just to prevent them from being
dropped I believe, which seems to have been unnecessary in some.
- The `SpriteBundle` still had a `Handle<Image>` field. I've removed
this.
- Tests in `bevy_sprite` incorrectly added a `Handle<Image>` field
outside of the `Sprite` component.
- A few examples were still inserting `Handle`s, switched those to their
corresponding wrappers.
- 2 examples that were still querying for `Handle<Image>` were changed
to query `Sprite`

## Testing

- I've verified that the changed example work now

## Migration Guide

`Handle` can no longer be used as a `Component`. All existing Bevy types
using this pattern have been wrapped in their own semantically
meaningful type. You should do the same for any custom `Handle`
components your project needs.

The `Handle<MeshletMesh>` component is now `MeshletMesh3d`.

The `WithMeshletMesh` type alias has been removed. Use
`With<MeshletMesh3d>` instead.
2024-10-09 21:10:01 +00:00
UkoeHB
a6be9b4ccd
Rename TextBlock to TextLayout (#15797)
# Objective

- Improve clarity when spawning a text block. See [this
discussion](https://github.com/bevyengine/bevy/pull/15591/#discussion_r1787083571).

## Solution

- Rename `TextBlock` to `TextLayout`.
2024-10-09 20:58:27 +00:00
UkoeHB
c2c19e5ae4
Text rework (#15591)
**Ready for review. Examples migration progress: 100%.**

# Objective

- Implement https://github.com/bevyengine/bevy/discussions/15014

## Solution

This implements [cart's
proposal](https://github.com/bevyengine/bevy/discussions/15014#discussioncomment-10574459)
faithfully except for one change. I separated `TextSpan` from
`TextSpan2d` because `TextSpan` needs to require the `GhostNode`
component, which is a `bevy_ui` component only usable by UI.

Extra changes:
- Added `EntityCommands::commands_mut` that returns a mutable reference.
This is a blocker for extension methods that return something other than
`self`. Note that `sickle_ui`'s `UiBuilder::commands` returns a mutable
reference for this reason.

## Testing

- [x] Text examples all work.

---

## Showcase

TODO: showcase-worthy

## Migration Guide

TODO: very breaking

### Accessing text spans by index

Text sections are now text sections on different entities in a
hierarchy, Use the new `TextReader` and `TextWriter` system parameters
to access spans by index.

Before:
```rust
fn refresh_text(mut query: Query<&mut Text, With<TimeText>>, time: Res<Time>) {
    let text = query.single_mut();
    text.sections[1].value = format_time(time.elapsed());
}
```

After:
```rust
fn refresh_text(
    query: Query<Entity, With<TimeText>>,
    mut writer: UiTextWriter,
    time: Res<Time>
) {
    let entity = query.single();
    *writer.text(entity, 1) = format_time(time.elapsed());
}
```

### Iterating text spans

Text spans are now entities in a hierarchy, so the new `UiTextReader`
and `UiTextWriter` system parameters provide ways to iterate that
hierarchy. The `UiTextReader::iter` method will give you a normal
iterator over spans, and `UiTextWriter::for_each` lets you visit each of
the spans.

---------

Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2024-10-09 18:35:36 +00:00
Emerson Coskey
7d40e3ec87
Migrate bevy_sprite to required components (#15489)
# Objective

Continue migration of bevy APIs to required components, following
guidance of https://hackmd.io/@bevy/required_components/

## Solution

- Make `Sprite` require `Transform` and `Visibility` and
`SyncToRenderWorld`
- move image and texture atlas handles into `Sprite`
- deprecate `SpriteBundle`
- remove engine uses of `SpriteBundle`

## Testing

ran cargo tests on bevy_sprite and tested several sprite examples.

---

## Migration Guide

Replace all uses of `SpriteBundle` with `Sprite`. There are several new
convenience constructors: `Sprite::from_image`,
`Sprite::from_atlas_image`, `Sprite::from_color`.

WARNING: use of `Handle<Image>` and `TextureAtlas` as components on
sprite entities will NO LONGER WORK. Use the fields on `Sprite` instead.
I would have removed the `Component` impls from `TextureAtlas` and
`Handle<Image>` except it is still used within ui. We should fix this
moving forward with the migration.
2024-10-09 16:17:26 +00:00
Christian Hughes
219b5930f1
Rename App/World::observe to add_observer, EntityWorldMut::observe_entity to observe. (#15754)
# Objective

- Closes #15752

Calling the functions `App::observe` and `World::observe` doesn't make
sense because you're not "observing" the `App` or `World`, you're adding
an observer that listens for an event that occurs *within* the `World`.
We should rename them to better fit this.

## Solution

Renames:
- `App::observe` -> `App::add_observer`
- `World::observe` -> `World::add_observer`
- `Commands::observe` -> `Commands::add_observer`
- `EntityWorldMut::observe_entity` -> `EntityWorldMut::observe`

(Note this isn't a breaking change as the original rename was introduced
earlier this cycle.)

## Testing

Reusing current tests.
2024-10-09 15:39:29 +00:00
Tim
700123ec64
Replace Handle<AnimationGraph> component with a wrapper (#15742)
# Objective

- Closes #15717 

## Solution

- Wrap the handle in a new wrapper component: `AnimationGraphHandle`.

## Testing

Searched for all instances of `AnimationGraph` in the examples and
updated and tested those

## Migration Guide

`Handle<AnimationGraph>` is no longer a component. Instead, use the
`AnimationGraphHandle` component which contains a
`Handle<AnimationGraph>`.
2024-10-08 22:41:24 +00:00
François Mockers
26813d9732
easing_functions example: draw point closer to its curve (#15744)
# Objective

- After #15711 which added a column to the example, the point of a curve
was too close to the next curve

## Solution

- Make it closer to its own
2024-10-08 22:40:40 +00:00
Matty
e563f86a1d
Simplified easing curves (#15711)
# Objective

Simplify the API surrounding easing curves. Broaden the base of types
that support easing.

## Solution

There is now a single library function, `easing_curve`, which constructs
a unit-parametrized easing curve between two values based on an
`EaseFunction`:
```rust
/// Given a `start` and `end` value, create a curve parametrized over [the unit interval]
/// that connects them, using the given [ease function] to determine the form of the
/// curve in between.
///
/// [the unit interval]: Interval::UNIT
/// [ease function]: EaseFunction
pub fn easing_curve<T: Ease>(start: T, end: T, ease_fn: EaseFunction) -> EasingCurve<T> { //... }
```

As this shows, the type of the output curve is generic only in `T`. In
particular, as long as `T` is `Reflect` (and `FromReflect` etc. — i.e.,
a standard "well-behaved" reflectable type), `EasingCurve<T>` is also
`Reflect`, and there is no special field handling nonsense. Therefore,
`EasingCurve` is the kind of thing that would be able to be easily
changed in an editor. This is made possible by storing the actual
`EaseFunction` on `EasingCurve<T>` instead of indirecting through some
kind of function type (which generally leads to issues with reflection).

The types that can be eased are those that implement a trait `Ease`:
```rust
/// A type whose values can be eased between.
///
/// This requires the construction of an interpolation curve that actually extends
/// beyond the curve segment that connects two values, because an easing curve may
/// extrapolate before the starting value and after the ending value. This is
/// especially common in easing functions that mimic elastic or springlike behavior.
pub trait Ease: Sized {
    /// Given `start` and `end` values, produce a curve with [unlimited domain]
    /// that:
    /// - takes a value equivalent to `start` at `t = 0`
    /// - takes a value equivalent to `end` at `t = 1`
    /// - has constant speed everywhere, including outside of `[0, 1]`
    ///
    /// [unlimited domain]: Interval::EVERYWHERE
    fn interpolating_curve_unbounded(start: &Self, end: &Self) -> impl Curve<Self>;
}
```

(I know, I know, yet *another* interpolation trait. See 'Future
direction'.)

The other existing easing functions from the previous version of this
module have also become new members of `EaseFunction`: `Linear`,
`Steps`, and `Elastic` (which maybe needs a different name). The latter
two are parametrized.

## Testing

Tested using the `easing_functions` example. I also axed the
`cubic_curve` example which was of questionable value and replaced it
with `eased_motion`, which uses this API in the context of animation:


https://github.com/user-attachments/assets/3c802992-6b9b-4b56-aeb1-a47501c29ce2


---

## Future direction

Morally speaking, `Ease` is incredibly similar to `StableInterpolate`.
Probably, we should just merge `StableInterpolate` into `Ease`, and then
make `SmoothNudge` an automatic extension trait of `Ease`. The reason I
didn't do that is that `StableInterpolate` is not implemented for
`VectorSpace` because of concerns about the `Color` types, and I wanted
to avoid controversy. I think that may be a good idea though.

As Alice mentioned before, we should also probably get rid of the
`interpolation` dependency.

The parametrized `Elastic` variant probably also needs some additional
work (e.g. renaming, in/out/in-out variants, etc.) if we want to keep
it.
2024-10-08 19:45:13 +00:00
Shane Celis
320d53c1d2
Vary transforms for custom_skinned_mesh example (#15710)
# Objective

Enhance the [custom skinned mesh
example](https://bevyengine.org/examples/animation/custom-skinned-mesh/)
to show some variety and clarify what the transform does to the mesh.

## Solution


https://github.com/user-attachments/assets/c919db74-6e77-4f33-ba43-0f40a88042b3

Add variety and clarity with the following changes:

- vary transform changes,
- use a UV texture,
- and show transform changes via gizmos.

(Maybe it'd be worth turning on wireframe rendering to show what happens
to the mesh. I think it'd be nice visually but might make the code a
little noisy.)

## Testing

I exercised it on my x86 macOS computer. It'd be good to have it
validated on Windows, Linux, and WASM.

---

## Showcase

- Custom skinned mesh example varies the transforms changes and uses a
UV test texture.
2024-10-08 12:37:46 +00:00
François Mockers
1869e45c49
fix some of the ease functions from interpolation (#15706)
# Objective

- Followup to #15675
- Some of the functions are wrong, noticed in #15703: `Sine`, `Elastic`
and `Back`

## Solution

- Fix them and make them deterministic


![ease-fixed-functions](https://github.com/user-attachments/assets/8a4d5c0c-36fa-4a49-a189-5b832dc24721)
2024-10-07 19:08:32 +00:00
François Mockers
01387101df
add example for ease functions (#15703)
# Objective

- Followup to #15675 
- Add an example showcasing the functions

## Solution

- Add an example showcasing the functions
- Some of the functions from the interpolation crate are messed up,
fixed in #15706


![ease](https://github.com/user-attachments/assets/1f3b2b80-23d2-45c7-8b08-95b2e870aa02)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2024-10-07 18:31:43 +00:00
poopy
d9190e4ff6
Add Support for Triggering Events via AnimationEvents (#15538)
# Objective

Add support for events that can be triggered from animation clips. This
is useful when you need something to happen at a specific time in an
animation. For example, playing a sound every time a characters feet
hits the ground when walking.

Closes #15494 

## Solution

Added a new field to `AnimationClip`: `events`, which contains a list of
`AnimationEvent`s. These are automatically triggered in
`animate_targets` and `trigger_untargeted_animation_events`.

## Testing

Added a couple of tests and example (`animation_events.rs`) to make sure
events are triggered when expected.

---

## Showcase

`Events` need to also implement `AnimationEvent` and `Reflect` to be
used with animations.

```rust
#[derive(Event, AnimationEvent, Reflect)]
struct SomeEvent;
```

Events can be added to an `AnimationClip` by specifying a time and
event.

```rust
// trigger an event after 1.0 second
animation_clip.add_event(1.0, SomeEvent);
```

And optionally, providing a target id.

```rust
let id = AnimationTargetId::from_iter(["shoulder", "arm", "hand"]);
animation_clip.add_event_to_target(id, 1.0, HandEvent);
```

I modified the `animated_fox` example to show off the feature.

![CleanShot 2024-10-05 at 02 41
57](https://github.com/user-attachments/assets/0bb47db7-24f9-4504-88f1-40e375b89b1b)

---------

Co-authored-by: Matty <weatherleymatthew@gmail.com>
Co-authored-by: Chris Biscardi <chris@christopherbiscardi.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-10-06 10:03:05 +00:00
Joona Aalto
25bfa80e60
Migrate cameras to required components (#15641)
# Objective

Yet another PR for migrating stuff to required components. This time,
cameras!

## Solution

As per the [selected
proposal](https://hackmd.io/tsYID4CGRiWxzsgawzxG_g#Combined-Proposal-1-Selected),
deprecate `Camera2dBundle` and `Camera3dBundle` in favor of `Camera2d`
and `Camera3d`.

Adding a `Camera` without `Camera2d` or `Camera3d` now logs a warning,
as suggested by Cart [on
Discord](https://discord.com/channels/691052431525675048/1264881140007702558/1291506402832945273).
I would personally like cameras to work a bit differently and be split
into a few more components, to avoid some footguns and confusing
semantics, but that is more controversial, and shouldn't block this core
migration.

## Testing

I ran a few 2D and 3D examples, and tried cameras with and without
render graphs.

---

## Migration Guide

`Camera2dBundle` and `Camera3dBundle` have been deprecated in favor of
`Camera2d` and `Camera3d`. Inserting them will now also insert the other
components required by them automatically.
2024-10-05 01:59:52 +00:00
Patrick Walton
0094bcbc07
Implement additive blending for animation graphs. (#15631)
*Additive blending* is an ubiquitous feature in game engines that allows
animations to be concatenated instead of blended. The canonical use case
is to allow a character to hold a weapon while performing arbitrary
poses. For example, if you had a character that needed to be able to
walk or run while attacking with a weapon, the typical workflow is to
have an additive blend node that combines walking and running animation
clips with an animation clip of one of the limbs performing a weapon
attack animation.

This commit adds support for additive blending to Bevy. It builds on top
of the flexible infrastructure in #15589 and introduces a new type of
node, the *add node*. Like blend nodes, add nodes combine the animations
of their children according to their weights. Unlike blend nodes,
however, add nodes don't normalize the weights to 1.0.

The `animation_masks` example has been overhauled to demonstrate the use
of additive blending in combination with masks. There are now controls
to choose an animation clip for every limb of the fox individually.

This patch also fixes a bug whereby masks were incorrectly accumulated
with `insert()` during the graph threading phase, which could cause
corruption of computed masks in some cases.

Note that the `clip` field has been replaced with an `AnimationNodeType`
enum, which breaks `animgraph.ron` files. The `Fox.animgraph.ron` asset
has been updated to the new format.

Closes #14395.

## Showcase


https://github.com/user-attachments/assets/52dfe05f-fdb3-477a-9462-ec150f93df33

## Migration Guide

* The `animgraph.ron` format has changed to accommodate the new
*additive blending* feature. You'll need to change `clip` fields to
instances of the new `AnimationNodeType` enum.
2024-10-04 22:13:22 +00:00
Patrick Walton
ca8dd06146
Impose a more sensible ordering for animation graph evaluation. (#15589)
This is an updated version of #15530. Review comments were addressed.

This commit changes the animation graph evaluation to be operate in a
more sensible order and updates the semantics of blend nodes to conform
to [the animation composition RFC]. Prior to this patch, a node graph
like this:

```
	    ┌─────┐
	    │     │
	    │  1  │
	    │     │
	    └──┬──┘
	       │
       ┌───────┴───────┐
       │               │
       ▼               ▼
    ┌─────┐         ┌─────┐
    │     │         │     │
    │  2  │         │  3  │
    │     │         │     │
    └──┬──┘         └──┬──┘
       │               │
   ┌───┴───┐       ┌───┴───┐
   │       │       │       │
   ▼       ▼       ▼       ▼
┌─────┐ ┌─────┐ ┌─────┐ ┌─────┐
│     │ │     │ │     │ │     │
│  4  │ │  6  │ │  5  │ │  7  │
│     │ │     │ │     │ │     │
└─────┘ └─────┘ └─────┘ └─────┘
```

Would be evaluated as (((4 ⊕ 5) ⊕ 6) ⊕ 7), with the blend (lerp/slerp)
operation notated as ⊕. As quaternion multiplication isn't commutative,
this is very counterintuitive and will especially lead to trouble with
the forthcoming additive blending feature (#15198).

This patch fixes the issue by changing the evaluation order to
postorder, with children of a node evaluated in ascending order by node
index.

To do so, this patch revamps `AnimationCurve` to be based on an
*evaluation stack* and a *blend register*. During target evaluation, the
graph evaluator traverses the graph in postorder. When encountering a
clip node, the evaluator pushes the possibly-interpolated value onto the
evaluation stack. When encountering a blend node, the evaluator pops
values off the stack into the blend register, accumulating weights as
appropriate. When the graph is completely evaluated, the top element on
the stack is *committed* to the property of the component.

A new system, the *graph threading* system, is added in order to cache
the sorted postorder traversal to avoid the overhead of sorting children
at animation evaluation time. Mask evaluation has been moved to this
system so that the graph only has to be traversed at most once per
frame. Unlike the `ActiveAnimation` list, the *threaded graph* is cached
from frame to frame and only has to be regenerated when the animation
graph asset changes.

This patch currently regresses the `animate_target` performance in
`many_foxes` by around 50%, resulting in an FPS loss of about 2-3 FPS.
I'd argue that this is an acceptable price to pay for a much more
intuitive system. In the future, we can mitigate the regression with a
fast path that avoids consulting the graph if only one animation is
playing. However, in the interest of keeping this patch simple, I didn't
do so here.

[the animation composition RFC]:
https://github.com/bevyengine/rfcs/blob/main/rfcs/51-animation-composition.md

# Objective

- Describe the objective or issue this PR addresses.
- If you're fixing a specific issue, say "Fixes #X".

## Solution

- Describe the solution used to achieve the objective above.

## Testing

- Did you test these changes? If so, how?
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Showcase

> This section is optional. If this PR does not include a visual change
or does not add a new feature, you can delete this section.

- Help others understand the result of this PR by showcasing your
awesome work!
- If this PR adds a new feature or public API, consider adding a brief
pseudo-code snippet of it in action
- If this PR includes a visual change, consider adding a screenshot,
GIF, or video
  - If you want, you could even include a before/after comparison!
- If the Migration Guide adequately covers the changes, you can delete
this section

While a showcase should aim to be brief and digestible, you can use a
toggleable section to save space on longer showcases:

<details>
  <summary>Click to view showcase</summary>

```rust
println!("My super cool code.");
```

</details>

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-10-03 00:36:42 +00:00
Tim
461305b3d7
Revert "Have EntityCommands methods consume self for easier chaining" (#15523)
As discussed in #15521

- Partial revert of #14897, reverting the change to the methods to
consume `self`
- The `insert_if` method is kept

The migration guide of #14897 should be removed
Closes #15521

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-10-02 12:47:26 +00:00
Tim
eb51b4c28e
Migrate scenes to required components (#15579)
# Objective

A step in the migration to required components: scenes!

## Solution

As per the [selected
proposal](https://hackmd.io/@bevy/required_components/%2FPJtNGVMMQhyM0zIvCJSkbA):
- Deprecate `SceneBundle` and `DynamicSceneBundle`.
- Add `SceneRoot` and `DynamicSceneRoot` components, which wrap a
`Handle<Scene>` and `Handle<DynamicScene>` respectively.

## Migration Guide
Asset handles for scenes and dynamic scenes must now be wrapped in the
`SceneRoot` and `DynamicSceneRoot` components. Raw handles as components
no longer spawn scenes.

Additionally, `SceneBundle` and `DynamicSceneBundle` have been
deprecated. Instead, use the scene components directly.

Previously:
```rust
let model_scene = asset_server.load(GltfAssetLabel::Scene(0).from_asset("model.gltf"));

commands.spawn(SceneBundle {
    scene: model_scene,
    transform: Transform::from_xyz(-4.0, 0.0, -3.0),
    ..default()
});
```
Now:
```rust
let model_scene = asset_server.load(GltfAssetLabel::Scene(0).from_asset("model.gltf"));

commands.spawn((
    SceneRoot(model_scene),
    Transform::from_xyz(-4.0, 0.0, -3.0),
));
```
2024-10-01 22:42:11 +00:00
Joona Aalto
54006b107b
Migrate meshes and materials to required components (#15524)
# Objective

A big step in the migration to required components: meshes and
materials!

## Solution

As per the [selected
proposal](https://hackmd.io/@bevy/required_components/%2Fj9-PnF-2QKK0on1KQ29UWQ):

- Deprecate `MaterialMesh2dBundle`, `MaterialMeshBundle`, and
`PbrBundle`.
- Add `Mesh2d` and `Mesh3d` components, which wrap a `Handle<Mesh>`.
- Add `MeshMaterial2d<M: Material2d>` and `MeshMaterial3d<M: Material>`,
which wrap a `Handle<M>`.
- Meshes *without* a mesh material should be rendered with a default
material. The existence of a material is determined by
`HasMaterial2d`/`HasMaterial3d`, which is required by
`MeshMaterial2d`/`MeshMaterial3d`. This gets around problems with the
generics.

Previously:

```rust
commands.spawn(MaterialMesh2dBundle {
    mesh: meshes.add(Circle::new(100.0)).into(),
    material: materials.add(Color::srgb(7.5, 0.0, 7.5)),
    transform: Transform::from_translation(Vec3::new(-200., 0., 0.)),
    ..default()
});
```

Now:

```rust
commands.spawn((
    Mesh2d(meshes.add(Circle::new(100.0))),
    MeshMaterial2d(materials.add(Color::srgb(7.5, 0.0, 7.5))),
    Transform::from_translation(Vec3::new(-200., 0., 0.)),
));
```

If the mesh material is missing, previously nothing was rendered. Now,
it renders a white default `ColorMaterial` in 2D and a
`StandardMaterial` in 3D (this can be overridden). Below, only every
other entity has a material:

![Näyttökuva 2024-09-29
181746](https://github.com/user-attachments/assets/5c8be029-d2fe-4b8c-ae89-17a72ff82c9a)

![Näyttökuva 2024-09-29
181918](https://github.com/user-attachments/assets/58adbc55-5a1e-4c7d-a2c7-ed456227b909)

Why white? This is still open for discussion, but I think white makes
sense for a *default* material, while *invalid* asset handles pointing
to nothing should have something like a pink material to indicate that
something is broken (I don't handle that in this PR yet). This is kind
of a mix of Godot and Unity: Godot just renders a white material for
non-existent materials, while Unity renders nothing when no materials
exist, but renders pink for invalid materials. I can also change the
default material to pink if that is preferable though.

## Testing

I ran some 2D and 3D examples to test if anything changed visually. I
have not tested all examples or features yet however. If anyone wants to
test more extensively, it would be appreciated!

## Implementation Notes

- The relationship between `bevy_render` and `bevy_pbr` is weird here.
`bevy_render` needs `Mesh3d` for its own systems, but `bevy_pbr` has all
of the material logic, and `bevy_render` doesn't depend on it. I feel
like the two crates should be refactored in some way, but I think that's
out of scope for this PR.
- I didn't migrate meshlets to required components yet. That can
probably be done in a follow-up, as this is already a huge PR.
- It is becoming increasingly clear to me that we really, *really* want
to disallow raw asset handles as components. They caused me a *ton* of
headache here already, and it took me a long time to find every place
that queried for them or inserted them directly on entities, since there
were no compiler errors for it. If we don't remove the `Component`
derive, I expect raw asset handles to be a *huge* footgun for users as
we transition to wrapper components, especially as handles as components
have been the norm so far. I personally consider this to be a blocker
for 0.15: we need to migrate to wrapper components for asset handles
everywhere, and remove the `Component` derive. Also see
https://github.com/bevyengine/bevy/issues/14124.

---

## Migration Guide

Asset handles for meshes and mesh materials must now be wrapped in the
`Mesh2d` and `MeshMaterial2d` or `Mesh3d` and `MeshMaterial3d`
components for 2D and 3D respectively. Raw handles as components no
longer render meshes.

Additionally, `MaterialMesh2dBundle`, `MaterialMeshBundle`, and
`PbrBundle` have been deprecated. Instead, use the mesh and material
components directly.

Previously:

```rust
commands.spawn(MaterialMesh2dBundle {
    mesh: meshes.add(Circle::new(100.0)).into(),
    material: materials.add(Color::srgb(7.5, 0.0, 7.5)),
    transform: Transform::from_translation(Vec3::new(-200., 0., 0.)),
    ..default()
});
```

Now:

```rust
commands.spawn((
    Mesh2d(meshes.add(Circle::new(100.0))),
    MeshMaterial2d(materials.add(Color::srgb(7.5, 0.0, 7.5))),
    Transform::from_translation(Vec3::new(-200., 0., 0.)),
));
```

If the mesh material is missing, a white default material is now used.
Previously, nothing was rendered if the material was missing.

The `WithMesh2d` and `WithMesh3d` query filter type aliases have also
been removed. Simply use `With<Mesh2d>` or `With<Mesh3d>`.

---------

Co-authored-by: Tim Blackbird <justthecooldude@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2024-10-01 21:33:17 +00:00
Joona Aalto
de888a373d
Migrate lights to required components (#15554)
# Objective

Another step in the migration to required components: lights!

Note that this does not include `EnvironmentMapLight` or reflection
probes yet, because their API hasn't been fully chosen yet.

## Solution

As per the [selected
proposals](https://hackmd.io/@bevy/required_components/%2FLLnzwz9XTxiD7i2jiUXkJg):

- Deprecate `PointLightBundle` in favor of the `PointLight` component
- Deprecate `SpotLightBundle` in favor of the `PointLight` component
- Deprecate `DirectionalLightBundle` in favor of the `DirectionalLight`
component

## Testing

I ran some examples with lights.

---

## Migration Guide

`PointLightBundle`, `SpotLightBundle`, and `DirectionalLightBundle` have
been deprecated. Use the `PointLight`, `SpotLight`, and
`DirectionalLight` components instead. Adding them will now insert the
other components required by them automatically.
2024-10-01 03:20:43 +00:00
Matty
429987ebf8
Curve-based animation (#15434)
# Objective

This PR extends and reworks the material from #15282 by allowing
arbitrary curves to be used by the animation system to animate arbitrary
properties. The goals of this work are to:
- Allow far greater flexibility in how animations are allowed to be
defined in order to be used with `bevy_animation`.
- Delegate responsibility over keyframe interpolation to `bevy_math` and
the `Curve` libraries and reduce reliance on keyframes in animation
definitions generally.
- Move away from allowing the glTF spec to completely define animations
on a mechanical level.

## Solution

### Overview

At a high level, curves have been incorporated into the animation system
using the `AnimationCurve` trait (closely related to what was
`Keyframes`). From the top down:

1. In `animate_targets`, animations are driven by `VariableCurve`, which
is now a thin wrapper around a `Box<dyn AnimationCurve>`.
2. `AnimationCurve` is something built out of a `Curve`, and it tells
the animation system how to use the curve's output to actually mutate
component properties. The trait looks like this:
```rust
/// A low-level trait that provides control over how curves are actually applied to entities
/// by the animation system.
///
/// Typically, this will not need to be implemented manually, since it is automatically
/// implemented by [`AnimatableCurve`] and other curves used by the animation system
/// (e.g. those that animate parts of transforms or morph weights). However, this can be
/// implemented manually when `AnimatableCurve` is not sufficiently expressive.
///
/// In many respects, this behaves like a type-erased form of [`Curve`], where the output
/// type of the curve is remembered only in the components that are mutated in the
/// implementation of [`apply`].
///
/// [`apply`]: AnimationCurve::apply
pub trait AnimationCurve: Reflect + Debug + Send + Sync {
    /// Returns a boxed clone of this value.
    fn clone_value(&self) -> Box<dyn AnimationCurve>;

    /// The range of times for which this animation is defined.
    fn domain(&self) -> Interval;

    /// Write the value of sampling this curve at time `t` into `transform` or `entity`,
    /// as appropriate, interpolating between the existing value and the sampled value
    /// using the given `weight`.
    fn apply<'a>(
        &self,
        t: f32,
        transform: Option<Mut<'a, Transform>>,
        entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle<AnimationGraph>)>,
        weight: f32,
    ) -> Result<(), AnimationEvaluationError>;
}
```
3. The conversion process from a `Curve` to an `AnimationCurve` involves
using wrappers which communicate the intent to animate a particular
property. For example, here is `TranslationCurve`, which wraps a
`Curve<Vec3>` and uses it to animate `Transform::translation`:
```rust
/// This type allows a curve valued in `Vec3` to become an [`AnimationCurve`] that animates
/// the translation component of a transform.
pub struct TranslationCurve<C>(pub C);
```

### Animatable Properties

The `AnimatableProperty` trait survives in the transition, and it can be
used to allow curves to animate arbitrary component properties. The
updated documentation for `AnimatableProperty` explains this process:
<details>
  <summary>Expand AnimatableProperty example</summary

An `AnimatableProperty` is a value on a component that Bevy can animate.

You can implement this trait on a unit struct in order to support
animating
custom components other than transforms and morph weights. Use that type
in
conjunction with `AnimatableCurve` (and perhaps
`AnimatableKeyframeCurve`
to define the animation itself). For example, in order to animate font
size of a
text section from 24 pt. to 80 pt., you might use:

```rust
#[derive(Reflect)]
struct FontSizeProperty;

impl AnimatableProperty for FontSizeProperty {
    type Component = Text;
    type Property = f32;
    fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> {
        Some(&mut component.sections.get_mut(0)?.style.font_size)
    }
}
```

You can then create an `AnimationClip` to animate this property like so:

```rust
let mut animation_clip = AnimationClip::default();
animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableKeyframeCurve::new(
        [
            (0.0, 24.0),
            (1.0, 80.0),
        ]
    )
    .map(AnimatableCurve::<FontSizeProperty, _>::from_curve)
    .expect("Failed to create font size curve")
);
```

Here, the use of `AnimatableKeyframeCurve` creates a curve out of the
given keyframe time-value
pairs, using the `Animatable` implementation of `f32` to interpolate
between them. The
invocation of `AnimatableCurve::from_curve` with `FontSizeProperty`
indicates that the `f32`
output from that curve is to be used to animate the font size of a
`Text` component (as
configured above).


</details>

### glTF Loading

glTF animations are now loaded into `Curve` types of various kinds,
depending on what is being animated and what interpolation mode is being
used. Those types get wrapped into and converted into `Box<dyn
AnimationCurve>` and shoved inside of a `VariableCurve` just like
everybody else.

### Morph Weights

There is an `IterableCurve` abstraction which allows sampling these from
a contiguous buffer without allocating. Its only reason for existing is
that Rust disallows you from naming function types, otherwise we would
just use `Curve` with an iterator output type. (The iterator involves
`Map`, and the name of the function type would have to be able to be
named, but it is not.)

A `WeightsCurve` adaptor turns an `IterableCurve` into an
`AnimationCurve`, so it behaves like everything else in that regard.

## Testing

Tested by running existing animation examples. Interpolation logic has
had additional tests added within the `Curve` API to replace the tests
in `bevy_animation`. Some kinds of out-of-bounds errors have become
impossible.

Performance testing on `many_foxes` (`animate_targets`) suggests that
performance is very similar to the existing implementation. Here are a
couple trace histograms across different runs (yellow is this branch,
red is main).
<img width="669" alt="Screenshot 2024-09-27 at 9 41 50 AM"
src="https://github.com/user-attachments/assets/5ba4e9ac-3aea-452e-aaf8-1492acc2d7fc">
<img width="673" alt="Screenshot 2024-09-27 at 9 45 18 AM"
src="https://github.com/user-attachments/assets/8982538b-04cf-46b5-97b2-164c6bc8162e">

---

## Migration Guide

Most user code that does not directly deal with `AnimationClip` and
`VariableCurve` will not need to be changed. On the other hand,
`VariableCurve` has been completely overhauled. If you were previously
defining animation curves in code using keyframes, you will need to
migrate that code to use curve constructors instead. For example, a
rotation animation defined using keyframes and added to an animation
clip like this:
```rust
animation_clip.add_curve_to_target(
    animation_target_id,
    VariableCurve {
        keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
        keyframes: Keyframes::Rotation(vec![
            Quat::IDENTITY,
            Quat::from_axis_angle(Vec3::Y, PI / 2.),
            Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
            Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
            Quat::IDENTITY,
        ]),
        interpolation: Interpolation::Linear,
    },
);
```

would now be added like this:
```rust
animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableKeyframeCurve::new([0.0, 1.0, 2.0, 3.0, 4.0].into_iter().zip([
        Quat::IDENTITY,
        Quat::from_axis_angle(Vec3::Y, PI / 2.),
        Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.),
        Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.),
        Quat::IDENTITY,
    ]))
    .map(RotationCurve)
    .expect("Failed to build rotation curve"),
);
```

Note that the interface of `AnimationClip::add_curve_to_target` has also
changed (as this example shows, if subtly), and now takes its curve
input as an `impl AnimationCurve`. If you need to add a `VariableCurve`
directly, a new method `add_variable_curve_to_target` accommodates that
(and serves as a one-to-one migration in this regard).

### For reviewers

The diff is pretty big, and the structure of some of the changes might
not be super-obvious:
- `keyframes.rs` became `animation_curves.rs`, and `AnimationCurve` is
based heavily on `Keyframes`, with the adaptors also largely following
suite.
- The Curve API adaptor structs were moved from `bevy_math::curve::mod`
into their own module `adaptors`. There are no functional changes to how
these adaptors work; this is just to make room for the specialized
reflection implementations since `mod.rs` was getting kind of cramped.
- The new module `gltf_curves` holds the additional curve constructions
that are needed by the glTF loader. Note that the loader uses a mix of
these and off-the-shelf `bevy_math` curve stuff.
- `animatable.rs` no longer holds logic related to keyframe
interpolation, which is now delegated to the existing abstractions in
`bevy_math::curve::cores`.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com>
2024-09-30 19:56:55 +00:00
Emerson Coskey
b04947d44f
Migrate bevy_transform to required components (#14964)
The first step in the migration to required components! This PR removes
`GlobalTransform` from all user-facing code, since it's now added
automatically wherever `Transform` is used.

## Testing

- None of the examples I tested were broken, and I assume breaking
transforms in any way would be visible *everywhere*

---

## Changelog

- Make `Transform` require `GlobalTransform`
~~- Remove `GlobalTransform` from all engine bundles~~
- Remove in-engine insertions of GlobalTransform and TransformBundle
- Deprecate `TransformBundle`
- update docs to reflect changes

## Migration Guide

Replace all insertions of `GlobalTransform` and/or `TransformBundle`
with `Transform` alone.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Tim <JustTheCoolDude@gmail.com>
2024-09-27 17:06:48 +00:00
Zachary Harrold
d70595b667
Add core and alloc over std Lints (#15281)
# Objective

- Fixes #6370
- Closes #6581

## Solution

- Added the following lints to the workspace:
  - `std_instead_of_core`
  - `std_instead_of_alloc`
  - `alloc_instead_of_core`
- Used `cargo +nightly fmt` with [item level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Item%5C%3A)
to split all `use` statements into single items.
- Used `cargo clippy --workspace --all-targets --all-features --fix
--allow-dirty` to _attempt_ to resolve the new linting issues, and
intervened where the lint was unable to resolve the issue automatically
(usually due to needing an `extern crate alloc;` statement in a crate
root).
- Manually removed certain uses of `std` where negative feature gating
prevented `--all-features` from finding the offending uses.
- Used `cargo +nightly fmt` with [crate level use
formatting](https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#Crate%5C%3A)
to re-merge all `use` statements matching Bevy's previous styling.
- Manually fixed cases where the `fmt` tool could not re-merge `use`
statements due to conditional compilation attributes.

## Testing

- Ran CI locally

## Migration Guide

The MSRV is now 1.81. Please update to this version or higher.

## Notes

- This is a _massive_ change to try and push through, which is why I've
outlined the semi-automatic steps I used to create this PR, in case this
fails and someone else tries again in the future.
- Making this change has no impact on user code, but does mean Bevy
contributors will be warned to use `core` and `alloc` instead of `std`
where possible.
- This lint is a critical first step towards investigating `no_std`
options for Bevy.

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-09-27 00:59:59 +00:00
Clar Fon
efda7f3f9c
Simpler lint fixes: makes ci lints work but disables a lint for now (#15376)
Takes the first two commits from #15375 and adds suggestions from this
comment:
https://github.com/bevyengine/bevy/pull/15375#issuecomment-2366968300

See #15375 for more reasoning/motivation.

## Rebasing (rerunning)

```rust
git switch simpler-lint-fixes
git reset --hard main
cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate
cargo fmt --all
git add --update
git commit --message "rustfmt"
cargo clippy --workspace --all-targets --all-features --fix
cargo fmt --all -- --unstable-features --config normalize_comments=true,imports_granularity=Crate
cargo fmt --all
git add --update
git commit --message "clippy"
git cherry-pick e6c0b94f6795222310fb812fa5c4512661fc7887
```
2024-09-24 11:42:59 +00:00
Patrick Walton
8154164f1b
Allow animation clips to animate arbitrary properties. (#15282)
Currently, Bevy restricts animation clips to animating
`Transform::translation`, `Transform::rotation`, `Transform::scale`, or
`MorphWeights`, which correspond to the properties that glTF can
animate. This is insufficient for many use cases such as animating UI,
as the UI layout systems expect to have exclusive control over UI
elements' `Transform`s and therefore the `Style` properties must be
animated instead.

This commit fixes this, allowing for `AnimationClip`s to animate
arbitrary properties. The `Keyframes` structure has been turned into a
low-level trait that can be implemented to achieve arbitrary animation
behavior. Along with `Keyframes`, this patch adds a higher-level trait,
`AnimatableProperty`, that simplifies the task of animating single
interpolable properties. Built-in `Keyframes` implementations exist for
translation, rotation, scale, and morph weights. For the most part, you
can migrate by simply changing your code from
`Keyframes::Translation(...)` to `TranslationKeyframes(...)`, and
likewise for rotation, scale, and morph weights.

An example `AnimatableProperty` implementation for the font size of a
text section follows:

     #[derive(Reflect)]
     struct FontSizeProperty;

     impl AnimatableProperty for FontSizeProperty {
         type Component = Text;
         type Property = f32;
fn get_mut(component: &mut Self::Component) -> Option<&mut
Self::Property> {
             Some(&mut component.sections.get_mut(0)?.style.font_size)
         }
     }

In order to keep this patch relatively small, this patch doesn't include
an implementation of `AnimatableProperty` on top of the reflection
system. That can be a follow-up.

This patch builds on top of the new `EntityMutExcept<>` type in order to
widen the `AnimationTarget` query to include write access to all
components. Because `EntityMutExcept<>` has some performance overhead
over an explicit query, we continue to explicitly query `Transform` in
order to avoid regressing the performance of skeletal animation, such as
the `many_foxes` benchmark. I've measured the performance of that
benchmark and have found no significant regressions.

A new example, `animated_ui`, has been added. This example shows how to
use Bevy's built-in animation infrastructure to animate font size and
color, which wasn't possible before this patch.

## Showcase


https://github.com/user-attachments/assets/1fa73492-a9ce-405a-a8f2-4aacd7f6dc97

## Migration Guide

* Animation keyframes are now an extensible trait, not an enum. Replace
`Keyframes::Translation(...)`, `Keyframes::Scale(...)`,
`Keyframes::Rotation(...)`, and `Keyframes::Weights(...)` with
`Box::new(TranslationKeyframes(...))`, `Box::new(ScaleKeyframes(...))`,
`Box::new(RotationKeyframes(...))`, and
`Box::new(MorphWeightsKeyframes(...))` respectively.
2024-09-23 17:14:12 +00:00
Benjamin Brienen
29508f065f
Fix floating point math (#15239)
# Objective

- Fixes #15236

## Solution

- Use bevy_math::ops instead of std floating point operations.

## Testing

- Did you test these changes? If so, how?
Unit tests and `cargo run -p ci -- test`

- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Execute `cargo run -p ci -- test` on Windows.

- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Windows

## Migration Guide

- Not a breaking change
- Projects should use bevy math where applicable

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: IQuick 143 <IQuick143cz@gmail.com>
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2024-09-16 23:28:12 +00:00
Taylor Neal
23a77ca5eb
Rename push children to add children (#15196)
# Objective

- Makes naming between add_child and add_children more consistent
- Fixes #15101 

## Solution

renamed push_children to add_children

## Testing

- Did you test these changes? If so, how?
Ran tests + grep search for any instance of `push_child`

- Are there any parts that need more testing?

- How can other people (reviewers) test your changes? Is there anything
specific they need to know?

- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
ran tests on WSL2

---

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes

rename any use of `push_children()` to the updated `add_children()`
2024-09-16 23:16:04 +00:00
Rob Parrett
86d5944b2e
Fix some examples having different instruction text positions (#15017)
# Objective

Thought I had found all of these... noticed some `10px` in #15013 and
did another sweep.

Continuation of #8478, #13583.

## Solution

- Position example text (and other elements) 12px from the edge of the
screen
2024-09-02 22:48:48 +00:00
Patrick Walton
d2624765d0
Implement animation masks, allowing fine control of the targets that animations affect. (#15013)
This commit adds support for *masks* to the animation graph. A mask is a
set of animation targets (bones) that neither a node nor its descendants
are allowed to animate. Animation targets can be assigned one or more
*mask group*s, which are specific to a single graph. If a node masks out
any mask group that an animation target belongs to, animation curves for
that target will be ignored during evaluation.

The canonical use case for masks is to support characters holding
objects. Typically, character animations will contain hand animations in
the case that the character's hand is empty. (For example, running
animations may close a character's fingers into a fist.) However, when
the character is holding an object, the animation must be altered so
that the hand grips the object.

Bevy currently has no convenient way to handle this. The only workaround
that I can see is to have entirely separate animation clips for
characters' hands and bodies and keep them in sync, which is burdensome
and doesn't match artists' expectations from other engines, which all
effectively have support for masks. However, with mask group support,
this task is simple. We assign each hand to a mask group and parent all
character animations to a node. When a character grasps an object in
hand, we position the fingers as appropriate and then enable the mask
group for that hand in that node. This allows the character's animations
to run normally, while the object remains correctly attached to the
hand.

Note that even with this PR, we won't have support for running separate
animations for a character's hand and the rest of the character. This is
because we're missing additive blending: there's no way to combine the
two masked animations together properly. I intend that to be a follow-up
PR.

The major engines all have support for masks, though the workflow varies
from engine to engine:

* Unity has support for masks [essentially as implemented here], though
with layers instead of a tree. However, when using the Mecanim
("Humanoid") feature, precise control over bones is lost in favor of
predefined muscle groups.

* Unreal has a feature named [*layered blend per bone*]. This allows for
separate blend weights for different bones, effectively achieving masks.
I believe that the combination of blend nodes and masks make Bevy's
animation graph as expressible as that of Unreal, once we have support
for additive blending, though you may have to use more nodes than you
would in Unreal. Moreover, separating out the concepts of "blend weight"
and "which bones this node applies to" seems like a cleaner design than
what Unreal has.

* Godot's `AnimationTree` has the notion of [*blend filters*], which are
essentially the same as masks as implemented in this PR.

Additionally, this patch fixes a bug with weight evaluation whereby
weights weren't properly propagated down to grandchildren, because the
weight evaluation for a node only checked its parent's weight, not its
evaluated weight. I considered submitting this as a separate PR, but
given that this PR refactors that code entirely to support masks and
weights under a unified "evaluated node" concept, I simply included the
fix here.

A new example, `animation_masks`, has been added. It demonstrates how to
toggle masks on and off for specific portions of a skin.

This is part of #14395, but I'm going to defer closing that issue until
we have additive blending.

[essentially as implemented here]:
https://docs.unity3d.com/560/Documentation/Manual/class-AvatarMask.html

[*layered blend per bone*]:
https://dev.epicgames.com/documentation/en-us/unreal-engine/using-layered-animations-in-unreal-engine

[*blend filters*]:
https://docs.godotengine.org/en/stable/tutorials/animation/animation_tree.html

## Migration Guide

* The serialized format of animation graphs has changed with the
addition of animation masks. To upgrade animation graph RON files, add
`mask` and `mask_groups` fields as appropriate. (They can be safely set
to zero.)
2024-09-02 17:10:34 +00:00
Shane
484721be80
Have EntityCommands methods consume self for easier chaining (#14897)
# Objective

Fixes #14883

## Solution

Pretty simple update to `EntityCommands` methods to consume `self` and
return it rather than taking `&mut self`. The things probably worth
noting:

* I added `#[allow(clippy::should_implement_trait)]` to the `add` method
because it causes a linting conflict with `std::ops::Add`.
* `despawn` and `log_components` now return `Self`. I'm not sure if
that's exactly the desired behavior so I'm happy to adjust if that seems
wrong.

## Testing

Tested with `cargo run -p ci`. I think that should be sufficient to call
things good.

## Migration Guide

The most likely migration needed is changing code from this:

```
        let mut entity = commands.get_or_spawn(entity);

        if depth_prepass {
            entity.insert(DepthPrepass);
        }
        if normal_prepass {
            entity.insert(NormalPrepass);
        }
        if motion_vector_prepass {
            entity.insert(MotionVectorPrepass);
        }
        if deferred_prepass {
            entity.insert(DeferredPrepass);
        }
```

to this:

```
        let mut entity = commands.get_or_spawn(entity);

        if depth_prepass {
            entity = entity.insert(DepthPrepass);
        }
        if normal_prepass {
            entity = entity.insert(NormalPrepass);
        }
        if motion_vector_prepass {
            entity = entity.insert(MotionVectorPrepass);
        }
        if deferred_prepass {
            entity.insert(DeferredPrepass);
        }
```

as can be seen in several of the example code updates here. There will
probably also be instances where mutable `EntityCommands` vars no longer
need to be mutable.
2024-08-26 18:24:59 +00:00
Rob Parrett
2c3f5a00ac
Add AnimationGraph::from_clips and simplify animated_fox example (#14853)
# Objective

Add a convenience constructor to make simple animation graphs easier to
build.

I've had some notes about attempting this since #11989 that I just
remembered after seeing #14852.

This partially addresses #14852, but I don't really know animation well
enough to write all of the documentation it's asking for.

## Solution

Add `AnimationGraph::from_clips` and use it to simplify `animated_fox`.

Do some other little bits of incidental cleanup and documentation .

## Testing

I ran `cargo run --example animated_fox`.
2024-08-25 14:16:04 +00:00
Matty
74cecb27bb
Disallow empty cubic and rational curves (#14382)
# Objective

Previously, our cubic spline constructors would produce
`CubicCurve`/`RationalCurve` output with no data when they themselves
didn't hold enough control points to produce a well-formed curve.
Attempting to sample the resulting empty "curves" (e.g. by calling
`CubicCurve::position`) would crash the program (😓).

The objectives of this PR are: 
1. Ensure that the curve output of `bevy_math`'s spline constructions
are never invalid as data.
2. Provide a type-level guarantee that `CubicCurve` and `RationalCurve`
actually function as curves.

## Solution

This has a few pieces. Firstly, the curve generator traits
`CubicGenerator`, `CyclicCubicGenerator`, and `RationalGenerator` are
now fallible — they have associated error types, and the
curve-generation functions are allowed to fail:
```rust
/// Implement this on cubic splines that can generate a cubic curve from their spline parameters.
pub trait CubicGenerator<P: VectorSpace> {
    /// An error type indicating why construction might fail.
    type Error;

    /// Build a [`CubicCurve`] by computing the interpolation coefficients for each curve segment.
    fn to_curve(&self) -> Result<CubicCurve<P>, Self::Error>;
}
```

All existing spline constructions use this together with errors that
indicate when they didn't have the right control data and provide curves
which have at least one segment whenever they return an `Ok` variant.

Next, `CubicCurve` and `RationalCurve` have been blessed with a
guarantee that their internal array of segments (`segments`) is never
empty. In particular, this field is no longer public, so that invalid
curves cannot be built using struct instantiation syntax. To compensate
for this shortfall for users (in particular library authors who might
want to implement their own generators), there is a new method
`from_segments` on these for constructing a curve from a list of
segments, failing if the list is empty:
```rust
/// Create a new curve from a collection of segments. If the collection of segments is empty,
/// a curve cannot be built and `None` will be returned instead.
pub fn from_segments(segments: impl Into<Vec<CubicSegment<P>>>) -> Option<Self> { //... }
```

All existing methods on `CyclicCurve` and `CubicCurve` maintain the
invariant, so the direct construction of invalid values by users is
impossible.

## Testing

Run unit tests from `bevy_math::cubic_splines`. Additionally, run the
`cubic_splines` example and try to get it to crash using small numbers
of control points: it uses the fallible constructors directly, so if
invalid data is ever constructed, it is basically guaranteed to crash.

---

## Migration Guide

The `to_curve` method on Bevy's cubic splines is now fallible (returning
a `Result`), meaning that any existing calls will need to be updated by
handling the possibility of an error variant.

Similarly, any custom implementation of `CubicGenerator` or
`RationalGenerator` will need to be amended to include an `Error` type
and be made fallible itself.

Finally, the fields of `CubicCurve` and `RationalCurve` are now private,
so any direct constructions of these structs from segments will need to
be replaced with the new `CubicCurve::from_segments` and
`RationalCurve::from_segments` methods.

---

## Design

The main thing to justify here is the choice for the curve internals to
remain the same. After all, if they were able to cause crashes in the
first place, it's worth wondering why safeguards weren't put in place on
the types themselves to prevent that.

My view on this is that the problem was really that the internals of
these methods implicitly relied on the assumption that the value they
were operating on was *actually a curve*, when this wasn't actually
guaranteed. Now, it's possible to make a bunch of small changes inside
the curve struct methods to account for that, but I think that's worse
than just guaranteeing that the data is valid upstream — sampling is
about as hot a code path as we're going to get in this area, and hitting
an additional branch every time it happens just to check that the struct
contains valid data is probably a waste of resources.

Another way of phrasing this is that even if we're only interested in
solving the crashes, the curve's validity needs to be checked at some
point, and it's almost certainly better to do this once at the point of
construction than every time the curve is sampled.

In cases where the control data is supplied dynamically, users would
already have to deal with empty curve outputs basically not working.
Anecdotally, I ran into this while writing the `cubic_splines` example,
and I think the diff illustrates the improvement pretty nicely — the
code no longer has to anticipate whether the output will be good or not;
it just has to handle the `Result`.

The cost of all this, of course, is that we have to guarantee that the
new invariant is actually maintained whenever we extend the API.
However, for the most part, I don't expect users to want to do much
surgery on the internals of their curves anyway.
2024-07-29 23:25:14 +00:00
François Mockers
5559632977
glTF labels: add enum to avoid misspelling and keep up-to-date list documented (#13586)
# Objective

- Followup to #13548
- It added a list of all possible labels to documentation. This seems
hard to keep up and doesn't stop people from making spelling mistake

## Solution

- Add an enum that can create all the labels possible, and encourage its
use rather than manually typed labels

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
2024-05-31 23:25:57 +00:00