neon-sprawl/docs/plans/NEO-91-implementation-plan.md

163 lines
13 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-91 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-91 |
| **Title** | E5M2-05: Prototype NPC instance registry + combat-target migration |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-91/e5m2-05-prototype-npc-instance-registry-combat-target-migration |
| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-05** |
| **Branch** | `NEO-91-npc-instance-registry-combat-target-migration` |
| **Precursor** | [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) — `INpcBehaviorDefinitionRegistry` + DI (**Done** on `main`) |
| **Pattern** | [NEO-83](https://linear.app/neon-sprawl/issue/NEO-83) — combat-targets GET + `PrototypeTargetRegistry`; [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) — injectable behavior registry |
| **Client counterpart** | [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) — Godot archetype markers + tab cycle (**out of scope** here; **breaking id change** must land in NEO-97 before playable Godot verification) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Registry shape** | New `PrototypeNpcRegistry` vs extend `PrototypeTargetRegistry`? | **New `PrototypeNpcRegistry` in `Game/Npc/`** — targeting/combat consume it; remove alpha/beta ids. | **User:** new `PrototypeNpcRegistry`. |
| **World anchors** | Where to place three NPC instances? | **Melee at alpha `(-3, 0.5, -3)`, ranged at beta `(3, 0, 3)`, elite at origin `(0, 0.5, 0)`; `SharedLockRadius` 6.0.** | **User:** preserve alpha/beta + elite at origin. |
| **Behavior binding** | Which behavior def per instance? | **1:1:** melee→`prototype_melee_pressure`, ranged→`prototype_ranged_control`, elite→`prototype_elite_mini_boss`. | **Adopted** (frozen naming in E5.M2). |
| **Bruno defeat spine** | Which NPC for defeat AC? | **`prototype_npc_melee`** (100 HP, 4× pulse — alpha replacement). | **User:** melee. |
## Goal, scope, and out-of-scope
**Goal:** Replace passive **`prototype_target_alpha` / `prototype_target_beta`** dummies with **three** NPC combat instances wired into targeting, cast resolution, and the combat-target HP store — each instance bound to a frozen `behaviorDefId` with **max HP from the behavior catalog**.
**In scope (from Linear + [E5M2-05](E5M2-prototype-backlog.md#e5m2-05--prototype-npc-instance-registry--combat-target-migration)):**
- **`PrototypeNpcRegistry`** in `Game/Npc/`: instance id, `behaviorDefId`, world anchor, shared lock radius.
- Migrate **`ICombatEntityHealthStore`** gates and max HP from flat 100 → per-instance **`maxHp`** via **`INpcBehaviorDefinitionRegistry`**.
- Replace **`PrototypeTargetRegistry`** alpha/beta usage across targeting, cast, combat-targets GET, dev fixture reset.
- Update server unit/integration tests and **all Bruno** smokes that reference alpha/beta.
- Document **breaking id change** + required client constant migration ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)).
**Out of scope (from Linear + backlog):**
- Behavior tick, aggro, telegraphs ([NEO-92+](E5M2-prototype-backlog.md)).
- Godot markers / tab cycle ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)).
- `docs/manual-qa/NEO-91.md` — server-only; Bruno + automated tests (NEO-83/90 pattern).
## Acceptance criteria checklist
- [x] Cast + combat-targets GET operate on **three NPC ids only** (alpha/beta removed).
- [x] Each instance references a valid frozen **`behaviorDefId`** resolvable via **`INpcBehaviorDefinitionRegistry`**.
- [x] Bruno defeat spine passes for **`prototype_npc_melee`** (4× pulse → defeated).
## Technical approach
1. **`PrototypeNpcRegistry`** (`Game/Npc/`):
- Stable id constants: **`prototype_npc_melee`**, **`prototype_npc_ranged`**, **`prototype_npc_elite`**.
- **`SharedLockRadius = 6.0`** (same convention as E1.M3/E5.M1 dummies).
- **`PrototypeNpcInstanceEntry`**: `BehaviorDefId`, anchor `X/Y/Z`, `LockRadius`.
- Frozen bindings (kickoff):
| Instance id | `behaviorDefId` | Anchor (X, Y, Z) |
|-------------|-----------------|------------------|
| `prototype_npc_melee` | `prototype_melee_pressure` | (-3, 0.5, -3) |
| `prototype_npc_ranged` | `prototype_ranged_control` | (3, 0, 3) |
| `prototype_npc_elite` | `prototype_elite_mini_boss` | (0, 0.5, 0) |
- **`TryGet(instanceIdLowercase, out entry)`**, **`GetInstanceIdsInOrder()`** (ordinal sort — tab cycle order for future NEO-97).
2. **Retire `PrototypeTargetRegistry`:** Remove alpha/beta ids and the class (or leave a one-release obsolete shim only if grep shows external consumers — prefer **delete** and update all server references). Targeting/combat import **`PrototypeNpcRegistry`** instead.
3. **`InMemoryCombatEntityHealthStore`:** Constructor-inject **`INpcBehaviorDefinitionRegistry`**. Gate rows on **`PrototypeNpcRegistry.TryGet`**. Lazy-init **`currentHp`** to behavior **`maxHp`**; **`CreateSnapshot`** / **`TryResetToFull`** use per-instance max from catalog. Remove dependence on **`PrototypeCombatConstants.MaxPrototypeTargetHp`** (delete constant class if unused).
4. **Targeting + cast paths:** **`PlayerTargetStateReader`**, **`TargetingApi`**, **`AbilityCastApi`** — swap **`PrototypeTargetRegistry.TryGet`** → **`PrototypeNpcRegistry.TryGet`** (range math unchanged).
5. **Combat-targets GET + fixture:** **`CombatTargetsWorldApi`**, **`CombatTargetFixtureApi`** — enumerate **`PrototypeNpcRegistry.GetInstanceIdsInOrder()`** (expect **3** rows).
6. **Bruno migration:** Replace every **`prototype_target_alpha`** / **`prototype_target_beta`** with NPC ids. Key updates:
- **`combat-targets/`** — assert **3** targets; defeat spine on **`prototype_npc_melee`** (same 4× pulse / 75→50→25→0 stepping).
- **`ability-cast/`** — happy cast on **`prototype_npc_ranged`** (beta slot/spine replacement); lock/defeat spines on melee.
- **`targeting/`** — select melee/ranged ids.
- **`gig-progression/Get gig progression after defeat spine.bru`** — melee defeat.
- **`scripts/combat-targets-reset-helper.js`** — unchanged route; resets all registry ids server-side.
7. **Docs:** **`server/README.md`** — combat targets section lists three NPC ids + per-archetype max HP; note **`PrototypeNpcRegistry`** + breaking change for **`client/scripts/prototype_target_constants.gd`** ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)). Update [E5_M2](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) instance registry bullet and [E5M2-prototype-backlog.md](E5M2-prototype-backlog.md) E5M2-05 checkboxes when landed.
### Expected snapshot values (fresh server, `prototype_pulse` = 25 damage)
| Instance | `maxHp` (from behavior def) | After 4× pulse on melee only |
|----------|------------------------------|------------------------------|
| `prototype_npc_elite` | 200 | 200 / 200 |
| `prototype_npc_melee` | 100 | 0 / 100, `defeated: true` |
| `prototype_npc_ranged` | 80 | 80 / 80 |
(GET row order: ascending **`targetId`** — elite, melee, ranged.)
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Npc/PrototypeNpcRegistry.cs` | Frozen three-instance registry: ids, behavior bindings, anchors, lock radius, ordered enumeration. |
| `server/NeonSprawl.Server.Tests/Game/Npc/PrototypeNpcRegistryTests.cs` | AAA unit tests: three ids in order, valid behavior bindings, anchor/lock radius parity, unknown id fails. |
| `docs/plans/NEO-91-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Combat/InMemoryCombatEntityHealthStore.cs` | Gate on `PrototypeNpcRegistry`; per-instance max HP from `INpcBehaviorDefinitionRegistry`. |
| `server/NeonSprawl.Server/Game/Combat/CombatEntityHealthServiceCollectionExtensions.cs` | Ensure health store resolves with behavior registry dependency (singleton ctor injection). |
| `server/NeonSprawl.Server/Game/Combat/CombatTargetsWorldApi.cs` | Enumerate NPC instance ids from `PrototypeNpcRegistry`. |
| `server/NeonSprawl.Server/Game/Combat/CombatTargetFixtureApi.cs` | Reset all three NPC instance rows. |
| `server/NeonSprawl.Server/Game/Combat/ICombatEntityHealthStore.cs` | XML remarks: NPC instance ids + catalog max HP (not alpha/beta / flat 100). |
| `server/NeonSprawl.Server/Game/Combat/PrototypeCombatConstants.cs` | Remove or narrow — flat 100 no longer authoritative. |
| `server/NeonSprawl.Server/Game/Targeting/PlayerTargetStateReader.cs` | Use `PrototypeNpcRegistry.TryGet` for validity/range. |
| `server/NeonSprawl.Server/Game/Targeting/TargetingApi.cs` | Target select gate on NPC registry. |
| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Cast target gate on NPC registry. |
| `server/NeonSprawl.Server/Game/Targeting/PrototypeTargetRegistry.cs` | **Delete** (after references migrated). |
| `server/README.md` | Document three NPC combat targets, registry, breaking id change, per-archetype HP. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatEntityHealthStoreTests.cs` | AAA tests for per-archetype max HP, melee/ranged/elite independence. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatTargetsWorldApiTests.cs` | Three targets, ordinal order, melee defeat spine, per-archetype maxHp assertions. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatOperationsTests.cs` | Replace alpha/beta ids with NPC ids. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatTargetFixtureApiTests.cs` | Three-instance reset. |
| `server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs` | NPC id select/range cases. |
| `server/NeonSprawl.Server.Tests/Game/Targeting/PlayerTargetStateReaderTests.cs` | NPC anchor range checks. |
| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | Cast against NPC ids. |
| `server/NeonSprawl.Server.Tests/Game/AbilityInput/CooldownSnapshotApiTests.cs` | Update target id literals if referenced. |
| `server/NeonSprawl.Server.Tests/Game/Gigs/GigProgressionGrantPersistenceIntegrationTests.cs` | Melee defeat id in integration spine. |
| `bruno/neon-sprawl-server/combat-targets/*.bru` | Three-target assertions; melee defeat spine. |
| `bruno/neon-sprawl-server/ability-cast/*.bru` | NPC target ids in lock/cast spines. |
| `bruno/neon-sprawl-server/targeting/*.bru` | NPC target ids in select requests. |
| `bruno/neon-sprawl-server/gig-progression/Get gig progression after defeat spine.bru` | Melee defeat spine. |
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | Instance registry landed note (when complete). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M2 row — NEO-91 instance registry. |
| `docs/plans/E5M2-prototype-backlog.md` | E5M2-05 acceptance checkboxes + landed note when complete. |
**Explicitly not modified (client — NEO-97):** `client/scripts/prototype_target_constants.gd`, Godot tests, manual QA. Plan documents the required client migration.
## Tests
| File | Coverage |
|------|----------|
| `server/NeonSprawl.Server.Tests/Game/Npc/PrototypeNpcRegistryTests.cs` | Three ids ordinal order; each `behaviorDefId` resolves via host registry; anchors + `SharedLockRadius`; unknown id fails `TryGet`. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatEntityHealthStoreTests.cs` | Lazy init to catalog maxHp (100/80/200); damage/defeat/reset per instance; independence across three rows. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatTargetsWorldApiTests.cs` | GET returns 3 targets in id order with correct maxHp; one pulse on melee → 75/100; four pulses → melee defeated, others full. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatOperationsTests.cs` | Resolve against NPC ids (spot-check melee + elite maxHp paths). |
| `server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs` | Select melee in range; unknown legacy alpha id → `unknown_target`. |
| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | Cast accept/deny with NPC target ids. |
Bruno **`combat-targets/Get combat targets after cast spine.bru`** is the AC defeat spine (melee, 4× pulse). No `docs/manual-qa/NEO-91.md` (server-only).
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|---------------------|--------|
| **Linear blockedBy NEO-89** | Proceed — NEO-89 **Done** on `main`. | **adopted** |
| **Client breakage until NEO-97** | Document in plan + README; cross-link NEO-97; Bruno-only verification is not prototype-complete per [full-stack epic decomposition](../../.cursor/rules/full-stack-epic-decomposition.md). | **adopted** |
| **Flat `MaxPrototypeTargetHp` removal** | Delete constant; tests assert catalog values per instance. | **adopted** |
| **Gig XP defeat grant** | No change — grant fires on any target defeat; melee spine still satisfies NEO-44 chain. | **adopted** |
| **Elite HP in future spines** | Defer 8× pulse Bruno until needed; melee spine satisfies AC. | **deferred** |
## Reconciliation (implementation)
- **`PrototypeNpcRegistry`** landed in `Game/Npc/`; **`PrototypeTargetRegistry`** and **`PrototypeCombatConstants`** removed.
- **`InMemoryCombatEntityHealthStore`** injects **`INpcBehaviorDefinitionRegistry`** for per-instance catalog max HP.
- Server tests + Bruno migrated to three NPC ids; melee 4× pulse defeat spine is AC verification.
- **Client not updated** — [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) owns `prototype_target_constants.gd` migration.