Files
stickman/docs/phase9_round1_bugfix_spec.md
T
ryan 6b273c049c 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.
2026-08-19 10:01:25 -04:00

19 KiB
Raw Blame History

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 189192) and .length (line 193) pivot is bbox center → wrong rotation origin
Scale factor scale_factor = bone_length / part_length, uniform float (lines 195198) applied to both axes → thickness explosion
Transform _transform_points() does (pt - pivot) * scale_factor (lines 258271) 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 203204).
  4. Clear geometry (unchanged): _reset_own_geometry(visual) + _clear_visual_children(visual).
  5. Read shapes (unchanged, lines 180188). Stop reading pivot (lines 189192) 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:

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

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 <= 0primary = 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:

out.append(Vector2((pt.x - anchor.x) * scale.x, (pt.y - anchor.y) * scale.y))

3h. New helper _reset_node_transform(visual: Node) -> void

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 167170); proportions absent → _bone_length_for() uses DEFAULT_PROPORTIONS (lines 214227).
  • pivot / length absent from a v1.0v1.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.0v1.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.

  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.