Do I need UnShared for my animations stored on a ScriptableObject?

[ADH-LukeBollam]

I'm a bit confused about when the rules about a Transition Asset apply. I have a scriptable object that defines the clip transitions for a weapon, so I can swap them out at runtime depending on which weapon is being used:

The Actions each look like this:

[Serializable] 
public class AnimationConfiguration
{
    [SerializeReference] List<ITransitionDetailed> _clips;
    [field: SerializeField] public int PlaybackLayer { get; private set; }

    public ReadOnlyCollection<ITransitionDetailed> Clips { get { return _clips.AsReadOnly(); } }
}

This seems like I meet the requirements for UnShared

My animation handler has optional events it can hookup to the clip each time it's played, which looks like this

    public void PlayAnimation(AnimationType type, Action onEndEvent)
    {
        var animConfig = _animationSet.Actions[type];
        var selectedAnimation = animConfig.Clips[UnityEngine.Random.Range(0, animConfig.Clips.Count)];

        var state = _animancer.Layers[animConfig.PlaybackLayer].Play(selectedAnimation, selectedAnimation.FadeDuration, FadeMode.FromStart);

        state.Events.OnEnd = () => FinishOverride(state, animConfig.PlaybackLayer, onEndEvent);
    }

    private void FinishOverride(AnimancerState state, int layer, Action onFinish)
    {
        state.Events.OnEnd = null;

        onFinish?.Invoke();
    }

Note how the event is set whenever it is played, and set to null again when the animation completes.

I'm confused though because running this with multiple enemies using the same animation set doesn't cause me any problems with the Events.OnEnd being shared. The enemies are all able to run / jump / attack at different or overlapping times and it all works fine. I thought I should be having issues with the shared events setting and cancelling each other.

I know it's not an issue now, but I don't want to shoot myself in the foot later if it turns out I need to make an UnShared version of everything once the scope of my game is bigger. Am I misunderstanding something about the limitations of a Transition Asset?

[KybernetikGames]

Your understanding seems correct, but you've run into an edge case which isn't really documented. The Clear Automatically section explains the difference in event handling between playing a clip directly and playing a transition, but actually if you play a transition that doesn't already have events then it won't bother allocating an empty event sequence to give the state so it will essentially function the same as playing a clip directly. Then after you play it, setting the end event like you are will give the state a spare event sequence from the object pool to hold it. So your setup would continue working fine unless you ever put events in the transitions.

The UnShared system probably won't help you here. It's a bit of a hack and if you look at the UnShared base classes you'll see that it takes quite a bit of complexity just to wrap the events and state reference of one transition asset containing one set of events. Doing the same for your AnimationSet would require similar complexity for every transition the set contains.

The simplest solution would be to just instantiate a copy of the AnimationSet for each character so they aren't sharing anything. That means an unfortunate amount of memory duplication overhead, but it should just work.

Another possibility would be for me to implement a method on states which basically says "if you got your events from a transition, get a new event sequence from the object pool and copy your current events into it". Then you could just play it, call that method, and add your end event knowing it won't cause event sharing issues. My main goal would be to support the recommended workflow of setting up transition events on startup rather than adding them on play like you are, but it should be possible to allow both approaches.

Thoughts?

Also, if you tick the Start Time toggle in the transition, you wouldn't need to manually feed in its fade duration and it would automatically use FadeMode.FromStart.

[ADH-LukeBollam]

I think intuitively I expected a State to be the state of the playing animation, including the .Events property. It could be made clearer that you're accessing the events of ClipTransition by hiding that access behind another property, something like state.BaseClipTransition.Events, but honestly I don't know your project well enough to suggest a solution.

I like the idea of taking a copy of the ClipTransition Events, but I don't think it needs to be a different method. If the public State.Events getter is lazy evaluated as a copy of the Cliptransition events then they won't even need to be instantiated most of the time, and private access could go to the ClipTransition.Events if the State.Events is null. Worst case scenario some people will be assigning events to the State each time when they could just be assigning it to the ClipTransition once, and it prevents users from hooking up the same event multiple times accidentally which judging by the warnings in the logs, that's a common issue you've had to deal with.

Something like this? There's a lot of inner workings I don't know but the lazy evaluation is what I was trying to show here

public class AnimancerState
{
    private ClipTransition _baseClipTransition;
    private Events _events;

    public Events Events {
        get
        {
            if (_events == null)
            {
                _events = _baseClipTransition.Events.ToList(); // not sure what the cloning process is
            }
            return _events;
        }
        set
        {
            _events = value;
        }
    }

    private void FireEvents()
    {
        if (_events == null)
        {
            _baseClipTransition.FireEvents;
        }
        else
        {
            _events.DoThings();
        }
    }
}

This means you can set your events in the ClipTransition and it won't cause any extra allocations of EventSequence's for no reason, or you can set events directly in the State which will copy the events from the ClipTransition the first time you access the state.Events property.

I think I can safely proceed as I am now, I don't really like setting events in the inspector so hopefully I won't be in any danger.

Also, if you tick the Start Time toggle in the transition, you wouldn't need to manually feed in its fade duration and it would automatically use FadeMode.FromStart.

Thanks for the tip, I actually tried to find a way to set FadeMode in the ClipTransition inspector but couldn't find any docs.

[KybernetikGames]

It could be made clearer that you're accessing the events of ClipTransition by hiding that access behind another property

OptionalWarning.DuplicateEvent and OptionalWarning.LockedEvents try to help with this issue, but an API design that made it clearer upfront would definitely be preferable over trying to react to the symptoms.

state.BaseClipTransition.Events

States don't currently have any connection back to the transition that created them and I'm reluctant to give them one because they can be created without transitions and I like the separation of concerns of keeping transitions as just a convenient way of grouping the details of how to play something. Also because having two ClipTransitions play the same clip will cause them to share the same state and it would very likely confuse people if a property implying "this state was created by X" either changed or was actually wrong half the time.

But what they do have is a bool indicating whether the event sequence was retrieved from the object pool (so it can be cleared and pooled afterwards instead of just dropping the reference if something else owns the sequence. And that would be enough to copy the way Unity handles Renderer.material and Renderer.sharedMaterial:

• Getting the state.SharedEvents could either work exactly as state.Events does now (return the assigned event sequence or get a pooled one if it's null) or maybe it should return null if nothing is assigned. I'm leaning towards the latter.
• Getting the state.Events would ensure that the event sequence isn't shared like the new method idea I mentioned earlier.
  • If a sequence is assigned and was retrieved from the pool, return it because it's owned by this state.
  • If a sequence is assigned and wasn't retrieved from the pool, replace it with a pooled one and copy over the contents.
  • If no sequence is assigned, get a pooled one.
  • That way, state.Events would always be owned by the state only. You could still have a script keep a reference to the sequence and mess things up by altering it after it gets re-pooled, but it's not really possible to prevent that without allocating new objects every time and not using object pooling which isn't a performance trade I'd want to make.
• Setting either property would work the same way it does now, simply flagging the event sequence as not pooled.

That should make things a bit more intuitive for users and I might have a go at implementing it sometime soon once the idea has had a bit more time to settle.

It would work for your case, but unfortunately it still wouldn't help with the generally recommended approach of configuring events only once on startup. The reason it's recommended is because it typically involves a bunch of memory allocations for any new callback delegates you assign and also searching through all events to do string comparisons if you're using named events. Without this new SharedEvents property it also means any event modifications on play accumulate in the transition which is disastrous, but the remaining reasons are still pretty significant. I just can't see any way of ensuring every character can assign their own callbacks to the transitions in your AnimationSet when they're sharing the same object and the character isn't even involved yet (because you're doing the setup before playing anything).

I don't really like setting events in the inspector so hopefully I won't be in any danger.

The main reason I like to use the Inspector is because event times should almost always be lined up visually with the animation preview rather than being mathematically derived or hard coded as magic numbers. I don't like putting actual logic in the Inspector though, so my recommended approach is to set up events with times and names (preferably with an Event Names Attribute) then just assign the callbacks in code.

I actually tried to find a way to set FadeMode in the ClipTransition inspector but couldn't find any docs.

It's explained in the Start Time tooltip and I'd like to display it directly in the GUI, but haven't thought of a good way to do so.

• The Fade Mode can't be an actual field because it's implicitly tied to the Start Time and I don't want to spend a whole extra line just to display a read-only value.
  • The x/s/f fields also negate the need for most of the modes. FromStart is used if there's a Start Time and the Normalized variants are unnecessary since you can just use the x field so the only meaningful options would be FixedDuration or FixedSpeed and the difference between them is so subtle that I'd rather most users not need to know about them. That's actually the only reason there's a Play method that takes a transition and lets you specify the FadeMode.
• I could merge the x/s/f fields into one when it's disabled to show "Continue from current time" or something, but that would prevent you from just clicking in one of those fields and typing a value if you want to set it. And just drawing that label over the top of them would look ugly.
• Changing the label to "Continue Time" (when it's disabled) might work, or might be even more misleading because it no longer makes any sense as a field name.

Sorry for the long post, I'm mostly just writing down my ideas in case that helps me spot something I've missed.

[ADH-LukeBollam]

I guess the problem I run into with setting events in the inspector is that it now binds the animation logic to the rest of my logic, where as passing in a callback allows me to do whatever the caller needs when the animation completes.

As a side note, I tested the performance of subscribing 10000 times to the EndEvent and the result was somewhere between 70 and 100 ms, and a single callback was around 0.0008 ms

// multiple
            var sw = new Stopwatch();
            sw.Start();
            for (var i = 0; i< 10000; i++)
            {
                state.Events.OnEnd += () => FinishOverride(state, animConfig.PlaybackLayer, onEndEvent);
            }
            sw.Stop();
            UnityEngine.Debug.Log($"time is {sw.ElapsedMilliseconds}");

// single
            var sw = new Stopwatch();
            sw.Start();
            state.Events.OnEnd = () => FinishOverride(state, animConfig.PlaybackLayer, onEndEvent);
            sw.Stop();
            UnityEngine.Debug.Log($"time is {sw.ElapsedTicks / 10000.0}");

[KybernetikGames]

That test won't capture the main performance costs.

• Every lambda assignment will create a new delegate and every += will create another one to combine them. That costs a bit of time upfront, but then will cost more when they all get garbage collected at some time in the future when they're no longer referenced by anything, which wouldn't happen until after your test.
• Assigning the End Event is also much faster than needing to search through all other events to find one with a particular name. The cost of that would vary based on the number of events, how early the target is found, and the length and similarity of their names.

[ADH-LukeBollam]

Hello, I'm bumping into an issue which I suppose is related to this but maybe not.

I'm trying to set the time that the EndEvent plays at using the inspector, I've set it to 0.5x here

I'm still setting the end event through code State.Events.OnEnd = () => FinishOverride(result.State, animConfig.PlaybackLayer, onEndAction);

It looks like the EndEvent uses the end time of the full length animation regardless of where I set End Time in the inspector, if I log the Transition.Length I get the correct 0.6. However the EndEvent plays at a NormalizedEndTime of 1, not the 0.5x like it shows in the inspector.

Any idea how I can get the End Event to fire at the End Time set in the inspector?

[KybernetikGames]

Animancer v8.0 is now available for Alpha Testing.

Animancer Events have been completely overhauled, including the state.SharedEvents change I explained above and removal of the UnShared system.

Also, it seems like I missed your last comment so please let me know if you're still experiencing that issue.

[KybernetikGames]

Animancer v8.0 is now fully released.