NEO-95: add implementation plan for NPC attack resolve + player HP

pull/133/head
VinPropane 2026-05-28 23:37:46 -04:00
parent 8bbf737b2d
commit ac4e1dff10
1 changed files with 176 additions and 0 deletions

View File

@ -0,0 +1,176 @@
# NEO-95 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-95 |
| **Title** | E5M2-09: NPC attack resolve + session player combat HP |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-95/e5m2-09-npc-attack-resolve-session-player-combat-hp |
| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-09** |
| **Branch** | `NEO-95-npc-attack-resolve-player-hp` |
| **Precursor** | [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) — TelegraphEvent snapshot GET + wire DTOs (**Done** on `main`) |
| **Pattern** | [NEO-80/91](https://linear.app/neon-sprawl/issue/NEO-91) — `ICombatEntityHealthStore` for NPC HP; [NEO-32](https://linear.app/neon-sprawl/issue/NEO-32) — player-scoped GET snapshot (`cooldown-snapshot`) |
| **Blocks** | [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) — telegraph HUD + player HP labels (polls this GET + npc-runtime-snapshot) |
| **Client counterpart** | [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) — player HP + incoming telegraph/attack feedback; capstone [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Player HP read model** | Nested on npc-runtime-snapshot vs dedicated player GET? | **`GET /game/players/{id}/combat-health`** — player-scoped route mirrors `cooldown-snapshot`; keeps world snapshot NPC-only. | **User:** dedicated GET. |
| **Manual QA doc** | Add `docs/manual-qa/NEO-95.md`? | **Skip** — server-only HTTP; Bruno + integration tests (NEO-94 pattern). | **User:** skip. |
## Goal, scope, and out-of-scope
**Goal:** When an NPC telegraph completes during lazy tick advance, apply catalog **`attackDamage`** to the aggro holder via **`IPlayerCombatHealthStore`**. Expose session player combat HP on a player GET for client HUD poll.
**In scope (from Linear + [E5M2-09](E5M2-prototype-backlog.md#e5m2-09--npc-attack-resolve--session-player-combat-hp)):**
- **`IPlayerCombatHealthStore`** + in-memory implementation (session-only, lazy init **100** HP).
- **`NpcAttackOperations.TryResolveTelegraphComplete`** invoked from **`NpcRuntimeOperations.AdvanceAll`** when windup completes.
- **`GET /game/players/{id}/combat-health`** — **`schemaVersion` 1**, **`playerId`**, **`maxHp`**, **`currentHp`**, **`defeated`**; **404** when player has no position row (same gate as cooldown-snapshot).
- Holder re-check at damage time — skip damage if aggro holder cleared during windup (leash clear).
- Comment-only **`player_death`** hook site when HP reaches **0** (no respawn loop).
- Extend dev **`POST /game/__dev/combat-targets-fixture`** to reset dev player combat HP to full.
- Integration tests (AAA) with **`InMemoryWebApplicationFactory.FakeClock`** — elite telegraph complete → holder HP **100 → 75**; GET read model matches store.
- Bruno folder **`bruno/neon-sprawl-server/combat-health/`** — baseline GET + post-attack HP smoke.
- **`server/README.md`** player combat HP section; update NPC runtime “attack_execute stub” note.
**Out of scope (from Linear + backlog):**
- Godot HUD ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)).
- Postgres persistence, healing, player defensive abilities.
- Telemetry instrumentation ([NEO-96](https://linear.app/neon-sprawl/issue/NEO-96)).
- `docs/manual-qa/NEO-95.md` (kickoff decision).
## Acceptance criteria checklist
- [ ] Elite archetype telegraph → attack deals catalog damage (**25**) to holder.
- [ ] Player HP read model matches store after NPC attack.
- [ ] No client-side damage math required for verification (Bruno or integration tests).
## Technical approach
### Damage resolve flow
1. **`NpcRuntimeOperations.AdvanceOne`** — on **`TelegraphWindup`** phase completion (transition to **`Recover`**), call **`NpcAttackOperations.TryResolveTelegraphComplete`** **before** writing the new state.
2. **`TryResolveTelegraphComplete(npcInstanceId, holderPlayerId, attackDamage, playerHealthStore, threatStore)`**:
- Re-verify holder via **`IThreatStateStore`** — if holder missing or differs, **no-op** (leash clear during windup).
- **`playerHealthStore.TryApplyDamage(holderPlayerId, attackDamage, out _)`**.
- When resulting **`currentHp == 0`**: comment-only **`// TODO(E9.M1): player_death`** (mirror NEO-84 combat hooks).
3. **`AdvanceAll`** gains optional **`IPlayerCombatHealthStore`** parameter (required in production callers — snapshot GET and tests pass the store from DI).
### Player combat health store
Mirror **`ICombatEntityHealthStore`** shape but keyed by **player id** (not NPC instance id):
| Method | Behavior |
|--------|----------|
| **`TryGet`** | Lazy-init **`currentHp = maxHp = 100`** on first access for any non-empty normalized player id. |
| **`TryApplyDamage`** | Non-negative damage; floor at zero; return snapshot with **`defeated`** when **`currentHp == 0`**. |
| **`TryResetToFull`** | Restore to **100** HP; create row if absent (dev fixture + tests). |
**Constant:** **`PlayerCombatHealthDefaults.MaxHp = 100`** — flat prototype default per E5M2 kickoff table (not catalog-derived).
### HTTP read model
**`GET /game/players/{id}/combat-health`** (NEO-95):
```json
{
"schemaVersion": 1,
"playerId": "dev-local-1",
"maxHp": 100,
"currentHp": 75,
"defeated": false
}
```
- Gate: **`IPositionStateStore.TryGetPosition(id)`** — **404** when unknown player (same as **`CooldownSnapshotApi`**).
- Lazy-init on GET — first poll returns full **100** HP without requiring prior NPC attack.
- **`NpcRuntimeSnapshotWorldApi`** unchanged — NPC-only world poll; NEO-97 polls both endpoints at ~1 Hz.
### Lazy tick wiring
**`NpcRuntimeSnapshotWorldApi`** handler injects **`IPlayerCombatHealthStore`** and passes it to **`AdvanceAll`** so poll-driven advance applies damage before building the NPC snapshot response.
### Example timeline (elite, integration test with **`FakeClock`**)
Elite: **`attackCooldownSeconds` 5.0**, **`telegraphWindupSeconds` 2.5**, **`attackDamage` 25**.
```
t=0 lock elite + damaging cast → GET combat-health currentHp 100
t=0 GET npc-runtime-snapshot → elite state "aggro"
t=5 GET npc-runtime-snapshot (AdvanceAll) → "telegraph_windup"
t=7.5 GET npc-runtime-snapshot (AdvanceAll) → windup complete, damage applied, state "recover"
GET combat-health → currentHp 75, defeated false
```
### Dev fixture extension
**`CombatTargetFixtureApi`** — after NPC HP + threat + runtime reset, call **`playerHealthStore.TryResetToFull(devPlayerId)`** using **`Game:DevPlayerId`** from **`IOptions<GamePositionOptions>`** so Bruno smoke starts at full player HP.
### Bruno smoke (E5M2-09 AC)
1. Reset fixture (**`combat-targets-reset-helper.js`**).
2. Bind slot 0 **`prototype_pulse`**, lock **`prototype_npc_elite`**, damaging cast.
3. Poll **`GET /game/world/npc-runtime-snapshot`** until elite past windup (real-time wait or sleep ≥ **7.5 s** against dev server).
4. **`GET /game/players/dev-local-1/combat-health`** — assert **`currentHp === 75`**.
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Combat/IPlayerCombatHealthStore.cs` | Session player HP contract (TryGet / TryApplyDamage / TryResetToFull). |
| `server/NeonSprawl.Server/Game/Combat/PlayerCombatHealthSnapshot.cs` | Immutable snapshot record (`playerId`, `maxHp`, `currentHp`, `defeated`). |
| `server/NeonSprawl.Server/Game/Combat/PlayerCombatHealthDefaults.cs` | **`MaxHp = 100`** prototype constant. |
| `server/NeonSprawl.Server/Game/Combat/InMemoryPlayerCombatHealthStore.cs` | Thread-safe in-memory implementation. |
| `server/NeonSprawl.Server/Game/Combat/PlayerCombatHealthServiceCollectionExtensions.cs` | DI registration singleton. |
| `server/NeonSprawl.Server/Game/Combat/NpcAttackOperations.cs` | **`TryResolveTelegraphComplete`** — holder re-check + damage apply + death hook comment. |
| `server/NeonSprawl.Server/Game/Combat/PlayerCombatHealthApi.cs` | **`GET /game/players/{id}/combat-health`**. |
| `server/NeonSprawl.Server/Game/Combat/PlayerCombatHealthDtos.cs` | Response envelope JSON DTO. |
| `server/NeonSprawl.Server.Tests/Game/Combat/PlayerCombatHealthStoreTests.cs` | Unit tests — lazy init, damage floor, reset, defeated flag. |
| `server/NeonSprawl.Server.Tests/Game/Combat/NpcAttackOperationsTests.cs` | Unit tests — resolve applies damage; no-op when holder cleared. |
| `server/NeonSprawl.Server.Tests/Game/Combat/PlayerCombatHealthApiTests.cs` | Integration tests — schema v1 baseline, elite telegraph → HP 75, 404 unknown player. |
| `bruno/neon-sprawl-server/combat-health/folder.bru` | Bruno folder metadata. |
| `bruno/neon-sprawl-server/combat-health/Get player combat health.bru` | Happy GET — schema v1 + full HP baseline. |
| `bruno/neon-sprawl-server/combat-health/Get combat health after elite attack.bru` | Lock elite → cast → poll → assert HP decreased. |
| `docs/plans/NEO-95-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs` | Pass **`IPlayerCombatHealthStore`** into **`AdvanceAll`** / **`AdvanceOne`**; invoke **`NpcAttackOperations.TryResolveTelegraphComplete`** on telegraph completion. |
| `server/NeonSprawl.Server/Game/Npc/NpcRuntimeSnapshotWorldApi.cs` | Inject **`IPlayerCombatHealthStore`**; pass to **`AdvanceAll`** on poll. |
| `server/NeonSprawl.Server/Game/Combat/CombatTargetFixtureApi.cs` | Reset dev player combat HP on fixture POST. |
| `server/NeonSprawl.Server/Program.cs` | Register **`AddPlayerCombatHealthStore()`** + **`MapPlayerCombatHealthApi()`**. |
| `server/README.md` | Add **`GET /game/players/{id}/combat-health`** section; update attack_execute stub + fixture reset notes. |
| `server/NeonSprawl.Server.Tests/Game/Npc/NpcRuntimeOperationsTests.cs` | Pass null or test store into updated **`AdvanceAll`** signature; add test that windup completion applies player damage. |
| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | NEO-95 landed note + player combat HP cross-link when complete. |
| `docs/plans/E5M2-prototype-backlog.md` | E5M2-09 acceptance checkboxes + landed note when complete. |
## Tests
| File | Coverage |
|------|----------|
| **`PlayerCombatHealthStoreTests.cs`** | **Lazy init:** first **`TryGet`** returns **`maxHp`/`currentHp` 100**, not defeated. **Damage:** **`TryApplyDamage(15)`** → **85** HP. **Floor:** damage below zero HP → **0**, **`defeated` true**. **Reset:** **`TryResetToFull`** restores 100. **Invalid:** empty player id returns false. |
| **`NpcAttackOperationsTests.cs`** | **Happy path:** holder set → **`TryResolveTelegraphComplete`** applies catalog damage. **Holder cleared:** leash clear before resolve → no HP mutation. **Zero damage:** no-op success. |
| **`NpcRuntimeOperationsTests.cs`** | **Windup → damage:** holder on melee, advance through windup with player store → holder HP decreased by **15**. |
| **`PlayerCombatHealthApiTests.cs`** | **Baseline GET:** **`dev-local-1`** returns schema v1, **100/100**, not defeated. **Elite attack (AC):** lock elite, cast, advance **`FakeClock`** **7.5 s**, GET snapshot then GET combat-health → **`currentHp` 75**. **Read model match:** store **`TryGet`** equals HTTP body. **404:** unknown player id. |
| **Bruno `combat-health/`** | CI Bruno step — baseline GET + elite attack HP smoke per E5M2-09 AC. |
No `docs/manual-qa/NEO-95.md`. No Godot tests.
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|---------------------|--------|
| **Linear blockedBy NEO-94** | Proceed — NEO-94 **Done** on `main`. | **adopted** |
| **Player HP on world snapshot** | Rejected at kickoff — dedicated player GET only. | **adopted** (kickoff) |
| **Holder cleared mid-windup** | Skip damage at resolve time if threat row empty or holder mismatch. | **adopted** |
| **Multiple NPCs same holder** | Each telegraph completion applies its own **`attackDamage`** independently — deterministic, no cap in prototype. | **adopted** |
| **`AdvanceAll` signature change** | Add **`IPlayerCombatHealthStore?`** or required param; update snapshot GET + all tests. | **adopted** |
| **`player_death` telemetry** | Comment-only hook at HP **0**; full event wiring deferred to E9.M1 / future player death story. | **adopted** |
| **Bruno real-time wait** | Integration tests own timing precision; Bruno may use **`sleep(8000)`** against dev server — document in `.bru` docs block (NEO-94 pattern). | **adopted** |