Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add animation reset track feature #43115

Merged
merged 2 commits into from
Dec 20, 2020

Conversation

RandomShaper
Copy link
Member

@RandomShaper RandomShaper commented Oct 27, 2020

This implements godotengine/godot-proposals#1597.

NOTE: This is barely tested, so testing is welcome.

A few notes follow, about aspects not included or not settled on the original proposal:


Any animation can be the reset animation.

Toggle Reset is used to make the current animation the reset one, or revert that:

When clicked, you get one of these:

The option to toggle reset is there and also asks for confirmation, to avoid accidental clicks that could happen if it was just a button in the animation dock. Also, there's no frequent need to deal with it.


The reset animation is marked as such in the dropdown:

image

If it's the autoplay animation at the same time, it looks like this:

image


If you have Create Reset Track(s) checked, and there's no reset animation, one will be created with the name RESET.


You can apply the reset animation any time, unless the it's the current one:

image


There's a new reset_on_save property:

image

The cool thing is that it helps in getting a cleaner diff and consistent saved scene, without breaking the workflow, since the current state in the editor is kept. If you need to see how its saved version looks, you can use Scene-> Reload Saved Scene.

@reduz
Copy link
Member

reduz commented Oct 27, 2020

My first feedback is that you should never create the reset track manually, or at much I would add an option in the New dialog like "Create RESET track" and make sure the name is always this (can't be something else and can't be renamed). This is unnecessary flexibility and I think users will appreciate more if this track has a standard name, this way they can quickly see it and identify it in tutorials, examples and documentation. The track will always feel like a special track and this is a good thing. If any track can be the reset track, it will just be more confusing.

image

Of course this option will be grayed out if the track already exists.

That said, my belief is that the common way to create this track would be to never do it manually. Instead, it should be a single checkbox in the new track create dialog:

image

This checkbox exists by default here and is turned on by default. If you don't want to use it, you toggle it off in the project settings.

image

This new setting also takes effect if you disable confirmation in this same section (when no confirmation is active and setting is enabled, RESET is track is always created).

@DDL-Blue
Copy link

Is it even a good idea to have a reset ANIMATION? Don't we want just reset KEYFRAME? I know this might fit the UI well, but it make no sense to me to be an animation. Or do I miss something?

@fire
Copy link
Member

fire commented Oct 27, 2020

Shouldn't this be [in addition] an import setting? For like FBX, GLTF etc [to create the track.]

@reduz
Copy link
Member

reduz commented Oct 27, 2020

@DDL-Blue This has been discussed in the original proposal. Besides it being easier to implement this way (can use the same editor) and offer a familiar way to edit it, it allows to queue it together with other animations (play something, then queue something, then queue reset, then another, or play an animation and queue reset once its finished). Users have already been doing this workflow manually in their projects, so the main goal of this approach is to just formalize it and make it easier to use and discover.

@reduz
Copy link
Member

reduz commented Oct 27, 2020

@fire Yes sure, would be nice to implement it once this is merged.

@DDL-Blue
Copy link

@reduz So when you save, it will (if enabled) play the whole animation? In some fast-forward way? Trigger all the function calls? Hopefully not play the sounds. I got your points, but this still seems a bit hacky to me.

@RandomShaper
Copy link
Member Author

@reduz, even with a fixed name for the reset animation (so you can't set any animation as reset), you'll still be able to insert reset tracks manually by editing the reset animation, because in the end it's just an animation.

I'm asking to clarify if that's what you want.

As a bonus, to have consistency between use Beziers and create insert tracks, use Beziers also gets a default via editor settings that is used when the confirmation dialog is disabled, instead of just falling back to creating non-Bezier tracks.
@RandomShaper
Copy link
Member Author

RandomShaper commented Dec 20, 2020

Updated with the requested changes.

One of the most important changes is this:
image
Both defaults will inform the initial checked state of their corresponding check boxes in the insert tracks dialog (one time. at the start of the editor session) and also will be used when the confirmation dialog is disabled. This keeps both insert track settings consistent, while also making it possible now to insert Bezier tracks when the confirmation dialog is disabled.

The insert track dialog looks like this by default:
image

@RandomShaper
Copy link
Member Author

So when you save, it will (if enabled) play the whole animation? In some fast-forward way? Trigger all the function calls?

It will play every key at time 0 in the RESET animation (the description of the setting has been updated to clarify that).

@akien-mga akien-mga merged commit bc2ac0b into godotengine:master Dec 20, 2020
@akien-mga
Copy link
Member

Thanks!

@Sslaxx
Copy link

Sslaxx commented Dec 20, 2020

Fixes #812?

@RandomShaper
Copy link
Member Author

Fixes #812?

I think so. I see @reduz already put there what we could call the first draft of the proposal. So, yes, this is finally the incarnation of that (if I've understood that issue correctly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants