Set animation step from importers. Increase default step from 10 to 30FPS #90894
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lyuma
force-pushed
the
animation_step_30
branch
from
bd575fe
to
c655836
Compare
lyuma
force-pushed
the
animation_step_30
branch
from
c655836
to
bb9674c
Compare
Calinou
reviewed
Apr 19, 2024
| @@ -602,7 +602,7 @@ | |||
| <member name="loop_mode" type="int" setter="set_loop_mode" getter="get_loop_mode" enum="Animation.LoopMode" default="0"> | |||
| Determines the behavior of both ends of the animation timeline during animation playback. This is used for correct interpolation of animation cycles, and for hinting the player that it must restart the animation. | |||
| </member> | |||
| <member name="step" type="float" setter="set_step" getter="get_step" default="0.1"> | |||
| <member name="step" type="float" setter="set_step" getter="get_step" default="0.0333333"> | |||
| The animation step value. | |||
There was a problem hiding this comment.
While we're at it, we can improve the description:
Suggested change
| The animation step value. | |
| The animation step value. This property affects snapping in the editor, but does not affect animation playback otherwise. |
It might make sense to do this in the future if no other animation software out there is displaying the step in seconds. The value would then always be displayed in FPS in the LineEdit, but the setting would control what's shown on the timeline instead. |
TokageItLab
approved these changes
Apr 22, 2024
|
Thanks! |
|
this makes sense as a change, however i still animate at 0.05 seconds snap. I just prefer it really, could it be possible to have an editor setting to change the default animation snap? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most framerates are a multiple of 30, and most animations also use 30 or 60 as a basis.
However, imported animations did not have a step assigned, leading editing imported animations to be extremely clunky since snap is on by default and the default step interval is at 10 fps.
This PR makes the default step in the Animation resource 1/30 (30 fps) and also uses the correct bake_fps value as step during import. Finally, the new button in the animation track editor will reuse the current animation step if one is loaded.
In a pedantic sense, this is compatibility breaking because I am changing the default value of an existing resource property: almost all existing animations will likely be increased to 30 FPS. I don't see this as a problem for these reasons:
stepvalue is currently only used in editor tooling. The exception would be cases where this value is used by a game script and that game uses 10 FPS animations that have not changed from the default value.If there is pushback to changing the Animation resource or the default FPS for new animations, I can remove those changes.
Finally, there was a comment in chat that it will look weird to have the Snap field default to 0.03333 since it doesn't look like a round number. My rebuttal is this is a consequence of being in the real world. We can't change that 30 FPS is a popular framerate, and we can't change the infinitely repeating decimal nature of 1/30. There is a FPS dropdown in the animation track editor and I'd be open to changing the default dropdown to be FPS, but this affects how the timeline looks and my goal is to not change the UX significantly. If I could do FPS just for the textbox but keep the track editor in seconds then maybe...