bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
36.39k stars 3.59k forks source link

AnimatedProperty and AnimatedPropertyOptional #16484

Open cart opened 2 days ago

cart commented 2 days ago

Objective

Animating component properties requires too much boilerplate at the moment:

#[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 AnimatedProperty and AnimatedPropertyOptional, enabling the following:

animation_clip.add_curve_to_target(
    animation_target_id,
    AnimatableCurve::new(
        AnimatedProperty::new(|font: &mut TextFont| &mut font.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 AnimatedProperty 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.

cart commented 2 days ago

I'd like to land this in 0.15 as it:

  1. Polishes up a feature being released in 0.15
  2. Is reasonably straightforward / risk-free
viridia commented 1 day ago

There are a few things that I would do differently here.

First, I would define a getter and setter rather than returning a mutable field. This would allow the property being animated to be a simplified abstraction of the underlying property. So for example, I could animate rotation_x as an angle instead of having to deal with a quat - the conversion from angle to quat could be done in the AnimatableProperty impl. Similarly, I could animate scale as a scalar affecting all dimensions instead of separate scale_x, scale_y, scale_z.

See also the discussion in #15725 - there's a difference between "keyframe" curves and "transition" curves (my terms, perhaps there's a better way of describing this), in that the animation target can change in mid-animation (like a button which inflates when hovered, but the mouse quickly enters and leaves, giving no time for the animation to complete). In thee cases, you want to avoid an ugly jump from resetting the animation back to start, and instead start the animation from whatever the current state is.