Phase 14 - Organized directories.
This commit is contained in:
@@ -0,0 +1,216 @@
|
||||
# Phase 4 Bug Fixes — High-Level Plan
|
||||
|
||||
## 1. Overview
|
||||
|
||||
This plan addresses eight defects discovered after Phase 4 shipped. Bug 1 requires a design decision (reactive-action interrupt policy); Bugs 2–8 are diagnosed defects with clear fix directions.
|
||||
|
||||
The plan is a design brief for the architect. Implementation details belong in the resulting spec, not here.
|
||||
|
||||
---
|
||||
|
||||
## 2. Bug 1 — Reactive Event Interrupt Policy
|
||||
|
||||
### 2.1. Problem Statement
|
||||
|
||||
A walk is in progress; a rule fires (`When [Stickman] collides with [Prop] → Ragdoll`). The reactive action is appended to the queue tail and only executes after the walk finishes. The user expects immediate response.
|
||||
|
||||
### 2.2. Design Decision — Three Interruption Roles
|
||||
|
||||
Every action — whether in the sequential queue or arriving reactively — fits one of three **roles**:
|
||||
|
||||
| Role | Behavior when triggered reactively |
|
||||
| -------------- | ------------------------------------------------------------------ |
|
||||
| **Sequential** | Cancel the current action; run this action; resume the queue after |
|
||||
| **Overlay** | Run alongside the current action; the queue is not touched |
|
||||
| **Takeover** | Cancel everything (including overlays); take over the body state |
|
||||
|
||||
### 2.3. Role Assignment Per Action Type
|
||||
|
||||
| Action | Role |
|
||||
| --------- | --------------------------------------------------------------------------------- |
|
||||
| `walk_to` | Sequential |
|
||||
| `wait` | Sequential |
|
||||
| `ragdoll` | Takeover |
|
||||
| `recover` | Takeover |
|
||||
| `speak` | **Overlay by default** — director can promote to Sequential via a per-rule toggle |
|
||||
|
||||
### 2.4. Director-Facing Behavior
|
||||
|
||||
The rule builder shows a single checkbox on **speak** actions only:
|
||||
|
||||
> **"Stop what he's doing?"** (default OFF)
|
||||
|
||||
- **Unchecked (default)** → speak is an Overlay. The stickman keeps walking / waiting / whatever he was doing. The speech plays alongside.
|
||||
- **Checked** → speak becomes Sequential. The stickman stops what he's doing, says his line, then resumes the queue.
|
||||
|
||||
No checkbox on `walk_to`, `wait`, `ragdoll`, or `recover` — their roles are fixed and unambiguous.
|
||||
|
||||
### 2.5. Why This Model
|
||||
|
||||
- **Covers every combination** — walk + speak, speak + walk, walk + ragdoll, speak + speak, wait + walk, etc. Each reduces to a role pairing.
|
||||
- **Kid-friendly** — one checkbox with plain-language wording on the only action where both behaviors are meaningful.
|
||||
- **Chain reactions handle "both at once"** — a director who wants "say 'Ouch!' AND walk away" adds two actions to the rule: overlay speak + sequential walk.
|
||||
- **Forward compatible** — the registry gains an `overlay_eligible` flag on action templates. Today only `speak` is marked overlay-eligible. Future actions like `gesture`, `emote`, or `aim` can be added later without changing the architecture.
|
||||
|
||||
### 2.6. Overlay Conflict Rule
|
||||
|
||||
When two overlay speaks arrive in the same window (e.g., two props hit at once), the new speech **replaces** the old — no queue pileup, one bubble at a time.
|
||||
|
||||
### 2.7. Takeover Cleanup
|
||||
|
||||
`ragdoll` and `recover` (as Takeover actions) explicitly cancel any active overlay speech on entry. The bubble hides and the queue pauses until the takeover state resolves.
|
||||
|
||||
### 2.8. Acceptance Criteria (Bug 1)
|
||||
|
||||
- [ ] `Walk → collides → Ragdoll` — ragdoll fires **on contact**, walk is cancelled
|
||||
- [ ] `Walk → collides → Speak "Ouch!"` (checkbox OFF) — walk continues, speech plays
|
||||
- [ ] `Walk → collides → Speak "Ouch!"` (checkbox ON) — walk stops, speech plays, then queue resumes
|
||||
- [ ] `Speak → collides → Walk` — speak cancels, walk starts
|
||||
- [ ] `Walk → collides → [Speak (overlay), Walk]` — speech and walk fire together
|
||||
- [ ] `Walk → collides → Ragdoll` while a speak overlay is active — speech cancels, ragdoll fires
|
||||
- [ ] Sequential queue resumes after every interrupt
|
||||
- [ ] `clear_reactive_actions()` still strips reactive actions on Edit-mode return
|
||||
|
||||
---
|
||||
|
||||
## 3. Bug 2 — Default Pose Changes on Placement
|
||||
|
||||
**Problem:** The stickman's default pose (both arms up) changes to arms-down after placement.
|
||||
|
||||
**Suspected area:** Placement path differences in IK target state, modification stack enabled timing, or animation player auto-start behavior between the reference rig and the spawned rig.
|
||||
|
||||
**Fix direction:** Ensure a newly placed stickman's pose matches the reference/default pose exactly at spawn time. Diagnose the delta between the reference rig and a spawned rig on the same frame.
|
||||
|
||||
---
|
||||
|
||||
## 4. Bug 3 — Animation Continues After Scene Stop
|
||||
|
||||
**Problem:** When the user switches from PLAY to DIRECT/EDIT mid-flinch, the head visibly tweens back to its resting position instead of snapping.
|
||||
|
||||
**Suspected area:** Reaction-decay tweens are not being cancelled on mode exit.
|
||||
|
||||
**Fix direction:** All in-flight pose tweens (reaction decay, stand-up, etc.) must be cancelled and the affected bones set directly to their captured base rotations on mode exit. No animation should play during state restoration.
|
||||
|
||||
---
|
||||
|
||||
## 5. Bug 4 — Props Cannot Be Moved or Rotated
|
||||
|
||||
**Problem:** A placed prop can be selected (rotate gizmo appears), but neither drag nor rotate works.
|
||||
|
||||
**Suspected area:** The edit-mode freeze state (`FREEZE_MODE_KINEMATIC`) may be blocking transform writes, or the drag/rotate input path is not routing to props, or another system is resetting the transform after the drag.
|
||||
|
||||
**Fix direction:** Audit the prop drag/rotate input path end-to-end. Verify the frozen state permits `global_position`/`global_rotation` writes for edit-time manipulation, and that nothing re-sets the transform after commit.
|
||||
|
||||
---
|
||||
|
||||
## 6. Bug 5 — Ghost Pose ≠ Placed Pose
|
||||
|
||||
**Problem:** The placement ghost shows the stickman with bent legs. The placed stickman has straight legs.
|
||||
|
||||
**Suspected area:** Shared root cause with Bug 2 — the ghost is a static render (modification stack disabled), while the placed rig is live and the IK solver drives a different pose.
|
||||
|
||||
**Fix direction:** The placed stickman's pose must match the ghost's pose at spawn. This is likely resolved by fixing Bug 2.
|
||||
|
||||
---
|
||||
|
||||
## 7. Bug 6 — Speech Bubble Mirrors with LEFT Profile
|
||||
|
||||
**Problem:** When facing LEFT (`Master.scale.x = -1`), the speech bubble and its text render mirrored — backwards text.
|
||||
|
||||
**Suspected area:** The speech bubble is a child of the rig root; the whole-rig X-mirror propagates into its `_draw()`.
|
||||
|
||||
**Fix direction:** Isolate the speech bubble from the rig's mirror. Either move it out of the mirrored subtree (world-space child of the stage, following the head bone) or apply a counter-mirror inside its draw. UI must not inherit anatomy mirroring.
|
||||
|
||||
---
|
||||
|
||||
## 8. Bug 7 — Snap Uses Object Size Instead of Grid
|
||||
|
||||
**Problem:** With snap enabled, dragging objects (steps, etc.) snaps at intervals equal to the object's own width/height, not the grid.
|
||||
|
||||
**Suspected area:** The drag-snap function is quantizing to the dragged object's AABB dimensions rather than the stage grid — likely residual block-unit paint stride logic applied incorrectly to the general drag path.
|
||||
|
||||
**Fix direction:** Object drags must snap to the stage grid (`_grid_size`) only. Block-unit stride is exclusive to terrain drag-painting; it must not leak into the general drag path.
|
||||
|
||||
---
|
||||
|
||||
## 9. Bug 8 — Single Terrain Placement Uses Block-Unit Snapping
|
||||
|
||||
**Problem:** Placing a single terrain block (one click) snaps to the block's own width/height intervals even when Snap is OFF.
|
||||
|
||||
**Suspected area:** Same as Bug 7 — the single-placement path reuses the block-unit paint stride instead of the grid or no-snap.
|
||||
|
||||
**Fix direction:** Single terrain placement must follow the standard object snapping rules:
|
||||
|
||||
- Snap ON → snap to grid
|
||||
- Snap OFF → no snapping; place at cursor exactly
|
||||
|
||||
Block-unit stride is for drag-painting only.
|
||||
|
||||
---
|
||||
|
||||
## 10. Bug Grouping
|
||||
|
||||
| Group | Bugs | Theme |
|
||||
| ------------------------ | ---- | --------------------------------------------- |
|
||||
| **A — Reactive model** | 1 | Design decision + runner/action role refactor |
|
||||
| **B — Pose consistency** | 2, 5 | Placement pose must match default/ghost |
|
||||
| **C — Animation stop** | 3 | Snap pose on mode exit, no tween |
|
||||
| **D — Prop interaction** | 4 | Restore drag/rotate in Edit |
|
||||
| **E — Mirror isolation** | 6 | UI must not inherit rig mirror |
|
||||
| **F — Snapping source** | 7, 8 | Grid vs. block-unit stride confusion |
|
||||
|
||||
---
|
||||
|
||||
## 11. Suggested Implementation Order
|
||||
|
||||
| Order | Group | Rationale |
|
||||
| ----- | -------------- | ---------------------------------------------------------------------------------------------------------------- |
|
||||
| 1 | **B (#2, #5)** | Likely one root cause; visible, high-impact on first-use impression |
|
||||
| 2 | **E (#6)** | Localized, low-risk, easy to verify |
|
||||
| 3 | **F (#7, #8)** | Likely one root cause; snapping is a daily-use frustration |
|
||||
| 4 | **D (#4)** | May need investigation; blocks prop interaction |
|
||||
| 5 | **C (#3)** | Interacts with reaction system; do after A's mechanism is understood |
|
||||
| 6 | **A (#1)** | Requires the reactive-role refactor; touches the runner, the registry, the rule builder, and the reaction system |
|
||||
|
||||
Bug 1 is last because it is the largest and touches the most systems. It also benefits from the clean foundation the other fixes restore.
|
||||
|
||||
---
|
||||
|
||||
## 12. Open Questions for the Architect
|
||||
|
||||
1. **Bug 1 — Overlay speak mechanics:** Should the overlay speak still respect the `_speech_active` state, or should the state be generalized to a list of active overlays? (Recommendation: single active overlay; new replaces old.)
|
||||
2. **Bug 1 — Chain reactions with mixed roles:** When a rule contains `[overlay speak, sequential walk]`, the overlay speak should fire first and the sequential walk should interrupt it. Confirm the ordering.
|
||||
3. **Bug 2/5 — Ghost vs. placed rig:** Should the ghost be a live rig (always matches placed) or a static preview (placed rig is initialized to match)? Live ghost is more accurate but heavier.
|
||||
4. **Bug 4 — Prop freeze:** Confirm whether the freeze state is the actual blocker, or whether the input path is broken independently.
|
||||
|
||||
---
|
||||
|
||||
## 13. Acceptance Summary
|
||||
|
||||
- Every bug has a clear repro, a fix direction, and verifiable acceptance criteria.
|
||||
- Bug 1's design is locked in: three roles (Sequential, Overlay, Takeover), one director-facing checkbox on speak, forward-compatible registry flag.
|
||||
- Bugs 2–8 are diagnosed defects with localized fixes.
|
||||
- Suggested order minimizes risk: local fixes first, cross-cutting refactor last.
|
||||
|
||||
## 14. Final implementation record
|
||||
|
||||
The approved implementation is complete. The runtime behavior is:
|
||||
|
||||
| Area | Final behavior |
|
||||
| --- | --- |
|
||||
| Reactive actions | `StickmanRig` keeps authored actions in the editor queue and runs rule actions in a private lane. Overlay speech runs concurrently in one replaceable slot; Sequential actions cancel the current primary action and continue with the authored future; Takeover actions cancel primary and overlay work before taking control. |
|
||||
| Prop transforms | `SandboxStage` routes the exact mouse event position through `_viewport_to_world()`. `StageGizmos` writes translation/rotation continuously, and frozen `RigidBody2D` props are synchronized through `PhysicsServer2D` during the drag and for three post-release settle frames. |
|
||||
| Grid snapping | Translation snap is based only on the configured grid size. The drag delta is accumulated from the original press, so sub-grid mouse events remain smooth instead of requiring an object-length drag. |
|
||||
| Walk completion | LEFT-facing walks stop on the exact current animation marker/bone frame when the queue reaches the end; no factory-pose restore occurs at arrival. Explicit stage reset (`stop_queue()` + `snap_to_standing()`) restores the captured factory spawn pose. |
|
||||
| Stop/flinch cleanup | Mode exit cancels reaction and recovery tweens, restores captured base rotations immediately, and collision dispatch is PLAY-only so delayed contacts cannot recreate a flinch after STOP. |
|
||||
| Walk To summary | Rule summaries read the waypoint from `params.target` and keep the actor instance id in the top-level `target`, with type guards for malformed/legacy payloads. |
|
||||
|
||||
Validation recorded for this hand-off:
|
||||
|
||||
- Focused Phase 4 final-bugs regression: **81/81 passed**.
|
||||
- Grid regression: **7/7 passed**.
|
||||
- `scenes/sandbox_stage.tscn` headless smoke test: **passed**.
|
||||
|
||||
---
|
||||
|
||||
**End of Phase 4 Bug Fixes — High-Level Plan**
|
||||
Reference in New Issue
Block a user