Sharing SpriteAnimation

[ufrshubham]

What could be improved

Would be better to warn users when same SpriteAnimation is shared between multiple SpriteAnimationComponents or SpriteAnimationGroupComponents.

Why should this be improved

Right now, if a SpriteAnimation is shared between multiple components, it gets ticked by all the components that are using it, causing the animation to essentially speed up.

Any risks?

None that I can think of.

More information

[spydon]

Since the animation have no sense of a parent, how do you plan to show the warning?

[ufrshubham]

Haven't thought this through, but I am thinking of extending SpriteAnimation from Component. That way we can add it as an child of SpriteAnimationComponent every time it is set.

[spydon]

SpriteAnimation is unfortunately a class that should be able to live outside of FCS, just like Sprite. I guess that it could still have a parent field that is manually set though, but it feels a bit dirty.

[st-pasha]

You could add the animationIndex in the SpriteAnimationComponent, and set it to be the same as in the SpriteAnimation. Then, whenever in the future you detect that the animation indices had diverged, it would be a clear indicator that the SpriteAnimation is being used improperly.

[ufrshubham]

animationIndex will diverge even when users explicitly modify SpriteAnimation's currentIndex

[ufrshubham]

I did some more digging into this and I think we can do some refactoring to actually allow shared sprite animations between multiple components.

As of now, SpriteAnimation take the responsibility of storing a list of animation frames as well as ticking itself. So if we just move out the ticking part (maybe to SpriteAnimationTicker), it is possible to share the same animation. It could be a standalone class or maybe a mixin too.

I did some quick prototyping and I think this will be a significant breaking change 😄.

[spydon]

@ufrshubham sounds great, maybe we should wait with it for v2.0? Or maybe it wouldn't break for that many since very few use the SpriteAnimation without the component?

[ufrshubham]

@ufrshubham sounds great, maybe we should wait with it for v2.0? Or maybe it wouldn't break for that many since very few use the SpriteAnimation without the component?

Yeah, if very few are using SpriteAnimation directly, then this should be less impactful. I am expecting members like onFrame, onComplete, etc to move out from SpriteAnimation in this change. So people that do use them, will have to do some migration.

I'll try to put up a PR as soon as I am happy with an implementation and then we all can take a call on when to merge it. Sooner will be better though. At least that will give us enough time to test and fix any bugs before v2.0. Do we have any rough ETA on v2.0?

[spydon]

No ETA for V2, several months away at least.