From d0d5dbdf1754f5abc9a37a38d6a83a9cd8fcb0bb Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 27 May 2026 21:45:57 -0400 Subject: [PATCH] NEO-92: Add kickoff implementation plan for aggro rule engine. IThreatStateStore, AggroOperations, cast/move hooks, and AAA test plan per E5M2-06 kickoff decisions. Co-authored-by: Cursor --- docs/plans/NEO-92-implementation-plan.md | 161 +++++++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 docs/plans/NEO-92-implementation-plan.md diff --git a/docs/plans/NEO-92-implementation-plan.md b/docs/plans/NEO-92-implementation-plan.md new file mode 100644 index 0000000..76a8f43 --- /dev/null +++ b/docs/plans/NEO-92-implementation-plan.md @@ -0,0 +1,161 @@ +# NEO-92 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-92 | +| **Title** | E5M2-06: AggroRule engine + `IThreatStateStore` | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-92/e5m2-06-aggrorule-engine-ithreatstatestore | +| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-06** | +| **Branch** | `NEO-92` | +| **Precursor** | [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) — `PrototypeNpcRegistry` + combat-target migration (**Done** on `main`) | +| **Pattern** | [NEO-80](https://linear.app/neon-sprawl/issue/NEO-80) — in-memory session store + DI; [NEO-82](https://linear.app/neon-sprawl/issue/NEO-82) — cast-layer player correlation | +| **Blocks** | [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) — NPC behavior state machine (needs aggro holder) | +| **Client counterpart** | None for this slice — threat projection lands in [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) (`npc-runtime-snapshot`); Bruno + automated tests verify server behavior | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Aggro acquire hook** | Where to run acquire after a damaging cast? | **`AbilityCastApi` after successful `TryResolve` when `DamageDealt > 0`** — player id at cast layer (NEO-30); keep `CombatOperations` engine-pure. | **User:** `AbilityCastApi`. | +| **Leash clear trigger** | When to invoke `TryClearOnLeash` at runtime? | **Player move (`POST /move` + `/move-stream`) and before acquire on cast** — deterministic clear without NEO-93 snapshot poll. | **User:** move + cast. | +| **Re-aggro model** | Proximity auto-aggro vs first-hit in NEO-92? | **First damaging cast only** — E5M2-06 AC; defer proximity auto-aggro to NEO-93. | **User:** first-hit only. | +| **Holder semantics** | Behavior when NPC already has a holder? | **No-op** — first hit wins until leash clear (prototype single-player). | **User:** no-op. | + +## Goal, scope, and out-of-scope + +**Goal:** Deterministic aggro — NPC acquires a **holder** (`playerId`) on the **first damaging player cast**; holder **clears** when that player leaves the behavior catalog **`leashRadius`** from the NPC world anchor. + +**In scope (from Linear + [E5M2-06](E5M2-prototype-backlog.md#e5m2-06--aggrorule-engine--ithreatstatestore)):** + +- **`IThreatStateStore`** + in-memory implementation keyed by NPC instance id. +- **`AggroOperations.TryAcquire`** / **`TryClearOnLeash`** using player **`PositionState`**, **`PrototypeNpcRegistry`** anchors, and **`INpcBehaviorDefinitionRegistry`** `leashRadius`. +- Hook acquire from **`AbilityCastApi`** after successful combat resolve with **`DamageDealt > 0`**. +- Wire leash clear on **`PositionStateApi`** move accept paths and before acquire on cast. +- Extend dev **`combat-targets-fixture`** to reset threat rows (Bruno isolation). +- Unit + integration tests (AAA). + +**Out of scope (from Linear + backlog):** + +- NPC attack loop, telegraphs, behavior state machine ([NEO-93](https://linear.app/neon-sprawl/issue/NEO-93)). +- HTTP DTO projection of threat state ([NEO-94](https://linear.app/neon-sprawl/issue/NEO-94)). +- Proximity auto-aggro inside `aggroRadius` without a damaging cast. +- Godot / client HUD ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)). +- `docs/manual-qa/NEO-92.md` — server-only; Bruno + automated tests (NEO-91 pattern). + +## Acceptance criteria checklist + +- [x] First damaging cast on NPC sets aggro holder to casting player id. +- [x] Holder clears when player leaves `leashRadius` (deterministic horizontal distance check). +- [x] Re-aggro inside radius after clear follows same first-hit rule (no proximity auto-aggro). + +## Technical approach + +1. **Threat store (`Game/Npc/`)** + - **`ThreatStateSnapshot`**: `AggroHolderPlayerId` (`string?`, lowercase trimmed route id or `null` when empty). + - **`IThreatStateStore`**: `TryGet(npcInstanceId, out snapshot)`, `TrySetHolder(npcInstanceId, playerId?)`, `TryClearHolder(npcInstanceId)` (or single `TrySetHolder` with null). + - **`InMemoryThreatStateStore`**: thread-safe lazy rows for known **`PrototypeNpcRegistry`** ids only; unknown ids fail `TryGet`/`TrySet`. + - **`ThreatStateServiceCollectionExtensions.AddThreatStateStore`**: register in-memory singleton (prototype session store — no Postgres). + +2. **`AggroOperations` (static, `Game/Npc/`)** + - **`TryAcquire(npcInstanceId, playerId, threatStore)`** → `bool` (true when holder was empty and now set): + - Normalize ids (trim + lowercase). + - Gate npc id via **`PrototypeNpcRegistry.TryGet`**. + - If current holder non-empty → **no-op**, return `false`. + - Else set holder to `playerId`, return `true`. + - **`TryClearOnLeashForPlayer(playerId, positions, behaviorRegistry, threatStore)`**: + - Load player position via **`IPositionStateStore.TryGetPosition`**; no-op if missing. + - For each prototype NPC instance id, if holder == `playerId`: + - Resolve behavior def → **`LeashRadius`**. + - Compare **`HorizontalReach.HorizontalDistance`** (player X/Z vs NPC anchor X/Z). + - Clear holder when distance **>** `leashRadius` (inclusive boundary still leashed — mirrors lock-radius tests). + - **`TryClearOnLeashForNpc(...)`** (optional internal helper for unit tests): single-NPC variant. + +3. **Cast hook (`AbilityCastApi`)** + - Inject **`IThreatStateStore`**. + - After combat success and **before** cooldown commit: + - If **`combatResult.DamageDealt > 0`**: call **`TryClearOnLeashForPlayer`** then **`TryAcquire(lookupKey, id.Trim(), threatStore)`**. + - Zero-damage abilities (guard/dash) skip acquire. + +4. **Move hook (`PositionStateApi`)** + - Inject **`IThreatStateStore`** + **`INpcBehaviorDefinitionRegistry`** on **`POST /move`** and **`POST /move-stream`** success paths. + - After **`TryApplyMoveTarget`** / stream apply succeeds, call **`TryClearOnLeashForPlayer`** for route `id`. + +5. **Dev fixture (`CombatTargetFixtureApi`)** + - Inject **`IThreatStateStore`**; after HP reset loop, clear all prototype NPC threat holders (fresh Bruno runs). + +6. **DI wiring** + - Call **`AddThreatStateStore()`** from **`Program.cs`** (or co-locate with NPC catalog extension). + - No change to **`CombatOperations`** signature — aggro stays outside the combat engine. + +7. **Docs** + - **`server/README.md`** — brief threat/aggro section: first-hit acquire, leash clear triggers, fixture reset, no HTTP read yet (NEO-94). + - Reconcile [E5M2-prototype-backlog.md](E5M2-prototype-backlog.md) E5M2-06 checkboxes when landed. + +### Leash radii (frozen catalog, for test expectations) + +| NPC instance | Behavior def | `leashRadius` | Anchor (X, Z) | +|--------------|--------------|---------------|---------------| +| `prototype_npc_melee` | `prototype_melee_pressure` | 16.0 | (-3, -3) | +| `prototype_npc_ranged` | `prototype_ranged_control` | 20.0 | (3, 3) | +| `prototype_npc_elite` | `prototype_elite_mini_boss` | 18.0 | (0, 0) | + +**Example (melee):** dev player at origin `(0, 0, 0)` is ~4.24 m from melee anchor — inside leash 16. Move to `(20, 0, 20)` → distance ~32.5 > 16 → holder clears. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Npc/ThreatStateSnapshot.cs` | Immutable aggro holder snapshot for one NPC instance. | +| `server/NeonSprawl.Server/Game/Npc/IThreatStateStore.cs` | Store contract keyed by NPC instance id. | +| `server/NeonSprawl.Server/Game/Npc/InMemoryThreatStateStore.cs` | Thread-safe in-memory prototype implementation. | +| `server/NeonSprawl.Server/Game/Npc/ThreatStateServiceCollectionExtensions.cs` | DI registration for `IThreatStateStore`. | +| `server/NeonSprawl.Server/Game/Npc/AggroOperations.cs` | `TryAcquire` + `TryClearOnLeashForPlayer` deterministic ops. | +| `server/NeonSprawl.Server.Tests/Game/Npc/AggroOperationsTests.cs` | AAA unit tests: acquire, no-op when held, leash clear, re-acquire after clear. | +| `server/NeonSprawl.Server.Tests/Game/Npc/InMemoryThreatStateStoreTests.cs` | AAA store tests: lazy rows, unknown id, clear/set. | +| `server/NeonSprawl.Server.Tests/Game/Npc/AggroThreatIntegrationTests.cs` | HTTP integration: cast acquire + move beyond leash clears holder via injected store. | +| `docs/plans/NEO-92-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register `AddThreatStateStore()`. | +| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Inject threat store; leash clear + acquire after damaging resolve. | +| `server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` | Leash clear on successful move + move-stream. | +| `server/NeonSprawl.Server/Game/Combat/CombatTargetFixtureApi.cs` | Reset threat holders in dev fixture alongside HP reset. | +| `server/README.md` | Document aggro acquire/clear behavior and fixture reset. | +| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | Assert holder set after pulse cast; zero-damage guard skips acquire. | +| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs` | Move beyond leash clears aggro holder (cast setup helper). | +| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | Threat store landed note when complete. | +| `docs/plans/E5M2-prototype-backlog.md` | E5M2-06 acceptance checkboxes + landed note when complete. | + +## Tests + +| File | Coverage | +|------|----------| +| `AggroOperationsTests.cs` | First acquire sets holder; second acquire no-op; clear when distance > leashRadius; no clear when inside; re-acquire after clear on next TryAcquire. | +| `InMemoryThreatStateStoreTests.cs` | Unknown npc id fails; set/get/clear; thread-safe spot check if trivial. | +| `AggroThreatIntegrationTests.cs` | End-to-end: bind pulse → lock melee → cast → store has `dev-local-1`; move far → holder cleared; cast again → re-acquire. | +| `AbilityCastApiTests.cs` | Damaging cast sets holder (via store resolve from factory); guard cast leaves holder empty. | +| `MoveCommandApiTests.cs` | After aggro via cast, validated move outside leash clears holder. | + +No Bruno changes required for AC (internal store not on wire); optional Bruno note in README only. No `docs/manual-qa/NEO-92.md`. + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|---------------------|--------| +| **Linear blockedBy NEO-91** | Proceed — NEO-91 **Done** on `main`. | **adopted** | +| **No HTTP read surface** | Accept — NEO-94 projects holder; tests inject `IThreatStateStore` from factory. | **adopted** | +| **Leash vs lock radius confusion** | Use catalog **`leashRadius`** for aggro clear; **`LockRadius` (6.0)** remains targeting-only. | **adopted** | +| **Multi-player holder steal** | Out of prototype scope; no-op when holder set (kickoff). | **adopted** | +| **Proximity re-aggro in NEO-93** | NEO-92 first-hit only; state machine may add radius checks later — do not implement proximity acquire here. | **deferred** | + +## Reconciliation (implementation) + +- **`IThreatStateStore`**, **`InMemoryThreatStateStore`**, and **`AggroOperations`** landed in `Game/Npc/`. +- Acquire wired in **`AbilityCastApi`** after damaging resolve; leash clear on **`PositionStateApi`** move paths and before acquire. +- Dev **`combat-targets-fixture`** clears threat holders alongside HP reset. +- Unit + integration tests cover acquire, leash clear, re-acquire, guard skip, and fixture reset.