KybernetikGames / animancer

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

How to implement transition sets? #80

Closed KybernetikGames closed 2 months ago

KybernetikGames commented 3 years ago

Use Case

Transitions are a very simple way to manage animation references alongside various details about how to play them (fade duration, speed, start time, and Animancer Events), but there is currently no standardised system for choosing different transition details depending on the previous animation you are transitioning from.

Solution

Implement a TransitionSet : ScriptableObject could store a list of from/to animation pairs with their own transition details so that at runtime you can do something like transitions.Play(animancer, animation) to have it determine which transition details to use based on the current state.

Problems

There are a few problems that need to be resolved before such a system could be implemented.

What information does the set actually store about each transition?

So how does the aforementioned transitions.Play(animancer, animation) actually work?

nehvaleem commented 3 years ago

This is something I would really like to have. What are the plans related to TransitionSets since the Feedback Wanted label was added quite long ago?

KybernetikGames commented 3 years ago

No plans currently, because I haven't come up with solutions to those problems so I don't know how the system would actually work. I'm open to suggestions, but for the moment it's just stalled.

KybernetikGames commented 9 months ago

Now that I've implemented Scriptable Objects as Event Names, Animancer Event Binding and Mixer Parameter Binding for the upcoming Animancer v8.0, I've also come up with solutions to the issues I mentioned above and started designing this feature more in depth to see if I can come up with a viable implementation. Here's what I've got so far:

Solutions to the above issues

System Overview

The goal of this system is to provide artists with a way to easily manage the appearance of specific transition combinations and possibly some other useful features without going too far towards Animator Controllers and running into all the same problems they have with restrictiveness and messy code.

I likely won't try to implement all these ideas for Animancer v8.0, but I'd like to at least have an understanding of what I might want to develop in the future.

Moar Feedback Pls

Whatever this system ends up looking like, it will undoubtedly be a big step towards the functionality of Animator Controllers, i.e. towards all the problems I made Animancer to avoid in the first place. For that reason, feedback on this topic is especially important so that I can come up with a system which meets your needs without necessarily forcing everyone into the way I think things should work. The more detail you can give me on what you would want to use this system for and how it might work, the better.

nscimerical commented 9 months ago

This is just my personal opinion but I think there is no need to do a lot of these (Overrides and Groups for example) because they should be done by the coder themselves. I think these should really be handled by FSM logic made by the coder. Having these handled by the Library would probably result in unintended animations and also harder to debug.

I do like the idea of Combining Libraries, however. I think it would be really helpful for different weapons as you've mentioned.

As for the networking aspect, didn't you mention something about Inertialization previously? I think the biggest benefit of it for networking would be that it flattens multiple state in a layer into one variable, making it significantly cheaper to sync over the network. The issue with the current Animator is that netcode libraries try to sync every parameter from it. Each float/int/bool value gets sent per tick so it quickly becomes really expensive (bandwidth-wise). This is on top of data that is needed for an animation transition like fade duration, start time, speed, etc. assuming they are all floats.

Though I do understand how hard this is to implement so it might not be practical.

KybernetikGames commented 9 months ago

Thanks for the feedback.

I have to disagree about Overrides since I see it as a core part of the whole system. If all you want is a list of Transitions then anyone could easily do that on their own but fine tuning the start time and fade duration of specific transition combinations isn't particularly easy to do cleanly in code, especially in an artist friendly way since those are art details, not logic.

The reason I'm considering Groups is a question of whether or not your scripts actually need to know the difference between Attack 1 or Attack 2. If you have a custom transition type containing all the attack data like damage and hit boxes then there might not be any logical difference between the different animations that your scripts should be aware of. Without any logic, it would just be data for designers and artists to configure.

The Inertialization comment you're thinking of is likely the one at the end of the Animancer v8.0 post. What you've said about the way Inertialization can simplify/optimize the necessary data is correct, but that's separate from the way Libraries would help with networking. The main issue currently is that even if you ignore fades and layers and stuff, there's nothing inherently network serializable about an AnimancerState which would allow it to be recreated on the other side. But with a Library that serializes everything in a list you would be able to look up the index of your current state and send that so the other side can look up the same Transition in their Library. So with that in place, Inertialization would make it even easier to sync the whole graph of a character.

daenius commented 9 months ago

Hot take:

The way you described all these features scream "these aren't just Transitions anymore". First of all I love all these ideas and most users have probably created various conceptually similar features for themselves, myself included, or modified what you did in the Animancer samples or Platform Game Kit (which i guess is sort of Animancer samples deluxe). You made a great point about the Overrides and the AttackTransition because I have ScriptableObjects that store groups of custom ITransition implementations that are basically just AttackTransition3D. Over time I started seeing AnimancerTransition<TState> as serialized templates or builders that reliably create AnimancerStates to be played.

That said, do you think it makes sense to call the library assets StateLibrary or AnimancerStateMachineAsset or any other name that denotes the concept of "each AnimancerTransition<TState> in here works like ONE state node (potentially with children) in an FSM"? It is ultimately saying "feedAnimationClip`s in here, and then you can define exactly how you want each of them to be played AND whether or how they might switch or blend with each other. You can chop it up, shorten it, play it at whatever speed, start it in the middle, do all that but in a Mixer, etc etc".

Regarding Overrides, because AnimancerTransition<TState>s are ultimately already super flexible "overrides" themselves for how your output animation would LOOK for that one node, the name "Override" is not very descriptive of what exactly are we overriding especially with artists being the potential intended audience and Unity already uses the term for something completely different. I fully understand and agree with how fine tuning each transition is part of the artistic work and your concept of Groups seem to take care of that need. This would leave "Overrides" to be denoting whether and how Groups and/or individual transitions could/would sequence or blend with each other, that sounds like an FSM, therefore making the Library asset some sort of state machine asset. Now "Overrides" sounds like it would be a set of conditions/predicates for your Groups and individual AnimancerTransition<TState>s, and perhaps calling it Selector like you did in Platform Game Kit might be more intuitive. I'm starting to see each Group as a container for all Transitions used in each of the CharacterStates with the overall Library as a container for all states in one FSM, and then you can swap FSMs on the go in code. We would essentially have an FSM asset that stores settings and references for animations along with selector predicates while still having the FSM logic be decoupled from the general use case of just playing animations because you are never forced to make one of these when you just want to play animations directly unlike Mecanim. Please do correct me if I understood this all wrong.

I m not understanding "Exit Time Transition" though because I always saw Animator Controller's "exit time" to be a worse version of setting an End Time in Animancer. If you wanted to create some sort of a general FSM or Behaviour Tree with a fixed duration that happens to also play animations, that's better left to be done by a separate system. For example, if I defined a state in say Visual Scripting that lasts 9 seconds but it invokes a Group from the Library that only lasts 6 seconds, what are you supposed to do about the remaining 3 seconds? Unity's Timeline addresses this by having you define pre and post extrapolation so that's one possible option, but notice how that behaviour is done in a different system because it's a different level of abstraction.

Big fan of Combining Libraries and funnily enough, Animancer even "fixes" that problem for Animator Controller already with ControllerTransitions. I always found it absolutely insane that Unity would introduce PlayableGRAPH and advertise how the graph allows you to nest Playables and show it with nested BlendTrees only to then undermine themselves by NOT respecting that rule when it comes to sub-statemachines inside: yes I'm talking about how you can't nest Animator Controllers. If an Animator Controller is in itself an FSM asset that can have nested FSMs, why doesn't it let you simply nest another Animator Controller?! Don't even get me started on AnimatorOverrideController... The normal controller can do Layers and it can even "synchronize" Layer states so they all have the same FSM and state names just with perhaps different AnimationClips or AvatarMask or whatever... hey wait a minute, that's EXACTLY what the other thing does, except the other thing does it worse because it ONLY lets you swap clips. You made a great point about Elden Ring because yeah, in that case Mecanim gives you the confusing dilemma of "which one am i supposed to use"? I get that people also use it to just swap clips for different Avatars to avoid duplicating the original controller and avoid rebinding graph, but that problem only exists because the FSM states and logic are tightly coupled with your animations instead of just being an FSM that works on its own just accepts matching Groups of animations for different instances. The kicker is they have also taken away the ability to simply play one of the clips inside the FSM, nope, better use a magic string or a hash you nerd! Damn I get so worked up about that because the whole thing reeks of haphazardly patching bad choices with more bad choices instead of fixing the original problem. Thankful to be here instead lol.

KybernetikGames commented 9 months ago

Over time I started seeing AnimancerTransition<TState> as serialized templates or builders that reliably create AnimancerStates to be played.

That's exactly what they're intended to be. They're called "Transitions" because you Play them when you want to transition into a state, but they also serve the purpose of creating those states in the first place since they aren't defined anywhere else like they would be in an Animator Controller.

They could be called something like "State Builders" to emphasize that part of their functionality instead, but that doesn't reflect how they're actually used in code. You don't (usually) tell a transition to manually create its state, you just play it when you want to and your script doesn't really care if the state already existed or not. Reminds me of this scene in Wallace and Grommit.

do you think it makes sense to call the library assets StateLibrary or AnimancerStateMachineAsset

StateLibrary would only make sense if Transitions were renamed to States which would be super confusing when they aren't the same thing.

And State Machine would only make sense if it defined the logic for a state machine to be able to manage its own behaviour, which isn't my goal for the system.

I picked Transition Library as the name because of the way you'd use it to look up transitions. You're at A and you say you want to go to B so the Library gives you an appropriate Transition to accomplish that based on the data it has been given. Transition Selector was another one I considered, but that doesn't sound as catchy to me and kind of implies that it's going to be selecting what to do on its own.

because AnimancerTransition<TState>s are ultimately already super flexible "overrides" themselves for how your output animation would LOOK for that one node, the name "Override" is not very descriptive of what exactly are we overriding

I think you misunderstood what I meant by Overrides.

Imagine a Library with Transitions for Idle, Walk, Attack, etc. The Idle Transition has a Start Time and Fade Duration which will be used by default whenever it's played. But the Library could also have a Transition Override targeting the Attack -> Idle pair so that if you're in the Attack animation and you play Idle it will instead use the Start Time and Fade Duration specified in that override. The Idle Transition is still only defined once, it just gets modified slightly when used in that particular situation. That's the main purpose of this system, to allow artists to preview and tweak the appearance of specific transition combinations like they could in an Animator Controller without needing any contact with the code. It's all purely about the visual details of how it blends.

I'm not understanding "Exit Time Transition" though because I always saw Animator Controller's "exit time" to be a worse version of setting an End Time in Animancer.

It would use the same End Events we currently have, it would just be automatically hooking up the callback to play something else for convenience so you don't need to do it in your scripts. That convenience seems nice at first glance, but the more I think about it the more I feel that it would just be giving control of important logic to the wrong system (and therefore the wrong users) when your scripts will almost always need to know what's going on so it's usually better to just give them control instead of making them check for changes made by other systems like you have to do with Animator Controllers.

daenius commented 9 months ago

Thanks for the clarifications! Very much appreciated and I got a good laugh out of that Wallace and Grommit scene for how well it illustrates the way you use Playable state nodes in the graph vs stock.

StateLibrary would only make sense if Transitions were renamed to States which would be super confusing when they aren't the same thing.

After your explanation I agree with TransitionLibrary being a more intuitive name.

I think you misunderstood what I meant by Overrides. Library could also have a Transition Override targeting the Attack -> Idle pair so that if you're in the Attack animation and you play Idle it will instead use the Start Time and Fade Duration specified in that override. The Idle Transition is still only defined once, it just gets modified slightly when used in that particular situation.

I totally misunderstood Override! I get it now, thanks for the great example and I'm happy with that name now too. I think your post here would serve as great documentation for when you release the new version.

It would use the same End Events we currently have, it would just be automatically hooking up the callback to play something else for convenience so you don't need to do it in your scripts. That convenience seems nice at first glance, but the more I think about it the more I feel that it would just be giving control of important logic to the wrong system (and therefore the wrong users) when your scripts will almost always need to know what's going on so it's usually better to just give them control instead of making them check for changes made by other systems like you have to do with Animator Controllers.

Agreed, I felt it would become wrong level of abstraction as well. I would say Exit Time Transition falls squarely under the YAGNI bucket unless we see an example use case that can only be uniquely solved by having that feature.

Exciting stuff, I look forward to the release!

KybernetikGames commented 8 months ago

I've run into a major roadblock in my implementation of this system which would significantly reduce its usefulness if I can't find a good way to resolve it:

How are End Events supposed to work?

Libraries will allow transitions to be shared among multiple characters and the new Event Binding System lets you avoid the current issues with events in shared transitions but it doesn't really address End Events.

I've come up with a few potential solutions, but none of them are particularly good.

1. Make another dictionary for End Events

The binding system has a dictionary of names mapped to callbacks, so just add another one for all the different End Events which you register with something like animancer.Events.SetOnEnd(_AttackKey, OnAnimationEnd);

What should be the key of that dictionary though?

2. Set the event after calling Play

var state = animancer.Play(_AttackKey);
state.Events.OnEnd = OnAttackEnd;

Nice and straightforward, but that allocates garbage every time which is highly undesirable.

To avoid that garbage you need to cache the callback yourself:

private readonly System.Action AttackEndEvent;

public ClassName()// Constructor.
{
    AttackEndEvent = OnAttackEnd;
}

...

var state = animancer.Play(_AttackKey);
state.Events.OnEnd = AttackEndEvent ;

But that's super verbose and annoying to maintain.

Either of those options will also be getting a new AnimancerEvent.Sequence from the ObjectPool and copying all the other events into it before you can add that custom End Event (otherwise you'd be modifying the source transition's events). Repeated every time you play something.

3. Something like the UnShared classes, but only for End Events

// Instead of this:
[SerializeField] private AnimancerTransition _Animation;

// Something like:
[SerializeField] private TransitionWithEndEvent _Animation;

private void Awake()
{
    _Animation.OnEnd = OnAnimationEnd;
}

...

_Animancer.Play(_Animation);// Plays the animation and assigns the event.

That avoids the garbage problem of #2, but still has the cost of copying the events into a new pooled sequence which the End Event can be safely added to.

But it also runs into the same issues that UnShared classes currently have. If you have a Weapon that's shared between characters, your AttackState won't be directly referencing the attack transitions so there's nowhere for it to actually use the TransitionWithEndEvent and you aren't any better off than if you just add an Action field to cache it manually.

4. Implement a new IEndEventHandler system

public class MyComponent : MonoBehaviour, IEndEventHandler 
{
    public void OnEndEvent()
    {
    }

...

var state = animancer.Play(_AttackKey)
state.AddEndEventHandler(this);

That method would get an object pooled IUpdatable which watches the state like the current event dispatcher in order to invoke the handler's OnEndEvent.

This would mean each script can only have one End Event callback so if it plays multiple different animations it would need to manually differentiate between them.

It would also be another completely new way of doing the same thing, which adds a lot of complexity to Animancer in terms of development cost and difficulty for users to understand.

Another notable downside is that if End Events aren't checked by the same system as other events, having both on a state would mean its NormalizedTime needs to be calculated twice per frame, which is particularly bad for mixers because they include the weighted times of all their children.

5. Move End Events out of the AnimancerEvent.Sequence

Some of the above issues could be avoided if the End Event (or IEndEventHandler) was stored in the AnimancerState instead of the AnimancerEvent.Sequence which might be shared, but all the options still have problems:

6. ???

If anyone has other ideas for how this could work please let me know.

The simplicity of being able to declare "Play this then do that" clearly in a script has always been one of Animancer's advantages, but the issues with the current implementation are already annoying and will only get worse with the introduction of Transition Libraries which are supposed to give artists more flexibility without turning your code into a mess.

KybernetikGames commented 7 months ago

I've come up with another idea that could alleviate the issue:

Remove the Event Auto Clear system and don't let Transitions share states

Remove the system that clears events automatically whenever you play an animation. Instead, states will keep any events you give them.

var state = animancer.Play(something);
state.Events.OnEnd ??= OnAnimationEnd;// Only assign the event if it was null.

The main reason for clearing automatically was so that each script could act as if it was the only thing using a particular animation, i.e. you could have an AttackComboState and WeaponSkillState which both happen to have the same LongSwordAttack1 animation assigned so clearing the events would ensure they don't conflict with each other. Except situations like that shouldn't be particularly common so instead of over-complicating the event system we could instead avoid sharing states between scripts:

I haven't thought of any reason why this approach wouldn't work yet, but I'll mull it over for a few more days before I go ahead and start implementing it and as always feedback is most welcome.

KybernetikGames commented 5 months ago

Time for a progress update.

I've implemented the idea from my previous post which is explained more in the Animancer v8.0 Change Log.

I've also been making good progress on the implementation of Transition Libraries. The runtime system is generally working and I'm currently working on the editor GUI side of things.

I started by implementing an editor window to show the transitions and overrides in the library, with the line thickness being proportional to the fade duration of a particular override:

image

Then I scrapped that whole system when I realised it would also need to show non-overridden transitions so you can preview and edit them, which would mean that you always end up with a uselessly dense spider web like an Animator Controller with every state connected to every other.

I came up with a few bad ideas trying to avoid ending up with a mess as soon as you get more than a few animations, one of which was to use a pathfinding algorithm to lay out the transition lines between nodes in a way that avoids going behind other nodes and minimises the number of lines that need to cross. That would have likely looked really cool, but wouldn't have actually solved the problem. Automatically arranging all the nodes in a circle was another discarded idea.

Eventually I realised I was being dumb and the proper way to represent data where every item has a relationship with every other item is to just use a nice old fashioned table:

image

That seems like a much better direction to go in. Up next, I'll be working on the Inspector so that you can click in any cell and get a preview of that transition combination. My estimated Release Timeline still seems reasonable, so I hope to have it ready for Closed Alpha testing in the next month or two.

KybernetikGames commented 4 months ago

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

KybernetikGames commented 2 months ago

Animancer v8.0 is now fully released.