godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 86 forks source link

Add a notion of Inheritance to Animations through Linked Tracks #3858

Open FreegleBarr opened 2 years ago

FreegleBarr commented 2 years ago

Describe the project you are working on

A 2D game with many partially identical animations.

Describe the problem or limitation you are having in your project

Currently, Animations, unlike other resources, have no concept of shared Sub-Resources. Their properties, the Tracks, seem always entirely built-in, and one cannot inherit a track without inheriting all of them.

In my game, and I assume many others, there are animations that are mostly the same for all entities, but with small variations such as hitbox position or whether to call an additional function. The first can't be done unless I make the animation unique, which "unlinks" its tracks from the parent animation, which means that any updates to "base" tracks must be manual. The latter can be done, but relies on that specific nodepath always existing with that specific function, or would need to add some kind of check in the root node for the nodepath and method before calling, which I think goes against the idea of encapsulation.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I think a nice way to circumvent that would be to add an icon besides any inherited tracks indicating that it's linked to its parent and any modifications from either direction will propagate to any linked tracks. Alternatively, the user may click to unlink the track and modify it without affecting the parent.

Any newly created tracks will be unlinked, to avoid creation of tracks of child-only properties. Then, users will be able to create base animations on base Scenes, and then refine those animations on inheriting scenes without having losing the ability to do more general updates to the base scene.

This is still limited to tracks, and particular moments/sections of the tracks would still not be inheritable. I am also not sure how this would work with 3D and external animation tools, as well as the new animation features recently introduced. This is also complementary to #57137 as in with this it would be equivalent to making all tracks unlinked or deleting all new tracks and reverting the old ones to parent.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

First and foremost I want to say that this solution probably breaks compatibility from my knowledge of the code. There could be better ways to implement this, which would be very welcome. This is just what I could come up with.

Firstly, the AnimationTrack would be converted into it's own resource, and would be a property of the Animation, similar to how the Animation is itself a property of the AnimationPlayer. Then an icon would be added to the track editor, showing the current state of each track. If the icon is clicked, it changes the track linked state and reverts it to default if linked.

This is a bit too ambitious for my liking in terms of the size of changes to internal behaviour, but would really help me a lot and I would gladly accept any feedback.

If this enhancement will not be used often, can it be worked around with a few lines of script?

This is mostly a convenience feature as it just reduces the work required to propagate changes in similar animations. This can be already be accomplished manually by either copying and pasting the base animation and repeating the child changes, or by not having a base at all and applying changes each animation individually. Both are cumbersome and error prone.

Is there a reason why this should be core and not an add-on in the asset library?

I'm not sure if this could be an add-on, but would be willing to implement some simpler solution as either core or add-on.

Calinou commented 2 years ago

I am also not sure how this would work with 3D and external animation tools, as well as the new animation features recently introduced

I'm concerned about performance when using this for 3D tracks, especially when animations are compressed. 3D tracks can have hundreds of tracks for position/rotation/scale of complex characters (each with ~30 keyframes per second). Supporting inheritance could make it slower.

FreegleBarr commented 2 years ago

That is a good point. Would it be worse at load time or run-time, and in-game or in-editor?

If in-game, a (really bad) work-around could be to save the final, fully inherited tracks in the save file, and adding a "linked" flag that would be used to determine if the editor should load from further up in the inheritance tree but be ignored if in-game. That would mean that there would be some need to load inherited animations if some change is made to the base. Editing of linked tracks could also be limited to the scene they're defined in to avoid having to make sure changes only have to be propagated down.

Now, in-editor, I can't really think of a solution.

FreegleBarr commented 2 years ago

(this was originally an edit but I thought it merited being its own comment)

After thinking some more on it, I think my problem can be solved with having two AnimationPlayers, one with the base shared animations and one with unique animations with the specific changes. It's still a bit clunky, having to keep switching between players, but seems doable with two AnimationTrees with the same StateMachine and playback, and I think would be efficient for 3D has it just needs to load and run two half-sized animations. Just make sure they don't conflict with each other or have different snap/length.

Maybe this could be a basis for a "linked" system, so instead of having full multi-level inheritance, in practice it's just two or more animations in parallel, specifically unable to contradict each other and with the same snap and length and edited in a single instance of the animation player editor, but I'm not sure how much better this is. I think that could be a plugin too, so no reason to add to core unless later on there is some demand.

I still think it would be worth it to have some kind of division between base tracks and , but if it's impossible not to harm performance for what are now minor gains in usability, I think this proposal can be closed, and I'll consider writing the second option as a plugin.