stickman/specs/phase_4_final_bugs.md

# Phase 4 Final Bugs — Architecture Specification

Source: `plans/PHASE_4_FINAL_BUGS.md`

## Scope and resolved decisions

This phase fixes the eight reported Sandbox Stage defects without changing the `.stk` format,
persisting stages/rules, or changing the master rig scene.

The four open questions are resolved as follows:

1. **One overlay slot per rig.** Do not generalize speech to an overlay list. A newer overlay
   speak replaces the prior overlay immediately; replacement/cancellation does not emit the old
   speech's completion signals.
2. **Mixed rule actions execute in authored order.** For `[overlay speak, sequential walk]`, the
   bubble starts first, then the walk interrupts the current *primary* action. Sequential actions
   do not cancel overlays, so the speak and walk continue together. Only a Takeover cancels an
   overlay. This interpretation is required by the plan's “say and walk away” acceptance case.
3. **The ghost stays static.** A live ghost would create ten animated collision proxies,
   navigation state, IK work, and animation work for a disposable preview. Instead, initialize the
   placed rig's IK targets from the same pre-solve skeleton pose displayed by the static ghost.
4. **Prop freeze remains kinematic.** `StageGizmos` already routes both translation and rotation
   to `global_position` / `global_rotation`, and `FREEZE_MODE_KINEMATIC` is the correct editable
   physics state. The competing writer is the stage's three-frame `_restore_authored_state()`
   pass. An edit drag must cancel that deferred restore before its first transform write, and the
   final transform must be synchronized to both the Node2D and physics body before it is saved.

## Runtime data contracts

### Action roles

Add to `ActionRegistry`:

```gdscript
enum ReactiveRole { SEQUENTIAL, OVERLAY, TAKEOVER }

static func reactive_role(action: Dictionary) -> ReactiveRole
static func is_overlay_eligible(type: String) -> bool
```

`ACTION_TEMPLATES` gains typed semantic metadata:

```gdscript
"walk_to": { ..., "reactive_role": ReactiveRole.SEQUENTIAL, "overlay_eligible": false }
"wait":    { ..., "reactive_role": ReactiveRole.SEQUENTIAL, "overlay_eligible": false }
"speak":   { ..., "reactive_role": ReactiveRole.OVERLAY,    "overlay_eligible": true  }
"ragdoll": { ..., "reactive_role": ReactiveRole.TAKEOVER,   "overlay_eligible": false }
"recover": { ..., "reactive_role": ReactiveRole.TAKEOVER,   "overlay_eligible": false }
```

For a reactive `speak`, `params.stop_current == true` promotes the resolved role from OVERLAY to
SEQUENTIAL. Missing/false is OVERLAY. The flag is ignored for every other type. Authored queue
actions remain primary sequential actions; roles govern only actions injected by a rule.

The transient rule-action payload remains:

```gdscript
{
    "type": String,
    "target": int,
    "params": {
        # existing type-specific fields,
        "stop_current": bool, # speak only; default false
    },
}
```

`ActionRegistry.to_rule_action()` / `from_rule_action()` / `summarize()` preserve this flag for
speak. The summary adds a short suffix such as `— stops current` only when true. Queue speak
actions do not retain the flag.

### Primary execution lane

`StickmanRig.action_queue` stays the authored, editor-visible queue. Reactive commands must no
longer be appended to its tail. Add a private runtime deque:

```gdscript
var _run_queue: Array[Dictionary] = []
# item: { "action": Dictionary, "queue_index": int, "reactive": bool }
var _current_run_item: Dictionary = {}
var _run_cursor: int = 0
```

At `start_queue()`, rebuild `_run_queue` from deep duplicates of `action_queue`; authored items
carry their original `queue_index`, reactive items use `-1`. This snapshot prevents editor
mutation from invalidating a running cursor. Existing `action_started` / `action_finished`
signals keep their signatures; reactive actions emit index `-1`. Canceled actions emit neither
`action_finished` nor `arrived`/`speech_finished` as a cancellation side effect.

Keep the public name and signature used by the stage:

```gdscript
func enqueue_reactive(actions: Array[Dictionary]) -> void
func clear_reactive_actions() -> void
```

`enqueue_reactive()` processes one rig's rule actions in array order:

- **Overlay:** start/replace the overlay slot immediately; do not mutate `_run_queue`, cursor,
  primary phase, walk, wait timer, or current primary speech.
- **Sequential:** cancel the active primary action without completion signals, prepend this and
  subsequent same-batch primary actions to the runtime deque, and start the first immediately.
  Already pending authored actions remain in order; the canceled current action is abandoned,
  not restarted. After injected actions finish, execution continues at the next authored item.
- **Takeover:** cancel the active primary action, clear pending reactive primary items, cancel the
  overlay slot, prepend the Takeover, and start it immediately. Pending authored items are retained
  and remain paused until the Takeover phase resolves. `ragdoll` resolves at the existing
  `is_ragdoll_at_rest()` boundary; `recover` resolves when state returns to `ANIMATED` (and remains
  a safe immediate completion if invoked while already animated).

If a batch contains an earlier primary followed by a Takeover, the Takeover discards/cancels that
earlier primary per its role. Actions after the Takeover are retained in authored order. An
overlay after a Takeover may start alongside it; an overlay before it is canceled on Takeover
entry. Later matching rules are processed after earlier rules and therefore may supersede them.

`_cancel_current_primary()` owns phase cleanup:

```gdscript
func _cancel_current_primary() -> void
```

It stops walk/animation without emitting `arrived`, clears wait state, cancels a primary speech
without emitting `speech_finished`, resets `_action_phase`, and clears `_current_run_item`. It
does not touch authored/pending actions or the overlay slot. `_finish_action()` must first clear
the current phase/item, then emit its natural completion signals; this prevents an event handler
that injects a new action synchronously from being overwritten by stale post-signal cleanup.

`stop_queue()` cancels the primary lane and overlay, clears `_run_queue`, and returns to IDLE.
`clear_reactive_actions()` clears any remaining runtime reactive items and defensively removes
legacy `{reactive=true}` entries from `action_queue`; it is safe/idempotent after `stop_queue()`.
Thus the existing Edit-mode sequence still guarantees that no reactive work survives PLAY.

### Speech ownership

Keep one rendered `SpeechBubble`, but separate logical ownership:

```gdscript
var _primary_speech: Dictionary = {} # {text, time_left}; only while SPEAKING
var _overlay_speech: Dictionary = {} # {text, time_left}; at most one
```

The overlay is the visible owner while present. If it expires while a primary speech remains,
redraw the primary text for its remaining duration; otherwise hide. Both timers may advance, but
there is never more than one overlay or one visible bubble. A new overlay replaces only
`_overlay_speech`. A new primary speech replaces/cancels only the current primary through normal
primary-interrupt rules. Natural expiry emits `speech_finished` for that speech; replacement,
interrupt, mode exit, and Takeover cleanup emit no false completion event.

## Target files and changes

### `scripts/action_registry.gd` — Bug 1

- Add `ReactiveRole`, template metadata, `reactive_role()`, and `is_overlay_eligible()`.
- Preserve `params.stop_current` through rule/flat conversion for speak only.
- `summarize()` reads a Walk To waypoint from `params.target` (the top-level `target` is the
  actor instance id) and type-guards malformed/legacy values before formatting the Vector2.
- Make all missing metadata/default flags backward compatible: old rules load as overlay speak;
  unknown action types resolve as SEQUENTIAL and are still skipped safely by the runner.

### `scripts/action_editor.gd` — Bug 1

Change the opening API to tell the shared editor whether it is editing a rule consequence:

```gdscript
func open_new(show_reactive_options: bool = false) -> void
func open_edit(action: Dictionary, show_reactive_options: bool = false) -> void
```

Add `_stop_current_check: CheckBox`. Create it only in the speak parameter section when
`show_reactive_options` and `ActionRegistry.is_overlay_eligible("speak")` are true. Label it
exactly **“Stop what he's doing?”**, default OFF, and prefill from `action.stop_current`.
`_on_ok_pressed()` writes `stop_current` only in this context. No checkbox appears for queue
actions or any non-speak type.

### `scripts/sandbox_stage.gd` — Bugs 1, 3, 4, 7, 8

**Reactive dispatch and UI**

- `_execute_rule_actions()` groups rule actions by target rig while preserving each rig's action
  order, normalizes them with `_action_for_rig()`, then calls `rig.enqueue_reactive(batch)` once
  per rig. This prevents multiple sequential actions from interrupting one another merely because
  the stage delivered the batch one item at a time.
- `_action_for_rig()` copies `params.stop_current` for speak.
- Call `ActionEditor.open_new/open_edit(show_reactive_options)` with true only for `rule_add` /
  `rule_edit`.
- The legacy quick rule-builder speak dialog gets `_speak_stop_current_check: CheckBox` beneath
  its LineEdit. It is visible only when `_rule_step == RuleStep.PARAMS`, resets OFF for a new rule
  action, and writes `params.stop_current`. Direct queue Speak continues to hide/ignore it.

**Immediate mode-exit restoration**

- Preserve `_enter_edit_shared()` order: `stop_queue()` then `snap_to_standing()` for every rig,
  then restore authored root transforms.
- Set `_restore_frames_left = 0` before starting any gizmo translation/rotation drag. Deferred
  physics-frame restoration is a mode-entry safety net, never an edit-time competing writer.
- `snap_to_standing()` (specified below) is the sole rig pose-cleanup call; do not start a recovery
  or decay tween on EDIT/DIRECT entry.
- `_on_prop_collided()` returns immediately outside `StageMode.PLAY`; a contact delivered after
  STOP cannot recreate a flinch or dispatch a reactive rule.

**Prop manipulation**

- Keep props `freeze_mode = FREEZE_MODE_KINEMATIC` and `freeze = true` throughout EDIT/DIRECT.
- Use the exact `InputEventMouseButton.position` / `InputEventMouseMotion.position`, converted by
  `_viewport_to_world()`, for gizmo press, drag, and release coordinates; do not poll a lagging
  global cursor position.
- On gizmo start, cancel deferred authored restoration. During drag, let `StageGizmos` continue to
  write the generic Node2D transform and synchronize frozen `RigidBody2D` nodes immediately.
- At `_on_transform_committed()`, for a frozen `RigidBody2D`, reassign its final
  `global_transform` and use `PhysicsServer2D.body_set_state(node.get_rid(),
  PhysicsServer2D.BODY_STATE_TRANSFORM, node.global_transform)` to synchronize the physics body.
  `_sync_pending_editable_physics_transforms()` reasserts that transform during physics and for
  three settle frames after release, then `_save_object_state()` records the committed transform.
  Terrain/navigation/grid/director dirty behavior remains unchanged.
- A null/freed RID is guarded; other Node2D types retain the current path.

**Snapping split**

- `_grid_size` is the only source for object placement and `StageGizmos` translation snap.
  `_terrain_stride()` is private to drag-paint cell generation/AABB classification.
- Add screen-stable press state:

  ```gdscript
  const TERRAIN_DRAG_THRESHOLD_PX := 6.0
  var _terrain_press_world: Vector2 = Vector2.ZERO
  var _terrain_painting: bool = false
  ```

- On terrain LMB press, record raw cursor world position but do not quantize/commit. Enter paint
  mode only after `(current_world - press_world).length() * camera.zoom.x >= threshold`.
- If released below threshold, place exactly once through standard placement semantics:
  Snap ON -> `_snap_to_grid(_terrain_press_world)`; Snap OFF -> raw press position. Do not call
  `_world_to_terrain_cell()` for this path.
- If threshold is crossed, initialize and update the existing Bresenham/block-stride drag path;
  drag-paint stays block-unit aligned independent of Snap.
- Idle terrain ghost position follows the same single-placement rule (grid when Snap ON, raw when
  OFF). Once painting begins, `_ghost_array` continues to preview block-stride cells.
- `_end_terrain_drag()` clears press/painting state and creates only one replacement cursor ghost.

### `scripts/stage_gizmos.gd` — Bugs 4 and 7

Add narrow APIs and keep all snap calculation scalar/grid based:

```gdscript
signal transform_started(nodes: Array[Node2D])
signal transform_updated(nodes: Array[Node2D])
func set_snap_size(size: float) -> void
```

Emit `transform_started` after either drag type is successfully initialized; the stage uses it to
zero `_restore_frames_left`. `set_snap_size()` clamps non-positive values to `0.0`. `_snap()` must
use only this value. `SandboxStage._apply_grid_settings()` passes `_grid_size` when Snap is ON and
`0.0` otherwise. Translation stores the press origin and all starting node positions, quantizes
the anchor's accumulated displacement from that origin, and applies the same delta to the rest of
the selection. This preserves smooth movement across sub-grid mouse events and prevents object
dimensions from affecting snap distance. `transform_updated` is emitted on every drag update.
No spawner/template/AABB dependency may be added to `StageGizmos`.

### `scripts/stickman_factory.gd` and `scripts/stickman_rig.gd` — Bugs 2 and 5

After `StkRigAdapter.apply()` and before the rig enters the scene tree, call:

```gdscript
func initialize_spawn_pose_from_skeleton() -> void
```

This method resolves the torso/head and four lower limb bones directly and computes the six
rig-local IK marker targets from the current adapted skeleton:

- Torso marker = current torso bone rig-local position/rotation.
- Hand/leg marker = corresponding lower bone's far endpoint. Compute the endpoint in global/rig
  space using the bone transform, `Bone2D.length`, and `bone_angle`; do not use hardcoded limb
  lengths or the pre-adapter scene dimensions.
- Head target = a point on the current head aim direction, preserving the authored head rotation.

Write those marker values before the first skeleton-modification solve, then deep-copy them into:

```gdscript
var _spawn_pose: Dictionary = {} # marker name -> {pos: Vector2, rot: float}
```

`_restore_standing_markers()` uses `_spawn_pose`, with existing `STAND_POSE` only as a malformed
rig/factory-bypass fallback. The method is idempotent and null-guarded with `StickmanRig:` warnings.
It does not enable/disable the stack or play animation.

The ghost remains configured exactly as today (IK stack disabled, animation stopped, collision
proxies disabled). Because the live rig's targets now preserve that same adapted skeleton pose,
its first solved frame, placement frame, later EDIT/DIRECT reset, and ghost silhouette match.
`master_rig.tscn` and `StkRigAdapter` are not modified.

### `scripts/stickman_rig.gd` — Bugs 1, 3, and 6

In addition to the queue/speech changes above:

**Pose cleanup**

- Expand `snap_to_standing()` into the authoritative no-animation restoration boundary. It kills
  `_stand_up_tween`, restores every active impact bone to its stored `base_rotation` before
  clearing `_impact_reactions`, restores `_spawn_pose` marker values directly, clears recovery
  caches/timers, restores IK/head modification state, shows the body, enables collision proxies,
  and publishes `ANIMATED` only after all writes are complete.
- `_cancel_recovery()` remains a kill (never `custom_step()`/finish). All mode-exit cancellation
  paths are silent: no `action_finished`, `speech_finished`, `arrived`, or `queue_finished`.
- Guard `_on_stand_up_finished()` against a killed/stale tween callback by checking that state is
  still `RECOVERING` and that the callback corresponds to the current tween generation/reference.
- `_finish_walk()` captures the current eight limb-bone rotations, calls `AnimationPlayer.stop(true)`,
  and leaves all six IK marker properties at their current values; no marker reset occurs at queue
  completion. A LEFT-facing idle disables mirrored TwoBoneIK while preserving that stopped
  marker/bone frame, so queue completion never restores the factory pose. The explicit stage reset
  path (`stop_queue()` followed by `snap_to_standing()`) restores the factory-captured `_spawn_pose`
  and limb rotations.

**LEFT speech mirror**

Add:

```gdscript
func _sync_speech_bubble_mirror() -> void
```

The bubble stays under the rig and at `SPEECH_BUBBLE_OFFSET`. Counter-mirror its local X scale:
`Vector2(-1, 1)` for LEFT and `Vector2.ONE` for RIGHT/FORWARD. Call the helper after lazy bubble
creation and from `_apply_profile()`. Do not move the bubble under `Body`, a bone, or a physics
node. This cancels only the root X reflection; the bubble still follows the rig and camera as
before. `stickman_speech_bubble.gd` needs no drawing/layout change.

## Signal flow

```text
collision/arrival/speech/action event
  -> SandboxStage._handle_event (all matching rules, stored order)
  -> SandboxStage._execute_rule_actions (ordered batches per target rig)
  -> StickmanRig.enqueue_reactive
       OVERLAY    -> replace/start overlay slot; primary untouched
       SEQUENTIAL -> cancel primary; prepend/start; authored future retained
       TAKEOVER   -> cancel primary + overlays + pending reactive; start now
  -> natural completion only
  -> existing action_finished / speech_finished / queue_finished signals
```

No existing public signal signature changes. `StageGizmos.transform_started` and
`StageGizmos.transform_updated` are the new gizmo signals.

## Cleanup and failure behavior

- Mode exit is idempotent even during walk, wait, primary/overlay speech, reaction decay,
  ragdoll, or recovery.
- Takeover and mode exit hide speech immediately without producing a `speech_finished` rule edge.
- A deleted/stale target rig is skipped by the stage. Unknown actions warn/complete safely.
- Reactive work never mutates the authored queue and cannot appear in the Queue Panel.
- Ghosts free their navigation/IK/collision children with the rig; no live-ghost processing is
  introduced.
- Frozen props remain physical objects with zero velocities; their committed transform becomes
  the new authored transform and survives later PLAY -> EDIT restoration.
- Terrain click cancel/Escape frees all previews and places nothing. A click places one block;
  a paint drag never also places the raw-click block.

## Layout and theme

The only UI addition is **“Stop what he's doing?”** under speak parameters in rule contexts:
both the shared `ActionEditor` and the legacy quick rule-builder dialog. Default is unchecked.
No scene-node, theme-token, font, cursor, grid-drawing, or speech-bubble style changes are needed;
the affected UI is built dynamically in scripts.

## Developer validation after approval

Add `tests/test_phase4_final_bugs.gd` (or split by subsystem if needed) and run Godot 4.7.1
`--check-only` on every changed script plus the test. Then run the new suite and the existing
Phase 3c/4b/4c/4.5 suites.

Required automated cases:

1. Registry role lookup, old-rule default overlay, speak promotion, payload round-trip, and
   checkbox visibility/default/prefill in queue vs rule contexts.
2. Walk + overlay speak keeps walking; replacing overlay leaves one slot; promoted speak cancels
   walk; after it finishes the next authored action runs.
3. Sequential reactive action during primary speak cancels it without `speech_finished`; mixed
   `[overlay speak, walk]` runs concurrently and in order.
4. Ragdoll Takeover is entered during the collision callback/update (not after walk), clears the
   overlay and pending reactive work, preserves authored future items, and resumes only after the
   Takeover boundary. Recover follows the same policy.
5. Natural completions emit once; all interrupt/Takeover/mode-exit cancellations emit none.
   `clear_reactive_actions()` leaves only authored queue actions and no runtime commands.
6. Factory-created rig pre-solve pose, static ghost pose, placed first-solved pose, and a later
   `snap_to_standing()` pose have matching hand/ankle/head/torso transforms within tolerance.
7. Collision reaction followed immediately by EDIT/DIRECT restores the exact captured base bone
   rotation in one frame; recovery tween is killed and a stale completion cannot mutate state.
8. A frozen crate/plank translates and rotates through gizmos, survives three physics frames,
   updates authored state on commit, and restores to that new transform after PLAY.
9. LEFT speech text has a positive world-transform determinant/readable orientation; toggling
   LEFT/RIGHT while visible updates immediately and does not move the anchor.
10. Object translate snap uses exactly configured `_grid_size` for terrain, prop, area, and rig,
    independent of each object's AABB. Snap OFF preserves drag delta.
11. Single terrain click: Snap OFF equals raw cursor; Snap ON equals stage grid; neither uses the
    block width/height. Idle ghost matches the committed position.
12. Terrain drag above threshold retains block-unit Bresenham tiling and existing occupancy
    classification with Snap both ON and OFF; click/drag paths each place exactly once.

Headless checks should include the exact skill command form, e.g.:

```powershell
& "C:\Godot4\Godot_v4.7.1-stable_win64_console.exe" --headless --check-only --script "res://scripts/stickman_rig.gd" --path "C:\Godot4\stickman"
& "C:\Godot4\Godot_v4.7.1-stable_win64_console.exe" --headless --path "C:\Godot4\stickman" --script "res://tests/test_phase4_final_bugs.gd"
```

## User verification in the Godot editor

Run `scenes/sandbox_stage.tscn` with F6 and verify:

1. The placement ghost and placed stickman have the same arms, bent legs, head, and silhouette;
   switching EDIT -> DIRECT does not change that pose.
2. Place and manipulate a crate/plank and a Step in EDIT: body drag and rotate ring both work;
   PLAY -> EDIT returns them to the newly committed transforms.
3. Set grid size to a visibly distinct value (for example 25): all selected objects translate on
   25 px increments, not their widths/heights. With Snap OFF, motion is continuous.
4. Single-click Ground/Ramp/Step with Snap OFF at an off-grid point, then with Snap ON; verify raw
   vs 25 px placement. Drag-paint still tiles by block footprint in both modes.
5. LEFT-facing speech remains readable and switches orientation correctly while the bubble is
   already visible.
6. Exercise every Bug 1 acceptance sequence from the source plan, including collision-triggered
   immediate ragdoll, overlay and promoted speech, speak interrupted by walk, mixed speak+walk,
   Takeover cleanup, and authored-queue continuation.
7. Trigger a visible collision flinch and immediately switch to DIRECT/EDIT: the rig snaps to its
   base pose with no visible head return tween.

## Final implementation and validation record

The feature was verified and the implementation is complete. The focused regression suite reports
**81/81 passed**, the grid regression suite reports **7/7 passed**, and the headless
`scenes/sandbox_stage.tscn` smoke test **passed**.

The final behavior includes exact event-coordinate prop translation/rotation, continuous frozen
body synchronization during drag plus three post-release settle frames, origin-based accumulated
grid snapping, current-frame preservation at LEFT walk queue completion, explicit factory-pose
restoration on stage reset, immediate silent flinch/recovery cleanup with PLAY-only collision
dispatch, reactive Overlay/Sequential/Takeover execution lanes, and type-safe Walk To summaries
that separate actor ids from waypoint vectors.