Investigate providing &mut world in tweenable callback vs. using events

[djeedai]

Currently a tweenable callback has little information. It contains a reference to the Tween<T>, and (change not merged yet) the Entity of the Animator<T> or AssetAnimator<T>.

Kero on Discord suggested providing a &mut world to allow the callback to do something useful, similar to what bevy-tick-timers does. But this forces the system ticking the animators to take a global lock and become exclusive, serializing execution and being a potential choke point for other Bevy systems, which can cause performance issues. This is particularly bad for bevy_tweening which has a series of such systems, once per component type:

bevy_tweening/src/plugin.rs

Lines 48 to 59 in 38e9939

	pub fn component_animator_system<T: Component>(
	time: Res<Time>,
	mut query: Query<(&mut T, &mut Animator<T>)>,
	) {
	for (ref mut target, ref mut animator) in query.iter_mut() {
	if animator.state != AnimatorState::Paused {
	if let Some(tweenable) = animator.tweenable_mut() {
	tweenable.tick(time.delta(), target);
	}
	}
	}
	}

A possible alternative which sounds more Bevy-esque would be to replace the callback with a Bevy event. This way the user app can design any system they want to read that event, with any resource / query they need. This however implies being able to write an event without knowing its type at build time, to avoid depending on the event type as an extra generic parameter for the system:

let mut tween = Tween::new(...);
tween.set_completed::<MyEvent>();

then

pub fn component_animator_system<T: Component>( 
    time: Res<Time>, 
    mut query: Query<(&mut T, &mut Animator<T>)>,
    mut writer: EventWriter<????>, //< Here we don't know the type
) { 
    for (ref mut target, ref mut animator) in query.iter_mut() { 
        if animator.state != AnimatorState::Paused { 
            if let Some(tweenable) = animator.tweenable_mut() { 
                tweenable.tick(time.delta(), target, writer); 
            } 
        } 
    } 
} 

[alice-i-cecile]

As a simpler alternative, create Events<TweenCompleted> (and so on) event streams, which stores the Entity that the tween is associated with along with other alternative information.

Then, users can follow that event stream, and respond to the animation events in their own systems.

[Kerollmops]

Just found out that it would maybe be better to insert a component to an entity and remove it when the animation is finished. It is the way benimator does it, it inserts the Play component to the animated entity (Sprite) and removes it when the animation was set to Once and is finished.

One can detect the removal of a component pretty easily by using the RemovedComponent special system parameter, the only "tricky" part would be on this crate side, you can read more on the Bevy cheat sheet.

It looks easier to implement in the sense that this crate only has to insert and remove a Play component and it is also maybe more bevy-idiomatic it is no good in terms of performance.

[djeedai]

Support has been merged via e31e76d and 8dcd3d2, so closing this. The sequence example provides a use case example, which given how easy it is to use feels like the approach is superior to add/remove components. We can revisit the question of &mut world and the dynamic event type if usage proves them useful.