feat(player)!: animation -> current_animation, and fix two generated track paths - #288
Merged
Merged
Conversation
…track paths Applies the stem the API conventions give the animation that is set up (SDK `20_design/40_api_conventions.md`). Godot names this same thing `AnimationPlayer.current_animation`; `AnimatedSprite2D.animation` is the selection on a sprite with no transport of its own. The exported property becomes `current_animation`, and its accessors stay deliberately asymmetric: `set_animation` is the verb the whole family publishes and `get_current_animation` is its read, and ADD_PROPERTY names the two independently. Inventing `set_current_animation` would give one operation a second name. Also fixes the generated AnimationPlayer tracks, which are PROPERTY paths: `.:animation` moves with this rename, and `.:frame` was left stale by the `frame` -> `frame_no` rename in #287. A value track pointing at a property that no longer exists is not an error -- it silently drives nothing -- so every `_anims.res` generated since that PR has had a dead playhead track, and neither the importer nor the editor would have said so.
This file contains hidden or 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
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.
Applies the stem the API conventions give the animation that is set up
(SpriteStudio-SDK#384).
Why
Godot names this same thing
AnimationPlayer.current_animation— a node thatowns a transport, like this one.
AnimatedSprite2D.animationis the selection on asprite that does not.
The other reason is the near-collision the conventions cite:
animation_namesitsone character from
animation_names, and those two are the pair a host usestogether.
The asymmetric accessors
ADD_PROPERTY(... "current_animation", "set_animation", "get_current_animation").Deliberate, and commented at the line.
set_animationis the verb the whole familypublishes;
get_current_animationis its read.ADD_PROPERTYnames the twoindependently, so the property reads as a Godot user expects without inventing a
set_current_animationthat would be a second name for one operation.player.current_animation = "walk"works and routes toset_animation.The part worth reviewing: two dead track paths
ss_resource_inspector.cppgenerates the_anims.resvalue tracks, and those areproperty paths:
.:framewas left behind by theframe→frame_norename in #287 — my miss.A value track pointing at a property that no longer exists is not an error; it
silently drives nothing. So every
_anims.resgenerated since that PR has had adead playhead track, and neither the importer nor the editor would have reported
it. Both paths are correct now, with a comment saying why they have to track the
exported names.
Anyone who generated a
.resbetween #287 and this PR needs to re-import to get aworking frame track.
Verification
All three changed translation units compile clean with the flags from the
checked-in
compile_commands.json(macOS arm64, GDExtension).Class reference, API docs and the AnimationPlayer workflow page follow; the last
one documents both property paths by name.
Breaking, and taken now while nothing is published so no aliases are needed.