Search Unity

OnStateMachineExit not fired from sub state machines when "Any State" transitions called

Discussion in 'Animation' started by edwon, Sep 28, 2016.

  1. edwon

    edwon

    Joined:
    Apr 24, 2011
    Posts:
    266
    OnStateMachineExit is not being fired on sub state machines whenever a transition out of "Any State" is fired. It only works when the sub state machine exits through the exit node.

    I need a method that gets fired on exit ALWAYS, no matter how I'm exiting.

    I've been digging through the forums, reddit, and done a ton of experiments, including trying out this fix:
    https://github.com/Baste-RainGames/AnimatorFix
    This is a well covered problem that is talked about here and other posts but apparently still has no fix:
    https://forum.unity3d.com/threads/statemachine-behaviour-review.423126/#post-2779394

    Nothing works.

    I really need this for my animation system as I've designed a huge amount of animation and systems using StateMachineBehaviors already. But I need to be able to clean up / reset certain things when I'm exiting a sub-state machine. Right now there is no reliable way to do that.

    Not only would it be great to have a method like OnStateMachineExit that actually fires whenever exiting, but also to have a true "exit" state that ALWAYS gets fired on exit that I could drop behaviours on that I can use to cleanup/reset stuff.
     
  2. CloudKid

    CloudKid

    Joined:
    Dec 13, 2015
    Posts:
    207
    To achieve the "true exit" you could try adding script directly on the BaseLayer of macanim.

    When you are in the animator layer go to the base layer (exit all the sub behaviours) and tap on an empty space. Then you can add code directly on the base layer (or any layer) like this. If you open the created class there is a comment which says:
    Now, I did not test it, but it should be called when any state in the machine exits. You can get info about the state that currenty ended from stateInfo.
     
  3. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    Oh S***, I should actually finish that! Sorry.

    Yeah, the functionality of OnStateMachineExit should still be changed to fire when the state machine exists. The current behavior is useless, so while it would be a "breaking change", nothing of value would be broken.
     
  4. edwon

    edwon

    Joined:
    Apr 24, 2011
    Posts:
    266
    Baste, awesome, yeah it would be amazing if Unity fixed it.

    In the mean time, might be cool to try CloudKids technique of adding a behaviour at the top level state machine, instead of adding it to the GameObject with the animator on it. That might also resolve your having to select the object every time you update it.
     
  5. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    The callback should be renamed to OnEntryNode/OnExitNode, the callback is badly named I agree but still the callback is useful to select what is gonna be the next state when you enter a Statemachine.

    https://blogs.unity3d.com/2015/02/11/having-fun-with-the-new-mecanim-features/

    the nice thing about this callback is that it get called while the statemachine is been evaluated and just before we evaluate all transitions so you can set parameter that will have a direct impact on which transition gonna get triggered.

    Maybe it not useful for you, but it's for some other users.

    As @CloudKid mentionned the only way to get a reliable StateMachine.OnStateExit/OnStateEnter is to add a behaviour directly on the statemachine itself.

    For the issue with interrupted transition not calling OnStateExit a fix was submited to trunk this week, if everything goes well I will backport this fix to 5.5.
     
  6. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    You would achieve exactly the same thing with a real OnStateMachineEnter, as long as AnimatorTransitionInfo gets an .machineTransition similar to .anyState. I'm not arguing that a callback that fires when you enter a state machine is not a good thing, I'm arguing that only calling it half the time you enter a state machine makes the system incredibly unwieldy.

    What does a OnEntryNode/OnExitNode achieve that a MachineEntry/Exit callback doesn't also achieve?
     
    IgorAherne likes this.
  7. edwon

    edwon

    Joined:
    Apr 24, 2011
    Posts:
    266
    I agree with Baste (and many others who have complained about this)

    So why not rename the current function to OnEnterNode/OnExitNode
    and also make an actual reliable OnStateMachineEnter/Exit function that gets fired always.

    I don't want to have this at the top level either, I want to be able to put it on the sub state machine so I can clean up or reset things when the sub state machine is exited for any reason in any wY (not just through exit)
     
  8. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    It the timing of callback, the OnEntryNode/OnExitNode is called while we do evaluate the state machine just before evaluating transitions conditions, all other statemachine behaviour callback are processed after the statemachine evaluation.

    So an OnEntryNode/OnExitNode callback can change the flow of evaluation of the current frame, all other callback can only influence the next frame.

    The real problem is not to argue against OnStateMachineEnter callback, we all agree that it would be a great addition.
    The problem is how we could change the system without breaking any project already using OnStateMachineEnter the way it was designed. We could say we don't care and change the behaviour but tomorow we will get a S*** tons of users with regression.

    So it only a matter of how to implement this without breaking project compatibility, the easiest solution is too choose another name for the callback. OnStateMachineStateEnter().

    I did bring this to the animation meeting this morning and we all agree that it need to be done it only a matter of how which I need to sort out with our scripting team
     
    Last edited: Oct 3, 2016
    Baste likes this.
  9. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    Thanks for the response! It's great to see that this is making progress.

    I also totally understand the problem with the evaluation order. I had forgotten that whole aspect about the callback.

    On that note, the callback should then be called OnBeforeEntryNode or just BeforeEntryNode or something like that. The ordering of these things are very important, and should be a part of the name so users notice it. OnEntryNode isn't a problem in itself - all examples will probably use the fact that it resolves early, so users will understand that.

    The problem is that if that method is named "OnEntry" and is evaluated before the machine, users will intuitively assume that the "On...Enter" methods also gets evaluated before the machine. If Before is a part of the EntryNode's callback name, the difference will be a part of the naming.
     
  10. edwon

    edwon

    Joined:
    Apr 24, 2011
    Posts:
    266
    Mecanim Dev, whatever you do, please do it as fast as possible. We are waiting (and posts about these issues go back to 2012)
     
  11. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    StateMachineBehaviours were announced mid-2014 and shipped in 5.0, which was in March 2015. So you're off by three years.

    Also nagging isn't going to get anything done!
     
  12. edwon

    edwon

    Joined:
    Apr 24, 2011
    Posts:
    266
    Sorry :( goofed up on the date, you're right Baste! I am just excited to see these changes happen.
     
  13. edwon

    edwon

    Joined:
    Apr 24, 2011
    Posts:
    266
    Ok now that I'm actually trying to implement the technique of putting a StateMachineBehaviour at the very top of the entire state machine, I have no idea how to actually detect which sub-state machine I'm in. I'd like to just be able to, in the OnStateExit check which sub-state machine I'm exiting from by name.
     
  14. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
  15. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    How would you go about making a script that automatically does that? It's possible to do it from a menu button or a shortcut or a button or whatever, but is it possible to make a thing that automatically notices that an AnimatorController has been updated and sets the correct tags?

    I have been able to have a MonoBehaviour's editor modify an Animator's controller, by using reflection to get at the controller, but I can't see how you'd automate the entire thing?

    Maybe from the SMB's editor, but can you get from a SMB to the animator that instance is attached to at editor time in any reasonable way?
     
  16. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    Like you said with a menu or button, it's not ideal but as a workaround it does work.

    We do have an internal message that we send when a controller is modified, we could expose it and that would allow you to do all kind of automatic setup when needed from a script

    OnDidModifyAnimatorController(AnimatorController controller);

    what do you think?
     
  17. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    That would be really nice!

    Then I could do things like ensuring that an AnimatorController that's meant to be an AI state machine only has zero-length transitions, and store metadata on StateMachineBehaviours.

    I've got some prefabs that break if I change the AnimatorController that's attached to the prefab unless I select the prefab and let a script's behaviour parse the changes to that AnimatorController. This would allow me to store the necessary metadata about the controller directly IN the controller instead of on the prefab.
     
  18. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    Look like we already have an internal System.Action callback on the AnimatorController class
    public sealed partial class AnimatorController : RuntimeAnimatorController
    {
    internal System.Action OnAnimatorControllerDirty;
    .....
    }

    with reflection you could find it and setup your own callback
    all our tools are hooked on this callback already so be careful and use += or -= when adding your own callback otherwise animation tools will stop responding.

    m_Controller.OnAnimatorControllerDirty += ControllerDirty;
     
    forestrf and Baste like this.
  19. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    @Baste let me know if you try it and it does work, I'm curious.
    There is alot of user operation that will trigger this callback, so i'm wondering if you could run into performance issue if the process that you run when you get this callback is heavy.
     
  20. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    I'll do that right away.

    In the meantime, as far as I can see, the only way to get at the animator controller of an animator is through reflection. I can get it through SerializedObject.FindProperty, but that seems to fail at times. Here's my code:

    Code (csharp):
    1. private bool TryFindAnimatorController(out AnimatorController controller) {
    2.     var anim = GetComponent<Animator>();
    3.     if (anim == null) {
    4.         controller = null;
    5.         return false;
    6.     }
    7.  
    8.     var serializedObject = new SerializedObject(anim);
    9.     var serializedProperty = serializedObject.FindProperty("m_Controller");
    10.     var objectReferenceValue = serializedProperty.objectReferenceValue;
    11.  
    12.     if (objectReferenceValue == null) {
    13.         controller = null;
    14.         return true;
    15.     }
    16.     controller = (AnimatorController) objectReferenceValue;
    17.     return false;
    18. }
    This manages to find the controller from OnBeforeSerialize, but if I run it in edit-mode Awake ([ExecuteInEditMode] MonoBehaviour), it fails.


    In that vein, it would be really nice to have this method:

    Code (csharp):
    1. AnimatorController AnimatorUtility.GetAnimatorController(Animator animator);
     
  21. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    Oh wait, looks like .runTimeAnimatorController the actual animatorController at edit time. Is that correct?
     
  22. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    Can't seem to get access to the event:

    Code (csharp):
    1. var updatedEvent = typeof (AnimatorController).GetEvent("OnAnimatorControllerDirty", BindingFlags.NonPublic | BindingFlags.Instance);
    2. Debug.Log("Found event: " + (updatedEvent != null)); //prints false!
    GetEvents returns empty arrays both with the Instance and the Static flags.

    Is the callback very recent? We're on 5.4.0 right now.
     
  23. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    yes, an AnimatorController derive from a RuntimeAnimatorController, as an AnimatorOverrideController derive from a RuntimeAnimatorController too.
    So watch because .runtimeAnimatorController could be either an AnimatorController or an AnimatorOverrideController.

    no has been there since 4.0 because it needed to update all the animation tools: Animator windows and all the inspector for AnimatorState, Transition, blend tree, etc ...
     
  24. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    @Baste try this it should work
    Code (CSharp):
    1.  
    2. FieldInfo myField = typeof(AnimatorController).GetField("OnAnimatorControllerDirty", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static);
    3.         Debug.Log(myField);
    4.  
     
  25. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,338
    Thanks, that was the missing piece!

    It works, but the reflection was a bitch to get working:

    Code (csharp):
    1. public void RegisterUpdateCallback(Animator animator) {
    2.     AnimatorController ac = animator.runtimeAnimatorController as AnimatorController;
    3.     if (ac == null) {
    4.         Debug.LogWarning("Not an AnimatorController (Probably an overrideController)!");
    5.         return;
    6.     }
    7.  
    8.     FieldInfo controllerDirtyInfo = typeof (AnimatorController).GetField("OnAnimatorControllerDirty", BindingFlags.NonPublic | BindingFlags.Instance);
    9.     if (controllerDirtyInfo == null) {
    10.         Debug.LogWarning("Unity changed the name of/removed the OnAnimatorControllerDirty event!");
    11.         return;
    12.     }
    13.  
    14.     Delegate originalDelegate = controllerDirtyInfo.GetValue(ac) as Delegate;
    15.     var debugMethod = GetType().GetMethod("PrintACChanged", BindingFlags.NonPublic | BindingFlags.Instance);
    16.     Delegate newDelegate = Delegate.CreateDelegate(controllerDirtyInfo.FieldType, this, debugMethod);
    17.     Delegate combined = Delegate.Combine(originalDelegate, newDelegate);
    18.     controllerDirtyInfo.SetValue(ac, combined);
    19. }
    20.  
    21. private void PrintACChanged() {
    22.     Debug.Log("AC Changed!!");
    23. }
    Now, that seems to get invoked for everything that happens to the animatorcontroller, except two things:
    - changing the position of a meta-node (so Any State, entry or exit) does not call the method.
    - setting values in a StateMachineBehaviour's inspector.

    Neither of those are a problem for me, but the second one might perhaps be a problem for someone?

    The delegate is also cleared on a script reload, which will need to be handled. Still, I could use this for interesting things.
     
    forestrf likes this.
  26. Mecanim-Dev

    Mecanim-Dev

    Joined:
    Nov 26, 2012
    Posts:
    1,675
    forestrf likes this.
  27. Vaitaliy

    Vaitaliy

    Joined:
    Jan 10, 2019
    Posts:
    11
    So OnStateMachineExit still doesn't work on any state even in 2021?
     
  28. unityJamesHPak

    unityJamesHPak

    Joined:
    Feb 22, 2021
    Posts:
    1
    Someone from Unity should implement new event called OnStateMachineExitAllCase
    or something like that, that gets calls regardless exiting through exit node or exiting through
    some external interruptions, then we don't have to worry about breaking all things that have been done.
    Without it, it's really awkward to implement state system.
     
    cassidycurtis likes this.
  29. cassidycurtis

    cassidycurtis

    Joined:
    Apr 8, 2019
    Posts:
    6
    Adding one more voice in support of this request. This missing functionality really seems like an oversight. It doesn't make much sense that OnStateMachineExit is invoked only when you pass through the specially anointed Exit node, whereas OnStateExit is invoked whenever you exit a state for any reason.

    I get that there may be legacy code that depends on that narrower interpretation of "Exit", so I see why Unity would be reluctant to change the existing method's behavior.

    But still, it would really help us all a lot if we could get the functionality we need. Regardless of what you decide to call it, it would be so nice if we could have an "OnStateMachineExitForAnyReason" method that works in all cases.
     
    sinedsem likes this.
  30. Vaitaliy

    Vaitaliy

    Joined:
    Jan 10, 2019
    Posts:
    11
    It's almost 2022 and we still don't have a reliable OnStateMachineExit call. Here's my awful code that I use to see if exit state happened even through AnyState.



    Code (CSharp):
    1. public class SetBoolOnStateMachineExit : StateMachineBehaviour {
    2.  
    3.     public string Parameter = "";
    4.     public bool Value;
    5.  
    6.     //This function is not reliable
    7.     public override void OnStateMachineExit(Animator animator, int stateMachinePathHash, AnimatorControllerPlayable controller) {
    8.       if (exitState != null)
    9.         StopCoroutine(exitState);
    10.       animator.SetBool(Parameter, Value);
    11.     }
    12.  
    13.     private Coroutine exitState;
    14.     public override void OnStateUpdate(Animator animator, AnimatorStateInfo stateInfo, int layerIndex, AnimatorControllerPlayable controller) {
    15.       if (exitState != null)
    16.         StopCoroutine(exitState);
    17.       exitState = StartCoroutine(DoAction(Time.deltaTime * 5f, () => {
    18.          if (animator) animator.SetBool(Parameter, Value);
    19.       }));
    20.     }
    21.  
    22.  
    23.   }
     
  31. silentslack

    silentslack

    Joined:
    Apr 5, 2013
    Posts:
    394
    @Vaitaliy how are you using a coroutine in a non mono here?