Skip to content

Support tuple structs in AnimatedField #16747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 34 additions & 8 deletions crates/bevy_animation/src/animation_curves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,18 +243,26 @@ where

impl<C: Typed, P, F: Fn(&mut C) -> &mut P + 'static> AnimatedField<C, P, F> {
/// Creates a new instance of [`AnimatedField`]. This operates under the assumption that
/// `C` is a reflect-able struct with named fields, and that `field_name` is a valid field on that struct.
/// `C` is a reflect-able struct, and that `field_name` is a valid field on that struct.
///
/// # Panics
/// If the type of `C` is not a struct with named fields or if the `field_name` does not exist.
/// If the type of `C` is not a struct or if the `field_name` does not exist.
pub fn new_unchecked(field_name: &str, func: F) -> Self {
let TypeInfo::Struct(struct_info) = C::type_info() else {
let field_index;
if let TypeInfo::Struct(struct_info) = C::type_info() {
field_index = struct_info
.index_of(field_name)
.expect("Field name should exist");
} else if let TypeInfo::TupleStruct(struct_info) = C::type_info() {
field_index = field_name
.parse()
.expect("Field name should be a valid tuple index");
if field_index >= struct_info.field_len() {
panic!("Field name should be a valid tuple index");
}
} else {
panic!("Only structs are supported in `AnimatedField::new_unchecked`")
};

let field_index = struct_info
.index_of(field_name)
.expect("Field name should exist");
}

Self {
func,
Expand Down Expand Up @@ -984,3 +992,21 @@ macro_rules! animated_field {
})
};
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_animated_field_tuple_struct_simple_uses() {
#[derive(Clone, Debug, Component, Reflect)]
struct A(f32);
let _ = AnimatedField::new_unchecked("0", |a: &mut A| &mut a.0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: What's the point of setting the field name if we have to specify this function?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super familiar with all the animation machinery, but for structs with named fields, the field name is used as part of the EvaluatorId, which I think identifies what part of the component is being animated? If you're manually constructing an AnimatedField (without the macro) you still have to do something like AnimatedField::new_unchecked("bar", |foo: &mut Foo| &mut foo.bar) for structs with named fields, so I figured it was fine to maintain the field name requirement, especially since I'm not quite sure what the EvaluatorId would be without being given the field name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yonzebu That is correct; in turn, it's important that the EvaluatorId incorporates the field index so that it can be known when two animatable properties are "the same" (this is used to dispatch the animation evaluators).


#[derive(Clone, Debug, Component, Reflect)]
struct B(f32, f64, f32);
let _ = AnimatedField::new_unchecked("0", |b: &mut B| &mut b.0);
let _ = AnimatedField::new_unchecked("1", |b: &mut B| &mut b.1);
let _ = AnimatedField::new_unchecked("2", |b: &mut B| &mut b.2);
}
}
Loading