Implement TryStableInterpolate.

[viridia]

Fixes: #20579

Testing

• Doc test
• Also tested extensively in a separate repo, however that code was not carried over (yet).

Note: Because docs.rs was down, I could not validate the doc link urls.

[viridia]

@LikeLakers2 Tried to add you as a reviewer but could not for some reason.

[LikeLakers2]

No worries. I hope I didn't frustrate you too much in the issue you made!

[LikeLakers2]

Huh, so this ended up working? I'm curious what was different about how you initially tested it, that caused the error you were reporting.

[viridia]

I think it has to do with the fact that the impls are in the same module as the target types instead of being in the same module as the trait definition. But to be honest, I'm not really sure why it works.

[LikeLakers2]

Me thinks it's best to let the user specify an error type, since interpolation can fail for reasons specific to the type being interpolated.

Suggested change

	pub trait TryStableInterpolate: Clone {
	/// Attempt to interpolate the value. This may fail if the two interpolation values have
	/// different units, or if the type is not interpolable.
	fn try_interpolate_stable(&self, other: &Self, t: f32) -> Result<Self, InterpolationError>;
	}
	pub trait TryStableInterpolate: Clone {
	/// The type returned in the event a stable interpolation cannot be performed.
	type Error;
	/// Attempt to interpolate the value.
	fn try_interpolate_stable(&self, other: &Self, t: f32) -> Result<Self, Self::Error>;
	}

This is similar to what TryFrom does, since conversion can fail for a variety of reasons, usually reasons specific to the types being converted.

[viridia]

Making the error type depend on the value type would defeat one of my purposes: to have a single uniform animation clip system that can mix both interpolable and non-interpolable types. Basically something like AnimatableProperty which can work for Val.

To be perfectly honest, we don't actually need an error at all, we could instead just make it an Option and return None. We never actually look at the error code, it's only there for documentation purposes and because the try_ name prefix suggest a Result return type. The animation only cares that the interpolation succeeded, it's not like it actually logs an error or anything.

[LikeLakers2]

Could you elaborate? I'm not sure how an error type that can be specified by each impl would defeat that purpose.

Also, I would recommend keeping it as a Result, even if our error type is (). This is because Option implies that trying will always succeed but may sometimes return nothing, whereas Result tells the user that the operation may fail.

[viridia]

I guess that it could work, since the error value is ignored, but it seems a bit dodgy. Here's the way the animation code looks now:

match self.start.try_interpolate(&self.end, t) {
    Ok(value) => value,
    Err(_) => self.end, // If we can't interpolate, then just skip to the end
}

Where start and end are generics of type TransitionProperty::ValueType:

/// Represents an animatable property such as `BackgroundColor` or `Width`.
pub trait TransitionProperty {
    /// The data type of the animated property.
    type ValueType: Copy + Send + Sync + PartialEq + 'static + TryStableInterpolate;

    /// The type of component that contains the animated property.
    type ComponentType: Component<Mutability = Mutable>;

    /// Read the value of the animatable property from the component.
    fn get(component: &Self::ComponentType) -> Self::ValueType;

    /// Update the value of the animatable property in the component.
    fn set(component: &mut Mut<Self::ComponentType>, value: Self::ValueType);
}

Adding an extra generic parameter for error type would mean that I wouldn't be able to treat different value types quite as uniformly as before.

[LikeLakers2]

Adding an extra generic parameter for error type would mean that I wouldn't be able to treat different value types quite as uniformly as before.

I'm not sure what's meant by this. Can you give me an example of what is meant here?

[viridia]

It's a bit difficult to reason about these cases because the way the Result is meant to be used is different than most Results. This isn't an error that gets logged or causes a panic; the intent is that the animation framework will see that the value cannot be interpolated and fall back to using a simple step. And this is the only response the animation framework can take, so the actual reason for the interpolation failure is irrelevant. Thus, it doesn't matter whether there's one failure code or a dozen.

This argument presumes that the only caller of this API would be an animation playback system of some kind. That might not be true, but at this point I can't imagine what other kind of code would call this.

[LikeLakers2]

So I did some thinking, and I think I have a good answer for both of you.

---

@kristoff3r, I will concede that MismatchedUnits would be a very common error type, and we should probably include it in Bevy as a result. But I don't think whether we can think of other error cases is relevant, as it's impossible to know every piece of code that will be written for TryStableInterpolate.

An associated type for the error would handle that issue, however. In the event that someone finds an error case that isn't covered by Bevy, an associated type would allow them to cover it themselves.

And should we find an error type becomes relatively common, we could add it as a built-in error type, for everyone to reuse.

---

@viridia, I don't think it makes sense to tailor TryStableInterpolate solely to the needs of the animation system. There can be plenty of cases where users may want to interpolate outside of the animation system - from physics to generating sprites, and so on. It's also possible that the user may wish to write their own animation system - perhaps they don't like some aspect of how Bevy's default animation system works.

In any one of those cases, a user may actually care about why an interpolation fails. If we build TryStableInterpolate to use Option instead of Result, or if we build TryStableInterpolate to only allow returning errors that Bevy has defined, we would force them to build their own interpolation traits to cover those deficiencies - taking time away from them that they could be using to build their game.

By adding an associated error type, we can solve that case before it comes up - even if the default animation system ignores the specific error given, and falls back to a simple step.

[viridia]

What would the associated error type be for types that have no error? What would TryStableInterpolate for f32 look like?

[LikeLakers2]

@viridia, core::convert::Infallible, the same used for the blanket TryFrom impl impl TryFrom<U> for T where U: Into<T> provided by the standard library (it even uses that blanket impl as an example).

It serves the same purpose as the ! "never" type, in that it's impossible to instantiate. So when used as an error type in a Result, the user is provided a guarantee by the type system that the Err variant will never be returned.

[LikeLakers2]

Woops, I forgot to answer your second question. TryStableInterpolate for f32 would probably look like this:

use core::convert::Infallible;

impl TryStableInterpolate for f32 {
	type Error = Infallible;

	fn try_interpolate_stable(&self, other: &Self, t: f32) -> Result<Self, Self::Error> {
		Ok(self.interpolate_stable(other, t))
	}
}

[LikeLakers2]

If this will always return an error, then I don't think it's an interpolate-able type.

Suggested change

	/// Boolean values can never be interpolated, but they can be animatable parameters (for things
	/// like enabling and disabling lighting).
	impl TryStableInterpolate for bool {
	fn try_interpolate_stable(&self, _other: &Self, _t: f32) -> Result<Self, InterpolationError> {
	Err(InterpolationError::NonContiguous)
	}
	}

[viridia]

Adding this was a bit of a social experiment to see what other people thought.

The use case here is to be able to have an AnimatableProperty that is a boolean rather than a continuous value - so for example you could use this to enable or disable an entity or component as part of an animation clip.

[kristoff3r]

Can we think of any examples that would have different error cases? It seems like mismatched units covers most of the cases where you'd like to use this. If we can then having the error be an associated type is the way to go.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21633

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.