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:

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:

"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:

{
    "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:

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:

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:

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:

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:

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:

  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:

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:

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:

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:

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

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.:

& "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.