Spine upgrade to AnimationState.cs leads to different animation behavior

abuki · 11 gün önce

Hi Harald,

Unfortunately, we discovered a new problem again related to this piece of code that was upgraded. It was not obvious on the first view, as it appears in some specific cases. As before, it is very challenging to share repro project, but I will try to explain what we see again as you might guess if that is in your code or mine.

We have a fairly complex track setup for the main characters, playing multiple animations in different tracks. And now it looks like when on one track the animation is interrupted and mixed to another that does not have keys for the bone the previous had, the pose for the bone is kept instead of mixing to the setup pose. Which should be the correct behavior, right?

Tl;dr I see visual changes in played animations with the same animations/track then before the update. I guess spine runtime should be consistent in interpreting animations after update, right?

Before we can be sure the behavior is consistent, I am downgrading back to an older version of spine runtime, which works fine (I kept the cleanup of my code, which was good anyway).

abuki · 11 gün önce

You can also update the discussion title of this to something like:
"Spine upgrade to AnimationState.cs leads to different animation behavior"

Harald · 11 gün önce

abuki We have a fairly complex track setup for the main characters, playing multiple animations in different tracks. And now it looks like when on one track the animation is interrupted and mixed to another that does not have keys for the bone the previous had, the pose for the bone is kept instead of mixing to the setup pose. Which should be the correct behavior, right?

If two animations are on the same track, then mixing out an animation by starting another one will mix out the changes of this animation if there is no key in the second animation. If there are animations on lower tracks, it's not mixed to the setup pose but the lower track will show. It's the same with the empty animation, which has no keys and mixes out the preceding animation, but does not apply the setup pose (which would be strange, overriding everything on lower tracks). If you want to mix to the setup pose on higher tracks overriding lower ones, you need to set a key identical to the setup pose, and have Animation clean up disabled in the export settings (otherwise these setup-pose keys will be removed again automatically during export).

abuki Which should be the correct behavior, right?

This sentence was a bit ambiguous. What do you assume to be the correct behaviour?

Nate · 11 gün önce

Harald have Animation clean up disabled in the export settings (otherwise these setup-pose keys will be removed again automatically during export).

Or check Layered!

abuki · 11 gün önce

Thanks for explaining.

It rises some questions, but if there is a detailed documentation for this, please give me the link, but I am unsure if I was able to find it.

To understand it properly, let's have the following example:
We have a bone A, animations1-3 and 3 tracks.
Track 0 - empty or unset (in our case, several empty tracks waiting for specific animations)
Track 1 - animation1 that animates bone A (has keys for A) which is interrupted in the middle, mixed to other animation2 which has no keys for A
Track 2 - another third animation3 that has no keys for A

What is the correct and expected behavior? After the mixing, should bone A have keys from setup or from interrupted animation1 (depending on when we started mixing)?

And please note that from my observation, this behavior changed with the previous code change in the first post. And that what I see in Spine Preview (trying to simulate something very similar) is different from the Unity runtime. There is still a small chance that something is bad in my code, but as I said it was working ok before runtime update. Also I need to be sure what is the correct behavior before diving deeper.

Another edge case might be what happens when on track 1 or 2 happens mixing during mixing, or calling update or anything special like that (there are some cases like this, difficult to simulate in Spine editor preview). May I ask if you have any test projects for testing these layering cases? Maybe I can try it on them?

Didn't know about the Layered tick box! That sounds very useful, can you please share a more detailed info or docs link on it?

Harald · 11 gün önce

abuki It rises some questions, but if there is a detailed documentation for this, please give me the link, but I am unsure if I was able to find it.

See:
https://esotericsoftware.com/spine-applying-animations
More specifically:
https://esotericsoftware.com/spine-applying-animations#Tracks
https://esotericsoftware.com/spine-applying-animations#Playback
https://esotericsoftware.com/spine-applying-animations#Empty-animations

abuki What is the correct and expected behavior? After the mixing, should bone A have keys from setup or from interrupted animation1 (depending on when we started mixing)?

"Track 2 - another third animation3 that has no keys for A" is irrelevant for bone A.
If the lower tracks don't key bone A as you've mentioned, and you're mixing-out the animation on track 1 which keys bone A to an animation which does not, you should end up with the setup pose.

If this is not your result, you might have discovered a bug. Could you please describe what result you get? Unfortunately it's a bit ambiguously written.

abuki Didn't know about the Layered tick box! That sounds very useful, can you please share a more detailed info or docs link on it?

When selecting an animation in the Tree view, you have options Export and Layered.

abuki · 10 gün önce

Harald If this is not your result, you might have discovered a bug. Could you please describe what result you get?

After the runtime update, I see the key from mixed out animation is preserved in the following example, no to the setup pose. It stays in the the position where mixing started. Now the question is if it is related purely to runtime or something in my code. I will try to investigate more.
Do you have any test examples pre-ready for these?

Harald When selecting an animation in the Tree view, you have options Export and Layered.

Yeah, I found it, just wanted to read some more detailed info how exactly the Layeredworks.

abuki · 10 gün önce

Ok, when trying to isolate the problem it looks like that in very basic scenario it works like intended. Now I need to dive deeper to see what is specific to my setup.

abuki · 10 gün önce

Here is code for an isolated problem, see the last lines with TODO and comments. I would like to hear your thoughts on this?

using Spine;
using Spine.Unity;
using UnityEngine;

public class AnimationTest : MonoBehaviour
{
    private const int MainTrack = 0;
    private const int HeadTrack = 1;

    [SerializeField] private SkeletonAnimation skeletonAnimation;
    [SerializeField] private AnimationReferenceAsset idleAnimation; // idle with keyed bone
    [SerializeField] private AnimationReferenceAsset walkAnimation; // walk without keyed bone
    [SerializeField] private AnimationReferenceAsset headAnimation; // head animation without keyed bone

    private void Start()
    {
        // Start idle
        skeletonAnimation.AnimationState.SetAnimation(MainTrack, idleAnimation, true);

        // Watch animations start
        skeletonAnimation.AnimationState.Start += AnimationStateOnStart;
    }

    private void Update()
    {
        if (Input.GetKeyDown(KeyCode.T))
        {
            // Start walking
            // AnimationStateOnStart will be fire and head animation set
            var mainTrackEntry = skeletonAnimation.AnimationState.SetAnimation(MainTrack, walkAnimation, true);
            mainTrackEntry.MixDuration = 0;
        }

        if (Input.GetKeyDown(KeyCode.U))
        {
            // Back to idle
            skeletonAnimation.AnimationState.SetAnimation(MainTrack, idleAnimation, true);
            skeletonAnimation.AnimationState.SetEmptyAnimation(HeadTrack, 0);
        }
    }

    private void AnimationStateOnStart(TrackEntry trackEntry)
    {
        // Checking if animation that needs independent head (like walk) started
        if (trackEntry.TrackIndex == MainTrack && trackEntry.Animation == walkAnimation.Animation)
        {
            var headTrackEntry = skeletonAnimation.AnimationState.SetAnimation(HeadTrack, headAnimation, true);
            headTrackEntry.MixDuration = 0;

            skeletonAnimation.Update(0); // TODO: This is causing the problem!
            // When Update(0) is called, the bone stays in idleAnimation position where it was interrupted
            // When Update(0) is not called, the bone mix to setup position as intended
            // This behavior changed after spine runtime update
            // Update(0) is here to prevent missing heads for one frame in some specific cases, but not sure if should be needed?
        }
    }
}

Nate · 10 gün önce

abuki just wanted to read some more detailed info how exactly the Layeredworks.

Normally animation clean up assumes the animation is applied on top of the setup pose -- in other words, that the animation is the first one to be applied. Clean up (among other things) will delete keys at the start of the animation that are the same as the setup pose.

When an animation is applied on top of another animation, those keys that are the same as the setup pose may be needed to override the animation below. Without this Layered check box, you can't use clean up because it will delete those keys. If you check the box you can still use clean up without losing keys at the start of the animation that are identical to the setup pose.

Layered is described in the docs under Clean Up:
https://esotericsoftware.com/spine-keys#Clean-Up

Here is the tooltip for layered:

When checked, clean up will preserve more keys by assuming this animation will be applied on top of other animations

Harald · 10 gün önce

abuki Yeah, I found it, just wanted to read some more detailed info how exactly the Layeredworks.

Note that you can also always hover over items longer or hit F1 to display the tooltip.

abuki Here is code for an isolated problem, see the last lines with TODO and comments. I would like to hear your thoughts on this?

Thanks for digging deeper! Your reproduction code can be simplified by removing anything on track 1:

using Spine;
using Spine.Unity;
using UnityEngine;

public class AnimationTest : MonoBehaviour
{
    [SerializeField] private SkeletonAnimation skeletonAnimation;
    [SerializeField] private AnimationReferenceAsset animWithKey; // animation with keyed bone
    [SerializeField] private AnimationReferenceAsset animWithoutKey; // animation without keyed bone

    private void Start () {
        skeletonAnimation.AnimationState.SetAnimation(0, animWithKey, true);
        skeletonAnimation.AnimationState.Start += AnimationStateOnStart;
    }

    private void Update () {
        // remark: hit U, wait a bit and then hit T and observe the leftover applied keys of anim0WithKey
        if (Input.GetKeyDown(KeyCode.U)) {
            skeletonAnimation.AnimationState.SetAnimation(0, animWithKey, true);
        }
        if (Input.GetKeyDown(KeyCode.T)) {
            var mainTrackEntry = skeletonAnimation.AnimationState.SetAnimation(0, animWithoutKey, true);
            mainTrackEntry.MixDuration = 0; // 0.001f fixes the issue
        }
    }

    private void AnimationStateOnStart (TrackEntry trackEntry) {
        if (trackEntry.Animation == animWithoutKey.Animation) {
            skeletonAnimation.Update(0); // TODO: This is causing the problem!
                                         // When Update(0) is called, the bone stays in idleAnimation position where it was interrupted
                                         // When Update(0) is not called, the bone mix to setup position as intended
                                         // This behavior changed after spine runtime update
                                         // Update(0) is here to prevent missing heads for one frame in some specific cases, but not sure if should be needed?
        }
    }
}

The problem seems to be due to skeletonAnimation.Update(0) being called from the callback which is called from within AnimationState.SetAnimation itself. While changing animation state from within callbacks must be done carefully (see this documentation section), we'll further investigate why exactly this is causing things to go wrong.

A quick fix BTW would be to set the mix duration to a tiny value other than 0 in the following line: mainTrackEntry.MixDuration = 0.001f;

abuki · 10 gün önce

Hi Harald,

Thanks, you are looking into it. The thing now is that I need to understand the change and how it works and if I should update my code or wait if you will be making some changes.
Because I can see that I can probably remove the Update call, which definitely was there for some reason, but I can't find any problems when removing it, so maybe something else was fixed in between?

So I don't need a quick fix, I better need to understand the issue properly and what is a general good practice.

abuki · 10 gün önce

The related question is if calls to Update(0) should ever be needed? I can see multiple calls to this in my code, usually fixing blinking for one frame as some logic changed something and animation is updated one frame later. So calling Update(0) fixes these and everything looks as it should. But maybe this is a bad practice in general and there is some better way?

Harald · 9 gün önce

abuki So I don't need a quick fix, I better need to understand the issue properly and what is a general good practice.

Of course, we will get back to you as soon as we've figured out what's going wrong.

abuki The related question is if calls to Update(0) should ever be needed? I can see multiple calls to this in my code, usually fixing blinking for one frame as some logic changed something and animation is updated one frame later. So calling Update(0) fixes these and everything looks as it should. But maybe this is a bad practice in general and there is some better way?

This is described in this documentation section. Calling SkeletonAnimation.Update(0) is in general a valid call to immediately reflect certain performend changes within the same frame. When bones remain unchanged, the call should be replaced by SkeletonAnimation.AnimationState.Apply(skeleton) instead, as described in the mentioned documentation section.

abuki · 9 gün önce

Harald in general a valid call to immediately reflect certain performend changes within the same frame

Well, thanks, then I am waiting for your investigation

Harald · 4 gün önce

@abuki Sorry for the long delay!

The problem is that you're first calling AnimationState.SetAnimation, then through the Start callback you're calling skeletonAnimation.Update(0) which calls AnimationState.Update(0) and after it has already been updated, you're calling mainTrackEntry.MixDuration = 0; which messes with the internal mixDuration state.

In general you shall always finish setting up the TrackEntry settings before calling AnimationState.Update. In most cases that's no problem, but in your case where the default mix is 0.2 and you're later setting mixDuration to 0.0, it unfortunately triggers a problem.

So to fix your issue, whenever calling SetAnimation, calling skeletonAnimation.Update(0); from the Start callback makes little sense, instead just call it like this:

 var mainTrackEntry = skeletonAnimation.AnimationState.SetAnimation(0, animWithoutKey, true);
mainTrackEntry.MixDuration = 0;
skeletonAnimation.Update(0);

Nate · 4 gün önce

Consider a different scenario, if the default mix was 0 and you did this:

var mainTrackEntry = skeletonAnimation.AnimationState.SetAnimation(0, animWithoutKey, true);
// SetAnimation above triggers your start callback which calls Update(0).
// If the default mix duration was 0, the mix has already completed!
mainTrackEntry.MixDuration = 0.2f; // Too late! This animation is already current.

You must ensure you do not call Update until after configuring your track entries. There is no reason to do it in a start callback. As Harald showed, call SetAnimation or AddAnimation, then configure the track entry, then call Update if needed.

Harald · 2 gün önce

@abuki We missed to reference the documentation earlier. This requirement is documented here as follows:

The mixDuration can be set manually rather than use the value from getMix. In that case, the mixDuration can be set for a new track entry only before update is first called.