ryan/stickman
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: address shape mounting issues in StkRigAdapter

Browse Source 
- 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.
This commit is contained in:
ryan
2026-08-19 10:01:25 -04:00
parent 1252728c06
commit 6b273c049c
5 changed files with 491 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/phase9_round1_bugfix_spec.md
+322
View File
@@ -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.