AnimationState.Complete does not fire

geowal-wondeluxe

According to the documentation page Spine Events and AnimationState callbacks 'AnimationState.Complete' should fire any time an animation completes its full duration whether or not another animation is queued to play after it. However what I'm experiencing is that the Complete event is only invoked when another animation is not queued to play after it.

For example:

// OnAnimationComplete invoked:
skeletonAnimation.state.Complete += OnAnimationComplete;
skeletonAnimation.state.SetAnimation(0, animation1, false);

// OnAnimationComplete not invoked after completion of animation1:
skeletonAnimation.state.Complete += OnAnimationComplete;
skeletonAnimation.state.SetAnimation(0, animation1, false);
skeletonAnimation.state.AddAnimation(0, animation2, true, 0f);

I have confirmed this with a number of logs, all other events appear to fire. Of note, during the End event the TrackEntry is not marked as complete and TrackTime is less than TrackEntry.Animation.Duration.

private void OnAnimationEnd(TrackEntry trackEntry)
{
   bool isComplete = trackEntry.IsComplete;// Returns false
   bool reachedDuration = (trackEntry.TrackTime >= trackEntry.Animation.Duration);// Evaluates to false
}

This leaves me with no reliable way to determine if an animation has completed or not, which is obviously problematic.

Harald

Sorry to hear you're encountering problems. Unfortunately we could not reproduce the behaviour you're describing, we normally receive complete callbacks and also trackEntry.IsComplete return true in an OnAnimationEnd delegate in our test.

Which version of the spine-unity runtime (name of the unitypackage, also listed in Assets/Spine/version.txt) are you using?

geowal-wondeluxe

I'm using:

MacOS Monterey version 12.6
Unity 2022.1.23f1
spine-csharp version 4.1.0
spine-unity Runtime version 4.1.0

Spine installed from a GIT URL using Package Manager as described on spine-unity Runtime Documentation, excluding the examples package.

I've created an empty project with a minimal reproduction case. I can send that through if it's helpful.

Misaki

Thank you for the information.

I've created an empty project with a minimal reproduction case. I can send that through if it's helpful.

Great, we would appreciate it if you could send it to us via email: contact@esotericsoftware.com

Harald

Thanks for sending your reproduction project, we received everything. The cause of your Complete event not being fired is due to trackEntry.MixDuration in the following code section:

TrackEntry trackEntry = SkeletonAnimation.state.AddAnimation(trackIndex, animation, loop, delay);
trackEntry.MixDuration = mixDuration;

If you only change trackEntry.MixDuration without also updating trackEntry.Delay, it keeps the transition start time (set via the delay one line earlier) as it was, essentially using your Default Mix Duration of 0.2 set at the SkeletonDataAsset. With an animation duration of 0.6, this sets the transition start time to 0.6 - 0.2 = 0.4. When you then change the MixDuration to 0, it still starts, but now also ends the transition at 0.4, never reaching 0.6.

The corrected code section above would look like this:

TrackEntry trackEntry = SkeletonAnimation.state.SetAnimation(trackIndex, animation, loop);
trackEntry.MixDuration = mixDuration;
if (trackEntry.Previous != null)
    trackEntry.Delay = trackEntry.Previous.TrackComplete - trackEntry.MixDuration;

You can find details about it here on the Spine Runtime API documentation page:
TrackEntry mixDuration

geowal-wondeluxe

Apologies Harald – I obviously missed that part of the documentation.

That is a cheeky gotcha though. It would nice if there was something in the API that could handle that a bit more elegantly. Perhaps an overload of SetAnimation and AddAnimation that takes a mixDuration parameter? For now I've implemented my own extension methods on AnimationState. For anyone that wants to see the implementation:

public static TrackEntry SetAnimation(this AnimationState animationState, int trackIndex, Animation animation, bool loop, float mixDuration)
{
   TrackEntry trackEntry = animationState.SetAnimation(trackIndex, animation, loop);
   trackEntry.MixDuration = mixDuration;

   return trackEntry;
}

public static TrackEntry AddAnimation(this AnimationState animationState, int trackIndex, Animation animation, bool loop, float delay, float mixDuration)
{
   TrackEntry trackEntry = animationState.AddAnimation(trackIndex, animation, loop, delay);
   trackEntry.MixDuration = mixDuration;

   if (trackEntry.Previous != null && delay <= 0f)
   {
      trackEntry.Delay = trackEntry.Previous.TrackComplete - mixDuration + delay;
   }

   return trackEntry;
}

Nate

I agree, it's a tricky gotcha. I have mixed feelings about more parameters. It's common to set mix duration, but not required. Might be best to have a setMixDuration(mixDuration, delay) method.

geowal-wondeluxe

Yes these decisions are always tricky. Personally I think it makes sense to overload SetAnimation and AddAnimation, but I still feel way that AddAnimation changes the given delay parameter only if it's <= 0 is a bit opaque. Perhaps what would be clearer to users is if TrackEntry had an additonal delayToStart property that is calculated the same way delay is currently calculated, but also stored the delay value supplied by AddAnimation or Delay. This would give a bit more consistency to how users interact with TrackEntry.

TrackEntry.Delay property:

setter sets internal delay backing field
setter sets internal delayToStart
getter returns delay backing field

TrackEntry.MixDuration property:

setter sets internal mixDuration backing field
setter sets internal delayToStart
getter returns mixDuration backing field

TrackEntry.DelayToStart property:

getter only, returns delayToStart value

I guess a change like this would break existing code and it might be getting too complicated, but just a thought.

Nate

That's a pretty good idea, to remember the delay parameter so we can keep the actual delay updated if mix duration is changed. Setting mix duration would only change delayToStart if delay is <= 0.

The names delay and delayToStart are pretty good, but maybe delayToStart could be better. delay is the absolute time (> 0) or relative offset (<= 0). delayToStart is the time remaining and gets decremented until <= 0, then the track entry is made the current entry. We do call that "start" in AnimationStateListener. delayToStart or delayTime? delayRemaining? startTime? Any other ideas?

A breaking change can be worthwhile sometimes. We'll give it some more thought!

geowal-wondeluxe

startDelay?

I wasn't thrilled with delayToStart either, it was just the first thing that came to my mind for the purpose of the discussion. What about changing the name of the existing delay property to initialDelay or something?

I feel like we could throw ideas around for this all day! 😃

Nate

Indeed we could, the beauty of API design. :nerd: Names are important though and getting rid of gotchas feels good.

It's looking like delay and delayRemaining are the favorites with our runtimes team. I like delay for the property the user sets, nice and clean. Then it's nice for the "countdown" value to start with the word "delay". delayRemaining does that and also indicates that the value diminishes over time.