gamedev-pro / dmotion

DMotion - A high level Animation Framework for Unity DOTS
Other
491 stars 47 forks source link

Move clips into visual editor too #11

Closed AVeryLostNomad closed 2 years ago

AVeryLostNomad commented 2 years ago

This PR moves AnimationClipAssets into the visual editor along with the preview window and related event inspectors. This unifies the experience and makes it easier to browse different assets (in my opinion).

Here's an example of the changes: image

gabrieldechichi commented 2 years ago

Hey @AVeryLostNomad, thanks for this PR. I was very impressive that you were able to jump into the code and add this, and I totally agree clips need a better integration right now.

I do have a plan for AnimationClipAssets though, which was the reason I believe it's better to keep them as separate assets. I'll share those here and I'd like to hear your thoughts on them, and if you think this could work for you:

1. One shot clips

One feature that Mechanim doesn't have, but DMotion does (and that I got from Unreal), is "one shot clips". Basically, they are clips you can just say "play", and the state machine will blend to it, and resume playing afterwards. Those are incredibly useful for playing things like attacks, spells, gestures, etc.

There are several ways to achieve this behaviour in Mechanim ("any states transitions", "triggers", "Animator.Play"), but to me none of them are scalable. Just imagine a large RPG with hundreds of abilities, each of them possibly with their own animation. Creating one state for each case is unmanageable.

Though we could still have state machine clips as subassets, and "one shot clips" as separate assets, I feel this makes for a non-consistent workflow, which I'd like to avoid because it gets users confused.

2. State Machine overrides

A coming soon feature is a version of Mechanim's AnimatorOverrides. Which is an State Machine asset that references another state machine, and allows you to override clips in specific states (a great way to reuse State Machine logic). I believe this feature is also more consistent with AnimationClipAssets as separate assets, because otherwise we get the same scenario as One Shot clips (some clips are subassets, and others are not)

In summary, I believe AnimationClipAssets are reusable whereas States and Parameters aren't. States and Parameters only make sense inside a given state machine, but there are use cases for Animation clips (and their events) being reused outside the state machine (points 1 and 2). Which is why I'd like to keep them separate.

That said, I totally agree the current editor experience needs to be better for clips, let me share what I have in mind in the next comment, and if you feel like implementing these, I'd be happy to take another look at the PR.

AVeryLostNomad commented 2 years ago

I'll definitely give a go at bringing it more in line with your ideas here, thanks for the feedback!

AVeryLostNomad commented 2 years ago

So, basically you outlined two paths for the inspector for improved UI:

1) Add additional fields to edit the internals of the AnimationClipAssets inside of the others. This is theoretically fine, and we could probably come up with some good solution here, but it starts to get messy really really fast with regards to displaying things cleanly inside of a CustomPropertyDrawer. You can't use LayoutGroups to make it easier to format either (since it happens in OnGUI).

2) Was the alternative you mentioned in the github feedback, which would be using the actual inspector for that (kind of like how the animator does). I think this is an interesting path:

image The biggest issues with this route:

I think I could be convinced that either route is the best way, but both are pretty out of scope for the PR, so I think I'll go ahead and close it. I'll keep an eye out for more things that need doing though!