From 6b273c049c4e533f0926e311e12e2d47198863d1 Mon Sep 17 00:00:00 2001 From: Ryan Kegel Date: Wed, 19 Aug 2026 10:01:25 -0400 Subject: [PATCH] fix: address shape mounting issues in StkRigAdapter - Implement anisotropic scaling for shape mounting to prevent distortion. - Replace bounding-box midpoint anchors with joint-based anchors for correct rotation. - Reset node transforms to ensure clean scaling and rotation before mounting shapes. - Introduce new helper functions for computing bounding boxes, anchors, part lengths, and scales. - Neutralize driver rotations to maintain a consistent frame of reference during shape mounting. - Update documentation to reflect changes and provide detailed bugfix specifications. --- AGENTS.md | 20 +- BUGS.md | 9 + README.md | 4 +- docs/phase9_round1_bugfix_spec.md | 322 ++++++++++++++++++++++++++++++ scripts/stk_rig_adapter.gd | 166 ++++++++++++--- 5 files changed, 491 insertions(+), 30 deletions(-) create mode 100644 docs/phase9_round1_bugfix_spec.md diff --git a/AGENTS.md b/AGENTS.md index e9ca886..f52d48c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,15 +110,29 @@ assembled in a "Whole Stickman" preview that supports translation, rotation, and - `scripts/stk_rig_adapter.gd` — `class_name StkRigAdapter`, `extends RefCounted`; a **standalone runtime adapter** (Phase 8, **not referenced by the editor**, extended by Phase 9). `static func apply(stk_data, rig)` fits an instantiated `master_rig.tscn` to a loaded `.stk` - dictionary, calling three private helpers in order: `_fit_bones` (re-fits the 8 limb `Bone2D` + dictionary, calling four private helpers in order: `_fit_bones` (re-fits the 8 limb `Bone2D` lengths + lower-bone origins), `_recalibrate_ik` (repositions the `IK_Targets/Left|Right_Hand` - and `Left|Right_Leg` targets), and `_mount_shapes` (mounts `.stk` shapes onto the `Body/*` - visual nodes — open → `Line2D`, closed → `Polygon2D` fill + `Line2D` outline, width 16). + and `Left|Right_Leg` targets), `_neutralize_driver_rotations` (sets `update_rotation = false` + on the 10 `Body/*` `RemoteTransform2D` drivers so the `Body/*` nodes stay in the clean + unrotated frame the mount math assumes; position/scale pushes are retained), and + `_mount_shapes` (mounts `.stk` shapes onto the `Body/*` visual nodes — open → `Line2D`, + closed → `Polygon2D` fill + `Line2D` outline, width 16). - **Phase 9 extension:** also fits the head bone (`Skeleton2D/Torso/Head.position.y = -proportions.torso_length`, x preserved) and mounts the head as **full geometry** like every other part — it clears the `Body/Head` node's inline `@tool` circle script via `set_script(null)` and mounts `.stk` head shapes as `Line2D`/`Polygon2D`. Dead helpers `_mount_head_circle`, `_compute_shapes_bbox`, and `_first_shape_color` were removed. + - **Phase 9 Round 1 bugfix (mount math):** `_mount_shapes()` no longer reads the file's + `pivot`/`length` fields — it recomputes a per-part bounding box at mount time via + `_compute_part_bbox()` (empty bbox → the part is skipped). `_compute_anchor()` derives a + **joint-based anchor** per part family: head → bottom-center `(cx, max_y)`; torso + legs → + top-center `(cx, min_y)`; left arms → `(max_x, cy)`; right arms → `(min_x, cy)`. Scaling is + **anisotropic** via `_compute_scale()`, applied as a `Vector2`: arms scale X only + `(bone_length/part_length, 1.0)`, legs/torso scale Y only `(1.0, bone_length/part_length)`, + head unscaled `(1.0, 1.0)`, with a `part_length <= 0` guard → `1.0`. `_bone_length_for()` + maps each part to its bone length (upper/lower arm/leg, torso); `X_AXIS_PARTS` const + identifies the four arm keys. `_reset_node_transform()` resets each `Body/*` container's + scale to `(1, 1)` and rotation to `0` (position untouched, owned by the driver). Targets `master_rig.tscn` node paths; every node lookup is null-guarded (missing node → `push_warning` + skip, never crash). Consumed by a future runtime pipeline. - `scripts/stickman_factory.gd` — `class_name StickmanFactory`, `extends RefCounted`; a **static diff --git a/BUGS.md b/BUGS.md index cf5c016..5d9e2f8 100644 --- a/BUGS.md +++ b/BUGS.md @@ -66,3 +66,12 @@ When it is hidden it should say "Show Pose Guide" ### Guide visibility The guide and joints should slightly 'ghost' in front of the objects so that the use can see how well the objects are aligned to the joints and limbs. + +## Stickman editor (Phase 9 Round 1) + +> FIXED — 2026 Round 1: implemented per docs/phase9_round1_bugfix_spec.md; anisotropic scaling, joint-based anchors, and node/driver transform neutralization; verified with a 37-assertion headless smoke test. + +### Fix Shape Mount Math & Point Scaling in StkRigAdapter.gd + +Problem Summary: +When applying .stk v1.4 data to master*rig.tscn, shapes are severely distorted:Giant Polygon Explosions (Legs): Uniformly scaling points using scale_factor = bone_length / part_length on both $X$ and $Y$ multiplies the shape's thickness, turning thin leg segments into screen-filling blocks.Misaligned Joint Rotations (Head/Torso): Using (min + max) / 2 sets the pivot to the geometric center of the shape instead of its joint connection (e.g., neck base or hip joint).Required Fixes in StkRigAdapter.gd (or ActorFactory.gd):Please refactor \_mount_shapes() / point calculation logic using the following rules:1. Anisotropic Scaling (Primary Axis Only)Do not apply scale_factor to both axes. Only scale points along the bone's primary directional axis; leave cross-axis thickness at a $1.0$ scale multiplier:Arms (Primary Axis: $X$):$$\text{point}*{\text{local}}.x = (P*x - \text{anchor}\_x) \times \left(\frac{\text{upper_arm_length}}{\text{part_length}}\right)$$$$\text{point}*{\text{local}}.y = P*y - \text{anchor}\_y$$Legs / Torso (Primary Axis: $Y$):$$\text{point}*{\text{local}}.x = P*x - \text{anchor}\_x$$$$\text{point}*{\text{local}}.y = (P*y - \text{anchor}\_y) \times \left(\frac{\text{upper_leg_length}}{\text{part_length}}\right)$$Head: Keep unscaled ($1.0$ factor) on both axes:$$\text{point}*{\text{local}} = P - \text{anchor}$$(Guard against division by zero if part_length <= 0 by defaulting the scale factor to 1.0.)2. Joint-Based Anchor Alignment (Not BBox Center)Replace bounding-box midpoint anchors with joint origins so shapes rotate correctly around the bone joints:Head Anchor: Bottom-center of bounding box (x = (min_x + max_x) / 2, y = max_y).Torso & Legs Anchor: Top-center of bounding box (x = (min_x + max_x) / 2, y = min_y).Arms Anchor: Joint-end connection (x = min_x for right arms, x = max_x for left arms, y = (min_y + max_y) / 2).3. Node Transform ResetEnsure target Body/\* container nodes have their local scale reset to Vector2(1, 1) and rotation = 0 so Godot's node hierarchy does not multiply the geometry scaling a second time.Deliverable:Please update StkRigAdapter.gd to implement these corrected coordinate transform calculations and return the full updated GDScript. diff --git a/README.md b/README.md index 199cce1..9580597 100644 --- a/README.md +++ b/README.md @@ -397,7 +397,7 @@ Behavior: | `res://scenes/stickman_editor.tscn` | **Main scene** — editor layout, File/Edit/View menu bar, dialogs (`GridConfigDialog` + SpinBox), column containers (unique-name nodes). | | `res://scenes/body_part_panel.tscn` | Reusable single body-part editor panel (title, drawing area, context menu, `ColorPickerPopup`); expands vertically in its column. | | `res://scripts/stickman_editor.gd` | Editor controller — File/Edit/View menu actions, save/load/clear, JSON v1.4 serialization with multi-shape/rotation/scale, `part_order`, and Phase 8 `proportions`/`pivot`/`length`, `settings.json` load/save, editor-wide shape clipboard (Copy/Paste across panels), broadcast of grid/snap settings to panels, Reset Views, populates panels, coordinates selection across panels. | -| `res://scripts/stk_rig_adapter.gd` | **Phase 8, extended by Phase 9.** Standalone runtime adapter (`class_name StkRigAdapter`, `static func apply(stk_data, rig)`): fits an instantiated `master_rig.tscn` to a loaded `.stk` by re-fitting the 8 limb bones (`Skeleton2D/Torso/...` `Bone2D` lengths + lower-bone origins), recalibrating the IK targets (`IK_Targets/Left|Right_Hand`, `Left|Right_Leg`), mounting the `.stk` shapes onto the `Body/*` visual nodes (open shapes → `Line2D`, closed → `Polygon2D` fill + `Line2D` outline, width 16), and (Phase 9) fitting the head bone (`Head.position.y = -proportions.torso_length`) while mounting the head as **full geometry** — it clears the head's inline `@tool` circle script and mounts `.stk` head shapes as `Line2D`/`Polygon2D`. **Not used by the editor** — consumed by the runtime pipeline. | +| `res://scripts/stk_rig_adapter.gd` | **Phase 8, extended by Phase 9 (Round 1 bugfix).** Standalone runtime adapter (`class_name StkRigAdapter`, `static func apply(stk_data, rig)`): fits an instantiated `master_rig.tscn` to a loaded `.stk` by re-fitting the 8 limb bones (`Skeleton2D/Torso/...` `Bone2D` lengths + lower-bone origins), recalibrating the IK targets (`IK_Targets/Left|Right_Hand`, `Left|Right_Leg`), neutralizing the `RemoteTransform2D` driver rotations (`update_rotation = false` on the 10 `Body/*` drivers), and mounting the `.stk` shapes onto the `Body/*` visual nodes (open shapes → `Line2D`, closed → `Polygon2D` fill + `Line2D` outline, width 16). Shape mounting recomputes each part's bounding box at mount time (file `pivot`/`length` are no longer trusted), derives **joint-based anchors** per part family (head bottom-center, torso/legs top-center, arms shoulder-end), applies **anisotropic scaling** along the bone's primary axis only (arms X, legs/torso Y; head unscaled; divide-by-zero guard → `1.0`), and resets each `Body/*` container's scale to `(1,1)` / rotation `0` (position untouched). (Phase 9) also fits the head bone (`Head.position.y = -proportions.torso_length`) while mounting the head as **full geometry** — it clears the head's inline `@tool` circle script and mounts `.stk` head shapes as `Line2D`/`Polygon2D`. **Not used by the editor** — consumed by the runtime pipeline. | | `res://scripts/stickman_factory.gd` | **Phase 9.** Runtime entry point (`class_name StickmanFactory`, `extends RefCounted`); a static factory that turns a `.stk` file into a live, rigged `master_rig.tscn` instance. `load_stk(path)` reads + parses the file (`{}` + `push_warning` on failure); `spawn_from_data(stk_data)` instantiates `res://master_rig.tscn` and calls `StkRigAdapter.apply(stk_data, rig)`; `spawn(path)` chains them (`null` on empty data). **Not used by the editor.** | | `res://scripts/test_harness.gd` | **Phase 9.** Standalone staging scene (run via **F6** on `res://scenes/test_harness.tscn`, not wired into the editor) for debugging bone scales, vector-drawing offsets, and IK limits in isolation. Top UI bar: "Open .stk…" / quick-select buttons (`stickmen/break.stk`, `stickmen/basic.stk`, `stickmen/test.stk`), "Show Bones" / "Show IK Handles" toggles, loaded-filename label. `SubViewport` world + enabled `Camera2D` (middle-mouse pan, wheel zoom, recenter on spawn); each load frees the previous rig and spawns a fresh one via `StickmanFactory.spawn()`. A world-space debug overlay draws bone lines and IK-target markers; the 4 limb `Marker2D` IK targets are click-draggable, flexing limbs live via `SkeletonModificationStack2D` TwoBoneIK (enabled after each spawn). | | `res://scenes/test_harness.tscn` | **Phase 9.** Standalone staging scene backing `scripts/test_harness.gd` (run via **F6**; not wired into the editor). | @@ -485,3 +485,5 @@ BodyPartPanel.shape_selected() ---(bound to part_name)---> stickman_editor > **Phase 8:** The `.stk` export evolved to **v1.4** with write-only metadata the editor never reads back. A top-level `proportions` object stores the 5 master-rig rest-pose bone lengths (`upper_arm_length` 168.0, `lower_arm_length` 200.0, `upper_leg_length` 200.0, `lower_leg_length` 200.0, `torso_length` 391.5) — hardcoded constants from `master_rig.tscn`, not measured from the user's shapes. Each `body_parts` entry gains `pivot` (local bounding-box center = rotation origin) and `length` (bbox extent along the segment axis: width for arms, height for torso/legs/head). v1.0–v1.3 files load unchanged and gain these keys on their next save. A new standalone `res://scripts/stk_rig_adapter.gd` (`class_name StkRigAdapter`) fits an instantiated `master_rig.tscn` to a loaded `.stk` (bone fitting + IK recalibration + visual shape mount); it is **not** used by the editor and is reserved for a future runtime pipeline. > **Phase 9:** Adds the **runtime pipeline** for turning a `.stk` file into a live, rigged `master_rig.tscn` instance, plus a standalone staging scene to debug it. A new `res://scripts/stickman_factory.gd` (`class_name StickmanFactory`) provides the runtime entry point: `load_stk(path)` reads/parses a `.stk` (`FileAccess` + `JSON.parse_string`, `{}` + `push_warning` on failure), `spawn_from_data(stk_data)` instantiates `master_rig.tscn` and applies `StkRigAdapter.apply(stk_data, rig)`, and `spawn(path)` chains them (`null` on empty data). `StkRigAdapter` is extended to also fit the head bone (`Head.position.y = -proportions.torso_length`) and to mount the head as **full geometry** like every other part — clearing its inline `@tool` circle script and mounting `.stk` head shapes as `Line2D`/`Polygon2D` (removed the dead `_mount_head_circle`, `_compute_shapes_bbox`, and `_first_shape_color` helpers). A new standalone scene/resource pair, `res://scenes/test_harness.tscn` + `res://scripts/test_harness.gd` (run via **F6**), loads `.stk` files (open dialog + quick-select for `stickmen/break.stk`, `stickmen/basic.stk`, `stickmen/test.stk`), toggles a world-space debug overlay (bone lines + IK-target markers), pans/zooms a `Camera2D`, and lets you click-drag the 4 limb IK targets with live TwoBoneIK flexing (`SkeletonModificationStack2D` enabled on spawn). Neither the factory nor the harness is wired into the editor. **No `.stk` format change** — `FILE_VERSION` stays `"1.4"`. + +> **Phase 9 Round 1 bugfix:** `StkRigAdapter._mount_shapes()` no longer trusts the file's `pivot`/`length`; it recomputes each part's bounding box at mount time and applies **joint-based anchors** (head bottom-center, torso/legs top-center, arms shoulder-end) with **anisotropic scaling** along the bone's primary axis only (arms X, legs/torso Y, head unscaled, divide-by-zero guard → `1.0`). `Body/*` container transforms are reset (scale `(1,1)`, rotation `0`, position untouched) and `_neutralize_driver_rotations()` sets `update_rotation = false` on the 10 `RemoteTransform2D` drivers before mounting. Per docs/phase9_round1_bugfix_spec.md; verified with a 37-assertion headless smoke test. diff --git a/docs/phase9_round1_bugfix_spec.md b/docs/phase9_round1_bugfix_spec.md new file mode 100644 index 0000000..7edfdc0 --- /dev/null +++ b/docs/phase9_round1_bugfix_spec.md @@ -0,0 +1,322 @@ +# Phase 9 Round 1 — Bugfix: Shape Mount Math & Point Scaling in `StkRigAdapter.gd` + +## Overview + +The runtime adapter `scripts/stk_rig_adapter.gd` mounts `.stk` v1.4 vector shapes onto +`master_rig.tscn`'s `Body/*` visual nodes. Two defects make the mounted result unusable: + +1. **Giant Polygon Explosions (legs)** — `_mount_shapes()` computes a single **uniform** + `scale_factor = bone_length / part_length` and applies it to **both** X and Y in + `_transform_points()`. This multiplies the shape's *thickness* (cross-axis extent) as well + as its *length*, turning thin leg segments into screen-filling blocks. +2. **Misaligned Joint Rotations (head/torso)** — the anchor is the file's `pivot` field, which + is the bounding-box **center** `(min+max)/2`, not the joint connection (neck base / hip). + Shapes therefore rotate around their geometric center instead of the bone joint. + +The fix is confined to `scripts/stk_rig_adapter.gd` (the file the bug's "ActorFactory.gd" +reference actually means; the factory `stickman_factory.gd` only calls it). It introduces +**anisotropic scaling** (scale only the bone's primary axis), **joint-based anchors** (computed +from the part bbox at mount time, not the file `pivot`), and a **node-transform reset** on the +`Body/*` containers. No `.stk` format change, no editor change, no `.tscn` change. + +### Scope + +- **Changed:** `scripts/stk_rig_adapter.gd` (the mount pipeline only — `_mount_shapes`, + `_mount_shape`, `_transform_points`, plus new bbox/anchor/scale helpers). +- **Unchanged:** `_fit_bones()`, `_recalibrate_ik()`, `_bone_length_for()`, all node-path + constants, `stickman_factory.gd`, `test_harness.gd`, `master_rig.tscn`. +- **Not touched:** the editor (`stickman_editor.gd`) keeps writing `pivot`/`length` exactly as + today — the adapter simply stops *trusting* `pivot` and `length` for anchoring/scaling. + +--- + +## 1. Current behavior vs required behavior + +### 1a. Current mount pipeline (`stk_rig_adapter.gd`) + +| Step | Current code | Problem | +|---|---|---| +| Read per-part data | `_mount_shapes()` reads `body_parts[part].pivot` (lines 189–192) and `.length` (line 193) | `pivot` is bbox **center** → wrong rotation origin | +| Scale factor | `scale_factor = bone_length / part_length`, uniform float (lines 195–198) | applied to **both** axes → thickness explosion | +| Transform | `_transform_points()` does `(pt - pivot) * scale_factor` (lines 258–271) | uniform, center-anchored | +| Node state | `_mount_shapes()` clears geometry but never resets the node's own `scale`/`rotation` | authored transforms (`rotation = -π` on `Body/Body`, near-unit `scale` noise on several nodes) persist | + +### 1b. Required behavior (bug report, verbatim) + +1. **Anisotropic Scaling (Primary Axis Only)** — scale only the bone's primary directional axis; + cross-axis multiplier stays `1.0`. Guard `part_length <= 0 → scale 1.0`. +2. **Joint-Based Anchor Alignment** — replace bbox-midpoint anchors with joint origins: + - Head: bottom-center `( (min_x+max_x)/2, max_y )` + - Torso & legs: top-center `( (min_x+max_x)/2, min_y )` + - Arms: joint-end connection `( x = min_x for right arms, x = max_x for left arms, y = (min_y+max_y)/2 )` +3. **Node Transform Reset** — target `Body/*` container nodes must have local `scale = Vector2(1,1)` + and `rotation = 0` so the node hierarchy doesn't multiply geometry scaling a second time. + +--- + +## 2. Precise algorithm per part family + +All anchors and extents are computed **per part over the bbox of all of that part's shapes** +(multi-shape parts are treated as one unit with one joint connection). If the part has no +shape points (empty bbox), mount **no** geometry (the node is left empty after reset) — the +current empty-part behavior already skips the shape loop; this is preserved. + +Let `bbox = {min_x, min_y, max_x, max_y}` over every point of every shape in the part, and +`cy = (min_y + max_y) / 2`. + +| Part family | Parts | Primary axis | Anchor (local) | `part_length` (extent) | Scale `(sx, sy)` | `bone_length` source | +|---|---|---|---|---|---|---| +| Head | `head` | — (unscaled) | `( (min_x+max_x)/2, max_y )` bottom-center | — | `(1.0, 1.0)` | `1.0` (unused) | +| Torso | `torso` | **Y** | `( (min_x+max_x)/2, min_y )` top-center | `max_y - min_y` | `(1.0, torso_length/part_length)` | `torso_length` | +| Upper arms | `left_upper_arm`, `right_upper_arm` | **X** | left `(max_x, cy)` · right `(min_x, cy)` | `max_x - min_x` | `(upper_arm_length/part_length, 1.0)` | `upper_arm_length` | +| Lower arms | `left_lower_arm`, `right_lower_arm` | **X** | left `(max_x, cy)` · right `(min_x, cy)` | `max_x - min_x` | `(lower_arm_length/part_length, 1.0)` | `lower_arm_length` | +| Upper legs | `left_upper_leg`, `right_upper_leg` | **Y** | `( (min_x+max_x)/2, min_y )` top-center | `max_y - min_y` | `(1.0, upper_leg_length/part_length)` | `upper_leg_length` | +| Lower legs | `left_lower_leg`, `right_lower_leg` | **Y** | `( (min_x+max_x)/2, min_y )` top-center | `max_y - min_y` | `(1.0, lower_leg_length/part_length)` | `lower_leg_length` | + +**Point transform** (replaces the uniform multiply at `stk_rig_adapter.gd:265/267/270`): + +``` +pt_local = Vector2( (P.x - anchor.x) * sx, (P.y - anchor.y) * sy ) +``` + +### 2a. Arm anchor direction (verified) + +The rig's arms extend **outward** from the torso: `LeftUpperArm` bone `bone_angle = -180` +(points −X, `master_rig.tscn:174`), `RightUpperArm` bone `bone_angle = 0` (points +X, +`master_rig.tscn:199`). The shoulder joint is therefore the **inner** end of each drawn arm: + +- **Left arms** (extend leftward): shoulder at the **right** end → anchor `x = max_x`. +- **Right arms** (extend rightward): shoulder at the **left** end → anchor `x = min_x`. + +This matches the bug report exactly. (Assumes the user drew the arm with the shoulder at the +torso-facing end — see §8 Q3.) + +--- + +## 3. Function-level change list (`scripts/stk_rig_adapter.gd`) + +### 3a. `_mount_shapes()` (line 166) — rewrite the per-part loop + +For each `part_name` in `PART_KEYS`: + +1. Resolve `visual` (unchanged null-guard, line 175). +2. **Reset the node transform** (new): `visual.scale = Vector2.ONE`, `visual.rotation = 0.0`. + Leave `position` untouched (the `RemoteTransform2D` driver sets it; see §6). +3. Head special case (unchanged): `visual.set_script(null)` (line 203–204). +4. Clear geometry (unchanged): `_reset_own_geometry(visual)` + `_clear_visual_children(visual)`. +5. Read `shapes` (unchanged, lines 180–188). **Stop reading** `pivot` (lines 189–192) and + `length` (line 193) — both are now recomputed. +6. Compute `bbox = _compute_part_bbox(shapes)`. If empty → `continue` (no geometry). +7. `anchor = _compute_anchor(bbox, part_name)` (§2). +8. `part_length = _compute_part_length(bbox, part_name)` (§2). +9. `bone_length = _bone_length_for(part_name, proportions)` (unchanged helper, line 214). +10. `scale = _compute_scale(part_name, part_length, bone_length)` (§2). +11. `for shape in shapes: _mount_shape(visual, shape, anchor, scale)`. + +### 3b. New helper `_compute_part_bbox(shapes: Array) -> Dictionary` + +Returns `{ "min_x", "min_y", "max_x", "max_y" }` over all points of all shapes, or an +`is_empty` flag (e.g. `min_x > max_x`). Mirrors the editor's `_compute_part_pivot_length()` +(`stickman_editor.gd:376-403`) but returns raw bounds instead of center/length. Handles both +`{x,y}` dictionaries and `Vector2` points (same tolerance as `_transform_points` today). + +### 3c. New helper `_compute_anchor(bbox: Dictionary, part_name: String) -> Vector2` + +Implements the §2 anchor table: + +```gdscript +var cx := (bbox.min_x + bbox.max_x) * 0.5 +var cy := (bbox.min_y + bbox.max_y) * 0.5 +match part_name: + "head": + return Vector2(cx, bbox.max_y) # neck base + "left_upper_arm", "left_lower_arm": + return Vector2(bbox.max_x, cy) # shoulder at right end + "right_upper_arm", "right_lower_arm": + return Vector2(bbox.min_x, cy) # shoulder at left end + _: # torso + all legs + return Vector2(cx, bbox.min_y) # hip/neck top-center +``` + +### 3d. New helper `_compute_part_length(bbox: Dictionary, part_name: String) -> float` + +`max_x - min_x` for the four arm keys, else `max_y - min_y` (matches the editor's +`X_AXIS_PARTS` convention, `stickman_editor.gd:399-402`). Add a `const X_AXIS_PARTS: +PackedStringArray` to the adapter mirroring the editor's (`stickman_editor.gd:65`). + +### 3e. New helper `_compute_scale(part_name: String, part_length: float, bone_length: float) -> Vector2` + +```gdscript +var primary := 1.0 +if part_name != "head" and part_length > 0.0001: + primary = bone_length / part_length +if X_AXIS_PARTS.has(part_name): + return Vector2(primary, 1.0) # arms: X is primary +return Vector2(1.0, primary) # legs/torso/head: Y is primary (head → (1.0, 1.0)) +``` + +`part_length <= 0` → `primary = 1.0` (the bug's divide-by-zero guard). + +### 3f. `_mount_shape()` (line 230) — signature change + +`_mount_shape(visual: Node, shape: Dictionary, anchor: Vector2, scale: Vector2) -> void` +— replaces the `scale_factor: float` parameter. Everything else (open→`Line2D`, +closed→`Polygon2D` fill + `Line2D` outline, `DEFAULT_LINE_WIDTH = 16.0`, color via +`Color.from_string`) is unchanged. + +### 3g. `_transform_points()` (line 258) — signature + math change + +`_transform_points(pts_var: Variant, anchor: Vector2, scale: Vector2) -> PackedVector2Array` +— per point: + +```gdscript +out.append(Vector2((pt.x - anchor.x) * scale.x, (pt.y - anchor.y) * scale.y)) +``` + +### 3h. New helper `_reset_node_transform(visual: Node) -> void` + +```gdscript +if visual is Node2D: + (visual as Node2D).scale = Vector2.ONE + (visual as Node2D).rotation = 0.0 +``` + +`Body/Head` is a `Node2D` (not `Line2D`/`Polygon2D`) so the `Node2D` check is required — +`_reset_own_geometry()` (line 274) only handles `Line2D`/`Polygon2D` and is left unchanged. + +### 3i. New helper `_neutralize_driver_rotations(rig: Node2D) -> void` (resolved Q1) + +Called from `apply()` before `_mount_shapes()`. For each `Body/*` visual node, find the +`RemoteTransform2D` that drives it (fixed node paths, mirroring the `master_rig.tscn` +structure) and set `update_rotation = false` so the `Body/*` nodes stay in the clean +unrotated frame the mount math assumes. Drivers keep pushing position/scale. All lookups +null-guarded (warning + skip) like every other adapter helper. + +--- + +## 4. Edge cases + +- **Empty part** (no shapes, or all shapes `< 2` points): bbox empty → skip mounting; node is + left geometry-cleared and transform-reset. Never divide by zero (`part_length <= 0 → primary 1.0`). +- **Multi-shape part**: anchor + `part_length` computed over the **union** of all shapes' points + (the part is one unit). A shape with `< 2` points after transform is skipped (existing + `_mount_shape` guard, line 232). +- **`part_length == 0` / `bone_length == 0`**: primary scale falls back to `1.0` (translation-only). +- **Missing `body_parts` / missing `proportions`**: existing guards unchanged — `body_parts` + invalid → whole mount no-ops with a warning (lines 167–170); `proportions` absent → + `_bone_length_for()` uses `DEFAULT_PROPORTIONS` (lines 214–227). +- **`pivot` / `length` absent from a v1.0–v1.3 file**: irrelevant now — the adapter no longer + reads them; bbox is recomputed from `shapes`, so old files mount identically. +- **`RightUpperLeg.length = 90.0`** (`master_rig.tscn:245`): unaffected — `_fit_bones()` already + overwrites both legs from `proportions` (line 106/109) before `_mount_shapes()` runs. +- **Negative/mirrored part `scale`** in `.stk`: the adapter does not consume part + `scale`/`rotation`/`position` (unchanged from Phase 8/9), so mirroring does not affect the + bbox-based anchor/scale. + +--- + +## 5. Test plan + +No automated test suite exists (no `test/` directory; the phase8_spec §8 smoke test is +aspirational and was never added as a file). Verification is manual + parse check. + +1. **Parse check** (per project convention, from `C:\Godot4\stickman`): + `..\Godot_v4.7.1-stable_win64_console.exe . --headless --check-only --quit` +2. **Harness visual check** (F6 on `res://scenes/test_harness.tscn`), load each of + `stickmen/basic.stk`, `stickmen/test.stk`, `stickmen/break.stk`: + - **Legs** are thin (no block explosion) — cross-axis thickness is preserved at drawn size; + only the long axis stretches to the bone length. + - **Head** is unscaled (identical to drawn size), anchored at the neck base. + - **Torso** scales along Y only; no horizontal blow-up. + - **Arms** scale along X only; shoulder end sits at the joint. + - No crash on a part with no shapes. +3. **Regression**: `_fit_bones()` / `_recalibrate_ik()` behavior unchanged — bone lengths and + IK-target positions identical to before (assert visually via "Show Bones" / "Show IK Handles"). +4. **Old-file compatibility**: `basic.stk` (v1.0) and `test.stk` (v1.1) still mount (they have + no `pivot`/`length` — confirms the recompute-from-bbox path). + +--- + +## 6. RemoteTransform2D interaction (critical finding) + +Every `Body/*` visual node is driven by a `RemoteTransform2D` under the matching bone +(`master_rig.tscn:160-162` head, `:184-191` left arm, `:209-215` right arm, `:233-239` left +leg, `:256-262` right leg, `:264-266` torso). **None** of these set `use_global_coordinates` or +any `update_remote_*` flag, so Godot defaults apply: `use_global_coordinates = false`, and +`update_remote_position/rotation/scale = true`. The drivers therefore push their **local** +`position`, `rotation`, **and** `scale` onto the `Body/*` node every internal-process frame. + +Consequences for fix #3: + +- The authored `scale`/`rotation` on the `Body/*` nodes are **overwritten at runtime** by the + driver (which itself carries `scale = (1,1)` and a rest-pose `rotation`: `π` for the torso, + `±π/2` for the arms, `-π/2` for the lower legs/arms, `0` for the upper legs/head). Resetting + the `Body/*` node's `scale`/`rotation` (fix #3) is therefore a **defensive normalization** of + the authored values (guarantees a clean frame in the editor and on the pre-tree-entry frame); + it does **not** by itself change the rendered orientation, because the driver re-applies its + own rotation. +- The bug's "primary-axis" convention (arms = X/horizontal, legs/torso = Y/vertical) is stated + in a **clean unrotated frame**. The rig's driver rotations are what orient the default + (vertical-authored) limbs. This is the one place where the bug's rules and the rig's + `RemoteTransform2D` setup may not fully reconcile — see §8 Q1. + +The mount pipeline does **not** touch the `RemoteTransform2D` drivers (out of the bug's literal +scope). The recommendation is to implement the bug as written, then confirm orientation in the +harness and resolve Q1 if limbs render rotated. + +--- + +## 7. Design decisions (summary) + +| # | Decision | One-line justification | +|---|---|---| +| D1 | Anchor + `part_length` computed **per part over all shapes' bbox**, not per shape | The part is one unit with one joint connection; all shapes must share a single pivot/scale so they rotate coherently. | +| D2 | Recompute bbox at mount time; **stop trusting** file `pivot` and `length` | Single source of truth; also robust for v1.0–v1.3 files that lack both fields. | +| D3 | Keep `_bone_length_for()` (upper→upper_arm, lower→lower_arm, etc.) | Correct per-part bone length; the bug's literal "upper_arm_length" for all arms is a shorthand for "the arm's bone length." | +| D4 | Scale as a `Vector2 (sx, sy)` with cross-axis `1.0` | Directly implements anisotropic scaling and replaces the uniform `float` scale_factor. | +| D5 | Reset `Body/*` `scale=(1,1)` + `rotation=0`, leave `position` | Matches the bug's "scale + rotation only"; position is owned by the `RemoteTransform2D` driver. | +| D6 | Head keeps `set_script(null)`, anchor bottom-center, scale `(1.0,1.0)` | Preserves the Phase 9 full-geometry head path; head is a circle (not a bone-length segment) so it stays unscaled. | +| D7 | `DEFAULT_LINE_WIDTH` stays `16.0` | `.stk` stores no width; `16.0` matches the rig's authored `Line2D` width. | +| D8 | No change to `stickman_factory.gd` / `test_harness.gd` / editor | The fix is internal to the adapter's public `apply()` contract, which they already call unchanged. | +| D9 | Neutralize the 10 `Body/*` `RemoteTransform2D` rotations via `update_rotation = false` (resolved Q1; Godot 4 property name, verified by the Tester) | Keeps the `Body/*` nodes in the clean unrotated frame the bug's primary-axis math assumes; position/scale pushes are preserved. | + +--- + +## 8. Open questions — RESOLVED (user-approved) + +1. **RemoteTransform2D rotation vs. the "primary-axis" convention (§6).** ✅ **Neutralize driver rotation**: set `update_rotation = false` on all 10 `Body/*` `RemoteTransform2D` drivers (a new helper `_neutralize_driver_rotations(rig)` in the adapter) so the `Body/*` nodes stay in the clean unrotated frame the bug's math assumes; the drivers keep pushing position (and scale). +2. **Torso anchor: top-center vs. the hip-driven `Body/Body` node.** ✅ **Top-center per the bug** (neck). If the harness shows the torso upside-down, flip to bottom-center in `_compute_anchor()` (one line). +3. **Arm anchor assumes the shoulder is drawn at the inner end.** ✅ **Shoulder-inward** — left arms extend leftward (`anchor.x = max_x`), right arms extend rightward (`anchor.x = min_x`). + +--- + +## 9. Files modified + +| File | Change | +|---|---| +| `scripts/stk_rig_adapter.gd` | Rewrite `_mount_shapes()` per §3a; add `_compute_part_bbox()`, `_compute_anchor()`, `_compute_part_length()`, `_compute_scale()`, `_reset_node_transform()`, `_neutralize_driver_rotations()`; change `_mount_shape()` and `_transform_points()` signatures to `anchor: Vector2, scale: Vector2`; add `X_AXIS_PARTS` const; call `_neutralize_driver_rotations()` from `apply()`. | +| `docs/phase9_round1_bugfix_spec.md` | This file. | + +No changes to `stickman_factory.gd`, `test_harness.gd`, `stickman_editor.gd`, `master_rig.tscn`, +`master_rig2.tscn`, `clear_pose.gd`, `master_rig_builder.gd`, or the `.stk` files. + +**Adjacent files checked, no impact:** +- `master_rig2.tscn` — node-for-node mirror of `master_rig.tscn` (same `Body/*` + `RemoteTransform2D` + layout, `master_rig2.tscn:72-286`); adapter targets `master_rig.tscn`, and the fix applies to + either since node names/paths are identical. +- `clear_pose.gd` — an `EditorScript` that resets bone `scale`/`rest` on the *edited* scene; not + consumed by the runtime adapter. +- `scripts/master_rig_builder.gd` — builds a **different** rig (`Sticky/Stickman/.../Hip` naming), + unrelated to `master_rig.tscn`'s `Master/Body/Skeleton2D/Torso` naming; not an adapter target. + +--- + +## 10. Recommended implementation order + +1. `scripts/stk_rig_adapter.gd` — add `X_AXIS_PARTS` const + `_compute_part_bbox()`. +2. `scripts/stk_rig_adapter.gd` — add `_compute_anchor()`, `_compute_part_length()`, + `_compute_scale()`, `_reset_node_transform()`, `_neutralize_driver_rotations()`. +3. `scripts/stk_rig_adapter.gd` — rewrite `_mount_shapes()`; change `_mount_shape()` / + `_transform_points()` signatures and point math; wire `_neutralize_driver_rotations()` into + `apply()`. +4. Parse check + harness visual verification (§5); flip the torso anchor if it renders upside-down. diff --git a/scripts/stk_rig_adapter.gd b/scripts/stk_rig_adapter.gd index 030dd5f..57f19bd 100644 --- a/scripts/stk_rig_adapter.gd +++ b/scripts/stk_rig_adapter.gd @@ -30,6 +30,11 @@ const PART_KEYS: PackedStringArray = [ "right_upper_leg", "right_lower_leg", ] +## Parts whose primary (bone-aligned) axis is horizontal (X). +const X_AXIS_PARTS: PackedStringArray = [ + "left_upper_arm", "left_lower_arm", "right_upper_arm", "right_lower_arm", +] + ## Bone node paths (relative to rig root), keyed by part name. const BONE_PATHS: Dictionary = { "left_upper_arm": "Skeleton2D/Torso/LeftUpperArm", @@ -56,6 +61,24 @@ const BODY_PATHS: Dictionary = { "right_lower_leg": "Body/RightLowerLeg", } +## RemoteTransform2D driver node paths (relative to rig root), keyed by part +## name. Each driver pushes its transform onto the matching Body/* visual node +## (see master_rig.tscn). We neutralize their rotation so the Body/* nodes stay +## in the clean unrotated frame the mount math assumes (position/scale pushes +## are preserved). +const DRIVER_PATHS: Dictionary = { + "head": "Skeleton2D/Torso/Head/RemoteTransform2D", + "torso": "Skeleton2D/Torso/RemoteTransform2D", + "left_upper_arm": "Skeleton2D/Torso/LeftUpperArm/RemoteTransform2D", + "left_lower_arm": "Skeleton2D/Torso/LeftUpperArm/LeftLowerArm/RemoteTransform2D", + "right_upper_arm": "Skeleton2D/Torso/RightUpperArm/RemoteTransform2D", + "right_lower_arm": "Skeleton2D/Torso/RightUpperArm/RightLowerArm/RemoteTransform2D", + "left_upper_leg": "Skeleton2D/Torso/LeftUpperLeg/RemoteTransform2D", + "left_lower_leg": "Skeleton2D/Torso/LeftUpperLeg/LeftLowerLeg/RemoteTransform2D", + "right_upper_leg": "Skeleton2D/Torso/RightUpperLeg/RemoteTransform2D", + "right_lower_leg": "Skeleton2D/Torso/RightUpperLeg/RightLowerLeg/RemoteTransform2D", +} + const IK_LEFT_HAND := "IK_Targets/Left_Hand" const IK_RIGHT_HAND := "IK_Targets/Right_Hand" const IK_LEFT_LEG := "IK_Targets/Left_Leg" @@ -70,6 +93,7 @@ const HEAD_BONE_PATH := "Skeleton2D/Torso/Head" static func apply(stk_data: Dictionary, rig: Node2D) -> void: _fit_bones(stk_data, rig) _recalibrate_ik(stk_data, rig) + _neutralize_driver_rotations(rig) _mount_shapes(stk_data, rig) # --------------------------------------------------------------------------- @@ -159,6 +183,18 @@ static func _recalibrate_ik(stk_data: Dictionary, rig: Node2D) -> void: else: push_warning("StkRigAdapter: missing IK target '%s'." % IK_RIGHT_HAND) +# --------------------------------------------------------------------------- +# Driver neutralization +# --------------------------------------------------------------------------- + +static func _neutralize_driver_rotations(rig: Node2D) -> void: + for part_name: String in DRIVER_PATHS: + var driver := rig.get_node_or_null(NodePath(DRIVER_PATHS[part_name])) as RemoteTransform2D + if driver == null: + push_warning("StkRigAdapter: missing RemoteTransform2D driver '%s' for part '%s'; skipped." % [DRIVER_PATHS[part_name], part_name]) + continue + driver.update_rotation = false + # --------------------------------------------------------------------------- # Visual shape mount # --------------------------------------------------------------------------- @@ -177,25 +213,10 @@ static func _mount_shapes(stk_data: Dictionary, rig: Node2D) -> void: push_warning("StkRigAdapter: missing Body node for part '%s'; skipped." % part_name) continue - var shapes: Array = [] - var pivot := Vector2.ZERO - var part_length := 0.0 - var part_data: Variant = body_parts.get(part_name, {}) - if part_data is Dictionary: - var pd := part_data as Dictionary - var shapes_var: Variant = pd.get("shapes", []) - if shapes_var is Array: - shapes = shapes_var as Array - var pivot_var: Variant = pd.get("pivot", {}) - if pivot_var is Dictionary: - var pv := pivot_var as Dictionary - pivot = Vector2(float(pv.get("x", 0.0)), float(pv.get("y", 0.0))) - part_length = float(pd.get("length", 0.0)) - - var bone_length := _bone_length_for(part_name, proportions) - var scale_factor := 1.0 - if part_name != "head" and part_length > 0.0001: - scale_factor = bone_length / part_length + # Reset the node's authored transform so the mount math starts from a + # clean unrotated, unit-scaled frame. Position is owned by the + # RemoteTransform2D driver and is left untouched. + _reset_node_transform(visual) # Phase 9: the Body/Head node is a plain Node2D carrying an inline # @tool circle-drawing script; clear it so the head is mounted with the @@ -206,9 +227,28 @@ static func _mount_shapes(stk_data: Dictionary, rig: Node2D) -> void: _reset_own_geometry(visual) _clear_visual_children(visual) + var shapes: Array = [] + var part_data: Variant = body_parts.get(part_name, {}) + if part_data is Dictionary: + var pd := part_data as Dictionary + var shapes_var: Variant = pd.get("shapes", []) + if shapes_var is Array: + shapes = shapes_var as Array + + # Recompute the joint anchor and part length from the shape bbox at + # mount time (the file's pivot/length fields are write-only metadata + # and are no longer trusted for anchoring/scaling). + var bbox := _compute_part_bbox(shapes) + if float(bbox["min_x"]) > float(bbox["max_x"]) or float(bbox["min_y"]) > float(bbox["max_y"]): + continue + var anchor := _compute_anchor(bbox, part_name) + var part_length := _compute_part_length(bbox, part_name) + var bone_length := _bone_length_for(part_name, proportions) + var scale := _compute_scale(part_name, part_length, bone_length) + for shape in shapes: if shape is Dictionary: - _mount_shape(visual, shape as Dictionary, pivot, scale_factor) + _mount_shape(visual, shape as Dictionary, anchor, scale) static func _bone_length_for(part_name: String, proportions: Dictionary) -> float: @@ -227,8 +267,81 @@ static func _bone_length_for(part_name: String, proportions: Dictionary) -> floa return 1.0 -static func _mount_shape(visual: Node, shape: Dictionary, pivot: Vector2, scale_factor: float) -> void: - var pts := _transform_points(shape.get("points", []), pivot, scale_factor) +static func _compute_part_bbox(shapes: Array) -> Dictionary: + var min_x := INF + var min_y := INF + var max_x := -INF + var max_y := -INF + for shape in shapes: + if not shape is Dictionary: + continue + var pts_var: Variant = (shape as Dictionary).get("points", []) + if pts_var is Array: + for p in pts_var as Array: + if p is Dictionary: + var d := p as Dictionary + var pt := Vector2(float(d.get("x", 0.0)), float(d.get("y", 0.0))) + min_x = minf(min_x, pt.x) + min_y = minf(min_y, pt.y) + max_x = maxf(max_x, pt.x) + max_y = maxf(max_y, pt.y) + elif p is Vector2: + var pt := p as Vector2 + min_x = minf(min_x, pt.x) + min_y = minf(min_y, pt.y) + max_x = maxf(max_x, pt.x) + max_y = maxf(max_y, pt.y) + elif pts_var is PackedVector2Array: + for pt in pts_var as PackedVector2Array: + min_x = minf(min_x, pt.x) + min_y = minf(min_y, pt.y) + max_x = maxf(max_x, pt.x) + max_y = maxf(max_y, pt.y) + return { + "min_x": min_x, + "min_y": min_y, + "max_x": max_x, + "max_y": max_y, + } + + +static func _compute_anchor(bbox: Dictionary, part_name: String) -> Vector2: + var cx := (float(bbox["min_x"]) + float(bbox["max_x"])) * 0.5 + var cy := (float(bbox["min_y"]) + float(bbox["max_y"])) * 0.5 + match part_name: + "head": + return Vector2(cx, float(bbox["max_y"])) # neck base + "left_upper_arm", "left_lower_arm": + return Vector2(float(bbox["max_x"]), cy) # shoulder at right end + "right_upper_arm", "right_lower_arm": + return Vector2(float(bbox["min_x"]), cy) # shoulder at left end + _: # torso + all legs + return Vector2(cx, float(bbox["min_y"])) # hip/neck top-center + + +static func _compute_part_length(bbox: Dictionary, part_name: String) -> float: + if X_AXIS_PARTS.has(part_name): + return float(bbox["max_x"]) - float(bbox["min_x"]) + return float(bbox["max_y"]) - float(bbox["min_y"]) + + +static func _compute_scale(part_name: String, part_length: float, bone_length: float) -> Vector2: + var primary := 1.0 + if part_name != "head" and part_length > 0.0001: + primary = bone_length / part_length + if X_AXIS_PARTS.has(part_name): + return Vector2(primary, 1.0) # arms: X is primary + return Vector2(1.0, primary) # legs/torso/head: Y is primary (head → (1.0, 1.0)) + + +static func _reset_node_transform(visual: Node) -> void: + if visual is Node2D: + (visual as Node2D).scale = Vector2.ONE + (visual as Node2D).rotation = 0.0 + + +static func _mount_shape(visual: Node, shape: Dictionary, anchor: Vector2, scale: Vector2) -> void: + var pts := _transform_points(shape.get("points", []), anchor, scale) if pts.size() < 2: return @@ -255,19 +368,20 @@ static func _mount_shape(visual: Node, shape: Dictionary, pivot: Vector2, scale_ visual.add_child(line) -static func _transform_points(pts_var: Variant, pivot: Vector2, scale_factor: float) -> PackedVector2Array: +static func _transform_points(pts_var: Variant, anchor: Vector2, scale: Vector2) -> PackedVector2Array: var out := PackedVector2Array() if pts_var is Array: for p in pts_var as Array: if p is Dictionary: var d := p as Dictionary var pt := Vector2(float(d.get("x", 0.0)), float(d.get("y", 0.0))) - out.append((pt - pivot) * scale_factor) + out.append(Vector2((pt.x - anchor.x) * scale.x, (pt.y - anchor.y) * scale.y)) elif p is Vector2: - out.append(((p as Vector2) - pivot) * scale_factor) + var pt := p as Vector2 + out.append(Vector2((pt.x - anchor.x) * scale.x, (pt.y - anchor.y) * scale.y)) elif pts_var is PackedVector2Array: for pt in pts_var as PackedVector2Array: - out.append((pt - pivot) * scale_factor) + out.append(Vector2((pt.x - anchor.x) * scale.x, (pt.y - anchor.y) * scale.y)) return out