Skip to content

feat(player)!: animation -> current_animation, and fix two generated track paths - #288

Merged
Naruto merged 1 commit into
developfrom
conventions/current-animation
Aug 26, 2026
Merged

feat(player)!: animation -> current_animation, and fix two generated track paths#288
Naruto merged 1 commit into
developfrom
conventions/current-animation

Conversation

@Naruto

@Naruto Naruto commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Applies the stem the API conventions give the animation that is set up
(SpriteStudio-SDK#384).

animation (exported property)  ->  current_animation
get_animation                  ->  get_current_animation

Why

Godot names this same thing AnimationPlayer.current_animation — a node that
owns a transport, like this one. AnimatedSprite2D.animation is the selection on a
sprite that does not.

The other reason is the near-collision the conventions cite: animation_name sits
one character from animation_names, and those two are the pair a host uses
together.

The asymmetric accessors

ADD_PROPERTY(... "current_animation", "set_animation", "get_current_animation").

Deliberate, and commented at the line. set_animation is the verb the whole family
publishes; get_current_animation is its read. ADD_PROPERTY names the two
independently, so the property reads as a Godot user expects without inventing a
set_current_animation that would be a second name for one operation.

player.current_animation = "walk" works and routes to set_animation.

The part worth reviewing: two dead track paths

ss_resource_inspector.cpp generates the _anims.res value tracks, and those are
property paths:

anim->track_set_path(track_anim,  NodePath(".:animation"));   // moves with this rename
anim->track_set_path(track_frame, NodePath(".:frame"));       // ALREADY STALE

.:frame was left behind by the frameframe_no rename 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.res generated since that PR has had a
dead 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 .res between #287 and this PR needs to re-import to get a
working frame track.

Verification

All three changed translation units compile clean with the flags from the
checked-in compile_commands.json (macOS arm64, GDExtension).

PR Build still cannot verify this — the cross-org submodule blocker is open.

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.

…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.
@Naruto
Naruto merged commit fdd4cbf into develop Aug 26, 2026
0 of 2 checks passed
@Naruto
Naruto deleted the conventions/current-animation branch August 26, 2026 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant