Implement rig animation controls in the test harness

- Added a new animation specification document for Phase 9 Task 5 detailing the requirements for rig animation controls.
- Introduced a new `StickmanRig` script to manage the facing direction and joint bending for the rig.
- Implemented UI elements in the test harness for selecting animations, controlling playback (play/pause/resume/stop), and toggling loop mode.
- Enhanced the `test_harness.gd` script to handle animation playback state and UI interactions.
- Updated documentation in `AGENTS.md`, `README.md`, and `RIGGING.md` to reflect the new animation features.
This commit is contained in:
2026-08-24 23:41:23 -04:00
parent 273090993e
commit 07bef66703
12 changed files with 1695 additions and 283 deletions
+471
View File
@@ -0,0 +1,471 @@
# Phase 9 Task 4 — Refactor: Facing Direction & Bone Bend into the Rig
## Overview
RIGGING.md Task 4: facing direction and per-joint bone bend must become **part of
`master_rig.tscn`**, not the test harness. A rigged stickman instance should carry its own facing
profile and bend flags so different instances can face different directions / bend differently at
the same time. These must be **exported controls** on the rig so they are easy to access from the
inspector and from runtime code.
Today the harness owns all of this:
- `enum FacingProfile { LEFT, RIGHT, FORWARD }` (Task 1),
- `PROFILE_FLAGS` — per-profile `flip_bend_direction` sets for the 4 TwoBoneIK joints,
- `Z_ORDER_BY_PROFILE` — per-profile `Body/*` draw-order tables (Task 2),
- `BEND_JOINTS` / `BEND_JOINT_BONE_PATHS` — the 4 upper↔lower limb connectors,
- `_facing_profile`, `_bend_joint_bones`, `_bend_modifications`, `_body_container` state,
- `_apply_facing_profile()`, `_apply_body_z_order()`, `_resolve_bend_joints()`, and
`_ensure_modification_stack_enabled()`.
Task 4 moves that state + logic into a new script attached to the `master_rig.tscn` root, and the
harness becomes a thin driver that reads/writes the rig's exported properties.
## 1. New rig script — `scripts/stickman_rig.gd`
A new `class_name StickmanRig` script, `extends Node2D`, attached to the `Master` root node of
`master_rig.tscn` (the root currently has **no** script). It is the single owner of facing profile
+ per-joint bend flags + `Body/*` z-order.
### 1a. Class declaration & enums
```gdscript
class_name StickmanRig
extends Node2D
## Facing profiles. Values are used directly as facing-menu item ids in the
## harness (0/1/2), so they must stay stable.
enum FacingProfile { LEFT, RIGHT, FORWARD }
## Per-joint bend direction. INVERTED == flip_bend_direction = true.
enum BendDirection { NORMAL, INVERTED }
```
The enums live **in the rig script** (not a shared script). The harness references them as
`StickmanRig.FacingProfile.LEFT` / `StickmanRig.BendDirection.INVERTED` (idiomatic `class_name`
enum access — no autoload or singleton needed).
### 1b. Constants (moved verbatim from `test_harness.gd`)
```gdscript
const SKELETON_PATH := "Skeleton2D"
const BODY_CONTAINER_PATH := "Body"
## Bend joints (upper↔lower limb connectors) whose TwoBoneIK "Flip Bend
## Direction" flag is user-controllable.
const BEND_JOINTS: Array[String] = ["LeftArm", "RightArm", "LeftLeg", "RightLeg"]
## Lower-bone NodePath (relative to Skeleton2D) per bend joint. Used both to
## resolve the TwoBoneIK modification (via joint_two_bone2d_node) and to report
## each joint's world position for hit-testing.
const BEND_JOINT_BONE_PATHS: Dictionary = {
"LeftArm": "Torso/LeftUpperArm/LeftLowerArm",
"RightArm": "Torso/RightUpperArm/RightLowerArm",
"LeftLeg": "Torso/LeftUpperLeg/LeftLowerLeg",
"RightLeg": "Torso/RightUpperLeg/RightLowerLeg",
}
## flip_bend_direction value per facing profile, keyed by bend-joint name.
const PROFILE_FLAGS: Dictionary = {
FacingProfile.LEFT: { "LeftArm": false, "RightArm": false, "LeftLeg": true, "RightLeg": true },
FacingProfile.RIGHT: { "LeftArm": true, "RightArm": true, "LeftLeg": false, "RightLeg": false },
FacingProfile.FORWARD: { "LeftArm": false, "RightArm": true, "LeftLeg": true, "RightLeg": false },
}
## Body/* visual part node names in draw order (back-to-front) per profile.
## First entry backmost, last frontmost. Upper limbs behind lower limbs; on the
## far (behind-torso) side the arm pair draws behind the leg pair, on the near
## side the arm pair draws in front; head always frontmost.
const Z_ORDER_BY_PROFILE: Dictionary = {
FacingProfile.FORWARD: [
"Body",
"LeftUpperLeg", "RightUpperLeg",
"LeftLowerLeg", "RightLowerLeg",
"LeftUpperArm", "RightUpperArm",
"LeftLowerArm", "RightLowerArm",
"Head",
],
FacingProfile.LEFT: [
"LeftUpperArm", "LeftLowerArm",
"LeftUpperLeg", "LeftLowerLeg",
"Body",
"RightUpperLeg", "RightLowerLeg",
"RightUpperArm", "RightLowerArm",
"Head",
],
FacingProfile.RIGHT: [
"RightUpperArm", "RightLowerArm",
"RightUpperLeg", "RightLowerLeg",
"Body",
"LeftUpperLeg", "LeftLowerLeg",
"LeftUpperArm", "LeftLowerArm",
"Head",
],
}
```
### 1c. Exported properties (the "exported controls")
```gdscript
## Facing preset. Setting it overwrites the four per-joint bend values from
## PROFILE_FLAGS and reorders Body/* children. Default FORWARD.
@export var facing_profile: FacingProfile = FacingProfile.FORWARD:
set(value):
if facing_profile == value:
return
facing_profile = value
_apply_profile()
## Per-joint bend direction (the actual flip_bend_direction source of truth).
## Defaults match the FORWARD profile. These are individually overridable after
## a profile is applied (drifting away from the preset, exactly like the current
## harness right-click behavior).
@export_group("Bend Direction")
@export_enum("Normal", "Inverted") var left_arm_bend: int = BendDirection.NORMAL:
set(value):
_set_joint_bend_inverted("LeftArm", value == BendDirection.INVERTED)
@export_enum("Normal", "Inverted") var right_arm_bend: int = BendDirection.INVERTED:
set(value):
_set_joint_bend_inverted("RightArm", value == BendDirection.INVERTED)
@export_enum("Normal", "Inverted") var left_leg_bend: int = BendDirection.INVERTED:
set(value):
_set_joint_bend_inverted("LeftLeg", value == BendDirection.INVERTED)
@export_enum("Normal", "Inverted") var right_leg_bend: int = BendDirection.NORMAL:
set(value):
_set_joint_bend_inverted("RightLeg", value == BendDirection.INVERTED)
```
Semantics:
- `facing_profile` is a **preset**. Its setter writes the four `*_bend` vars (through their own
setters, so mod flags stay in sync) and then reorders `Body/*` children, then emits
`facing_profile_changed`. It early-outs if the value is unchanged (prevents redundant re-applies,
e.g. re-spawning with the default profile).
- The four `*_bend` vars are the **per-joint source of truth**. Each setter stores the value,
updates the resolved `SkeletonModification2DTwoBoneIK.flip_bend_direction`, and emits
`bend_flag_changed`. `BendDirection.INVERTED``flip_bend_direction = true`.
- All setters are **guarded by a `_nodes_ready` flag** (set at the end of `_ready()`): before the
rig has resolved its children (e.g. while `PackedScene.instantiate()` is still hydrating the
serialized exports), the setter only stores the value; `_ready()` then applies the full state
once. This makes the apply path order-independent and robust against setter timing during
instantiation.
### 1d. Signals
```gdscript
## Emitted when the facing preset changes (after flags + z-order are applied).
signal facing_profile_changed(profile: int)
## Emitted when a single joint's bend direction changes. `flipped` is the new
## flip_bend_direction value (true == inverted).
signal bend_flag_changed(joint: String, flipped: bool)
```
The harness primarily refreshes menu labels on `about_to_popup` (the established pattern used by
the editor's snap/guide menus), so the signals are not strictly required for label sync — but they
are emitted for programmatic consumers and match the repo's signal-driven data-flow convention.
### 1e. Public methods
```gdscript
## Facing preset accessors.
func set_facing_profile(profile: int) -> void
func get_facing_profile() -> int
## Per-joint bend accessors (bool form mirrors flip_bend_direction directly).
func set_joint_bend_flipped(joint: String, flipped: bool) -> void
func get_joint_bend_flipped(joint: String) -> bool
## The list of bend-joint names (["LeftArm", "RightArm", "LeftLeg", "RightLeg"]).
func get_bend_joints() -> Array[String]
## World position of a bend joint (its lower bone's global_position) for
## right-click hit-testing. Unknown joint → push_warning + Vector2.ZERO.
func get_bend_joint_global_position(joint: String) -> Vector2
```
`set_facing_profile` / `set_joint_bend_flipped` are the runtime API; the exported var setters funnel
into the same internal apply functions, so there is exactly one mutation path (no drift between the
inspector values, the internal state, and the live mod flags).
### 1f. Internal state & resolution
```gdscript
var _nodes_ready: bool = false
var _skeleton: Skeleton2D = null
var _body_container: Node2D = null
var _bend_joint_bones: Dictionary = {} # { String : Bone2D } (lower bones)
var _bend_modifications: Dictionary = {} # { String : SkeletonModification2DTwoBoneIK }
```
- `_ready()`: resolves `_skeleton` (`get_node_or_null(SKELETON_PATH)`), `_body_container`
(`get_node_or_null(BODY_CONTAINER_PATH)`), the 4 lower `Bone2D`s via `BEND_JOINT_BONE_PATHS`, and
the 4 TwoBoneIK modifications; enables the modification stack (`stack.enabled = true`); then
applies the current `facing_profile` once (`_apply_profile()`) and sets `_nodes_ready = true`.
- `_resolve_bend_modifications()` mirrors the current harness `_resolve_bend_joints()` matching: walk
`_skeleton.modification_stack`, and for each `SkeletonModification2DTwoBoneIK` match by
`joint_two_bone2d_node == NodePath(BEND_JOINT_BONE_PATHS[joint])` (**no hardcoded stack index**).
Missing bone/mod → `push_warning` + skip (never crash).
- `_apply_profile()`: writes the four `*_bend` vars from `PROFILE_FLAGS[profile]` (through their
setters), then `_apply_body_z_order()`.
- `_apply_body_z_order()`: preserves the Task 2 algorithm — walk the profile's ordered part names
**back-to-front** and `move_child(part, get_child_count() - 1)` for each existing child; unknown/
extra children stay at the back; missing parts skipped silently.
### 1g. Editor-time vs runtime
- **The script is NOT `@tool`.** Exported properties still appear in the editor inspector (they are
serialized into `master_rig.tscn` when the scene is saved), but the script body — node resolution,
`flip_bend_direction` writes, `Body/*` reordering, mod-stack enabling — runs **only at runtime**
(`_ready()` + setters on a live instance).
- Rationale: `SkeletonModificationStack2D` only solves at runtime, so an in-editor live preview of
bend direction would not be meaningful, and mutating `Body/*` child order / mod sub-resources in
the editor would dirty the scene and fight the editor's undo/ownership. Runtime-only application
is the safe minimum the task explicitly permits ("at minimum runtime"). Making it `@tool` later
(gated behind `Engine.is_editor_hint()`) is a possible future enhancement, not part of this task.
### 1h. Failure handling
Matches repo style throughout (`get_node_or_null`, `push_warning`, skip):
- Missing `Skeleton2D`, `Body`, a bend-joint bone, or a TwoBoneIK mod → `push_warning` (prefixed
`"StickmanRig: ..."`) and skip that piece; the rig never crashes.
- Unknown joint name in `set_joint_bend_flipped` / `get_joint_bend_flipped` /
`get_bend_joint_global_position``push_warning` and no-op / `false` / `Vector2.ZERO`
respectively.
- Empty `Body` container or missing part nodes → z-order reorder silently skips (as today).
## 2. `master_rig.tscn` changes
1. **Attach the script** to the root `Master` node: `script = ExtResource("…")` for
`res://scripts/stickman_rig.gd`. The exported defaults get serialized (`facing_profile = 2`,
`left_arm_bend = 0`, `right_arm_bend = 1`, `left_leg_bend = 1`, `right_leg_bend = 0`).
2. **Align authored state to the FORWARD default** (Q1 **resolved: yes**). So the
scene's as-authored state == the runtime default (and the editor view of the raw scene is
coherent):
- TwoBoneIK `flip_bend_direction` (currently authored = the **RIGHT** profile, see §6):
- `LeftArm` (`…TwoBoneIK_f0s26`, `target_nodepath = ../IK_Targets/Left_Hand`): `true`**remove the flag / `false`**.
- `RightArm` (`…TwoBoneIK_yvxej`): `true`**unchanged**.
- `RightLeg` (`…TwoBoneIK_ylko5`): absent → **unchanged** (stays `false`).
- `LeftLeg` (`…TwoBoneIK_2leu7`): absent → **add `flip_bend_direction = true`**.
- `Body/*` child order: the authored order is `Head, Body, LeftUpperLeg, RightUpperLeg,
LeftLowerLeg, RightLowerLeg, LeftUpperArm, RightUpperArm, LeftLowerArm, RightLowerArm`. Move
the `Body/Head` node to the **end** → order becomes exactly the FORWARD table. (Safe: the
`RemoteTransform2D` drivers and the adapter reference `Body/*` by name, never by index.)
Both edits are pure scene text edits. Runtime behavior is unchanged either way (the rig applies
FORWARD on `_ready`), so this only affects the as-authored appearance and makes the "default
leaves the scene as-authored" verification true.
## 3. Harness changes — `scripts/test_harness.gd`
The harness stops owning bend/facing state and drives the rig script. It keeps a thin UI mirror for
menu labels + re-application across respawns (the rig remains the authority over the actual flags).
### 3a. Removed
| Harness element | Disposition |
|---|---|
| `enum FacingProfile` | Delete — use `StickmanRig.FacingProfile`. |
| `PROFILE_FLAGS`, `Z_ORDER_BY_PROFILE` | Delete — moved to `StickmanRig`. |
| `BEND_JOINTS`, `BEND_JOINT_BONE_PATHS` | Delete — the rig exposes `get_bend_joints()` + `get_bend_joint_global_position()`. |
| `BODY_CONTAINER_PATH` | Delete — only the rig needs it. |
| `_body_container` | Delete (resolve + clear). |
| `_bend_joint_bones`, `_bend_modifications` | Delete (resolve + clear). |
| `_apply_facing_profile()` | Delete — call `_rig_script.set_facing_profile()`. |
| `_apply_body_z_order()` | Delete — moved to the rig. |
| `_resolve_bend_joints()` | Delete — moved to the rig's `_resolve_bend_modifications()`. |
| `_ensure_modification_stack_enabled()` | Delete — the rig enables its own stack in `_ready()`. |
Note: `SKELETON_PATH` **stays** in the harness (still used to resolve `_skeleton` for the bone
overlay and the coordinates panel). `_facing_profile` **stays** but is repurposed as a UI mirror
(§3b).
### 3b. State (changed / added)
```gdscript
var _rig_script: StickmanRig = null
var _facing_profile: int = StickmanRig.FacingProfile.FORWARD # UI mirror only
var _context_joint: String = ""
```
- `_facing_profile` is now only "the user's last-selected facing profile", used for the `[√] ` menu
prefix and to re-apply after each spawn. The rig's exported `facing_profile` is the actual state.
- `_context_joint` is unchanged (which joint the right-click menu targets).
### 3c. Spawn flow — `_load_and_spawn()`
1. `_free_current_rig()` (unchanged except it clears `_rig_script` instead of the deleted state).
2. `var rig := StickmanFactory.spawn(path)`; `_rig_script = rig as StickmanRig` (null-guarded:
foreign rig without the script → `push_warning` and disable facing/bend UI).
3. Connect signals **before** `add_child`:
`_rig_script.facing_profile_changed.connect(_on_facing_profile_changed)`,
`_rig_script.bend_flag_changed.connect(_on_bend_flag_changed)` (both may be no-ops; label sync
is done on `about_to_popup`, but connecting keeps future consumers working).
4. `_world.add_child(_rig)` (rig `_ready` runs here, applies its default FORWARD and enables its
mod stack).
5. `_resolve_rig_nodes()` — now only resolves `_skeleton`, `_ik_handles`, `_coord_bones` (drops
`_body_container` and `_resolve_bend_joints()`).
6. `_rig_script.set_facing_profile(_facing_profile)` — re-applies the remembered selection (no-op
when it equals the rig's FORWARD default).
### 3d. Facing menu
- Menu item ids stay `StickmanRig.FacingProfile.LEFT/RIGHT/FORWARD`.
- `_on_facing_menu_id_pressed(id)`: `_facing_profile = id`; if `_rig_script` valid →
`_rig_script.set_facing_profile(id)`; then `_update_facing_menu_labels()`.
- `_facing_menu_label(profile)`: unchanged (`[√] ` prefix based on `_facing_profile`).
### 3e. Right-click bend toggle
- `_hit_test_bend_joint(world_pos)`: iterate `_rig_script.get_bend_joints()`; position from
`_rig_script.get_bend_joint_global_position(joint)`; same `JOINT_HIT_RADIUS_PX / _camera.zoom.x`
nearest-joint selection.
- `_handle_right_click()`: unchanged flow (set `_context_joint`, set label, popup).
- `_context_menu_label(joint)`: `"Normal Bend" if _rig_script.get_joint_bend_flipped(joint) else "Invert Bend"`
(fallback `"Invert Bend"` when the rig script is missing).
- `_on_context_menu_id_pressed(_id)`: if `_context_joint` non-empty and `_rig_script` valid →
`_rig_script.set_joint_bend_flipped(_context_joint, not _rig_script.get_joint_bend_flipped(_context_joint))`.
### 3f. Signal handlers (optional but included)
```gdscript
func _on_facing_profile_changed(_profile: int) -> void:
_facing_profile = _rig_script.get_facing_profile() if _rig_script else _facing_profile
_update_facing_menu_labels()
_debug_overlay.queue_redraw()
func _on_bend_flag_changed(_joint: String, _flipped: bool) -> void:
_debug_overlay.queue_redraw()
```
(These keep the `[√] ` prefix in sync if the profile is ever changed from outside the menu; the
menu still refreshes on `about_to_popup` as the primary mechanism.)
## 4. Factory / adapter impact
- `stickman_factory.gd`: **no functional change.** `spawn_from_data` / `spawn` still return the
rig root (now carrying the `StickmanRig` script); narrow the return type to `StickmanRig`
(`return RIG_SCENE.instantiate() as StickmanRig`) for stronger typing (Q6 **resolved: yes**).
- `stk_rig_adapter.gd`: **no change.** The adapter mounts shapes onto `Body/*` by node path and
does not read or write bend/facing state. The rig's `_ready` (z-order + flags + stack-enable) runs
**after** `StkRigAdapter.apply()` (which happens inside `spawn_from_data`, before the rig enters
the tree), so mounted shape children are already present when the rig reorders `Body/*` — moving a
part node moves its whole shape group, exactly as today.
## 5. `master_rig_builder.gd`, `master_rig2.tscn`
Out of scope and untouched (per `docs/phase9_round1_bugfix_spec.md`): `master_rig_builder.gd` builds
a **different** rig (`Sticky/Stickman/.../Hip` naming), and `master_rig2.tscn` is a pose-override
variant not referenced by the factory. The new `StickmanRig` script targets `master_rig.tscn` only.
## 6. Defaults & backward compatibility
- **Default profile = FORWARD** (`facing_profile = FacingProfile.FORWARD`), matching the current
harness default `_facing_profile = FacingProfile.FORWARD`. On a fresh spawn the rig applies
FORWARD flags `{LeftArm:false, RightArm:true, LeftLeg:true, RightLeg:false}` and the FORWARD
z-order — **identical to today's runtime behavior**.
- **Authored `master_rig.tscn` flags are NOT currently FORWARD** — they are the **RIGHT** profile
(`LeftArm:true, RightArm:true, LeftLeg:false, RightLeg:false`, see `master_rig.tscn` lines 23/31
and the absent flags on the two leg mods). The harness already overwrites these at runtime, so
behavior is unchanged before/after this refactor; the mismatch only matters for the "as-authored
== default" goal (§2 / Q1).
- **No `.stk` format change.** `FILE_VERSION` stays `"1.5"`. Bend/facing state is rig-instance
state, not figure data; it is never serialized into `.stk`.
- **No `settings.json` change.** The harness facing selection stays non-persistent (as today).
- Old `.stk` files, foreign rigs, and partial figures load unchanged (null-guarded resolution).
## 7. Files modified
| File | Changes |
|---|---|
| `scripts/stickman_rig.gd` | **New.** `class_name StickmanRig extends Node2D` — enums, constants, exported properties, signals, methods, runtime resolution/apply. |
| `master_rig.tscn` | Attach `StickmanRig` to root `Master`; (recommended) align authored TwoBoneIK flags + `Body/*` order to FORWARD (§2). |
| `scripts/test_harness.gd` | Delete bend/facing ownership (§3a); add `_rig_script` + repurposed `_facing_profile`; rewire spawn, facing menu, right-click toggle (§3c3f). |
| `scripts/stickman_factory.gd` | Narrow `spawn_from_data`/`spawn` return type to `StickmanRig` (Q6). |
| `docs/phase9_task4_refactor_spec.md` | This file. |
| `AGENTS.md` | Add `scripts/stickman_rig.gd` bullet; update `test_harness.gd` bullet (facing/bend now drive the rig); note `master_rig.tscn` root script. |
| `README.md` | Project-structure table row for `stickman_rig.gd`; update test-harness + factory bullets. |
| `RIGGING.md` | Mark Task 4 implemented. |
## 8. Edge cases
- **No rig script / foreign rig** (`rig as StickmanRig` == null): `push_warning`; facing menu and
right-click bend toggle become no-ops; bone overlay/coords still work.
- **Missing `Skeleton2D` / `Body` / bones / mods**: `push_warning` + skip per section; rig never
crashes.
- **Setters before `_ready`** (during instantiation): guarded by `_nodes_ready`; `_ready` applies the
full state once — no ordering bug.
- **Re-spawn**: the rig is freed and a fresh one spawns; the harness re-applies the remembered
`_facing_profile` (§3c). `_free_current_rig()` clears `_rig_script` (not the deleted state).
- **Manual bend override then profile change**: setting a profile overwrites all four per-joint
flags (the preset wins), exactly like the current `_apply_facing_profile()`.
- **Unknown `Body/*` children**: stay at the back during reorder (unchanged Task 2 semantics).
- **Head always frontmost**: preserved in all three `Z_ORDER_BY_PROFILE` tables.
## 9. Design decisions
| # | Decision | Justification |
|---|---|---|
| D1 | New `class_name StickmanRig extends Node2D` on the `master_rig.tscn` root | The rig is the natural owner of per-instance facing/bend; a `class_name` script makes it a typed, reusable API for the harness and future consumers. |
| D2 | Enums live in the rig script (`StickmanRig.FacingProfile` / `StickmanRig.BendDirection`) | No autoload/singleton; idiomatic `class_name` enum access; single source of truth both sides can reference. |
| D3 | `facing_profile` is a preset; the four `*_bend` enum exports are the per-joint source of truth | Matches the harness's existing preset-then-override behavior (RIGGING.md: "user is free to change bend manually"); the enum dropdowns give readable "Normal"/"Inverted" inspector labels (the task's "per-joint bend enums" hint). |
| D4 | Public bool-form methods (`set_joint_bend_flipped`/`get_joint_bend_flipped`) alongside the enum exports | `flip_bend_direction` is a bool; the bool form is the engine-accurate contract and keeps the harness context-menu label logic unchanged. |
| D5 | TwoBoneIK resolved by `joint_two_bone2d_node` NodePath matching, not stack index | Preserves the existing (Task 1) resolution approach; robust to stack reordering. |
| D6 | Non-`@tool` script; apply only at runtime | IK doesn't solve in-editor; mutating `Body/*` order / mod flags in-editor would dirty the scene; runtime-only is the task's permitted minimum. |
| D7 | Harness keeps a `_facing_profile` **UI mirror** but not the bend authority | Preserves current UX (selection persists across respawns, `[√] ` label) without the harness owning flags/z-order. |
| D8 | Rig enables its own mod stack in `_ready()` | Facing/bend are meaningless until the stack is live; the rig should self-enable at runtime (single consumer today always enables it). |
| D9 | Rig exposes `get_bend_joint_global_position()` | Lets the harness drop `BEND_JOINT_BONE_PATHS`/`_bend_joint_bones` entirely; the rig owns the whole bend domain. |
## 10. Test plan
1. **Parse check** (same as prior tasks):
`..\Godot_v4.7.1-stable_win64_console.exe . --headless --check-only --quit` — no errors.
2. **Headless SceneTree verification** (temporary script, deleted after; pattern from Round 7):
- `StickmanFactory.spawn("res://stickmen/basic.stk")` → cast root to `StickmanRig` (non-null);
`add_child` it; assert `get_facing_profile() == FacingProfile.FORWARD` and the 4 mods' flags
equal the FORWARD set; assert `Body` child order == `Z_ORDER_BY_PROFILE[FORWARD]` and `Body/Head`
is last.
- `set_facing_profile(FacingProfile.LEFT)` → assert flags + `Body` order match LEFT (left pairs
before torso, head last); repeat `RIGHT`.
- `set_joint_bend_flipped("LeftArm", true)` → assert `get_joint_bend_flipped("LeftArm") == true`,
the matched TwoBoneIK mod `flip_bend_direction == true`, and the other three unchanged.
- Instantiate a rig **without** adding it to a tree, set `facing_profile` before `add_child`, then
`add_child` → assert the pre-set profile was honored by `_ready` (guards D-setters).
3. **Harness code review**: facing menu ids use `StickmanRig.FacingProfile`; spawn connects signals
before `add_child` and re-applies `_facing_profile`; right-click toggle reads/writes the rig
script, not a mod directly; `_resolve_rig_nodes()` no longer touches bend/z-order state.
4. **Manual F6** (`res://scenes/test_harness.tscn`):
- Load `basic.stk`; figure starts FORWARD (unchanged from before).
- Facing menu → Left/Right: limbs tuck behind the torso correctly, head stays frontmost; `[√] `
moves; context menu labels reflect the new per-joint flags.
- Right-click an elbow/knee → "Invert Bend"/"Normal Bend" toggles the rig property and the label
flips; the limb visibly bends the other way.
- Load another `.stk` → the remembered facing profile re-applies to the fresh rig.
5. **Cleanup** temp verification files.
## 11. Implementation order
1. `scripts/stickman_rig.gd` — enums, constants, exports, signals, methods, runtime apply.
2. `master_rig.tscn` — attach script; align authored flags + `Body/*` order to FORWARD (Q1).
3. `scripts/test_harness.gd` — remove ownership, add `_rig_script`, rewire spawn/menus/toggle.
4. `scripts/stickman_factory.gd` — (optional) narrow return type.
5. Parse check + headless verification (temp, then removed).
6. Docs: `AGENTS.md`, `README.md`, `RIGGING.md`, this spec.
## 12. Open questions — RESOLVED (user approval)
- **Q1 — Authored scene ≠ FORWARD default.** **Resolved: YES — edit `master_rig.tscn`** to align
the authored TwoBoneIK flags and `Body/*` order to FORWARD (§2).
- **Q2 — Per-joint export representation.** **Resolved: enum form** — `BendDirection` with
`@export_enum("Normal","Inverted")` dropdowns.
- **Q3 — `@tool` vs runtime-only.** **Resolved: runtime-only** (non-`@tool`; exports still
inspector-editable).
- **Q4 — Mod-stack enabling.** **Resolved: YES** — move `stack.enabled = true` into the rig's
`_ready()`; harness drops `_ensure_modification_stack_enabled()`.
- **Q5 — Harness `_facing_profile` UI mirror.** **Resolved: YES** — keep the lightweight mirror
for the `[√] ` label + respawn re-application; the rig is the authority.
- **Q6 — Factory return type.** **Resolved: YES** — narrow `spawn_from_data`/`spawn` return type
to `StickmanRig`.
+398
View File
@@ -0,0 +1,398 @@
# Phase 9 Task 5 — Feature: Rig Animation in the Test Harness
## Overview
RIGGING.md Task 5: `master_rig.tscn` already ships an `AnimationPlayer` and an `AnimationTree`.
The test harness must let the user **select** an animation from a dropdown, **play / pause /
resume / stop** it, and **toggle loop vs. play-once**. Concretely:
1. A **dropdown** listing the rig's animations (from `AnimationPlayer.get_animation_list()`).
2. A **play/pause/resume** button (single button whose label reflects playback state) plus a
**Stop** button. Per RIGGING.md, **Play always restarts from the beginning**; **Stop resets to
the start** so the next Play restarts.
3. A **Loop** checkbox that makes the selected animation loop or play just once.
This is a **harness-only** change. `master_rig.tscn` is **not** modified (the two animation nodes
already exist and are sufficient). `scripts/stickman_rig.gd` is **not** modified (the harness
resolves the `AnimationPlayer` directly by node path, exactly as it already resolves `Skeleton2D`
and the IK handles). Only `scripts/test_harness.gd` changes.
## 1. Findings — `master_rig.tscn` animation nodes
### 1a. Node paths (both are direct children of the `Master` rig root)
| Node | Path (rig-relative) | Properties |
|---|---|---|
| `AnimationPlayer` | `AnimationPlayer` | `libraries/ = AnimationLibrary_t75yq`; no `active` override (defaults to `true`). |
| `AnimationTree` | `AnimationTree` | `active = false`; `tree_root = AnimationNodeStateMachine_6rw38`; `anim_player = NodePath("../AnimationPlayer")`. |
The harness reaches the player via `_rig.get_node_or_null(NodePath("AnimationPlayer"))` — the
rig root is the `Master` node (`_rig`, a `StickmanRig`), and `AnimationPlayer` is a direct child
(`parent="."`).
### 1b. Animation library — two animations (not one)
The `AnimationLibrary_t75yq._data` dictionary holds **two** animations (RIGGING.md says "currently
just 'walk_right'", but there is also a pose-reset helper):
| Name | Length | `loop_mode` | Tracks |
|---|---|---|---|
| `RESET` | `0.001` | *(absent → `LOOP_NONE`)* | `.:facing_profile` (discrete, value `2` = `FacingProfile.FORWARD`) |
| `walk_right` | `0.8` | `1` (`LOOP_LINEAR`) | 6 IK-target position tracks + `.:facing_profile` (discrete, value `1` = `RIGHT`) |
`AnimationPlayer.get_animation_list()` therefore returns `["RESET", "walk_right"]` (library
dictionary insertion order). The dropdown lists both; the harness prefers `walk_right` as the
initially-selected item (see §5b).
### 1c. What `walk_right` animates
`walk_right` does **not** key `Bone2D` rotations directly. It animates the **6 `IK_Targets`
`Marker2D` positions** (the `TwoBoneIK`/`LookAt` solvers then flex the bones) plus a discrete
`facing_profile` set on the rig root:
- `IK_Targets/Torso:position` (5 keys, cubic interp) — bobbing; the Torso marker's child
`RemoteTransform2D` (`remote_path = ../../../Skeleton2D/Torso`) translates the whole skeleton.
- `IK_Targets/Head:position`, `IK_Targets/Right_Leg:position`, `IK_Targets/Left_Leg:position`,
`IK_Targets/Right_Hand:position`, `IK_Targets/Left_Hand:position` (5 keys each, cubic interp).
- `.:facing_profile` (discrete, `update = 1`, value `1` = `FacingProfile.RIGHT`).
The `.:facing_profile` track writes through the `StickmanRig.facing_profile` export setter
(animation tracks write via `Object.set()`, which triggers the setter), so **playing an animation
can change the facing profile** and emits `facing_profile_changed` — which the harness already
handles via `_on_facing_profile_changed` (updates the `_facing_profile` mirror, the `[√] ` menu
prefix, and the debug redraw). No special harness handling is required; it is documented behavior.
`RESET` likewise sets `facing_profile = FORWARD`.
### 1d. Is `AnimationTree` configured? — **No (placeholder)**
`AnimationTree` has `active = false` and an **empty** `AnimationNodeStateMachine` root
(`AnimationNodeStateMachine_6rw38` has no states, no transitions, and no `start_node`). There is
**no** `AnimationNodeAnimation`, no `AnimationNodeBlendTree`, and no output node set. It is a
placeholder.
**Decision (D1): the harness drives `AnimationPlayer` directly; configuring `AnimationTree` is
out of scope.** Justification: the task only needs select/play/pause/stop/loop, all of which
`AnimationPlayer` provides directly; a state-machine/blend-tree setup adds nothing for a single
animation stream and would require editing `master_rig.tscn`. The `AnimationTree` node is left
untouched for a future blending phase.
## 2. Godot 4 API notes (verified for 4.7)
- `AnimationPlayer.get_animation_list() -> PackedStringArray` — animation names.
- `AnimationPlayer.get_animation(name: StringName) -> Animation` — the `Animation` resource.
- `AnimationPlayer.play(name: StringName, ...)` — if the player is **stopped**, calling `play(name)`
**restarts from position 0**. If the player is **paused** on the same animation, `play(name)` (or
`play()` with no args) **resumes**. We rely on the documented distinction: *"the assigned
animation will resume playing if it was paused, or restart if it was stopped."*
- `AnimationPlayer.pause()` — pauses, keeps position.
- `AnimationPlayer.stop()` — default `keep_state = false`: stops and **resets position to 0**.
- `Animation.loop_mode``Animation.LOOP_NONE` (0) / `Animation.LOOP_LINEAR` (1). Loop is a
property of the **`Animation` resource**, not of `play()`, so the toggle writes
`anim.loop_mode` on the selected animation before playing.
- `AnimationPlayer.animation_finished(anim_name: StringName)` — emitted when an animation reaches
its end and stops. **Not** emitted on `pause()`/`stop()`. For **looping** animations the emit-on-
wrap behavior varies across 4.x versions, so the handler ignores the signal while `_loop` is true
(see §7 D4) — this is safe under either engine behavior.
## 3. UI design
New controls in the top-bar `HBox`, inserted **immediately after the "Facing" `MenuButton` and
before the "Open .stk…" button** (keeps the two rig-behavior control clusters — Facing + Animation
— adjacent at the left edge, and leaves the load / debug-display clusters untouched):
```
[ Facing ][ AnimDropdown ][ Play/Pause ][ Stop ][ ☑ Loop ][ Open .stk… ][ Break ][ Basic ][ Test ][ Show Bones ][ Show IK Handles ][ Show Coords ] …status…
```
| Node | Type | Text / state | Purpose |
|---|---|---|---|
| `_anim_dropdown` | `OptionButton` | populated per spawn | Select the animation. |
| `_play_button` | `Button` | `"Play"` / `"Pause"` / `"Resume"` (label swaps) | Play-from-start / pause / resume. |
| `_stop_button` | `Button` | `"Stop"` | Stop and reset to start. |
| `_loop_check` | `CheckBox` | `"Loop"`, `button_pressed = true` | Loop vs. play-once. |
Controls are **always enabled** (matching the harness's existing "Facing" menu / checkbox style);
each handler no-op-guards on a missing `AnimationPlayer` instead of disabling the control.
## 4. Implementation — `scripts/test_harness.gd`
### 4a. Constants
```gdscript
## AnimationPlayer node path (relative to rig root).
const ANIMATION_PLAYER_PATH := "AnimationPlayer"
## Initially-selected animation in the dropdown (RIGGING.md default).
const DEFAULT_ANIMATION := "walk_right"
```
### 4b. Enum
```gdscript
## Harness-tracked playback state (the harness is the sole driver of the
## AnimationPlayer, so it tracks state authoritatively via button handlers and
## the animation_finished signal rather than polling is_playing()).
enum PlaybackState { STOPPED, PLAYING, PAUSED }
```
### 4c. Runtime-built node references (added to the existing block)
```gdscript
var _anim_dropdown: OptionButton
var _play_button: Button
var _stop_button: Button
var _loop_check: CheckBox
```
### 4d. State (added to the existing block)
```gdscript
var _anim_player: AnimationPlayer = null
var _selected_animation: String = ""
var _playback_state: int = PlaybackState.STOPPED
var _loop: bool = true # harness-level, persists across respawns (like _show_coords)
```
### 4e. UI construction — insert in `_build_ui()`
Insert after `hbox.add_child(_facing_button)` and before `var open_btn := Button.new()`:
```gdscript
_anim_dropdown = OptionButton.new()
_anim_dropdown.item_selected.connect(_on_anim_dropdown_selected)
hbox.add_child(_anim_dropdown)
_play_button = Button.new()
_play_button.text = "Play"
_play_button.pressed.connect(_on_play_pressed)
hbox.add_child(_play_button)
_stop_button = Button.new()
_stop_button.text = "Stop"
_stop_button.pressed.connect(_on_stop_pressed)
hbox.add_child(_stop_button)
_loop_check = CheckBox.new()
_loop_check.text = "Loop"
_loop_check.button_pressed = true
_loop_check.toggled.connect(_on_loop_toggled)
hbox.add_child(_loop_check)
```
### 4f. Resolution — `_resolve_anim_player()` (new)
Called from `_resolve_rig_nodes()` (add the call at its end, after `_resolve_coord_bones()`):
```gdscript
func _resolve_anim_player() -> void:
_anim_player = _rig.get_node_or_null(NodePath(ANIMATION_PLAYER_PATH)) as AnimationPlayer
_populate_animation_dropdown()
if _anim_player == null:
push_warning("TestHarness: missing '%s' node in rig." % ANIMATION_PLAYER_PATH)
return
_anim_player.animation_finished.connect(_on_animation_finished)
```
```gdscript
func _populate_animation_dropdown() -> void:
_anim_dropdown.clear()
_selected_animation = ""
_playback_state = PlaybackState.STOPPED
_update_play_button()
if _anim_player == null:
return
var preferred_idx := 0
var anim_list: PackedStringArray = _anim_player.get_animation_list()
for i: int in anim_list.size():
var anim_name: String = anim_list[i]
_anim_dropdown.add_item(anim_name)
if anim_name == DEFAULT_ANIMATION:
preferred_idx = i
if _anim_dropdown.item_count > 0:
_anim_dropdown.select(preferred_idx)
_selected_animation = _anim_dropdown.get_item_text(preferred_idx)
```
### 4g. Handlers
```gdscript
func _on_anim_dropdown_selected(index: int) -> void:
_selected_animation = _anim_dropdown.get_item_text(index)
# Changing selection stops any in-progress playback (Play restarts it).
if _anim_player != null and is_instance_valid(_anim_player):
_anim_player.stop()
_playback_state = PlaybackState.STOPPED
_update_play_button()
func _on_play_pressed() -> void:
if _anim_player == null or not is_instance_valid(_anim_player):
return
if _selected_animation.is_empty():
return
match _playback_state:
PlaybackState.STOPPED:
_apply_loop_mode()
_anim_player.play(_selected_animation) # restart from position 0
_playback_state = PlaybackState.PLAYING
PlaybackState.PLAYING:
_anim_player.pause()
_playback_state = PlaybackState.PAUSED
PlaybackState.PAUSED:
_anim_player.play() # resume the assigned (paused) animation
_playback_state = PlaybackState.PLAYING
_update_play_button()
func _on_stop_pressed() -> void:
if _anim_player == null or not is_instance_valid(_anim_player):
return
_anim_player.stop() # resets position to 0 and stops
_playback_state = PlaybackState.STOPPED
_update_play_button()
func _on_loop_toggled(pressed: bool) -> void:
_loop = pressed
_apply_loop_mode()
func _on_animation_finished(_anim_name: StringName) -> void:
if _loop:
return # looping: never treat a wrap as "finished"
_playback_state = PlaybackState.STOPPED
_update_play_button()
```
### 4h. Helpers
```gdscript
func _apply_loop_mode() -> void:
if _anim_player == null or not is_instance_valid(_anim_player):
return
if _selected_animation.is_empty():
return
var anim: Animation = _anim_player.get_animation(_selected_animation)
if anim != null:
anim.loop_mode = Animation.LOOP_LINEAR if _loop else Animation.LOOP_NONE
func _update_play_button() -> void:
if _play_button == null:
return
match _playback_state:
PlaybackState.STOPPED:
_play_button.text = "Play"
PlaybackState.PLAYING:
_play_button.text = "Pause"
PlaybackState.PAUSED:
_play_button.text = "Resume"
```
### 4i. Lifecycle
- **`_resolve_rig_nodes()`** — add `_resolve_anim_player()` after `_resolve_coord_bones()`. Each
spawn re-resolves the player, repopulates the dropdown (fresh `AnimationPlayer` → fresh list),
resets `_playback_state` to `STOPPED`, and re-connects `animation_finished`.
- **`_free_current_rig()`** — add:
```gdscript
_anim_player = null
_anim_dropdown.clear()
_selected_animation = ""
_playback_state = PlaybackState.STOPPED
_update_play_button()
```
(The old player is `queue_free`d with the rig; its `animation_finished` connection dies with it.
`_loop` is **not** reset — it is harness-level state that persists across respawns, like
`_show_coords` / `_facing_profile`.)
- **`_process(delta)`** — **unchanged.** The coordinates readout keeps updating during playback
(it reads `global_position`/`global_rotation` every frame), and `AnimationPlayer` self-animates
independent of the harness `_process`. No playback-state polling is added (state is tracked via
handlers + `animation_finished`).
## 5. Files modified
| File | Changes |
|---|---|
| `scripts/test_harness.gd` | `ANIMATION_PLAYER_PATH`, `DEFAULT_ANIMATION`, `PlaybackState`, `_anim_dropdown`/`_play_button`/`_stop_button`/`_loop_check`, `_anim_player`/`_selected_animation`/`_playback_state`/`_loop`, UI block in `_build_ui()`, `_resolve_anim_player()`, `_populate_animation_dropdown()`, `_on_anim_dropdown_selected()`, `_on_play_pressed()`, `_on_stop_pressed()`, `_on_loop_toggled()`, `_on_animation_finished()`, `_apply_loop_mode()`, `_update_play_button()`; lifecycle hooks in `_resolve_rig_nodes()` + `_free_current_rig()`. |
| `docs/phase9_task5_animation_spec.md` | This file. |
| `AGENTS.md` | Test-harness section: "Phase 9 Task 5 rig animation" bullet (dropdown + play/pause/stop + loop, drives `AnimationPlayer` directly). |
| `README.md` | Test-harness bullet: animation select/play/pause/stop/loop controls. |
| `RIGGING.md` | Mark Task 5 implemented. |
**Not modified:** `master_rig.tscn`, `scripts/stickman_rig.gd`, `scripts/stickman_factory.gd`,
`scripts/stk_rig_adapter.gd`.
## 6. Edge cases
- **No rig loaded / no `AnimationPlayer`** (foreign rig): `_resolve_anim_player()` warns once;
the dropdown is empty; `_on_play_pressed`/`_on_stop_pressed`/`_on_loop_toggled` no-op-guard.
- **Re-spawn**: dropdown repopulated, `_playback_state` reset to `STOPPED`, play button label back
to `"Play"`; `_loop` toggle persists and is re-applied on the next play (via `_apply_loop_mode()`).
- **Changing the dropdown selection mid-play**: the current animation stops and state → `STOPPED`
(the newly selected animation is not auto-started).
- **Non-looping animation finishes**: `animation_finished` → state → `STOPPED`, button → `"Play"`.
- **Looping animation**: `animation_finished` (if emitted on wrap in this engine version) is
ignored by the `_loop` guard; the button stays `"Pause"` indefinitely.
- **Manual IK dragging during playback**: not blocked; but the animated tracks overwrite the dragged
handles' positions on the next frame, so dragging an animated handle while playing has no lasting
effect (expected; documented, not "fixed").
- **Animation changes the facing profile**: `walk_right` → `RIGHT`, `RESET` → `FORWARD`; flows
through the rig setter and the existing `_on_facing_profile_changed` (menu `[√] ` + redraw).
- **Stopping does not restore the rest pose**: `stop()` resets the *playhead* to 0 but leaves
properties at their last keyed values. The `RESET` animation is available to restore facing;
full rest-pose restoration on stop is out of scope.
- **The 0.001s `RESET` animation + loop ON**: selecting it with loop ON makes a harmless tight
loop (facing stays FORWARD). Not special-cased.
## 7. Design decisions
| # | Decision | Justification |
|---|---|---|
| D1 | Drive `AnimationPlayer` directly; `AnimationTree` out of scope | `AnimationTree` is an unconfigured placeholder (`active = false`, empty state machine). Select/play/pause/stop/loop are all first-class `AnimationPlayer` APIs; wiring a blend tree would require editing `master_rig.tscn` for no gain here. |
| D2 | Single play/pause/resume button + separate Stop button | Matches RIGGING.md "pause/resume" (toggling) vs "stop (restart on play)" as distinct states; the label swap is the harness's existing dynamic-text pattern (cf. the editor's snap/guide menus). |
| D3 | Track playback state via `_playback_state` + `animation_finished`, not `is_playing()` polling | The harness is the sole driver, so state is deterministic. `animation_finished` reliably fires for non-looping end-of-play and never fires on `pause()`/`stop()`; the `_loop` guard makes the looping-wrap ambiguity moot. Avoids adding per-frame polling to `_process`. |
| D4 | Loop = write `Animation.loop_mode` on the selected `Animation` before playing | Loop is an `Animation`-resource property, not a `play()` argument; this is the only way to override the authored value. Re-applied on every play so the harness `_loop` toggle is authoritative regardless of authored `loop_mode`. |
| D5 | Loop default **ON** | Matches the authored `walk_right` (`loop_mode = 1`), and a walk cycle is the natural looping case. |
| D6 | Dropdown populated dynamically per spawn from `get_animation_list()` | The list comes from the rig's own library, so future animations appear automatically; no hardcoded list. |
| D7 | Prefer `walk_right` as initial selection (`DEFAULT_ANIMATION`) | Matches RIGGING.md's "currently just 'walk_right'" default even though the library also contains `RESET`. |
| D8 | No `stickman_rig.gd` change | The harness already resolves rig children by node path (`SKELETON_PATH`, `IK_HANDLE_PATHS`); `ANIMATION_PLAYER_PATH` follows that established pattern. A rig-level accessor is unnecessary. |
| D9 | Controls never disabled; handlers no-op-guard | Matches the existing harness style (the "Facing" menu and checkboxes are always enabled). |
## 8. Verification
Only `scripts/test_harness.gd` changes, so the syntax checks target that script.
1. **Whole-project parse check** (established form used by every prior phase — the plain
`--check-only` form hangs on renderer init in 4.7.x, so use the `--headless --check-only --quit`
variant). Run from the project dir `C:\Godot4\stickman`:
```
..\Godot_v4.7.1-stable_win64_console.exe . --headless --check-only --quit
```
2. **Single-script check** (user-specified form; run from anywhere):
```
& "C:\Godot4\Godot_v4.7.1-stable_win64_console.exe" --headless --path "C:\Godot4\stickman" --check-only --script "res://scripts/test_harness.gd"
```
3. **Manual F6 check** (`res://scenes/test_harness.tscn`):
- Load `stickmen/basic.stk` → dropdown lists `RESET` and `walk_right`, `walk_right` selected,
play button shows `"Play"`, Loop checked.
- Press **Play** → button `"Pause"`; the figure walks (IK targets animate, legs/arms swing,
facing menu flips to `[√] Right`). Coordinates readout updates live.
- Press **Pause** → button `"Resume"`; figure freezes. Press again → resumes.
- Press **Stop** → figure stops, button `"Play"`; press **Play** → restarts from the beginning
(not from the paused position).
- Uncheck **Loop** → press **Play** → animation plays once, then the button returns to `"Play"`
by itself.
- Select `RESET` → Play → facing returns to `[√] Forward`.
- Load a different `.stk` → dropdown repopulated, playback reset, Loop checkbox state kept.
## 9. Implementation order
1. `scripts/test_harness.gd` — constants, enum, node refs, state, `_build_ui()` block, handlers,
helpers, lifecycle hooks.
2. Parse checks (§8 items 12) + manual F6 (§8 item 3).
3. Docs: `AGENTS.md`, `README.md`, `RIGGING.md`, this spec.