From 6fa99fb9256b99cf69ed7c9915192f963d4e0a2a Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Sun, 13 Sep 2026 23:12:36 +0900 Subject: [PATCH 1/2] fix(player): an override reaches a frame that did not move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An override changes what the current frame draws without changing which frame it is, and every redraw path dedups on exactly that. So a host that set one and did not happen to step the animation saw nothing: `advance(0.0)`, a frame held at a section end, a paused player, a stopped one — and in `ANIMATION_PROCESS_MANUAL` the node never ticks the player at all. `_overrides_dirty` is set by every override mutator and cleared by `_drawAnimation`, which both the owner's seek and the parent's child walk go through. It defeats the dedup in `update()`, takes the stopped / finished early-return down its own redraw, and joins `_inherited_mask_dirty` in `_redraw_child_if_frame_changed`. MANUAL picks it up from the idle notification, which that mode already keeps: `advance()` is playback and an override is not. `_push_coverage_screen_scale()` moves out of the IDLE branch while it is there, which is what `set_animation_process_mode` already says happens. --- AGENTS.md | 2 +- docs/en/workflow/usage_scripting.md | 1 + docs/ja/workflow/usage_scripting.md | 1 + .../doc_classes/SpriteStudioPlayer2D.xml | 1 + ss_player/ss_internal_player.cpp | 45 ++++++++++++++----- ss_player/ss_internal_player.h | 16 +++++++ ss_player/ss_player_node_2d.cpp | 10 ++++- test_gdextension/suites/test_overrides.gd | 40 +++++++++++++++++ 8 files changed, 103 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8d84629..a956265 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,7 +84,7 @@ Callouts (`> [!NOTE]`) are parsed natively — `mkdocs-callouts` is gone. Two co ### The headless suite `test_gdextension/` is a Godot project that loads the built addon and drives -`SpriteStudioPlayer2D` from GDScript — 35 cases over the bound API, the part +`SpriteStudioPlayer2D` from GDScript — 38 cases over the bound API, the part override layer and the five signals. It is not a sample and does not live under `examples/`: the samples are what a reader is shown, and one project cannot be both that and a scratch pad (MAINTAINING_PLAYERS.md). It wears the diff --git a/docs/en/workflow/usage_scripting.md b/docs/en/workflow/usage_scripting.md index 24bcae2..4048e72 100644 --- a/docs/en/workflow/usage_scripting.md +++ b/docs/en/workflow/usage_scripting.md @@ -291,6 +291,7 @@ Color and cell overrides conflict with the animation, so they take a `priority` ### Notes +- An override lands on the player's **next tick**, not on the call: it changes what the current frame draws without changing which frame it is, so a read-back (`is_part_hidden()`, for instance) before that tick reports the frame computed before it. The tick does not have to advance the animation — a held frame, a paused or stopped player, and `ANIMATION_PROCESS_MANUAL` all pick it up. - **Color** applies to normal parts, **cell** to normal and mask parts; other part types silently ignore the override (the call still returns `true`). - Colors are interpreted in the same 8-bit sRGB space as the authored Part Color, and alpha is pre-multiplied by the runtime — pass the color as authored, without converting it yourself. - A cell override is resolved when you set it, so an unknown cell map / cell name fails immediately (returns `false`). diff --git a/docs/ja/workflow/usage_scripting.md b/docs/ja/workflow/usage_scripting.md index a3c0cd6..f7ebf7d 100644 --- a/docs/ja/workflow/usage_scripting.md +++ b/docs/ja/workflow/usage_scripting.md @@ -290,6 +290,7 @@ print(ss_player.get_cell_names("Ringo")) # → ["effect3", ...] ### 注意点 +- オーバーライドが反映されるのは、呼び出した瞬間ではなくプレーヤの**次のティック**です。オーバーライドは「今のフレームが何を描くか」を変えるだけでフレーム番号を動かさないため、ティックの前に読み戻すと（たとえば `is_part_hidden()`）1 つ前に計算されたフレームの答えが返ります。ティックはアニメーションを進めるものである必要はなく、フレームを止めたまま・一時停止中・停止中・`ANIMATION_PROCESS_MANUAL` のいずれでも反映されます。 - **カラー**は通常パーツ、**セル**は通常パーツとマスクパーツに適用されます。それ以外の種別のパーツでは黙って無視されます（呼び出し自体は `true` を返します）。 - 色はオーサリングされた Part Color と同じ 8bit sRGB 空間として解釈され、アルファはランタイム側で pre-multiply されます。自前で変換せず、オーサリングどおりの色を渡してください。 - セルオーバーライドは設定時に解決されるため、存在しないセルマップ名 / セル名はその場で失敗します（`false` を返します）。 diff --git a/ss_player/doc_classes/SpriteStudioPlayer2D.xml b/ss_player/doc_classes/SpriteStudioPlayer2D.xml index 09470cc..8c72a6e 100644 --- a/ss_player/doc_classes/SpriteStudioPlayer2D.xml +++ b/ss_player/doc_classes/SpriteStudioPlayer2D.xml @@ -7,6 +7,7 @@ Node that plays a SpriteStudio animation imported as an [code].ssab[/code] resource ([SSABResource]). Assign a resource to [member ssab], pick an animation with [member current_animation], then drive playback from code with [method play], [method pause], [method resume] and [method stop], or preview it in the editor from the [b]SpriteStudio[/b] bottom panel (shown while the node is selected). [b]Editor preview:[/b] with the node selected, the bottom panel provides transport controls (play from start / play from current / stop), a frame scrubber, loop and speed. Its keyboard shortcuts mirror the AnimationPlayer editor ([kbd]D[/kbd] play from current, [kbd]Shift+D[/kbd] play from start, [kbd]S[/kbd] stop). + [b]Part overrides:[/b] the [code]set_part_*_override[/code] family changes what the current frame draws without changing which frame it is, and lands on the next tick rather than on the call — so a value read straight back reports the frame computed before it. The tick does not have to advance the animation: a held frame, a paused or stopped player, and [constant ANIMATION_PROCESS_MANUAL] all pick the override up. diff --git a/ss_player/ss_internal_player.cpp b/ss_player/ss_internal_player.cpp index 8796a75..5aec69e 100644 --- a/ss_player/ss_internal_player.cpp +++ b/ss_player/ss_internal_player.cpp @@ -412,15 +412,20 @@ String SsInternalPlayer::get_part_name(int p_part_index) const { // ---- Override Layer (Phase 2) -------------------------------------------- +bool SsInternalPlayer::_override_applied(bool p_ok) { + if (p_ok) _overrides_dirty = true; + return p_ok; +} + bool SsInternalPlayer::set_part_visibility_override(int p_part_index, bool p_force_hidden, bool p_cascade) { if (p_part_index < 0 || runtime_ctx == nullptr) return false; // force: 0 = clear/revert, 1 = force-hide (2 = force-visible is reserved). - return ss_runtime_set_part_visibility_override(runtime_ctx, (uint32_t)p_part_index, p_force_hidden ? 1 : 0, p_cascade ? 1 : 0); + return _override_applied(ss_runtime_set_part_visibility_override(runtime_ctx, (uint32_t)p_part_index, p_force_hidden ? 1 : 0, p_cascade ? 1 : 0)); } bool SsInternalPlayer::clear_part_visibility_override(int p_part_index) { if (p_part_index < 0 || runtime_ctx == nullptr) return false; - return ss_runtime_clear_part_visibility_override(runtime_ctx, (uint32_t)p_part_index); + return _override_applied(ss_runtime_clear_part_visibility_override(runtime_ctx, (uint32_t)p_part_index)); } // Godot Color (gamma/sRGB 0..1 under Compatibility 2D) -> packed 0xRRGGBBAA, @@ -432,7 +437,7 @@ static uint32_t pack_color_rgba(const Color& p_color) { bool SsInternalPlayer::set_part_color_override(int p_part_index, const Color& p_color, int p_blend_op, int p_priority) { if (p_part_index < 0 || runtime_ctx == nullptr) return false; - return ss_runtime_set_part_color_override(runtime_ctx, (uint32_t)p_part_index, (unsigned char)p_blend_op, pack_color_rgba(p_color), (unsigned char)p_priority); + return _override_applied(ss_runtime_set_part_color_override(runtime_ctx, (uint32_t)p_part_index, (unsigned char)p_blend_op, pack_color_rgba(p_color), (unsigned char)p_priority)); } bool SsInternalPlayer::set_part_color_override_corners(int p_part_index, const Color& p_left_top, const Color& p_right_top, @@ -446,12 +451,12 @@ bool SsInternalPlayer::set_part_color_override_corners(int p_part_index, const C pack_color_rgba(p_left_bottom), pack_color_rgba(p_right_bottom), }; - return ss_runtime_set_part_color_override_corners(runtime_ctx, (uint32_t)p_part_index, (unsigned char)p_blend_op, rgba, (unsigned char)p_priority); + return _override_applied(ss_runtime_set_part_color_override_corners(runtime_ctx, (uint32_t)p_part_index, (unsigned char)p_blend_op, rgba, (unsigned char)p_priority)); } bool SsInternalPlayer::clear_part_color_override(int p_part_index) { if (p_part_index < 0 || runtime_ctx == nullptr) return false; - return ss_runtime_clear_part_color_override(runtime_ctx, (uint32_t)p_part_index); + return _override_applied(ss_runtime_clear_part_color_override(runtime_ctx, (uint32_t)p_part_index)); } bool SsInternalPlayer::set_part_cell_override(int p_part_index, const String& p_cellmap_name, const String& p_cell_name, int p_priority) { @@ -462,17 +467,17 @@ bool SsInternalPlayer::set_part_cell_override(int p_part_index, const String& p_ CharString cn = p_cell_name.utf8(); uint32_t cellmap_hash = ss_runtime_hash_string(cm.get_data()); uint32_t cell_hash = ss_runtime_hash_string(cn.get_data()); - return ss_runtime_set_part_cell_override(runtime_ctx, (uint32_t)p_part_index, cellmap_hash, cell_hash, (unsigned char)p_priority); + return _override_applied(ss_runtime_set_part_cell_override(runtime_ctx, (uint32_t)p_part_index, cellmap_hash, cell_hash, (unsigned char)p_priority)); } bool SsInternalPlayer::clear_part_cell_override(int p_part_index) { if (p_part_index < 0 || runtime_ctx == nullptr) return false; - return ss_runtime_clear_part_cell_override(runtime_ctx, (uint32_t)p_part_index); + return _override_applied(ss_runtime_clear_part_cell_override(runtime_ctx, (uint32_t)p_part_index)); } bool SsInternalPlayer::clear_all_part_overrides() { if (runtime_ctx == nullptr) return false; - return ss_runtime_clear_all_part_overrides(runtime_ctx); + return _override_applied(ss_runtime_clear_all_part_overrides(runtime_ctx)); } void SsInternalPlayer::onSSABReloaded() { @@ -786,7 +791,15 @@ void SsInternalPlayer::update(float delta_seconds) { if (_subtree_borrow_stale()) { onSSABReloaded(); } - if (!ss_runtime_is_playing(runtime_ctx)) return; + if (!ss_runtime_is_playing(runtime_ctx)) { + // A stopped or finished player does not reach the draw-dedup at the + // bottom at all, so an override taken here would wait for the next + // play(). Redraw the frame it is already holding. (A *paused* player + // is still "playing" to the runtime and goes the other way, where the + // dedup is what the override has to defeat.) + redraw_pending_overrides(); + return; + } auto d = delta_seconds * 1000.0f; float frame_no = ss_runtime_update(runtime_ctx, d); @@ -933,11 +946,18 @@ void SsInternalPlayer::update(float delta_seconds) { } float draw_frame = _sub_frame_enabled ? frame_no : floorf(frame_no); - if (previous_frame_no == draw_frame && !_needs_continuous_update()) return; + // An override changes what the frame draws without changing which frame it + // is, so it has to defeat the dedup the same way a held-frame effect does. + if (previous_frame_no == draw_frame && !_needs_continuous_update() && !_overrides_dirty) return; _seek_and_redraw(frame_no, delta_seconds, was_looped); } +void SsInternalPlayer::redraw_pending_overrides() { + if (!_overrides_dirty || runtime_ctx == nullptr) return; + _seek_and_redraw(ss_runtime_get_frame_no(runtime_ctx), 0.0f, false); +} + bool SsInternalPlayer::_build_mask_writers(const DrawFrame& f) { _mask_writers.clear(); // Clear before any early return: a part that stopped masking this frame must @@ -1612,6 +1632,9 @@ void SsInternalPlayer::_drawAnimation(float frame_no, float delta_seconds, bool _inherited_mask_materials.clear(); _inherited_mask_children.clear(); _inherited_mask_dirty = false; + // This draw is the one that picks the override layer up, whichever path + // reached here — the owner's seek or the parent's child walk. + _overrides_dirty = false; DrawFrame f = {}; f.rs = RenderingServer::get_singleton(); @@ -2064,7 +2087,7 @@ void SsInternalPlayer::_redraw_child_if_frame_changed(SsInternalPlayer* child, f // A changed inherited mask context has to rebuild even on a held frame: the // composed polarity is baked into the child's per-part materials. if (child->previous_frame_no == draw_frame && !child->_needs_continuous_update() - && !child->_inherited_mask_dirty) return; + && !child->_inherited_mask_dirty && !child->_overrides_dirty) return; child->previous_frame_no = draw_frame; child->_drawAnimation(draw_frame, delta_seconds, parent_looped); } diff --git a/ss_player/ss_internal_player.h b/ss_player/ss_internal_player.h index d5349b5..3bc69d5 100644 --- a/ss_player/ss_internal_player.h +++ b/ss_player/ss_internal_player.h @@ -258,6 +258,12 @@ class SsInternalPlayer { bool set_part_cell_override(int p_part_index, const String& p_cellmap_name, const String& p_cell_name, int p_priority); bool clear_part_cell_override(int p_part_index); bool clear_all_part_overrides(); + // Draw a frame that is already current because an override changed it. + // The override layer lives in the runtime, so nothing about the frame + // number moves when one is set — and the redraw paths dedup on exactly + // that. A no-op when no override is waiting, so it is safe to call every + // tick; `update()` covers the modes that tick the animation themselves. + void redraw_pending_overrides(); private: #ifdef SPRITESTUDIO_GODOT_EXTENSION @@ -390,6 +396,12 @@ class SsInternalPlayer { float previous_frame_no = -1.0f; bool _sub_frame_enabled = false; bool _parent_driven = false; + // An override was set or cleared since the last draw. Every redraw path + // dedups on the draw frame, which an override does not move — and a + // stopped player's frame never moves at all — so without this the change + // sits in the runtime, unseen, until playback happens to step. Cleared by + // `_drawAnimation`, which both the owner's and the child's path go through. + bool _overrides_dirty = false; SsPlayerEventSink* _event_sink = nullptr; @@ -760,6 +772,10 @@ class SsInternalPlayer { void _fetchAnimation(); void _drawAnimation(float frame_no, float delta_seconds = 0.0f, bool parent_looped = false); bool _needs_continuous_update() const; + // Every override mutator returns through here: a call the runtime accepted + // is a change the next draw has to pick up. Passes the result through so + // the mutators stay one line. + bool _override_applied(bool p_ok); // Instance slot emit: re-parent the child's _root_ci under this slot's // batch CI and apply the slot's world matrix as the child's root // transform. The child's own draw + simulation already happened earlier diff --git a/ss_player/ss_player_node_2d.cpp b/ss_player/ss_player_node_2d.cpp index d05da3b..0b719ca 100644 --- a/ss_player/ss_player_node_2d.cpp +++ b/ss_player/ss_player_node_2d.cpp @@ -841,12 +841,20 @@ void SpriteStudioPlayer2D::_notification(int p_notification) { if (_audio_controller) _audio_controller->stop_all(); break; case NOTIFICATION_INTERNAL_PROCESS: + // Pushed whichever mode is running: MANUAL does not advance the + // animation here, but it still has to report its on-screen scale + // for the mask coverage pass. + _push_coverage_screen_scale(); if (_process_mode == ANIMATION_PROCESS_IDLE) { - _push_coverage_screen_scale(); _internal->update(get_process_delta_time()); // Post-update: world matrices are final this tick, so part // attachments can mirror their parts in the same frame. emit_signal(SNAME("frame_updated"), _internal->getFrameNo()); + } else { + // MANUAL. `advance()` is playback, and an override is not, so a + // project that drives its own frames still gets the override on + // screen without having to step the animation to see it. + _internal->redraw_pending_overrides(); } // Audio voices advance independently of the animation's play/pause // state (fire-and-forget), so tick every frame the node processes. diff --git a/test_gdextension/suites/test_overrides.gd b/test_gdextension/suites/test_overrides.gd index c0b4eff..f34cae1 100644 --- a/test_gdextension/suites/test_overrides.gd +++ b/test_gdextension/suites/test_overrides.gd @@ -6,6 +6,10 @@ ## reads it back without stepping sees the old answer and concludes the call did ## nothing. Every case here steps once after setting, deliberately, and ## `test_an_override_lands_on_the_next_update` is the one that states it. +## +## **That update does not have to move the frame**, though, which is the half a +## host driving its own playback runs into: the two cases after it hold the +## frame still and pause the player, and expect the override on screen anyway. extends "res://test_base.gd" const BASIC := "res://ssab_generated/overall/Basic.ssab" @@ -36,6 +40,42 @@ func test_an_override_lands_on_the_next_update() -> void: ok(player.is_part_hidden(part), "hidden once a frame has been computed") +## The update it lands on does not have to be one that *moves* the frame. Every +## redraw path dedups on the draw frame, so a project driving its own playback — +## `advance(0.0)`, a frame held at a section end — used to leave the override in +## the runtime, unseen, until something happened to step. +func test_an_override_lands_on_a_frame_that_did_not_move() -> void: + var before: float = player.get_frame_no() + player.set_part_visibility_override(part, true) + player.advance(0.0) + eq(player.get_frame_no(), before, "the frame did not move") + ok(player.is_part_hidden(part), "and the override still reached the draw") + + +## Same rule with playback switched off: a paused player's frame never moves +## again on its own, so gating the redraw on a change of frame would hold the +## override until the next resume(). +func test_an_override_lands_on_a_paused_player() -> void: + player.pause() + player.advance(dt) + ok(player.is_pausing(), "paused") + player.set_part_visibility_override(part, true) + player.advance(dt) + ok(player.is_part_hidden(part), "the override reached the draw while paused") + + +## And with playback switched off at the runtime rather than held: a stopped +## player takes the other early-return in `update`, ahead of the frame step. +func test_an_override_lands_on_a_stopped_player() -> void: + player.stop() + player.advance(dt) + not_ok(player.is_playing(), "stopped") + not_ok(player.is_part_hidden(part), "'%s' is visible where it stopped" % part) + player.set_part_visibility_override(part, true) + player.advance(dt) + ok(player.is_part_hidden(part), "the override reached the draw while stopped") + + func test_clearing_gives_the_part_back() -> void: player.set_part_visibility_override(part, true) player.advance(dt) From deb0ef6a06d6ee2638632687560cf5eaf49c3239 Mon Sep 17 00:00:00 2001 From: Naruto TAKAHASHI Date: Sun, 13 Sep 2026 23:12:41 +0900 Subject: [PATCH 2/2] docs: roadmap the pure mask an Instance part drops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_drawAnimation` renders mask coverage only when `!_parent_driven`, and `_bubble_child_clip_writers` carries clipping writers up and nothing else, so a pure mask written inside a sub-animation is neither rasterised nor carried: the pack clips correctly played on its own and draws unclipped through an Instance part. `40_mask.md` §2-7 has a pure mask closing within the sub-animation, so this is a gap rather than the design. It earns a slot because a sub-animation is the only way SpriteStudio can express more than one independent clipping group — scope is draw priority and nothing else — and Adobe Animate's `Clpb` has no such limit, which is what holds the conversion on the SSProjectGenerator side. --- ROADMAP.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/ROADMAP.md b/ROADMAP.md index af4a676..c8d7e3e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -82,6 +82,29 @@ SSPlayerForGodot leverages Godot's `CanvasItem` API and `Node2D` paradigms. Feat - **Done when**: a demo screen puts a player inside a `VBoxContainer`, resizes with the window, receives `gui_input` on its own artwork, and changes animation as a `Button` is hovered and pressed. +## ☐ A pure mask inside an Instance part (Player-only) + +- **Goal**: a mask part written inside a sub-animation clips that sub-animation's own parts, the way it + does when the same pack is played directly. +- **Key fact**: `_drawAnimation` calls `_render_mask_coverage` only when `!_parent_driven`, so an Instance + child never rasterises its own writers, and `_bubble_child_clip_writers` carries **clipping** writers up + and nothing else. A pure mask inside an instance is therefore dropped: the pack clips correctly played + on its own and draws unclipped through an Instance part. The SDK's `40_mask.md` §2-7 has a pure mask + closing *within* the sub-animation, so this is a gap rather than the design. +- **Why it earns a slot**: a sub-animation is the only way SpriteStudio can express **more than one + independent clipping group** in one animation — scope is draw priority and nothing else, so a second + mask reaches the first one's targets. Adobe Animate's `Clpb` has no such limit and real exports carry + several, which is what holds the conversion in `SSProjectGenerator/ROADMAP.md`. +- **Steps**: + 1. Decide where the coverage comes from: a private pass for the child (a second borrowed target, and the + owner's UV transform no longer describing it), or the child's pure-mask writers bubbled into the + owner's coverage with a scope confined to the child's own draw-order window — `ss_mask_meta` carries + `(slot, bit, op, is_clipping)` today and would need the window as well. + 2. Whichever it is, keep `§2-6`: the instance part's composed `mask_influence` / `visible_inside_mask` + still decide whether the *owner's* mask reaches in, independently of the child's own writers. +- **Done when**: a pack of one drawing part plus the mask that clips it draws the same mounted on an + Instance part as it does played directly. + --- ## 🕒 Deferred ("あとで")