KybernetikGames / animancer

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

A more abstract way of using Animancer events #262

Closed FlorianBarnier closed 1 month ago

FlorianBarnier commented 1 year ago

Use Case

Working with animations that need lots of events can quickly become quite heavy. You have to search through the animation's events and compare their strings to subscribe to them, which runs into a lot of the same problems as the ones described in #257: it's not very performance-friendly and not very reliable/scalable.

Another issue with the current system is that two events cannot have the same name on an animation. What if you use events to trigger sounds, and have an animation that needs to play the same sound 3 times? Having MyCoolSound1, MyCoolSound2 and MyCoolSound3 isn't exactly the cleanest.

It becomes even worse when trying to work with nested Mixers, where you now have to go through n levels of nested Mixers to find all the correct events. For example, if I wanted to have a character with a set of movement animations for multiple speeds (Idle, Walk, Run, Sprint) and multiple ground slopes (up, down, left, right), I would need a Linear Mixer (speed), containing multiple 2D Mixers (slope), each containing a set of ClipTransitionAssets. You can see how that quickly becomes quite a mess!

Alternatives

For now, the alternative is to go through all the animations or sub-mixers contained on the mixer you just started, go through all of their respective events and compare their names to get the ones you need.

Solution

Similarly to what's described in the Solution part of #257, an Animancer event could take a ScriptableObject as its key instead of a string, which means:

KybernetikGames commented 1 year ago

Well there goes my whole morning :)

Looks like there's a few different feature suggestions here so I've split them out to separate issues:

That leaves the main idea as:

animancer.SetCallback(key, method)

Regardless of whether the keys are strings or Scriptable Objects, the general idea would be to give the AnimancerComponent (or really its root AnimancerPlayable) a Dictionary<key, Action> so that:

For example:

For debugging purposes, I'd probably want to give a log warning if an event can't find a callback but if you register a callback that happens to not be used I'd only show an indication in the Inspector wherever I end up listing all registered callbacks.

The main aspect I'm not certain about is the best way to determine when to connect an event to its registered callback:

Automatic Binding

Every named event automatically looks in the dictionary for a callback.

Toggled Binding

Add an "Auto Bind" toggle to each event in the Inspector.

If we switch to Scriptable Object keys instead of strings, the Scriptable Object could potentially contain the bool indicating whether its events should auto-bind, but that's probably still too fiddly for a potential tiny performance gain.

Manual Binding

You need to call a bind method on every transition which will go through all events (recursively) to find anything with a name but no callback and set it to LookInTheDictionaryToGetTheRealCallbackAndInvokeIt.

Having written all that out, I'm leaning towards automatic bindings.

Nolkeg commented 1 year ago

It becomes even worse when trying to work with nested Mixers, where you now have to go through n levels of nested Mixers to find all the correct events. For example, if I wanted to have a character with a set of movement animations for multiple speeds (Idle, Walk, Run, Sprint) and multiple ground slopes (up, down, left, right), I would need a Linear Mixer (speed), containing multiple 2D Mixers (slope), each containing a set of ClipTransitionAssets. You can see how that quickly becomes quite a mess!

For events that are reused a lot and can happen multiple times in the animation like FootStepSound and Weapon vfx sound, I personally embedded the event using Unity's AnimationEvent and it works quite well for me. I wonder what's your use case is here ?

KybernetikGames commented 1 year ago

That's a very good point. Implementing this sort of system would take time away from other features I could be working on and most use cases can probably already be covered by Animation Events (even if they're a bit annoying to use).

KybernetikGames commented 1 year ago

I'm currently looking at possible features to work on for the next major version of Animancer and this idea has some merit, but I've realised a limitation that might actually make it worse than Unity's Animation Events: the lack of parameters.

In the Footstep Events Example:

But if you want to call animancer.SetCallback(name, method) then the method can't have parameters and there's nowhere to store them with the events anyway. You would need to have a different name for each different parameter value which would be very inconvenient, especially with other parameter types. You'd be writing the real Method(value) as well as Method0() and Method1() to call it with the right parameters, then you'd also need to do both animancer.SetCallback("Footstep0", Method0) and animancer.SetCallback("Footstep1", Method1).

Unfortunately, I haven't been able to come up with any viable solutions to this issue and it likely won't be worth the effort of implementing if the result is going to be missing such a crucial part.

KybernetikGames commented 8 months ago

I've now implemented this feature in the upcoming Animancer v8.0.

Unfortunately, this is a breaking change. Any event callbacks set up in an older version of Animancer will be lost. Event names will also be lost due to changing them from raw strings to ScriptableObjects.

Here's a summary of how it works:

As an example, the Footstep Events example looks like this in the current system:

image

private void OnEnable()
{
    _Animancer.Play(_Walk);
}

public void PlaySound(AudioSource source) ...

In the new system it could still be set up like that, or it could look like this:

image

private void OnEnable()
{
    _Animancer.Events.AddNew<AudioSource>("Footstep", PlaySound);

    _Animancer.Play(_Walk);
}

public void PlaySound(AudioSource source) ...

Obviously there will be advantages and disadvantages to each approach, but I can see a lot of potential in the new system and it will be necessary if I manage to implement Transition Sets because the caller wouldn't be able to safely initialise transition events in the same way when those transitions aren't owned by the caller.

The system isn't fully implemented yet, but the general proof of concept is done so I can confirm it will be in the upcoming Animancer v8.0.

jarrodspurrier commented 8 months ago

Really looking forward to these changes, thanks for this! Will the new AnimancerEventParameters allow us to avoid the closure allocations when binding to event callbacks? I find that we have to do something like the following a lot in our code base and because the callback doesn't pass back any context about the event, we always have to pay the closure allocation cost.

events.SetCallback(
    i,
    () => PlayFootstep(eventName)
);

Another feature I would love to have is an Action<NamedKey> AnyEvent callback. I have a few systems that bind to every event in the animation requiring me to loop through each event in the sequence. Would be nice to be able to bind to a single callback that gets triggered when any event is triggered that passes along the NamedKey. It would be much easier to work with, and in most cases prevent the closure allocations since you would be providing the relevant context as to which event was triggered.

KybernetikGames commented 8 months ago

AnimancerEvent.CurrentEvent and CurrentState are static properties which give you access to the context of an event. The current system doesn't include the event name in that (because it wasn't used for anything at invocation time) so you would need to use AnimancerEvent.CurrentState.Events.IndexOf(AnimancerEvent.CurrentEvent) then get the name with that index, but the new system does include the event name in the context so you'll be able to access it directly, without any involvement from parameters.

If you wanted a different value other than the name then you could use a parameter like the AudioSource in the example I gave in my previous post.

I don't think an AnyEvent callback would be useful in enough cases to be worth the (admittedly small) performance cost for everyone. But it should be pretty easy for you to add yourself, just add the event in AnimancerPlayable and trigger it in AnimancerEvent.Invoke via CurrentState.Root.AnyEvent. I wonder if there's a way I could make modifications like that easier to do without needing you to modify Animancer's scripts directly.

KybernetikGames commented 3 months ago

Animancer v8.0 is now available for Alpha Testing and includes this new system..

KybernetikGames commented 1 month ago

Animancer v8.0 is now fully released.