diff --git a/docs/plans/NEO-84-implementation-plan.md b/docs/plans/NEO-84-implementation-plan.md new file mode 100644 index 0000000..80cd5b4 --- /dev/null +++ b/docs/plans/NEO-84-implementation-plan.md @@ -0,0 +1,106 @@ +# NEO-84 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-84 | +| **Title** | E5M1-09: Slice 1 combat telemetry hook sites | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-84/e5m1-09-slice-1-combat-telemetry-hook-sites | +| **Module** | [E5.M1 — CombatRulesEngine](../decomposition/modules/E5_M1_CombatRulesEngine.md) · Epic 5 Slice 1 · backlog **E5M1-09** | +| **Branch** | `NEO-84-combat-telemetry-hooks` | +| **Blocked by** | [NEO-82](https://linear.app/neon-sprawl/issue/NEO-82) — cast POST + `combatResolution` (**Done** on `main`); [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44) gig XP on defeat (**Done** on `main`) | +| **Pattern** | [NEO-71](https://linear.app/neon-sprawl/issue/NEO-71) — engine-only comment hooks in `*Operations`; [NEO-30](https://linear.app/neon-sprawl/issue/NEO-30) — separate E1.M4 funnel names in `AbilityCastApi` (keep, do not duplicate E5 names there) | +| **Client counterpart** | None — comment-only server story; combat HUD is [NEO-85](https://linear.app/neon-sprawl/issue/NEO-85) | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Manual QA doc** | Add `docs/manual-qa/NEO-84.md`? | **Skip** — server-only; comments-only; no API/response change (NEO-71 / NEO-83 pattern). | **User:** skip. | +| **Hook anchor** | Engine only vs engine + `AbilityCastApi`? | **`CombatOperations.TryResolve` only** — HTTP delegates to engine; NEO-30 stays for `ability_cast_*` funnel events (NEO-71/64/56 precedent). | **User:** engine only. | +| **`ability_used` scope** | Zero-damage successful resolves (`prototype_guard`, `prototype_dash`)? | **Yes** — every successful `TryResolve` return (ability still consumed in combat path). | **Adopted** (backlog “cast accept path” + readable prototype semantics). | +| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only** + `TODO(E9.M1)` (NEO-40/71 precedent). | **Adopted** (repo precedent). | +| **Reserved sites** | Where for `encounter_start` / `player_death`? | **Class-level** reserved hook block on `CombatOperations` — encounters (E5.M3) and player HP deferred per [E5M1 kickoff defaults](E5M1-prototype-backlog.md#kickoff-decisions-decomposition-defaults). | **Adopted** (backlog default). | + +## Goal, scope, and out-of-scope + +**Goal:** Document and place **comment-only hook sites** on the server-authoritative combat resolve path for Epic 5 Slice 1 product telemetry names **`ability_used`** and **`enemy_defeat`**, plus **reserved** sites for **`encounter_start`** and **`player_death`**. No E9.M1 ingest. + +**In scope (from Linear + [E5M1-09](E5M1-prototype-backlog.md#e5m1-09--slice-1-combat-telemetry-hook-sites)):** + +- Hook placement in **`CombatOperations.TryResolve`** for **`ability_used`** (every successful resolve) and **`enemy_defeat`** (defeat transition only). +- Reserved **`encounter_start`** / **`player_death`** comment sites (no executable path in Slice 1). +- **`TODO(E9.M1)`** markers; **`server/README.md`** combat telemetry subsection; [E5_M1 module](../decomposition/modules/E5_M1_CombatRulesEngine.md) landed line. +- E5M1 backlog + alignment register updates when implementation completes. + +**Out of scope (from Linear + backlog):** + +- Production logging, dashboards, metrics (**E9.M1**). +- `ILogger` / dev-only log lines. +- Duplicate hook comments in **`AbilityCastApi`** (NEO-30 funnel hooks remain; E5 combat vocabulary lives in engine only). +- Godot client — **[NEO-85](https://linear.app/neon-sprawl/issue/NEO-85)**. +- **`docs/manual-qa/NEO-84.md`** (kickoff decision). + +## Acceptance criteria checklist + +- [ ] Hook names match [epic_05 Slice 1](../decomposition/epics/epic_05_pve_combat.md#slice-1---combat-rules-mvp) vocabulary (`ability_used`, `enemy_defeat`, reserved `encounter_start`, `player_death`). +- [ ] **`enemy_defeat`** site documents emit only on transition to defeated (not re-hit / overkill — enforced by `target_defeated` deny before damage). + +## Technical approach + +1. **Authoritative surface:** **`CombatOperations.TryResolve`** — all combat damage and defeat transitions funnel here; **`AbilityCastApi`** invokes after E1.M4 gates and commits cooldown / gig XP without duplicating E5 hook comments. + +2. **Class summary + reserved sites (NEO-84):** Extend **`CombatOperations`** XML summary with NEO-84 telemetry anchor. Add a **reserved hook sites** comment block (no `TODO` emit) documenting: + - **`encounter_start`** — future E5.M3 encounter spawn / engage path (not wired in Slice 1). + - **`player_death`** — future player HP / death resolution (out of scope Slice 1 per backlog). + +3. **Hook site A — `ability_used`:** On **every successful** return path (zero-damage branch and post-`TryApplyDamage` success): + - Comment block names future E9.M1 event **`ability_used`**. + - **`TODO(E9.M1): catalog emit`** (NEO-71 style). + - Planned payload fields: `abilityId`, `targetId`, `damageDealt`, `targetRemainingHp`, `targetDefeated`; note **`playerId`** is not in engine scope — E9.M1 ingest at **`AbilityCastApi`** accept layer should correlate route `id` with this event (document in README, do not add API duplicate hook). + +4. **Hook site B — `enemy_defeat`:** Immediately after **`TryApplyDamage`** when **`after.Defeated`** is **`true`** (lethal hit transition only): + - Comment block names **`enemy_defeat`**. + - **`TODO(E9.M1): catalog emit`** — once per target defeat transition. + - Planned payload fields: `targetId`, `abilityId` (killing blow), `damageDealt`, final HP **0**; correlate `playerId` at cast layer as above. + - **AC guarantee:** Re-hit on defeated targets returns **`target_defeated`** deny **before** damage — hook never reached on overkill. + +5. **NEO-30 coexistence:** Keep existing **`ability_cast_requested`** / **`ability_cast_denied`** comments in **`AbilityCastApi`** (E1.M4 funnel). **`ability_used`** is the Epic 5 combat-outcome catalog name at resolve success — document both in README under separate subsections. + +6. **NEO-44 / gig XP:** No new hooks in **`CombatDefeatGigXpGrant`** — defeat side effects stay separate from telemetry vocabulary; **`enemy_defeat`** anchor remains on the HP transition in **`TryResolve`**. + +7. **Docs (minimal):** + - **`server/README.md`:** new **NEO-84 combat telemetry hooks** subsection under combat engine (hook table + “no production ingest”). + - **`CombatResult.cs`:** one-line summary cross-reference to NEO-84 hook anchor in **`CombatOperations`** (optional wording tweak). + - **`E5_M1_CombatRulesEngine.md`:** E5M1-09 landed line with README link. + - **`documentation_and_implementation_alignment.md`** + **`E5M1-prototype-backlog.md`:** E5M1-09 checkboxes / landed note when complete. + +## Files to add + +| Path | Purpose | +|------|---------| +| `docs/plans/NEO-84-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Combat/CombatOperations.cs` | **NEO-84:** reserved-site class comment; **`ability_used`** on both success paths; **`enemy_defeat`** on `after.Defeated` transition only. | +| `server/NeonSprawl.Server/Game/Combat/CombatResult.cs` | Cross-reference NEO-84 hook anchor in type summary (wording tweak if needed). | +| `server/README.md` | Document combat telemetry hook placement (NEO-84); distinguish from NEO-30 cast funnel hooks. | +| `docs/decomposition/modules/E5_M1_CombatRulesEngine.md` | E5M1-09 telemetry hooks landed line + README link. | +| `docs/plans/E5M1-prototype-backlog.md` | E5M1-09 acceptance checkboxes + landed note when complete. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M1 row — E5M1-09 complete when landed. | + +## Tests + +| Path | Change | +|------|--------| +| **None required** | Comments-only markers; existing **`CombatOperationsTests`** and **`AbilityCastApiTests`** cover resolve/defeat behavior unchanged. | + +**Manual verification:** Code review that comment blocks cite **`ability_used`** / **`enemy_defeat`** / reserved names, include **`TODO(E9.M1)`**, and that **`enemy_defeat`** sits only on the post-damage defeat branch. Optional Bruno cast + combat-targets regression (no request/response change). + +## Open questions / risks + +**None.** Precursors NEO-82 and NEO-44 are complete; production logging risk avoided by **comments-only** kickoff decisions.