From 874c5720d96cd10def8f867443608167c367cf61 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 27 May 2026 21:59:40 -0400 Subject: [PATCH] NEO-93: Add kickoff implementation plan for NPC behavior state machine. Co-authored-by: Cursor --- docs/plans/NEO-93-implementation-plan.md | 154 +++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 docs/plans/NEO-93-implementation-plan.md diff --git a/docs/plans/NEO-93-implementation-plan.md b/docs/plans/NEO-93-implementation-plan.md new file mode 100644 index 0000000..25845bf --- /dev/null +++ b/docs/plans/NEO-93-implementation-plan.md @@ -0,0 +1,154 @@ +# NEO-93 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-93 | +| **Title** | E5M2-07: NPC behavior state machine + lazy tick advance | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-93/e5m2-07-npc-behavior-state-machine-lazy-tick-advance | +| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-07** | +| **Branch** | `NEO-93-npc-behavior-state-machine-lazy-tick` | +| **Precursor** | [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) — `IThreatStateStore` + aggro acquire/leash clear (**Done** on `main`); [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) — `INpcBehaviorDefinitionRegistry` (**Done** on `main`) | +| **Pattern** | [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) — in-memory session store + static ops + DI; no HTTP wire until downstream story | +| **Blocks** | [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) — snapshot GET + DTO projection; [NEO-96](https://linear.app/neon-sprawl/issue/NEO-96) — telemetry hook sites | +| **Client counterpart** | None for this slice — runtime projection lands in [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94); Godot poll + HUD in [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Proximity auto-aggro** | Add idle→aggro when player enters `aggroRadius` without a damaging cast? | **Holder-only** — drive idle→aggro from `IThreatStateStore` only (NEO-92 first-hit acquire); defer proximity to a follow-up issue. | **User:** holder-only. | +| **Lazy tick wiring** | Wire `AdvanceAll` to HTTP in NEO-93? | **Engine + DI only** — no HTTP; [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) invokes `AdvanceAll` from `GET /game/world/npc-runtime-snapshot`. | **User:** engine-only. | +| **attack_execute semantics** | Apply player damage in NEO-93? | **Instant pass-through** — transition attack_execute → recover in the same advance step; no `IPlayerCombatHealthStore` ([NEO-95](https://linear.app/neon-sprawl/issue/NEO-95)). | **Adopted** — E5M2-09 owns damage. | +| **First telegraph after acquire** | Wait full `attackCooldownSeconds` or telegraph immediately? | **Wait full cooldown** — E5M2-07 AC: “enters telegraph after `attackCooldownSeconds` elapses from last attack”; treat first cycle as cooldown starting at aggro entry. | **Adopted** — backlog AC. | +| **Recover phase** | Duration when no catalog field? | **`recover` is the post-attack cooldown wait** — hold until `attackCooldownSeconds` elapses, then → `telegraph_windup` while holder remains. | **Adopted** — closes idle→aggro→telegraph→attack→recover loop. | +| **Telemetry events** | Emit `npc_state_transition` now? | **Comment-only hook sites** at transition points; full instrumentation in [NEO-96](https://linear.app/neon-sprawl/issue/NEO-96). | **Adopted** — E5M2-10 scope. | + +## Goal, scope, and out-of-scope + +**Goal:** Server-owned NPC behavior state machine with **lazy tick advance** — prototype states **`idle`** → **`aggro`** → **`telegraph_windup`** → **`attack_execute`** → **`recover`**, with phase durations from the behavior catalog. + +**In scope (from Linear + [E5M2-07](E5M2-prototype-backlog.md#e5m2-07--npc-behavior-state-machine--lazy-tick-advance)):** + +- `INpcRuntimeStateStore` + in-memory implementation for all **`PrototypeNpcRegistry`** instance ids. +- `NpcRuntimeOperations.AdvanceAll` (monotonic clock, per-advance **delta cap**) reading **`IThreatStateStore`** + **`INpcBehaviorDefinitionRegistry`**. +- Holder sync: empty holder → **`idle`**; holder set while **`idle`** → **`aggro`** (no proximity acquire). +- Catalog-driven timings: **`attackCooldownSeconds`**, **`telegraphWindupSeconds`**. +- Internal active-telegraph snapshot while in **`telegraph_windup`** (for NEO-94 DTO mapping). +- Comment-only telemetry hook sites at state transitions. +- Extend dev **`combat-targets-fixture`** to reset runtime rows. +- Unit tests with injected **`TimeProvider`** / controlled advance deltas (AAA). + +**Out of scope (from Linear + backlog):** + +- HTTP DTO projection — **`GET /game/world/npc-runtime-snapshot`** ([NEO-94](https://linear.app/neon-sprawl/issue/NEO-94)). +- Player damage on attack complete ([NEO-95](https://linear.app/neon-sprawl/issue/NEO-95)). +- Proximity auto-aggro inside **`aggroRadius`** without damaging cast. +- Godot / client HUD ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)). +- `docs/manual-qa/NEO-93.md` — server-only; unit tests verify AC (NEO-92 pattern). + +## Acceptance criteria checklist + +- [ ] Aggro'd NPC enters telegraph after `attackCooldownSeconds` elapses from last attack (or from aggro entry on first cycle). +- [ ] Windup duration matches `telegraphWindupSeconds` from behavior def. +- [ ] Idle NPC with no aggro holder does not telegraph. +- [ ] Full transition chain **`idle` → `aggro` → `telegraph_windup` → `attack_execute` → `recover`** is reachable with holder set (deterministic injected clock). + +## Technical approach + +### State machine (per prototype NPC instance) + +| State | Enter when | Exit when | +|-------|------------|-----------| +| **`idle`** | No aggro holder | Holder set → **`aggro`** (phase start = now) | +| **`aggro`** | Holder set from **`idle`**, or after **`recover`** cooldown (optional shortcut: recover → telegraph directly) | Elapsed ≥ **`attackCooldownSeconds`** → **`telegraph_windup`** | +| **`telegraph_windup`** | Cooldown complete while holder set | Elapsed ≥ **`telegraphWindupSeconds`** → **`attack_execute`** | +| **`attack_execute`** | Windup complete | Same advance step → **`recover`** (no damage in NEO-93) | +| **`recover`** | Attack step complete | Elapsed ≥ **`attackCooldownSeconds`** → **`telegraph_windup`** | + +**Holder cleared** (leash break / fixture reset) from any state → **`idle`**, clear active telegraph. + +**No proximity acquire:** `AdvanceAll` never sets holders; only reads **`IThreatStateStore`**. + +### Lazy tick advance + +1. **`NpcRuntimeOperations.AdvanceAll(nowUtc, runtimeStore, threatStore, behaviorRegistry, maxDeltaSeconds = 5.0)`** + - Compute `delta = min(nowUtc - runtimeStore.LastAdvancedUtc, maxDeltaSeconds)`; skip when `delta <= 0`. + - Update **`LastAdvancedUtc = nowUtc`** after processing all instances. + - For each **`PrototypeNpcRegistry`** id: load threat holder + behavior def + runtime row; apply holder sync then phase transitions consuming **`delta`** (may multi-step within one advance when delta spans phases — loop until delta exhausted or waiting on timer). + +2. **`INpcRuntimeStateStore`** + - **`DateTimeOffset LastAdvancedUtc`** (lazy-init to `DateTimeOffset.MinValue` so first advance applies full delta). + - **`TryGet(npcInstanceId, out NpcRuntimeStateSnapshot)`** — `BehaviorState`, `PhaseStartedUtc`, optional **`ActiveTelegraph`** (`TelegraphId`, `WindupStartedUtc`). + - **`TryResetPrototypeRow(npcInstanceId)`** / **`ResetAllPrototypeRows()`** for fixture. + +3. **`NpcBehaviorState`** enum — wire string values: `idle`, `aggro`, `telegraph_windup`, `attack_execute`, `recover` (stable for NEO-94 JSON). + +4. **DI** — **`AddNpcRuntimeStateStore()`** singleton alongside **`AddThreatStateStore()`** in **`Program.cs`**. + +5. **Dev fixture** — after HP + threat reset, call **`NpcRuntimeOperations.ResetAllPrototypeRows(runtimeStore)`**. + +6. **Docs** — **`server/README.md`** NPC runtime section: states, lazy advance (NEO-94 wires poll), fixture reset, no HTTP yet. + +### Catalog timings (test expectations) + +| Instance | Behavior def | `attackCooldownSeconds` | `telegraphWindupSeconds` | +|----------|--------------|---------------------------|--------------------------| +| `prototype_npc_melee` | `prototype_melee_pressure` | 3.0 | 1.5 | +| `prototype_npc_ranged` | `prototype_ranged_control` | 4.0 | 2.0 | +| `prototype_npc_elite` | `prototype_elite_mini_boss` | 5.0 | 2.5 | + +### Example cycle (melee, holder already set) + +``` +t=0 idle + holder → aggro +t=3 aggro → telegraph_windup +t=4.5 telegraph_windup → attack_execute → recover (instant attack stub) +t=7.5 recover → telegraph_windup (next cycle) +``` + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Npc/NpcBehaviorState.cs` | Enum of prototype behavior states (stable wire names). | +| `server/NeonSprawl.Server/Game/Npc/NpcRuntimeStateSnapshot.cs` | Immutable runtime row + optional active telegraph snapshot. | +| `server/NeonSprawl.Server/Game/Npc/INpcRuntimeStateStore.cs` | Store contract + `LastAdvancedUtc` for lazy tick. | +| `server/NeonSprawl.Server/Game/Npc/InMemoryNpcRuntimeStateStore.cs` | Thread-safe in-memory prototype implementation. | +| `server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs` | `AdvanceAll`, `ResetAllPrototypeRows`, holder sync + phase transitions. | +| `server/NeonSprawl.Server/Game/Npc/NpcRuntimeServiceCollectionExtensions.cs` | DI registration for `INpcRuntimeStateStore`. | +| `server/NeonSprawl.Server.Tests/Game/Npc/NpcRuntimeOperationsTests.cs` | AAA unit tests with fake `TimeProvider` / manual advance timestamps. | +| `server/NeonSprawl.Server.Tests/Game/Npc/InMemoryNpcRuntimeStateStoreTests.cs` | AAA store tests: lazy rows, reset, unknown id. | +| `docs/plans/NEO-93-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register `AddNpcRuntimeStateStore()`. | +| `server/NeonSprawl.Server/Game/Combat/CombatTargetFixtureApi.cs` | Reset runtime state rows alongside HP + threat on dev fixture. | +| `server/README.md` | Document NPC runtime state machine, lazy advance, fixture reset; note NEO-94 HTTP projection. | +| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | NEO-93 landed note when complete. | +| `docs/plans/E5M2-prototype-backlog.md` | E5M2-07 acceptance checkboxes + landed note when complete. | + +## Tests + +| File | Coverage | +|------|----------| +| `NpcRuntimeOperationsTests.cs` | Idle + no holder stays idle; idle + holder → aggro; no telegraph without holder; aggro → telegraph after melee `attackCooldownSeconds` (3.0); windup duration matches melee `telegraphWindupSeconds` (1.5); full chain through attack_execute → recover → second telegraph; holder clear mid-windup → idle; elite/ranged catalog timings spot check; delta cap does not overshoot single phase in one advance. | +| `InMemoryNpcRuntimeStateStoreTests.cs` | Unknown npc id fails; lazy init rows; reset all prototype ids; `LastAdvancedUtc` update. | +| `CombatTargetFixtureApiTests.cs` | Extend existing fixture test — after reset, runtime rows are idle (no active telegraph). | + +No Bruno changes (no HTTP wire). No `docs/manual-qa/NEO-93.md`. + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|---------------------|--------| +| **Linear blockedBy NEO-92 / NEO-89** | Proceed — both **Done** on `main`. | **adopted** | +| **No HTTP in NEO-93** | Accept — NEO-94 calls `AdvanceAll` before snapshot read; unit tests drive advance directly. | **adopted** (kickoff) | +| **Proximity auto-aggro** | Deferred — holder-only per kickoff; E5M2 default proximity re-aggro is a follow-up if still desired. | **deferred** | +| **Multi-step advance in one poll** | Loop phase transitions until delta consumed or phase needs more time — prevents stuck states when client polls slowly. | **adopted** | +| **attack_execute duration** | Zero-duration stub; NEO-95 adds damage resolve at telegraph complete. | **adopted** | +| **Thread safety** | Mirror `InMemoryThreatStateStore` lock pattern for three fixed prototype ids. | **adopted** |