164 lines
11 KiB
Markdown
164 lines
11 KiB
Markdown
# Phase 4c — Trigger Area Reliability
|
|
|
|
## Scope
|
|
|
|
Repair and regression-test `entered_area` rules in the standalone Sandbox Stage. This is a targeted correction to the existing geometric event engine; it does not add trigger types, persistence, physics sensors, or new UI.
|
|
|
|
## Architecture diagnosis
|
|
|
|
Read-only inspection and a Godot 4.7.1 `--check-only` pass establish that the diagnostic plan was written against contracts that differ from the current implementation:
|
|
|
|
- `SandboxStage._physics_process()` already calls `_update_area_entry()` only in `StageMode.PLAY`.
|
|
- `StageSpawner._spawn_area()` already adds the committed `TriggerArea` directly to `World`.
|
|
- `_update_area_entry()` already keys overlap state by both area and movable instance IDs (`"<area_id>:<movable_id>"`) and clears that state on PLAY/EDIT transitions.
|
|
- `TriggerArea.get_area_rect()` intentionally returns a **local-space** centered `Rect2`; `_update_area_entry()` converts it once with `area.global_transform`. Changing the getter to world space would double-transform detection and make `_draw()` incorrect.
|
|
- Rules intentionally store integer instance IDs. The rule builder, `RuleEditor`, cleanup, visualizer, filters, and action executor all share this representation. Replacing only an area target with `WeakRef` would violate the rule payload contract and break `int(trigger.target)`, `instance_from_id()`, deep-copy/edit, and deletion-cleanup paths.
|
|
- The stickman event position is already its feet. `StickmanRig.FOOT_OFFSET` is defined as `Vector2(0, -385)` in the feet-to-root equation `root = feet + FOOT_OFFSET`; therefore the inverse is `feet = root - FOOT_OFFSET`. The diagnostic plan's proposed `root + FOOT_OFFSET` has the wrong sign. Entry containment itself uses the movable's visible world AABB, while this feet point remains the event payload and point-like fallback.
|
|
- Both rule creation paths already commit `entered_area` as a rule in `_event_rules`.
|
|
|
|
The verified correctness defects were in two parts of the area path. `SandboxStage._rule_matches()` checked only `trigger.target` (the area) and ignored `trigger.source` (the selected stickman/prop), allowing another movable to consume the apparent entry moment and run the consequence before A arrived. `_update_area_entry()` also tested only the movable's origin/feet point, so a visible leg or body could pass through an area while its anchor remained outside. The final implementation matches both IDs and tests visible world-AABB overlap; the corrected prop enumeration scope keeps unfrozen props in the movable set without changing the area loop. The new end-to-end regression suite and editor verification cover these boundaries.
|
|
|
|
## Data contracts
|
|
|
|
Keep the existing transient rule payload:
|
|
|
|
```gdscript
|
|
{
|
|
"id": int,
|
|
"trigger": {
|
|
"type": "entered_area",
|
|
"source": int, # Object.get_instance_id(); -1 means any movable
|
|
"target": int, # TriggerArea.get_instance_id()
|
|
"params": Dictionary,
|
|
},
|
|
"actions": Array[Dictionary],
|
|
}
|
|
```
|
|
|
|
Keep the emitted internal event payload:
|
|
|
|
```gdscript
|
|
{
|
|
"type": "entered_area",
|
|
"source": Node2D, # movable that crossed the edge
|
|
"target": TriggerArea, # crossed area
|
|
"position": Vector2, # feet for StickmanRig; global_position otherwise
|
|
"action": Dictionary,
|
|
}
|
|
```
|
|
|
|
No serialized format or public signal changes are required.
|
|
|
|
## Target files and changes
|
|
|
|
### `scripts/editors/stage/sandbox_stage.gd`
|
|
|
|
1. Preserve the PLAY-only call in `_physics_process(_delta: float) -> void`.
|
|
2. Preserve `_update_area_entry() -> void` as the owner of area discovery, movable discovery, transformed world-AABB overlap, pairwise edge state, and event emission.
|
|
3. Correct `_rule_matches(rule: Dictionary, event: Dictionary) -> bool` so `entered_area` matches **both** ends of the trigger:
|
|
|
|
```gdscript
|
|
"entered_area":
|
|
return _ids_match(source_id, source) and _ids_match(target_id, target)
|
|
```
|
|
|
|
This retains the existing `-1` wildcard behavior through `_ids_match()` while ensuring a rule authored from Stickman A does not fire for Stickman B or a prop.
|
|
4. Keep `_movable_point(node: Node2D) -> Vector2` unchanged for stickmen:
|
|
|
|
```gdscript
|
|
return node.global_position - StickmanRig.FOOT_OFFSET
|
|
```
|
|
|
|
5. Use `StageSelection.get_world_aabb(node)` for visible stickman/prop overlap. If a future movable has no bounds, fall back to `area_rect.has_point(_movable_point(node))`. Keep the area's local rectangle contract and transform it once with `area.global_transform` before comparing AABBs.
|
|
|
|
6. Keep instance IDs as the single rule-reference representation. Stale action targets remain null-guarded by `instance_from_id(...) as StickmanRig`; deleted stage objects continue to remove referencing rules through `_cleanup_rules_for_nodes()`.
|
|
7. Do not ship the four unconditional diagnostic prints proposed in `docs/archive/plans/PHASE_4c_AREA.md`. Automated assertions cover each boundary without per-physics-frame console spam. If interactive tracing is temporarily needed during development, remove it before hand-off or guard it behind a default-false debug constant.
|
|
|
|
### `tests/test_phase4c_area.gd` (new)
|
|
|
|
Add a headless regression suite that instantiates `res://scenes/stage/sandbox_stage.tscn`, uses real `StageSpawner` objects, and drives the event engine deterministically. Tests may call internal methods, matching the existing phase suites.
|
|
|
|
Required cases:
|
|
|
|
1. Spawning `"area"` returns a `TriggerArea` whose parent is `stage._world`.
|
|
2. An `entered_area` rule built with live instance IDs is retained in `_event_rules` with non-empty actions.
|
|
3. A source stickman crossing from outside to inside adds exactly one reactive action to the configured target rig.
|
|
4. Repeated polls while still inside do not add another action; an outside poll followed by another inside poll adds one more.
|
|
5. A different stickman or an unfrozen prop entering the same area does **not** match a source-specific rule.
|
|
6. A `source == -1` rule remains a deliberate wildcard and fires independently for multiple movables.
|
|
7. Two areas and two source stickmen use independent pair keys; entering one pair does not suppress another.
|
|
8. Moving/rotating/scaling an area uses its current global transform for containment.
|
|
9. A stickman's event `position` equals `rig.global_position - StickmanRig.FOOT_OFFSET` (feet), not its root or a bone position; containment uses the visible world AABB so the tested overlap can be leg/body-level.
|
|
10. An area with no matching rule is a no-op with no errors.
|
|
11. A stale/deleted action target is skipped without a crash. Deleting through the stage path removes rules referencing the deleted area.
|
|
12. Exercise at least one visible consequence payload (`speak`) and assert that the correct rig receives a normalized reactive queue action; the editor user-verification pass confirms the speech bubble itself.
|
|
|
|
Tests must restore/avoid global side effects and queue-free their instantiated stage.
|
|
|
|
## Nodes/classes affected
|
|
|
|
- `SandboxStage` (`scenes/stage/sandbox_stage.tscn` root): source+target matcher correction, visible-AABB overlap helper, and prop-enumeration scope correction; no node-tree change.
|
|
- `TriggerArea`: exercised but not modified.
|
|
- `StageSpawner`, `RuleEditor`, `StageDirectorVisuals`, and `StickmanRig`: exercised through their existing contracts; no implementation change expected.
|
|
|
|
## Signal flow
|
|
|
|
```text
|
|
PLAY physics tick
|
|
-> SandboxStage._update_area_entry()
|
|
-> new (area_id, movable_id) containment edge
|
|
-> SandboxStage._handle_event(event)
|
|
-> SandboxStage._rule_matches(rule, event)
|
|
source instance ID AND area instance ID must match
|
|
-> SandboxStage._execute_rule_actions(rule)
|
|
-> StickmanRig.enqueue_reactive(actions)
|
|
-> existing StickmanRig queue runner / speech bubble
|
|
```
|
|
|
|
No new signals are introduced. Existing queue signals (`queue_changed`, `action_started`, `action_finished`, `queue_finished`) continue unchanged.
|
|
|
|
## Layout/theme
|
|
|
|
No layout, scene, cursor, label, or theme changes are required. Trigger area drawing remains a local-space translucent rectangle with a dashed outline.
|
|
|
|
## Validation
|
|
|
|
Developer validation:
|
|
|
|
```powershell
|
|
& "C:\Godot4\Godot_v4.7.1-stable_win64_console.exe" --headless --check-only --script "res://scripts/editors/stage/sandbox_stage.gd" --path "C:\Godot4\stickman"
|
|
& "C:\Godot4\Godot_v4.7.1-stable_win64_console.exe" --headless --path "C:\Godot4\stickman" --script "res://tests/test_phase4c_area.gd"
|
|
```
|
|
|
|
The baseline syntax check passed before implementation. Post-implementation validation passed as follows:
|
|
|
|
- `sandbox_stage.gd` and `tests/test_phase4c_area.gd` passed Godot `--check-only` syntax validation.
|
|
- `tests/test_phase4c_area.gd` reported `RESULT: ALL PASSED (19 assertions)`, covering source-specific and wildcard matching, one-shot/re-entry edges, independent pairs, transformed areas, visible leg-level overlap with feet outside, speech queue normalization and visible speech execution, no-rule no-op behavior, stale targets, and deletion cleanup.
|
|
- Existing Phase 4b suites remained passing: 16/16 logic assertions and 53/53 stage/UI assertions.
|
|
|
|
User verification in the Godot editor (`scenes/stage/sandbox_stage.tscn`, F6) completed successfully:
|
|
|
|
1. Place Stickman A, Stickman B, and an Area.
|
|
2. In DIRECT, queue A to walk through the Area.
|
|
3. Author `When A enters Area -> B speaks`.
|
|
4. Enter PLAY: B speaks once as A's visible body overlaps the area, even when the feet anchor remains outside.
|
|
5. Keep A inside for five seconds: no repeat.
|
|
6. Return to EDIT/DIRECT, author a route that exits and re-enters, then PLAY: B speaks once on each entry.
|
|
7. Add another stickman/area pair and confirm independent firing with no warnings/errors.
|
|
|
|
The reported editor result was that Stickman A walking completely through the area caused Stickman B's configured speech bubble to appear, confirming visible-body overlap rather than feet-only containment.
|
|
|
|
## Design decisions and trade-offs
|
|
|
|
- **Instance IDs, not weakrefs:** consistent with all current rule UI/runtime consumers and adequate for session-only rules. A future persisted-stage format will need stable authored object IDs; that is outside Phase 4c.
|
|
- **Local area rectangle, transformed by the stage:** keeps drawing, hit-testing, AABB calculation, and overlap detection on one local geometry contract and supports moved/rotated/scaled areas.
|
|
- **Visible AABB overlap, not physics shape overlap:** a stickman or prop enters when its visible world AABB overlaps the area's transformed world AABB. This catches leg/body-level crossings while keeping the event engine geometric and lightweight; exact polygon/collision-shape overlap remains out of scope.
|
|
- **Source-specific by default, wildcard explicitly supported:** UI-authored rules retain the selected source. `-1` remains available for existing/programmatic "any movable" rules.
|
|
|
|
## Out of scope / documentation follow-up
|
|
|
|
- `exited_area` and `stay_in_area` triggers.
|
|
- Physics `Area2D` conversion or exact polygon/collision-shape overlap.
|
|
- Disk persistence/stable IDs for rules.
|
|
- Project documentation was updated after user verification; the stable-ID persistence limitation is recorded in `docs/architecture/tech_debt_and_optimizations.md`.
|