Support tuple structs in AnimatedField #16747
Conversation
|
Welcome, new contributor! Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨ |
IQuick143
added
A-Animation
alice-i-cecile
added
C-Usability
| 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); |
There was a problem hiding this comment.
Question: What's the point of setting the field name if we have to specify this function?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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).
alice-i-cecile
added
S-Ready-For-Final-Review
# Objective Partially fixes #16736. ## Solution `AnimatedField::new_unchecked` now supports tuple struct fields. `animated_field!` is unchanged. ## Testing Added a test to make sure common and simple uses of `AnimatedField::new_unchecked` with tuple structs don't panic. --------- Co-authored-by: yonzebu <yonzebu@gmail.com>
# Objective Partially fixes bevyengine#16736. ## Solution `AnimatedField::new_unchecked` now supports tuple struct fields. `animated_field!` is unchanged. ## Testing Added a test to make sure common and simple uses of `AnimatedField::new_unchecked` with tuple structs don't panic. --------- Co-authored-by: yonzebu <yonzebu@gmail.com>
# Objective Allow tuple structs in the animated_field macro. - for example `animated_field!(MyTupleStruct::0)`. Fixes #16736 - This issue was partially fixed in #16747, where support for tuple structs was added to `AnimatedField::new_unchecked`. ## Solution Change the designator for `$field` in the macro from `ident` to `tt`. ## Testing Expanded the doc test on `animated_field!` to include an example with a tuple struct.
# Objective Allow tuple structs in the animated_field macro. - for example `animated_field!(MyTupleStruct::0)`. Fixes bevyengine#16736 - This issue was partially fixed in bevyengine#16747, where support for tuple structs was added to `AnimatedField::new_unchecked`. ## Solution Change the designator for `$field` in the macro from `ident` to `tt`. ## Testing Expanded the doc test on `animated_field!` to include an example with a tuple struct.
Objective
Partially fixes #16736.
Solution
AnimatedField::new_uncheckednow supports tuple struct fields.animated_field!is unchanged.Testing
Added a test to make sure common and simple uses of
AnimatedField::new_uncheckedwith tuple structs don't panic.