KybernetikGames / animancer

Documentation for the Animancer Unity Plugin.
63 stars 8 forks source link

Guidance on handling AnimationSet assets with MultiState Implementation #311

Closed RustedGames closed 11 months ago

RustedGames commented 11 months ago

Hello @KybernetikGames, In my video tutorial I'm building a locomotion system that can be changed depending of the equipped weapon, for that purpose I have created a scriptableobject called AnimationSet where I have a reference to individual ClipTransitions and MixerTransition2D for directional movements. I'm using a combination of the Platformer Sample and Weapons Example where I have a MultiState CharacterState holding the DodgeState, LocomotionState and StrafeState.

The problem I have is if I change the AnimationSet when I'm in any of States above, E.g DodgState, then locomotionState keeps playing the previous Animation.

Here showcasing equipping and unequipping weapons the "standard locomotion" works, but as soon I play the BackStep inside the DodgeState and it transitions correctly to the defaultState but the animation is not updated with correct animationSet. https://gyazo.com/fc0c9c5ed1d1a5380e6f8b02e904d90e

I have followed related questions here, but I can't find what I should change to make this to work. The MultiState code is basically a copy of the MultiState from the Platformer Demo.

Here's the code from the DodgeState state, but I think the problem is in the my MulttiState implementation, I have no clue how I can tell it to sync the other States with the right AnimationSets

using UnityEngine;
using Animancer;

namespace RustedGames
{
    public sealed class DodgeState : CharacterState
    {
        [SerializeField]
        private ClipTransition _Backstep;

        [SerializeField]
        private ClipTransition _Roll;

        [SerializeField]
        private MixerTransition2D _Dash;

        public override bool CanEnterState => Character.CharacterMovement.Dodge;

        public override bool CanExitState => true;

        public ITransition CurrentAnimation
        {
            get
            {
                if (!Character.CharacterMovement.LockOnTarget)
                {

                    if (Character.MovementDirection != Vector2.zero)
                    {
                        return _Roll;
                    }
                    else
                    {
                        return _Backstep;
                    }
                }
                else
                {
                    if (_Dash.State != null)
                    {
                        _Dash.State.Parameter = Character.MovementDirection;
                    }
                    return _Dash;
                }
            }
        }

        public override void OnEnterState()
        {
            base.OnEnterState();
            Character.CharacterAnimancerComponent.Play(CurrentAnimation);
        }

        private void Awake()
        {
            Weapon.OnWeaponChanged += UpdateAnimationSet;
            ChangeAnimationSet();
        }

        private void OnAnimationEnded()
        {
            Debug.Log(_Backstep.Clip.name + " ended");
            Character.CharacterMovement.VariableHandler.SetWithoutNotify
                (GameConstants.DodgeObjectVariableKey, false);
            Character.StateMachine.TryResetDefaultState();
        }

        private void Update()
        {
            if (_Dash.State != null)
            {
                _Dash.State.Parameter = Character.MovementDirection;
            }
        }

        private void ChangeAnimationSet()
        {
            _Backstep.CopyFrom(Character.CurrentAnimationSet.Backstep);
            _Roll.CopyFrom(Character.CurrentAnimationSet.Roll);
            _Dash.CopyFrom(Character.CurrentAnimationSet.Dash);

            _Backstep.Events.OnEnd = OnAnimationEnded;
            _Roll.Events.OnEnd  = OnAnimationEnded;
            _Dash.Events.OnEnd = OnAnimationEnded;
        }

        private void UpdateAnimationSet()
        {
            ChangeAnimationSet();
        }

        private void OnDestroy()
        {
            Weapon.OnWeaponChanged -= UpdateAnimationSet;
        }

    }
}
KybernetikGames commented 11 months ago

You're registering for OnWeaponChanged in Awake but un-registering in OnDisable so the first time that state ends it will stop listening for weapon changes. Using OnDestroy should avoid that (or you might not need to bother if it's only going to happen when the whole character is destroyed anyway).

RustedGames commented 11 months ago

Hi @KybernetikGames you were faster than me, I need to rephrase the question since I got it all wrong, I hope the description is clearer now. https://gyazo.com/631742871718c2eafee973ded655332c

KybernetikGames commented 11 months ago

A MixerTransition2D uses itself as the Key, meaning that even if you copy its contents from another transition, when you play it again it will still use the same key and play the same state instead of creating a new state with its new contents. The easiest way to avoid that would be to use Character.CharacterAnimancerComponent.States.Destroy to destroy the old state during ChangeAnimationSet so the next time it's played it will create the new one.

Other than that, nothing jumps out at me as looking wrong. Try putting in some more logs to make sure the new details swapped in by ChangeAnimationSet are actually working as intended and that you're playing the right animations.

RustedGames commented 11 months ago

Thanks a lot @KybernetikGames the solution was in front of me the whole time, given I have 3 main Sates, the solution was to store the AnimationSet reference when entering the sate for the first time and then checking if it was changed when entering the state the second time then proceed accordingly to update the clips.

public class LocomotionState : CharacterState
{
//**********************
 private AnimationSet _LocalCurrentAnimationSet => Character.CurrentAnimationSet;

        public override void OnEnterState()
        {
            base.OnEnterState();

            if (_LocalCurrentAnimationSet != Character.CurrentAnimationSet)
            {
                ChangeAnimationSet();
            }

            Character.CharacterAnimancerComponent.Play(CurrentAnimation);
        }

        private void ChangeAnimationSet()
        {
             _Idle.CopyFrom(Character.CurrentAnimationSet.Idle);
            _Walk.CopyFrom(Character.CurrentAnimationSet.Walk);
            _Jog.CopyFrom(Character.CurrentAnimationSet.Jog);
            _Sprint.CopyFrom(Character.CurrentAnimationSet.Sprint);
        }
//*********************
}
RustedGames commented 11 months ago

Project sample can be found here: https://github.com/IndieTutorials/DarkARPGProject.git

RustedGames commented 11 months ago

Hi @KybernetikGames, After deep testing MixerTransition2D is not working as I wanted, I'm afraid I'll be forced to handle the directional animations as simple ClipTransitions :(

For reference, in the ChangeAnimationSet method I ended up testing this code, the Destroy method always returns false, though I can see the animations changing in the inspector, but if I keep switching different AnimationSet at some point it gets stuck again.

            for (int i = 0; i < _StrafeWalkMovement.Animations.Length -1; i++)
            {                
                Character.CharacterAnimancerComponent.States.Destroy(_StrafeWalkMovement.Animations[i]);
            }

            for (int i = 0; i < _StrafeJogMovement.Animations.Length - 1; i++)
            {
               Character.CharacterAnimancerComponent.States.Destroy(_StrafeJogMovement.Animations[i]);
            }

            _StrafeWalkMovement.CopyFrom(Character.CurrentAnimationSet.Strafe_Walk);
            _StrafeJogMovement.CopyFrom(Character.CurrentAnimationSet.Strafe_Jog);

And after stopping Playmode in Editor I get this warning: Assets/Plugins/Animancer/AnimancerComponent.cs(315): PlayableGraph was not destroyed.

KybernetikGames commented 11 months ago

States.Destroy won't destroy mixer children because they aren't registered in the States dictionary.

You should be able to just destroy the mixers which will include destroying their children:

Character.CharacterAnimancerComponent.States.Destroy(_StrafeWalkMovement);
Character.CharacterAnimancerComponent.States.Destroy(_StrafeJogMovement);

PlayableGraph was not destroyed.

The only way I know of for that warning to occur is if you use an AnimancerComponent while it's disabled and never enable it because that means Unity doesn't call OnDestroy on that object. If that's not the case and it still happens after you fix the other issue you're having, please post another thread for it because that's definitely worth looking into.

RustedGames commented 11 months ago

Before the code I posted with the States.Destroy I already tested with your suggestion by passing the Mixer, for some reason the method always returns false = it didn't found the state, so I can't enclose it in a if statement, however the result is the same I'm afraid. I only get the Playable warning message if I implement any of the States.Destroy approaches

KybernetikGames commented 11 months ago

I can't think of any reason why Destroy wouldn't be able to find the states.

Do you think you could make a minimal reproduction project and send it to animancer@kybernetik.com.au?

RustedGames commented 11 months ago

The project is at https://github.com/IndieTutorials/DarkARPGProject.git but I need to update it with the changes discussed here

KybernetikGames commented 11 months ago

I'm not going to debug your project for you. If you can narrow down a specific situation which demonstrates that Animancer is the source of the problem then I can take a look at it, but it's not reasonable for you to just throw your whole project at me and expect me to fix it because the problem happens to be related to Animancer.

You also shouldn't be redistributing assets from the Asset Store. Even if they're free, the Asset Store license doesn't give you permission to do that.

RustedGames commented 11 months ago

I believe I got you in the wrong mood, I was not asking you to debug the project for me! 1) I only have time to work on this prototypes during weekends 2) I have requested permission to all the asset creators including you, to use their free versions of their assets for the tutorials and redistribute them to viewers. If you want to revoke that permission I will be happy to take remove the Animancer lite from the project.

KybernetikGames commented 11 months ago

Yes, posting a link to a git repo with the implication that I should just check it out and find your problem does tend to get me in a bad mood. I'm happy to spend my free time (emphasis on free) trying to help people use Animancer but there are inevitably people who try to abuse that kindness like you just did. It doesn't matter how often you can work on your project, you can keep this thread open until the topic is resolved and simply post in it when you actually have something that you reasonably believe should be brought to my attention such as a question or bug report.

I have no problem with you including Animancer Lite in the repo, but your LICENSE.md says your repo is covered by the MIT license. I didn't give you permission to change the license of Animancer and I suspect neither did any of those other asset creators. You need to explicitly specify which assets are covered by which license. I'd also recommend including a note about when/how permission was given for you to redistribute so users know you actually have permission and so the creators can easily cross check that with their records.

I can also see that you have a Mixamo folder but unfortunately they don't allow you to redistribute their raw assets (and as far as I know they don't even have a way to directly link users to a specific animation or model so it's not easy to include them in setup steps for users either).