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

148 lines
12 KiB
Markdown
Raw 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-81 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-81 |
| **Title** | E5M1-06: CombatOperations.TryResolve + CombatResult |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-81/e5m1-06-combatoperationstryresolve-combatresult |
| **Module** | [E5.M1 — CombatRulesEngine](../decomposition/modules/E5_M1_CombatRulesEngine.md) · Epic 5 Slice 1 · backlog **E5M1-06** |
| **Branch** | `NEO-81-combat-operations-try-resolve` |
| **Precursor** | [NEO-80](https://linear.app/neon-sprawl/issue/NEO-80) — `ICombatEntityHealthStore` (**Done** on `main`); [NEO-79](https://linear.app/neon-sprawl/issue/NEO-79) — `IAbilityDefinitionRegistry` (**Done** on `main`) |
| **Pattern** | [NEO-62](https://linear.app/neon-sprawl/issue/NEO-62) / [NEO-69](https://linear.app/neon-sprawl/issue/NEO-69) — static `*Operations.Try*` + `*Result` envelope + `*ReasonCodes`; NEO-80 store handoff for defeated re-hit deny |
| **Blocks** | [NEO-82](https://linear.app/neon-sprawl/issue/NEO-82) — wire `ability-cast` into combat engine + extend cast response |
| **Client counterpart** | None (server-only internal ops); wire **`CombatResolution`** on cast POST is [NEO-82](https://linear.app/neon-sprawl/issue/NEO-82); HP HUD is [NEO-85](https://linear.app/neon-sprawl/issue/NEO-85) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Defeated-target deny scope** | Deny all abilities on defeated target, including zero-damage utility? | **Deny all** — E5M1-07 uses `target_defeated` on cast at defeated dummy regardless of ability kind. | **User:** deny all. |
| **Unknown id validation** | Validate ability + target inside `TryResolve` vs assume E1.M4 pre-validated? | **Validate inside** — backlog requires stable deny for unknown ids; mirrors `GatherOperations` / `CraftOperations` self-contained validation. | **User:** validate inside. |
| **Zero-damage success path** | `TryGet` only vs `TryApplyDamage(target, 0)`? | **Success with `damageDealt = 0`; `TryGet` snapshot only** — no HP mutation when catalog damage is 0; still lazy-inits via `TryGet`. | **User:** success, TryGet only. |
## Goal, scope, and out-of-scope
**Goal:** Provide pure server combat resolution: known ability + known prototype target → deterministic catalog damage, remaining HP, and defeated flag. Return structured denies (no silent no-op) when the target is already defeated or ids are unknown.
**In scope (from Linear + [E5M1-06](E5M1-prototype-backlog.md#e5m1-06--combatoperationstryresolve--combatresult)):**
- `CombatOperations.TryResolve` using **`IAbilityDefinitionRegistry`** + **`ICombatEntityHealthStore`**.
- `CombatResult` readonly record struct with **`DamageDealt`**, **`TargetRemainingHp`**, **`TargetDefeated`**, **`Success`**, **`ReasonCode`**.
- `CombatReasonCodes` stable deny strings aligned with E1.M4 / E5M1-07 vocabulary.
- Unit tests (AAA): happy path (`prototype_pulse` → 25 dmg), zero-damage utility success, defeated-target deny (including guard/dash), unknown ability/target denies.
- `server/README.md` combat engine section.
**Out of scope (from Linear + backlog):**
- HTTP wiring / `AbilityCastResponse` extension ([NEO-82](https://linear.app/neon-sprawl/issue/NEO-82)).
- Per-ability cooldown commit ([NEO-82](https://linear.app/neon-sprawl/issue/NEO-82)).
- Gig XP on defeat ([NEO-44](https://linear.app/neon-sprawl/issue/NEO-44)).
- Telemetry hooks (reserved for follow-up like NEO-64 / NEO-71 pattern).
- Godot / client changes.
- Bruno (no HTTP surface in this story).
## Acceptance criteria checklist
- [x] **`prototype_pulse`** against full-HP dummy deals catalog damage (25) deterministically.
- [x] Second resolve on defeated target returns structured deny (no silent no-op).
- [x] Zero-damage utility abilities succeed with **`damageDealt = 0`** on live targets.
- [x] Unknown ability / unknown target return stable deny codes from **`CombatOperations`**.
- [x] Defeated-target deny applies to zero-damage utility abilities.
## Technical approach
1. **`CombatReasonCodes`** — static constants in `Game/Combat/`:
- **`TargetDefeated = "target_defeated"`** — target HP is already 0 (checked **before** damage application; no store mutation).
- **`UnknownAbility = "unknown_ability"`** — ability id fails **`IAbilityDefinitionRegistry.TryNormalizeKnown`** (matches cast/hotbar vocabulary).
- **`UnknownTarget = "unknown_target"`** — target id fails **`ICombatEntityHealthStore.TryGet`** (store gates **`PrototypeTargetRegistry`** ids; matches E1.M3 targeting vocabulary).
2. **`CombatResult`** — readonly record struct (mirror [`CraftResult`](../../server/NeonSprawl.Server/Game/Crafting/CraftResult.cs)):
- **`Success`** — `true` when damage resolution committed (including zero-damage success).
- **`ReasonCode`** — `null` on success; stable string on deny.
- **`DamageDealt`** — catalog **`AbilityDefRow.BaseDamage`** on success; **0** on deny.
- **`TargetRemainingHp`** — authoritative HP after resolve on success; **null** on deny (or current HP on `target_defeated` deny — see step 4).
- **`TargetDefeated`** — `true` when post-resolve HP is 0 on success; **false** on deny (even when target was already defeated — caller uses **`ReasonCode`**).
3. **`CombatOperations`** — static class (mirror [`GatherOperations`](../../server/NeonSprawl.Server/Game/Gathering/GatherOperations.cs)):
```csharp
public static CombatResult TryResolve(
string abilityId,
string targetId,
IAbilityDefinitionRegistry abilityRegistry,
ICombatEntityHealthStore healthStore)
```
- **Ability gate:** `abilityRegistry.TryNormalizeKnown(abilityId, out var normalizedAbility)` → deny **`unknown_ability`** when false.
- **Definition load:** `abilityRegistry.TryGetDefinition(normalizedAbility, out var definition)` — should succeed after normalize; treat miss as **`unknown_ability`** (defensive).
- **Target read:** `healthStore.TryGet(targetId, out var snapshot)` → deny **`unknown_target`** when false (lazy-inits live targets).
- **Defeated pre-check:** when **`snapshot.Defeated`** → deny **`target_defeated`** **without** calling **`TryApplyDamage`** (NEO-80 handoff; applies to all abilities including zero-damage).
- **Zero-damage path:** when **`definition.BaseDamage == 0`** → success with **`DamageDealt = 0`**, **`TargetRemainingHp = snapshot.CurrentHp`**, **`TargetDefeated = snapshot.Defeated`** (always false for live target after pre-check).
- **Damage path:** `healthStore.TryApplyDamage(targetId, definition.BaseDamage, out var after)` → success with **`DamageDealt = definition.BaseDamage`**, **`TargetRemainingHp = after.CurrentHp`**, **`TargetDefeated = after.Defeated`**.
- Private **`Deny(string reasonCode)`** helper returns **`Success = false`**, empty damage fields.
4. **`target_defeated` deny payload:** On defeated pre-check, set **`TargetRemainingHp = 0`** (from snapshot) so callers/tests can observe current HP without re-querying store; **`DamageDealt = 0`**, **`TargetDefeated = false`** on envelope (deny is **`ReasonCode`**-driven — same pattern as gather depletion deny carrying **`RemainingGathers`**).
5. **No DI registration** — static ops class; tests construct **`InMemoryCombatEntityHealthStore`** + in-memory registry helper (same as **`AbilityDefinitionRegistryTests`**). Host already registers both dependencies via **`AddAbilityDefinitionCatalog`**.
6. **`server/README.md`** — new **Combat engine (NEO-81)** section after combat entity health: document **`CombatOperations.TryResolve`**, **`CombatResult`** fields, reason-code table, and cross-link **NEO-82** cast wiring.
### Expected prototype resolve values
| Ability | Target | Initial HP | Result |
|---------|--------|------------|--------|
| `prototype_pulse` | `prototype_target_alpha` | 100 (lazy) | Success: **`damageDealt` 25**, **`targetRemainingHp` 75**, **`targetDefeated` false** |
| `prototype_pulse` × 4 | same | — | 4th success: **`targetRemainingHp` 0**, **`targetDefeated` true** |
| `prototype_pulse` | same (defeated) | 0 | Deny: **`target_defeated`**, **`damageDealt` 0** |
| `prototype_guard` | live alpha | 100 | Success: **`damageDealt` 0**, HP unchanged |
| `prototype_guard` | defeated alpha | 0 | Deny: **`target_defeated`** |
| `unknown_ability` | alpha | — | Deny: **`unknown_ability`** |
| `prototype_pulse` | `not_a_target` | — | Deny: **`unknown_target`** |
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Combat/CombatReasonCodes.cs` | Stable deny reason strings for combat ops. |
| `server/NeonSprawl.Server/Game/Combat/CombatResult.cs` | Server-internal combat resolution envelope. |
| `server/NeonSprawl.Server/Game/Combat/CombatOperations.cs` | Static **`TryResolve`** orchestrating registry + health store. |
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatOperationsTests.cs` | AAA unit tests for resolve happy/deny paths. |
| `docs/plans/NEO-81-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/README.md` | Document **`CombatOperations.TryResolve`**, **`CombatResult`**, reason codes, and NEO-82 handoff. |
## Tests
| File | Coverage |
|------|----------|
| `server/NeonSprawl.Server.Tests/Game/Combat/CombatOperationsTests.cs` | **Happy path:** `prototype_pulse` on fresh alpha → 25 dmg, 75 HP. **Deterministic chain:** four pulses → defeated on 4th. **Defeated deny:** 5th pulse → **`target_defeated`**, no HP change. **Zero-damage:** `prototype_guard` on live target → success, 0 dmg, HP unchanged. **Defeated + utility:** `prototype_guard` on defeated → **`target_defeated`**. **Unknown ability:** deny **`unknown_ability`**. **Unknown target:** deny **`unknown_target`**. **Burst:** `prototype_burst` → 40 dmg once. Use in-memory registry helper + **`InMemoryCombatEntityHealthStore`**; optional host test resolving registries from **`InMemoryWebApplicationFactory`**. **AAA** per [csharp-style](../../.cursor/rules/csharp-style.md). |
No Bruno or manual QA for this story (server-only internal ops; no HTTP or client-facing change).
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|---------------------|--------|
| **Wire naming: CombatResult vs CombatResolution** | **`CombatResult`** server-internal; NEO-82 promotes to wire **`CombatResolution`** / cast response block (CraftResult → CraftResponse precedent). | **adopted** |
| **Defeated deny on zero-damage** | **Deny all abilities** on defeated target (user kickoff). | **adopted** |
| **Telemetry hook site** | **Defer** comment-only hooks to NEO-82 or dedicated telemetry story when cast path wires in. | **deferred** |
| **Player-as-target / PvP** | **Out of scope** — prototype registry has NPC stubs only; PvP gates are E6.M1. | **adopted** |
## Decisions (kickoff)
- **Defeated pre-check** before damage; no **`TryApplyDamage`** on defeated targets.
- **Zero-damage** abilities succeed via **`TryGet`** only (no store mutation).
- **Unknown ability/target** validated inside **`TryResolve`** with stable reason codes.
- **No HTTP** in NEO-81; cast wiring is [NEO-82](https://linear.app/neon-sprawl/issue/NEO-82).
## Reconciliation (implementation)
- **`CombatReasonCodes`**, **`CombatResult`**, **`CombatOperations.TryResolve`** added under **`Game/Combat/`**.
- Defeated pre-check denies before damage; zero-damage path uses **`TryGet`** only.
- **9** AAA tests in **`CombatOperationsTests`** (all green), including host DI resolve.
- **`server/README.md`** — combat engine section with reason-code table and NEO-82 handoff.