From dedd9a9c188178b5d3d259d204594d5d4015520f Mon Sep 17 00:00:00 2001 From: VinPropane Date: Fri, 29 May 2026 20:47:21 -0400 Subject: [PATCH 1/4] NEO-96: add E5M2-10 NPC telemetry hook sites implementation plan --- docs/plans/NEO-96-implementation-plan.md | 129 +++++++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 docs/plans/NEO-96-implementation-plan.md diff --git a/docs/plans/NEO-96-implementation-plan.md b/docs/plans/NEO-96-implementation-plan.md new file mode 100644 index 0000000..99bbb95 --- /dev/null +++ b/docs/plans/NEO-96-implementation-plan.md @@ -0,0 +1,129 @@ +# NEO-96 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-96 | +| **Title** | E5M2-10: Slice 2 NPC telemetry hook sites | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-96/e5m2-10-slice-2-npc-telemetry-hook-sites | +| **Module** | [E5.M2 — NpcAiAndBehaviorProfiles](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) · Epic 5 Slice 2 · backlog **E5M2-10** | +| **Branch** | `NEO-96-e5m2-10-npc-telemetry-hooks` | +| **Precursor** | [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) — NPC runtime state machine + lazy tick (**Done** on `main`; left `(NEO-96)` placeholder comments) | +| **Pattern** | [NEO-64](https://linear.app/neon-sprawl/issue/NEO-64) / [NEO-84](https://linear.app/neon-sprawl/issue/NEO-84) — comment-only engine anchors + `TODO(E9.M1): catalog emit` + planned payload fields | +| **Client counterpart** | None — server-only comment hooks; Godot poll/HUD is [NEO-97](https://linear.app/neon-sprawl/issue/NEO-97) | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only** — `TODO(E9.M1)`; no production logging (NEO-64/NEO-84 precedent). | **Adopted** (repo precedent; not asked). | +| **Hook anchor** | Engine only vs engine + snapshot HTTP API? | **Engine only** in **`NpcRuntimeOperations`** — snapshot GET calls `AdvanceAll`; no duplicate API-layer hooks (NEO-56/NEO-64 precedent). | **Adopted** (repo precedent; not asked). | +| **`telegraph_fired` timing** | At telegraph start vs windup complete / attack resolve? | **At each `telegraph_windup` entry** — aggro→telegraph and recover→telegraph (E5M2 backlog: “at telegraph start”). | **Adopted** (backlog AC). | +| **`npc_state_transition` scope** | Every `WriteState` commit including holder clear → idle? | **Yes** — emit anchor on every committed state change, including **`WriteIdle`** when aggro holder clears. | **Adopted** (backlog: “on state transition commit”). | +| **Manual QA doc** | `docs/manual-qa/NEO-96.md`? | **No** — no client-facing change; NEO-93 server-only pattern (NEO-64 added manual QA for gather hooks with broader doc surface). | **User:** no manual QA — not client-facing. | + +## Goal, scope, and out-of-scope + +**Goal:** Formalize **comment-only** E9.M1 telemetry hook sites for Epic 5 Slice 2 product events **`telegraph_fired`** and **`npc_state_transition`** on the server-authoritative NPC runtime path. + +**In scope (from Linear + [E5M2-10](E5M2-prototype-backlog.md#e5m2-10--slice-2-npc-telemetry-hook-sites)):** + +- Replace NEO-93 `(NEO-96)` stub comments in **`NpcRuntimeOperations`** with full NEO-84-style blocks: event name, **`TODO(E9.M1): catalog emit`**, planned payload fields. +- **`telegraph_fired`** at each **`telegraph_windup`** entry (telegraph start). +- **`npc_state_transition`** at every state commit (idle↔aggro, phase transitions, holder clear → idle). +- **`server/README.md`** NPC telemetry subsection + [E5_M2 module](../decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md) landed note. +- E5M2 backlog acceptance checkboxes + alignment register bump when complete. + +**Out of scope (from Linear + backlog):** + +- Production telemetry ingest, dashboards (**E9.M1**). +- `ILogger` / metrics / dev-only log lines. +- Godot / client hooks ([NEO-97](https://linear.app/neon-sprawl/issue/NEO-97)). +- Duplicate hook comments in **`NpcRuntimeSnapshotWorldApi`** or **`NpcAttackOperations`** (engine orchestration layer only). +- `docs/manual-qa/NEO-96.md` (kickoff decision). + +## Acceptance criteria checklist + +- [ ] **`TODO(E9.M1)`** markers present at **`telegraph_fired`** and **`npc_state_transition`** hook sites in NPC runtime path. +- [ ] Hook names match [epic_05 Slice 2](../decomposition/epics/epic_05_pve_combat.md#slice-2---npc-archetypes-and-telegraphs) vocabulary (`telegraph_fired`, `npc_state_transition`). +- [ ] No runtime behavior change beyond comments / doc cross-links. + +## Technical approach + +### Authoritative surface + +**`NpcRuntimeOperations.AdvanceAll` → `AdvanceOne`** — all prototype NPC state transitions and telegraph scheduling funnel here. **`GET /game/world/npc-runtime-snapshot`** invokes **`AdvanceAll`** before read ([NEO-94](https://linear.app/neon-sprawl/issue/NEO-94)); no HTTP-layer hooks. + +### Hook site A — `telegraph_fired` + +Immediately after **`WriteState(..., TelegraphWindup, ...)`** (both paths): + +1. **aggro → telegraph_windup** — cooldown elapsed while holder set. +2. **recover → telegraph_windup** — post-attack cooldown elapsed while holder set. + +Comment block names future E9.M1 event **`telegraph_fired`**. **`TODO(E9.M1): catalog emit`** — once per telegraph start (each windup cycle). + +Planned payload fields: `npcInstanceId`, `telegraphId`, `behaviorDefId`, `archetypeKind`, `aggroHolderPlayerId`, `windupStartedUtc`, `telegraphWindupSeconds`. + +### Hook site B — `npc_state_transition` + +Immediately after every **`WriteState`** / **`WriteIdle`** commit: + +| Transition | `fromState` → `toState` | +|------------|-------------------------| +| Holder set while idle | `idle` → `aggro` | +| Cooldown complete | `aggro` → `telegraph_windup` | +| Windup complete | `telegraph_windup` → `recover` (logical attack; no visible `attack_execute` phase) | +| Defensive `AttackExecute` branch | `attack_execute` → `recover` | +| Recover cooldown complete | `recover` → `telegraph_windup` | +| Holder cleared / fixture reset | any → `idle` | + +Comment block names future E9.M1 event **`npc_state_transition`**. **`TODO(E9.M1): catalog emit`** — once per committed transition. + +Planned payload fields: `npcInstanceId`, `behaviorDefId`, `fromState`, `toState`, `phaseStartedUtc`, optional `aggroHolderPlayerId`, optional `telegraphId` when entering/leaving windup. + +**Note:** At recover→telegraph, emit **both** `npc_state_transition` and `telegraph_fired` (state commit then telegraph start) — analogous to gather success + depletion hooks sharing a path but naming distinct catalog events. + +### Cross-references + +1. **`NpcRuntimeOperations.cs`** — extend class summary with NEO-96 telemetry hook anchor (NEO-84 style). +2. **`server/README.md`** — new **NEO-96 NPC telemetry hooks** subsection under NPC runtime behavior (after NEO-93 block). +3. **`E5_M2_NpcAiAndBehaviorProfiles.md`** — NEO-96 landed line when complete. +4. **`E5M2-prototype-backlog.md`** — E5M2-10 checkboxes + landed note. +5. **`documentation_and_implementation_alignment.md`** — E5.M2 row bump when E5M2-10 lands. + +### NEO-93 placeholder replacement + +Replace terse `// telemetry: … (NEO-96)` stubs with full `--- Telemetry hook site (NEO-96): … ---` blocks matching **`CombatOperations`** / **`GatherOperations`** formatting. + +## Files to add + +| Path | Purpose | +|------|---------| +| `docs/plans/NEO-96-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs` | NEO-96: formalize `telegraph_fired` + `npc_state_transition` comment-only hook sites at transition/telegraph anchors; extend class summary. | +| `server/README.md` | Document NPC runtime telemetry hook sites + payload notes; link plan and epic vocabulary. | +| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | NEO-96 landed note under Related implementation slices. | +| `docs/plans/E5M2-prototype-backlog.md` | E5M2-10 acceptance checkboxes + landed note when complete. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M2 alignment register — E5M2-10 complete when shipped. | + +## Tests + +| File | Coverage | +|------|----------| +| *(none — no new or changed test files)* | Comment-only change; existing **`NpcRuntimeOperationsTests`** continue to verify state machine behavior. No runtime/API contract change to assert. | + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|---------------------|--------| +| **Linear blockedBy NEO-93** | Proceed — **Done** on `main`; placeholders already in place. | **adopted** | +| **Dual emit on recover→telegraph** | Accept — distinct catalog events at same code point (state transition + telegraph start). | **adopted** | +| **No manual QA doc** | Accept per kickoff — server-only comments; verify via code review + existing unit tests green. | **adopted** (kickoff) | +| **No new unit tests for comments** | Accept — NEO-64/NEO-84 did not add comment-assertion tests; behavior unchanged. | **adopted** | From 5e1e37423f2741e9d37cc88e958b1664ab4e1c79 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Fri, 29 May 2026 20:48:50 -0400 Subject: [PATCH 2/4] NEO-96: formalize NPC runtime telemetry hook sites Replace NEO-93 placeholder comments with full telegraph_fired and npc_state_transition E9.M1 anchors in NpcRuntimeOperations; document in server README and E5.M2 module alignment. --- .../modules/E5_M2_NpcAiAndBehaviorProfiles.md | 2 + ...umentation_and_implementation_alignment.md | 2 +- docs/plans/E5M2-prototype-backlog.md | 6 ++- docs/plans/NEO-96-implementation-plan.md | 13 +++-- .../Game/Npc/NpcRuntimeOperations.cs | 52 ++++++++++++++++--- server/README.md | 4 ++ 6 files changed, 67 insertions(+), 12 deletions(-) diff --git a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md index fc58734..b305970 100644 --- a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md +++ b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md @@ -91,6 +91,8 @@ The **first shipped three-archetype NPC spine** under `content/npc-behaviors/*.j **Session player combat HP (NEO-95):** **`IPlayerCombatHealthStore`** + **`NpcAttackOperations.TryResolveTelegraphComplete`** in `server/NeonSprawl.Server/Game/Combat/` — telegraph complete applies catalog **`attackDamage`** to aggro holder; **`GET /game/players/{id}/combat-health`** read model for client HUD. Plan: [NEO-95 implementation plan](../../plans/NEO-95-implementation-plan.md); [server README — Session player combat HP (NEO-95)](../../../server/README.md#session-player-combat-hp-neo-95); Bruno `bruno/neon-sprawl-server/combat-health/`. +**NPC runtime telemetry hooks (NEO-96):** comment-only **`telegraph_fired`** + **`npc_state_transition`** hook sites in **`NpcRuntimeOperations.AdvanceAll`** (`TODO(E9.M1)`; no ingest). Plan: [NEO-96 implementation plan](../../plans/NEO-96-implementation-plan.md); [server README — NPC runtime telemetry hooks (NEO-96)](../../../server/README.md#npc-runtime-telemetry-hooks-neo-96). + **NPC behavior definitions HTTP (NEO-90):** **`GET /game/world/npc-behavior-definitions`** — versioned read-only projection (`schemaVersion` **1**, **`npcBehaviors`**) backed by **`INpcBehaviorDefinitionRegistry`**. Plan: [NEO-90 implementation plan](../../plans/NEO-90-implementation-plan.md); [server README — NPC behavior definitions (NEO-90)](../../../server/README.md#npc-behavior-definitions-neo-90); Bruno `bruno/neon-sprawl-server/npc-behavior-definitions/`. ## Risks and telemetry diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 73ea7b7..8c9b078 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -57,7 +57,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E3.M3 | In Progress | **NEO-50 landed:** frozen prototype six-item catalog in [`content/items/prototype_items.json`](../../../content/items/prototype_items.json); [`item-def.schema.json`](../../../content/schemas/item-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py). **NEO-51 landed:** fail-fast server load of `content/items/*_items.json` at startup — `server/NeonSprawl.Server/Game/Items/` ([NEO-51](../../plans/NEO-51-implementation-plan.md)); [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). **NEO-52 landed:** injectable **`IItemDefinitionRegistry`** + lookup tests ([NEO-52](../../plans/NEO-52-implementation-plan.md)). **NEO-53 landed:** **`GET /game/world/item-definitions`** — `ItemDefinitionsWorldApi` + DTOs in `Game/Items/` ([NEO-53](../../plans/NEO-53-implementation-plan.md), [`NEO-53` manual QA](../../manual-qa/NEO-53.md)); [server README — Item definitions (NEO-53)](../../../server/README.md#item-definitions-neo-53); Bruno `bruno/neon-sprawl-server/item-definitions/`. **NEO-54 landed:** **`IPlayerInventoryStore`** + **`PlayerInventoryOperations`** — 24 bag + 1 equipment slots, stack rules, `V005` migration ([NEO-54](../../plans/NEO-54-implementation-plan.md)). **NEO-55 landed:** **`GET`/`POST /game/players/{id}/inventory`** — `PlayerInventoryApi` + DTOs in `Game/Items/` ([NEO-55](../../plans/NEO-55-implementation-plan.md)); [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http); Bruno `bruno/neon-sprawl-server/inventory/`. **NEO-56 landed:** comment-only **`item_created`** / **`inventory_transfer_denied`** telemetry hook sites in [`PlayerInventoryOperations`](../../../server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs) ([NEO-56](../../plans/NEO-56-implementation-plan.md), [`NEO-56` manual QA](../../manual-qa/NEO-56.md)); no E9.M1 ingest. **NEO-72 landed:** client inventory HUD — **`inventory_client.gd`**, **`item_definitions_client.gd`**, **`InventoryLabel`**, boot hydrate + **`I`** refresh ([NEO-72](../../plans/NEO-72-implementation-plan.md), [`NEO-72` manual QA](../../manual-qa/NEO-72.md)); `client/README.md` inventory HUD section. **NEO-75 landed:** **`prototype_economy_hud_section.gd`** collapse toggle + capstone manual QA ([NEO-75](../../plans/NEO-75-implementation-plan.md), [`NEO-75` manual QA](../../manual-qa/NEO-75.md)); Epic 3 Slice 5 client complete. | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [NEO-53](../../plans/NEO-53-implementation-plan.md), [NEO-54](../../plans/NEO-54-implementation-plan.md), [NEO-55](../../plans/NEO-55-implementation-plan.md), [NEO-56](../../plans/NEO-56-implementation-plan.md), [NEO-72](../../plans/NEO-72-implementation-plan.md), [NEO-75](../../plans/NEO-75-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) | | E3.M2 | Ready | **NEO-65 landed:** frozen prototype eight-recipe catalog in [`content/recipes/prototype_recipes.json`](../../../content/recipes/prototype_recipes.json); [`recipe-def.schema.json`](../../../content/schemas/recipe-def.schema.json) + [`recipe-io-row.schema.json`](../../../content/schemas/recipe-io-row.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) Slice 3 gates ([NEO-65](../../plans/NEO-65-implementation-plan.md)). **NEO-66 landed:** fail-fast server load of `content/recipes/*_recipes.json` at startup — `server/NeonSprawl.Server/Game/Crafting/` ([NEO-66](../../plans/NEO-66-implementation-plan.md)); [server README — Recipe catalog](../../../server/README.md#recipe-catalog-contentrecipes-neo-66). **NEO-67 landed:** injectable **`IRecipeDefinitionRegistry`** + lookup tests ([NEO-67](../../plans/NEO-67-implementation-plan.md)). **NEO-68 landed:** **`GET /game/world/recipe-definitions`** — `RecipeDefinitionsWorldApi` + DTOs in `Game/Crafting/` ([NEO-68](../../plans/NEO-68-implementation-plan.md), [`NEO-68` manual QA](../../manual-qa/NEO-68.md)); [server README — Recipe definitions (NEO-68)](../../../server/README.md#recipe-definitions-neo-68); Bruno `bruno/neon-sprawl-server/recipe-definitions/`. **NEO-69 landed:** **`CraftOperations.TryCraft`** + **`CraftResult`** — pre-flight inputs/outputs, inventory mutation, refine XP ([NEO-69](../../plans/NEO-69-implementation-plan.md)); [server README — Craft engine (NEO-69)](../../../server/README.md#craft-engine-neo-69-and-craft-http-neo-70). **NEO-70 landed:** **`POST /game/players/{id}/craft`** — `PlayerCraftApi` + wire **`CraftResponse`**; Bruno gather→refine→make spine ([NEO-70](../../plans/NEO-70-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/craft/`. **NEO-71 landed:** comment-only **`item_crafted`** / **`craft_failed`** telemetry hook sites in **`CraftOperations.TryCraft`** ([NEO-71](../../plans/NEO-71-implementation-plan.md)); [server README — Craft telemetry hooks (NEO-71)](../../../server/README.md#craft-telemetry-hooks-neo-71). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** — wired from craft engine success path. Epic 3 Slice 3 server backlog **E3M2-01**–**E3M2-07** complete. Upstream: E3.M1 **Ready**, E3.M3 inventory landed. **NEO-74 landed:** client craft UI — **`recipe_definitions_client.gd`**, **`craft_client.gd`**, **`craft_recipe_panel.gd`**, **`CraftFeedbackLabel`**, **refine** skill row ([NEO-74](../../plans/NEO-74-implementation-plan.md), [`NEO-74` manual QA](../../manual-qa/NEO-74.md)); `client/README.md` craft section. **NEO-75 landed:** capstone playable gather→refine→make loop — [`NEO-75` manual QA](../../manual-qa/NEO-75.md), **`prototype_economy_hud_section.gd`**; Epic 3 Slice 5 client complete. | [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-65](../../plans/NEO-65-implementation-plan.md), [NEO-66](../../plans/NEO-66-implementation-plan.md), [NEO-67](../../plans/NEO-67-implementation-plan.md), [NEO-68](../../plans/NEO-68-implementation-plan.md), [NEO-69](../../plans/NEO-69-implementation-plan.md), [NEO-70](../../plans/NEO-70-implementation-plan.md), [NEO-71](../../plans/NEO-71-implementation-plan.md), [NEO-74](../../plans/NEO-74-implementation-plan.md), [NEO-75](../../plans/NEO-75-implementation-plan.md), [E3M2-prototype-backlog](../../plans/E3M2-prototype-backlog.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M2](E3_M2_RefinementAndRecipeExecution.md) | | E5.M1 | Ready | **NEO-76 landed:** frozen prototype four-ability catalog in [`content/abilities/prototype_abilities.json`](../../../content/abilities/prototype_abilities.json); [`ability-def.schema.json`](../../../content/schemas/ability-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M1 four-id gate ([NEO-76](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` at startup — `server/NeonSprawl.Server/Game/Combat/` ([NEO-77](../../plans/NEO-77-implementation-plan.md)); [server README — Ability catalog](../../../server/README.md#ability-catalog-contentabilities-neo-77). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79](../../plans/NEO-79-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/hotbar-loadout/` + `ability-cast/` unknown-ability deny smokes. **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78](../../plans/NEO-78-implementation-plan.md)); [server README — Ability definitions (NEO-78)](../../../server/README.md#ability-definitions-neo-78); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **NEO-80 landed:** **`ICombatEntityHealthStore`** / **`InMemoryCombatEntityHealthStore`** — lazy init (Slice 1 flat 100 HP; **superseded by [NEO-91](../../plans/NEO-91-implementation-plan.md)** per-archetype catalog max HP + three NPC instance ids); DI via **`AddCombatEntityHealthStore()`** ([NEO-80](../../plans/NEO-80-implementation-plan.md)); [server README — Combat entity health (NEO-80, NEO-91)](../../../server/README.md#combat-entity-health-neo-80-neo-91). **NEO-81 landed:** **`CombatOperations.TryResolve`** + **`CombatResult`** + **`CombatReasonCodes`** — defeated pre-check deny, catalog damage ([NEO-81](../../plans/NEO-81-implementation-plan.md)); [server README — Combat engine (NEO-81)](../../../server/README.md#combat-engine-neo-81). **NEO-82 landed:** **`AbilityCastApi`** → **`CombatOperations.TryResolve`**; nested wire **`combatResolution`** on accept; per-ability catalog cooldown; **`target_defeated`** cast deny ([NEO-82](../../plans/NEO-82-implementation-plan.md)); [server README — Ability cast (NEO-82)](../../../server/README.md#ability-cast-neo-31-neo-82); Bruno `bruno/neon-sprawl-server/ability-cast/` defeat spine. **NEO-83 landed:** **`GET /game/world/combat-targets`** — `CombatTargetsWorldApi` + DTOs; authoritative HP snapshot from **`ICombatEntityHealthStore`** ([NEO-83](../../plans/NEO-83-implementation-plan.md)); [server README — Combat targets snapshot (NEO-83)](../../../server/README.md#combat-targets-snapshot-neo-83); Bruno `bruno/neon-sprawl-server/combat-targets/`. **NEO-44 landed:** **`CombatDefeatGigXpGrant`** on cast **`targetDefeated`** — **25** gig XP to **`breach`** via **`IPlayerGigProgressionStore`** (V007); **`GET …/gig-progression`** ([NEO-44](../../plans/NEO-44-implementation-plan.md), [`NEO-44` manual QA](../../manual-qa/NEO-44.md)); [server README — Gig progression (NEO-44)](../../../server/README.md#gig-progression-snapshot-neo-44); Bruno `bruno/neon-sprawl-server/gig-progression/`. **NEO-84 landed:** comment-only **`ability_used`** / **`enemy_defeat`** telemetry hook sites in **`CombatOperations.TryResolve`**; reserved **`encounter_start`** / **`player_death`** ([NEO-84](../../plans/NEO-84-implementation-plan.md)); [server README — Combat telemetry hooks (NEO-84)](../../../server/README.md#combat-telemetry-hooks-neo-84). **NEO-85 landed:** client combat HUD — **`ability_cast_client.gd`** parses **`combatResolution`**; **`combat_targets_client.gd`**, **`CastFeedbackLabel`** resolution copy, **`CombatTargetHpLabel`**; event-driven GET refresh ([NEO-85](../../plans/NEO-85-implementation-plan.md), [`NEO-85` manual QA](../../manual-qa/NEO-85.md)); `client/README.md` combat HUD section. **NEO-86 landed:** playable combat capstone — **`gig_progression_client.gd`**, **`GigXpLabel`**, defeat-triggered gig GET refresh; capstone manual QA [`NEO-86`](../../manual-qa/NEO-86.md); `client/README.md` end-to-end combat loop section ([NEO-86](../../plans/NEO-86-implementation-plan.md)). **Epic 5 Slice 1 client capstone complete.** **Backlog decomposed:** Epic 5 Slice 1 — **E5M1-01**–**E5M1-12** landed. **Precursor landed:** E1.M4 cast funnel (NEO-28/31/32). | [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1](E5_M1_CombatRulesEngine.md), [NEO-77](../../plans/NEO-77-implementation-plan.md), [NEO-79](../../plans/NEO-79-implementation-plan.md), [NEO-78](../../plans/NEO-78-implementation-plan.md), [NEO-80](../../plans/NEO-80-implementation-plan.md), [NEO-81](../../plans/NEO-81-implementation-plan.md), [NEO-82](../../plans/NEO-82-implementation-plan.md), [NEO-83](../../plans/NEO-83-implementation-plan.md), [NEO-44](../../plans/NEO-44-implementation-plan.md), [NEO-84](../../plans/NEO-84-implementation-plan.md), [NEO-85](../../plans/NEO-85-implementation-plan.md), [NEO-86](../../plans/NEO-86-implementation-plan.md), [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) | -| E5.M2 | In Progress | **NEO-87 landed:** frozen prototype three-behavior catalog in [`content/npc-behaviors/prototype_npc_behaviors.json`](../../../content/npc-behaviors/prototype_npc_behaviors.json); [`npc-behavior-def.schema.json`](../../../content/schemas/npc-behavior-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M2 three-id gate ([NEO-87](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` at startup — `server/NeonSprawl.Server/Game/Npc/` ([NEO-88](../../plans/NEO-88-implementation-plan.md)); [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). **NEO-89 landed:** injectable **`INpcBehaviorDefinitionRegistry`** + DI; **`PrototypeNpcBehaviorRegistry`** id constants ([NEO-89](../../plans/NEO-89-implementation-plan.md)). **NEO-90 landed:** **`GET /game/world/npc-behavior-definitions`** — `NpcBehaviorDefinitionsWorldApi` + DTOs in `Game/Npc/` ([NEO-90](../../plans/NEO-90-implementation-plan.md)); [server README — NPC behavior definitions (NEO-90)](../../../server/README.md#npc-behavior-definitions-neo-90); Bruno `bruno/neon-sprawl-server/npc-behavior-definitions/`. **NEO-91 landed:** **`PrototypeNpcRegistry`** + combat-target migration — three NPC instance ids replace alpha/beta; per-archetype max HP from behavior catalog ([NEO-91](../../plans/NEO-91-implementation-plan.md)); [server README — Combat entity health (NEO-91)](../../../server/README.md#combat-entity-health-neo-80-neo-91). **NEO-92 landed:** **`IThreatStateStore`** + **`AggroOperations`** — first-hit acquire on damaging cast, leash clear on move/cast; cast/move/fixture hooks ([NEO-92](../../plans/NEO-92-implementation-plan.md)); [server README — Threat / aggro state (NEO-92)](../../../server/README.md#threat--aggro-state-neo-92). **NEO-93 landed:** **`INpcRuntimeStateStore`** + **`NpcRuntimeOperations.AdvanceAll`** — catalog-driven behavior state machine, lazy tick advance with delta cap; dev fixture reset ([NEO-93](../../plans/NEO-93-implementation-plan.md)); [server README — NPC runtime behavior state (NEO-93)](../../../server/README.md#npc-runtime-behavior-state-neo-93). **NEO-94 landed:** **`GET /game/world/npc-runtime-snapshot`** — `NpcRuntimeSnapshotWorldApi` + DTOs; lazy **`AdvanceAll`** on poll; nested **`activeTelegraph`** with server-computed **`windupRemainingSeconds`** ([NEO-94](../../plans/NEO-94-implementation-plan.md)); [server README — NPC runtime snapshot (NEO-94)](../../../server/README.md#npc-runtime-snapshot-neo-94); Bruno `bruno/neon-sprawl-server/npc-runtime-snapshot/`. **NEO-95 landed:** **`IPlayerCombatHealthStore`** + **`NpcAttackOperations.TryResolveTelegraphComplete`** — telegraph complete applies catalog **`attackDamage`** to aggro holder; **`GET /game/players/{id}/combat-health`** read model ([NEO-95](../../plans/NEO-95-implementation-plan.md)); [server README — Session player combat HP (NEO-95)](../../../server/README.md#session-player-combat-hp-neo-95); Bruno `bruno/neon-sprawl-server/combat-health/`. **Backlog decomposed:** Epic 5 Slice 2 — [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98); client capstone **NEO-98**. Three archetypes (`prototype_melee_pressure`, `prototype_ranged_control`, `prototype_elite_mini_boss`); replaces alpha/beta dummies with **`prototype_npc_melee` / `ranged` / `elite`**. Owns **`ThreatState`**, **`TelegraphEvent`**, **`GET …/npc-runtime-snapshot`**, session **`IPlayerCombatHealthStore`**. Upstream **E5.M1 Ready**. | [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2](E5_M2_NpcAiAndBehaviorProfiles.md), [NEO-87](../../plans/NEO-87-implementation-plan.md), [NEO-88](../../plans/NEO-88-implementation-plan.md), [NEO-89](../../plans/NEO-89-implementation-plan.md), [NEO-90](../../plans/NEO-90-implementation-plan.md), [NEO-91](../../plans/NEO-91-implementation-plan.md), [NEO-92](../../plans/NEO-92-implementation-plan.md), [NEO-93](../../plans/NEO-93-implementation-plan.md), [NEO-94](../../plans/NEO-94-implementation-plan.md), [NEO-95](../../plans/NEO-95-implementation-plan.md), label **`E5.M2`** on NEO-87–NEO-98 | +| E5.M2 | In Progress | **NEO-87 landed:** frozen prototype three-behavior catalog in [`content/npc-behaviors/prototype_npc_behaviors.json`](../../../content/npc-behaviors/prototype_npc_behaviors.json); [`npc-behavior-def.schema.json`](../../../content/schemas/npc-behavior-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M2 three-id gate ([NEO-87](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` at startup — `server/NeonSprawl.Server/Game/Npc/` ([NEO-88](../../plans/NEO-88-implementation-plan.md)); [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). **NEO-89 landed:** injectable **`INpcBehaviorDefinitionRegistry`** + DI; **`PrototypeNpcBehaviorRegistry`** id constants ([NEO-89](../../plans/NEO-89-implementation-plan.md)). **NEO-90 landed:** **`GET /game/world/npc-behavior-definitions`** — `NpcBehaviorDefinitionsWorldApi` + DTOs in `Game/Npc/` ([NEO-90](../../plans/NEO-90-implementation-plan.md)); [server README — NPC behavior definitions (NEO-90)](../../../server/README.md#npc-behavior-definitions-neo-90); Bruno `bruno/neon-sprawl-server/npc-behavior-definitions/`. **NEO-91 landed:** **`PrototypeNpcRegistry`** + combat-target migration — three NPC instance ids replace alpha/beta; per-archetype max HP from behavior catalog ([NEO-91](../../plans/NEO-91-implementation-plan.md)); [server README — Combat entity health (NEO-91)](../../../server/README.md#combat-entity-health-neo-80-neo-91). **NEO-92 landed:** **`IThreatStateStore`** + **`AggroOperations`** — first-hit acquire on damaging cast, leash clear on move/cast; cast/move/fixture hooks ([NEO-92](../../plans/NEO-92-implementation-plan.md)); [server README — Threat / aggro state (NEO-92)](../../../server/README.md#threat--aggro-state-neo-92). **NEO-93 landed:** **`INpcRuntimeStateStore`** + **`NpcRuntimeOperations.AdvanceAll`** — catalog-driven behavior state machine, lazy tick advance with delta cap; dev fixture reset ([NEO-93](../../plans/NEO-93-implementation-plan.md)); [server README — NPC runtime behavior state (NEO-93)](../../../server/README.md#npc-runtime-behavior-state-neo-93). **NEO-94 landed:** **`GET /game/world/npc-runtime-snapshot`** — `NpcRuntimeSnapshotWorldApi` + DTOs; lazy **`AdvanceAll`** on poll; nested **`activeTelegraph`** with server-computed **`windupRemainingSeconds`** ([NEO-94](../../plans/NEO-94-implementation-plan.md)); [server README — NPC runtime snapshot (NEO-94)](../../../server/README.md#npc-runtime-snapshot-neo-94); Bruno `bruno/neon-sprawl-server/npc-runtime-snapshot/`. **NEO-95 landed:** **`IPlayerCombatHealthStore`** + **`NpcAttackOperations.TryResolveTelegraphComplete`** — telegraph complete applies catalog **`attackDamage`** to aggro holder; **`GET /game/players/{id}/combat-health`** read model ([NEO-95](../../plans/NEO-95-implementation-plan.md)); [server README — Session player combat HP (NEO-95)](../../../server/README.md#session-player-combat-hp-neo-95); Bruno `bruno/neon-sprawl-server/combat-health/`. **NEO-96 landed:** comment-only **`telegraph_fired`** + **`npc_state_transition`** hook sites in **`NpcRuntimeOperations.AdvanceAll`** ([NEO-96](../../plans/NEO-96-implementation-plan.md)); [server README — NPC runtime telemetry hooks (NEO-96)](../../../server/README.md#npc-runtime-telemetry-hooks-neo-96). **Backlog decomposed:** Epic 5 Slice 2 — [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98); client capstone **NEO-98**. Three archetypes (`prototype_melee_pressure`, `prototype_ranged_control`, `prototype_elite_mini_boss`); replaces alpha/beta dummies with **`prototype_npc_melee` / `ranged` / `elite`**. Owns **`ThreatState`**, **`TelegraphEvent`**, **`GET …/npc-runtime-snapshot`**, session **`IPlayerCombatHealthStore`**. Upstream **E5.M1 Ready**. | [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2](E5_M2_NpcAiAndBehaviorProfiles.md), [NEO-87](../../plans/NEO-87-implementation-plan.md), [NEO-88](../../plans/NEO-88-implementation-plan.md), [NEO-89](../../plans/NEO-89-implementation-plan.md), [NEO-90](../../plans/NEO-90-implementation-plan.md), [NEO-91](../../plans/NEO-91-implementation-plan.md), [NEO-92](../../plans/NEO-92-implementation-plan.md), [NEO-93](../../plans/NEO-93-implementation-plan.md), [NEO-94](../../plans/NEO-94-implementation-plan.md), [NEO-95](../../plans/NEO-95-implementation-plan.md), [NEO-96](../../plans/NEO-96-implementation-plan.md), label **`E5.M2`** on NEO-87–NEO-98 | --- diff --git a/docs/plans/E5M2-prototype-backlog.md b/docs/plans/E5M2-prototype-backlog.md index 289c6ec..85e1d4d 100644 --- a/docs/plans/E5M2-prototype-backlog.md +++ b/docs/plans/E5M2-prototype-backlog.md @@ -322,8 +322,10 @@ Working backlog for **Epic 5 — Slice 2** ([NPC archetypes and telegraphs](../d **Acceptance criteria** -- [ ] Hook names match [epic_05 Slice 2](../decomposition/epics/epic_05_pve_combat.md#slice-2---npc-archetypes-and-telegraphs) vocabulary. -- [ ] No runtime behavior change beyond comments. +- [x] Hook names match [epic_05 Slice 2](../decomposition/epics/epic_05_pve_combat.md#slice-2---npc-archetypes-and-telegraphs) vocabulary. +- [x] No runtime behavior change beyond comments. + +**Landed ([NEO-96](https://linear.app/neon-sprawl/issue/NEO-96)):** comment-only **`telegraph_fired`** + **`npc_state_transition`** hook sites in **`NpcRuntimeOperations`**; plan [NEO-96-implementation-plan.md](NEO-96-implementation-plan.md). --- diff --git a/docs/plans/NEO-96-implementation-plan.md b/docs/plans/NEO-96-implementation-plan.md index 99bbb95..a33c85c 100644 --- a/docs/plans/NEO-96-implementation-plan.md +++ b/docs/plans/NEO-96-implementation-plan.md @@ -45,9 +45,16 @@ ## Acceptance criteria checklist -- [ ] **`TODO(E9.M1)`** markers present at **`telegraph_fired`** and **`npc_state_transition`** hook sites in NPC runtime path. -- [ ] Hook names match [epic_05 Slice 2](../decomposition/epics/epic_05_pve_combat.md#slice-2---npc-archetypes-and-telegraphs) vocabulary (`telegraph_fired`, `npc_state_transition`). -- [ ] No runtime behavior change beyond comments / doc cross-links. +- [x] **`TODO(E9.M1)`** markers present at **`telegraph_fired`** and **`npc_state_transition`** hook sites in NPC runtime path. +- [x] Hook names match [epic_05 Slice 2](../decomposition/epics/epic_05_pve_combat.md#slice-2---npc-archetypes-and-telegraphs) vocabulary (`telegraph_fired`, `npc_state_transition`). +- [x] No runtime behavior change beyond comments / doc cross-links. + +## Implementation reconciliation (shipped) + +- NEO-93 `(NEO-96)` stub comments replaced with full NEO-84-style hook blocks in **`NpcRuntimeOperations.AdvanceAll`**. +- **`telegraph_fired`** at aggro→telegraph and recover→telegraph entries; **`npc_state_transition`** on every state commit including holder clear → idle. +- **`server/README.md`**, **E5_M2** module doc, **E5M2** backlog, and alignment register updated. +- No new tests (comment-only); existing NPC runtime tests unchanged. ## Technical approach diff --git a/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs b/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs index 17e40e9..c43ae3a 100644 --- a/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs +++ b/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs @@ -2,7 +2,11 @@ using NeonSprawl.Server.Game.Combat; namespace NeonSprawl.Server.Game.Npc; -/// Deterministic prototype NPC behavior state machine + lazy tick advance (NEO-93). +/// +/// Deterministic prototype NPC behavior state machine + lazy tick advance (NEO-93). +/// NEO-96 telemetry hook sites: state transition commits and telegraph starts in +/// (telegraph_fired, npc_state_transition). +/// public static class NpcRuntimeOperations { /// Default per-advance simulation cap (seconds) for lazy poll ticks. @@ -114,6 +118,10 @@ public static class NpcRuntimeOperations if (snapshot.BehaviorState != NpcBehaviorState.Idle || snapshot.ActiveTelegraph is not null) { WriteIdle(runtimeStore, npcInstanceId); + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- + // TODO(E9.M1): catalog emit — holder cleared or fixture reset (any → idle). + // Planned payload fields: npcInstanceId, behaviorDefId, fromState, toState (idle), + // phaseStartedUtc, optional aggroHolderPlayerId (previous holder when known). } return; @@ -127,7 +135,10 @@ public static class NpcRuntimeOperations NpcBehaviorState.Aggro, windowStart, activeTelegraph: null); - // telemetry: npc_state_transition (NEO-96) + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- + // TODO(E9.M1): catalog emit — idle → aggro when aggro holder is set. + // Planned payload fields: npcInstanceId, behaviorDefId, fromState (idle), toState (aggro), + // phaseStartedUtc, aggroHolderPlayerId. if (!runtimeStore.TryGet(npcInstanceId, out snapshot)) { return; @@ -139,7 +150,16 @@ public static class NpcRuntimeOperations { if (!TryHasAggroHolder(npcInstanceId, threatStore, out aggroHolderPlayerId)) { + if (!runtimeStore.TryGet(npcInstanceId, out snapshot)) + { + return; + } + WriteIdle(runtimeStore, npcInstanceId); + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- + // TODO(E9.M1): catalog emit — holder cleared mid-cycle (any → idle). + // Planned payload fields: npcInstanceId, behaviorDefId, fromState, toState (idle), + // phaseStartedUtc, optional aggroHolderPlayerId (previous holder). return; } @@ -170,7 +190,14 @@ public static class NpcRuntimeOperations NpcBehaviorState.TelegraphWindup, cursor, telegraph); - // telemetry: npc_state_transition (NEO-96) + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- + // TODO(E9.M1): catalog emit — aggro → telegraph_windup. + // Planned payload fields: npcInstanceId, behaviorDefId, fromState (aggro), + // toState (telegraph_windup), phaseStartedUtc, aggroHolderPlayerId, telegraphId. + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `telegraph_fired` --- + // TODO(E9.M1): catalog emit — once per telegraph start (each windup cycle). + // Planned payload fields: npcInstanceId, telegraphId, behaviorDefId, archetypeKind, + // aggroHolderPlayerId, windupStartedUtc, telegraphWindupSeconds. break; } @@ -195,7 +222,10 @@ public static class NpcRuntimeOperations NpcBehaviorState.Recover, cursor, activeTelegraph: null); - // telemetry: telegraph_fired / npc_state_transition (NEO-96); logical attack_execute → recover + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- + // TODO(E9.M1): catalog emit — telegraph_windup → recover (logical attack_execute instant). + // Planned payload fields: npcInstanceId, behaviorDefId, fromState (telegraph_windup), + // toState (recover), phaseStartedUtc, aggroHolderPlayerId, telegraphId (completed windup). break; } @@ -213,7 +243,10 @@ public static class NpcRuntimeOperations NpcBehaviorState.Recover, cursor, activeTelegraph: null); - // telemetry: npc_state_transition (NEO-96) + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- + // TODO(E9.M1): catalog emit — attack_execute → recover (defensive visible phase). + // Planned payload fields: npcInstanceId, behaviorDefId, fromState (attack_execute), + // toState (recover), phaseStartedUtc, aggroHolderPlayerId. break; case NpcBehaviorState.Recover: @@ -232,7 +265,14 @@ public static class NpcRuntimeOperations NpcBehaviorState.TelegraphWindup, cursor, telegraph); - // telemetry: npc_state_transition (NEO-96) + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- + // TODO(E9.M1): catalog emit — recover → telegraph_windup. + // Planned payload fields: npcInstanceId, behaviorDefId, fromState (recover), + // toState (telegraph_windup), phaseStartedUtc, aggroHolderPlayerId, telegraphId. + // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `telegraph_fired` --- + // TODO(E9.M1): catalog emit — once per telegraph start (each windup cycle). + // Planned payload fields: npcInstanceId, telegraphId, behaviorDefId, archetypeKind, + // aggroHolderPlayerId, windupStartedUtc, telegraphWindupSeconds. break; } diff --git a/server/README.md b/server/README.md index fc410fd..d5fb166 100644 --- a/server/README.md +++ b/server/README.md @@ -188,6 +188,10 @@ Per-NPC behavior states live in **`Game/Npc/`** as **`INpcRuntimeStateStore`** + Plan: [NEO-93 implementation plan](../../docs/plans/NEO-93-implementation-plan.md). +### NPC runtime telemetry hooks (NEO-96) + +Comment-only hook sites in **`NpcRuntimeOperations.AdvanceAll`** for future E9.M1 catalog events — **`telegraph_fired`** at each **`telegraph_windup`** entry (aggro→telegraph and recover→telegraph); **`npc_state_transition`** on every committed state change (including holder clear → **`idle`**). **`TODO(E9.M1)`** — no production ingest. Snapshot GET invokes **`AdvanceAll`** before read (no duplicate API-layer hooks). Plan: [NEO-96 implementation plan](../../docs/plans/NEO-96-implementation-plan.md). + ## NPC runtime snapshot (NEO-94) **`GET /game/world/npc-runtime-snapshot`** returns a versioned JSON body (`schemaVersion` **1**, **`serverTimeUtc`**, **`npcInstances`**) backed by **`INpcRuntimeStateStore`** + **`IThreatStateStore`** after **`NpcRuntimeOperations.AdvanceAll`** (lazy tick, **5.0 s** delta cap). Each row includes **`npcInstanceId`**, **`behaviorDefId`**, **`state`** (stable snake_case), **`aggroHolderPlayerId`**, and optional nested **`activeTelegraph`** during **`telegraph_windup`**. All three **`PrototypeNpcRegistry`** instances are always returned in ascending **`npcInstanceId`** order. Plan: [NEO-94 implementation plan](../../docs/plans/NEO-94-implementation-plan.md); Bruno: `bruno/neon-sprawl-server/npc-runtime-snapshot/`. From fedb735e535ecd0ad441e143d3cc0838234f0d77 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Fri, 29 May 2026 20:49:41 -0400 Subject: [PATCH 3/4] NEO-96: add code review for E5M2-10 telemetry hook sites --- docs/reviews/2026-05-29-NEO-96.md | 54 +++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 docs/reviews/2026-05-29-NEO-96.md diff --git a/docs/reviews/2026-05-29-NEO-96.md b/docs/reviews/2026-05-29-NEO-96.md new file mode 100644 index 0000000..59d3b0f --- /dev/null +++ b/docs/reviews/2026-05-29-NEO-96.md @@ -0,0 +1,54 @@ +# Code review — NEO-96 (E5M2-10) + +**Date:** 2026-05-29 +**Scope:** Branch `NEO-96-e5m2-10-npc-telemetry-hooks` vs `origin/main` — commits `dedd9a9` … `5e1e374` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-96 replaces NEO-93 telemetry stubs in **`NpcRuntimeOperations.AdvanceAll`** with full NEO-84-style comment blocks for Epic 5 Slice 2 catalog events **`telegraph_fired`** (at each **`telegraph_windup`** entry) and **`npc_state_transition`** (on every committed state change, including holder clear → **`idle`**). Class summary, **`server/README.md`**, E5.M2 module doc, E5M2 backlog E5M2-10, alignment register, and implementation plan are updated. No production ingest, no new tests — appropriate for comment-only scope. **`NpcRuntimeOperations`** tests pass (**21/21**). One mid-cycle holder-clear path adds a defensive **`TryGet`** guard beyond pure comments; dev fixture **`ResetAllPrototypeRows`** bypasses hook anchors (pre-existing gap, comment wording slightly overstates coverage). + +## Documentation checked + +| Path | Result | +|------|--------| +| `docs/plans/NEO-96-implementation-plan.md` | **Matches** — AC checked; kickoff decisions (comments-only, engine-only, telegraph at windup start, every state commit, no manual QA) reflected in code and docs; reconciliation section accurate. | +| `docs/plans/E5M2-prototype-backlog.md` (E5M2-10) | **Matches** — AC checked, landed note present; no client counterpart (NEO-97 deferred). | +| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Partially matches** — Related implementation slices landed line present; Summary **Status** row still jumps **E5M2-09** → **E5M2-12** without **E5M2-10 / NEO-96 landed** (same gap NEO-95 review closed for E5M2-09). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M2 row includes **NEO-96 landed** with README/plan links. | +| `docs/decomposition/epics/epic_05_pve_combat.md` (Slice 2 telemetry) | **Matches** — hook vocabulary `telegraph_fired`, `npc_state_transition`. | +| `docs/decomposition/modules/client_server_authority.md` | **N/A** — no authority change; server-only comment anchors. | +| `server/README.md` | **Matches** — NPC runtime telemetry hooks subsection with event timing and E9.M1 deferral. | + +## Blocking issues + +None. + +## Suggestions + +1. **E5.M2 Summary status table** — Append **E5M2-10 / NEO-96 telemetry hooks landed** to the module Summary **Status** row (between E5M2-09 and E5M2-12), matching the pattern used when NEO-95 landed. + +2. **Fixture reset vs hook comments** — Plan transition table and the holder-clear hook comment cite “fixture reset,” but **`ResetAllPrototypeRows`** writes idle directly in the store (dev fixture API) without passing through **`AdvanceOne`** hook sites. Either add a comment-only anchor on the reset path when E9.M1 lands, or narrow comment/plan wording to “holder cleared during **`AdvanceAll`**” so implementers do not assume fixture reset is already instrumented. + +## Nits + +- Nit: Mid-cycle holder-clear path (inside the **`while`** loop) adds **`TryGet`** before **`WriteIdle`** — slightly exceeds “comments only” AC; **`snapshot`** from the top of the same iteration should already be valid, so the guard is redundant unless retained for future emit code that reads **`fromState`** from **`snapshot`**. + +- Nit: **`telegraph_fired`** payload comments list **`archetypeKind`** but engine scope has **`behaviorDefId`** only — fine for planned fields; E9.M1 implementer will map via behavior catalog (same as snapshot DTO projection). + +## Verification + +```bash +# NEO-96-adjacent runtime tests (passed 2026-05-29) +dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \ + --filter "FullyQualifiedName~NpcRuntimeOperations" + +# Full server suite (Postgres persistence tests require local DB) +dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj +``` + +Manual: none required (server-only comment hooks; kickoff decision). From 8a774857c6b3bf6120e1eb0b7026d9809404d117 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Fri, 29 May 2026 20:50:47 -0400 Subject: [PATCH 4/4] =?UTF-8?q?NEO-96:=20address=20code=20review=20?= =?UTF-8?q?=E2=80=94=20fixture=20wording,=20E5.M2=20status?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Narrow holder-clear hook comments to AdvanceAll scope; document ResetAllPrototypeRows bypass with optional E9.M1 anchor; remove redundant TryGet; update E5.M2 Summary status and review follow-up. --- .../modules/E5_M2_NpcAiAndBehaviorProfiles.md | 2 +- docs/plans/NEO-96-implementation-plan.md | 7 +++++-- docs/reviews/2026-05-29-NEO-96.md | 10 ++++++---- .../Game/Npc/NpcRuntimeOperations.cs | 15 +++++++-------- server/README.md | 2 +- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md index b305970..34c489e 100644 --- a/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md +++ b/docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md @@ -7,7 +7,7 @@ | **Module ID** | E5.M2 | | **Epic** | [Epic 5 — PvE Combat](../epics/epic_05_pve_combat.md) | | **Stage target** | Prototype | -| **Status** | In Progress — Slice 2 backlog [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md): **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) catalog + CI landed · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) server load landed · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) registry + DI landed · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) behavior-definitions GET landed · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) NPC instance registry + combat-target migration landed · **E5M2-06** [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) aggro rule + threat store landed · **E5M2-07** [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) NPC runtime state machine landed · **E5M2-08** [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) runtime snapshot GET landed · **E5M2-09** [NEO-95](https://linear.app/neon-sprawl/issue/NEO-95) NPC attack resolve + player combat HP landed → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) | +| **Status** | In Progress — Slice 2 backlog [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md): **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) catalog + CI landed · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) server load landed · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) registry + DI landed · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) behavior-definitions GET landed · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) NPC instance registry + combat-target migration landed · **E5M2-06** [NEO-92](https://linear.app/neon-sprawl/issue/NEO-92) aggro rule + threat store landed · **E5M2-07** [NEO-93](https://linear.app/neon-sprawl/issue/NEO-93) NPC runtime state machine landed · **E5M2-08** [NEO-94](https://linear.app/neon-sprawl/issue/NEO-94) runtime snapshot GET landed · **E5M2-09** [NEO-95](https://linear.app/neon-sprawl/issue/NEO-95) NPC attack resolve + player combat HP landed · **E5M2-10** [NEO-96](https://linear.app/neon-sprawl/issue/NEO-96) NPC runtime telemetry hooks landed → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) | | **Linear** | Label **`E5.M2`** · [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) · **E5M2-02** [NEO-88](https://linear.app/neon-sprawl/issue/NEO-88) · **E5M2-03** [NEO-89](https://linear.app/neon-sprawl/issue/NEO-89) · **E5M2-04** [NEO-90](https://linear.app/neon-sprawl/issue/NEO-90) · **E5M2-05** [NEO-91](https://linear.app/neon-sprawl/issue/NEO-91) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) | ## Purpose diff --git a/docs/plans/NEO-96-implementation-plan.md b/docs/plans/NEO-96-implementation-plan.md index a33c85c..bf091bb 100644 --- a/docs/plans/NEO-96-implementation-plan.md +++ b/docs/plans/NEO-96-implementation-plan.md @@ -52,7 +52,8 @@ ## Implementation reconciliation (shipped) - NEO-93 `(NEO-96)` stub comments replaced with full NEO-84-style hook blocks in **`NpcRuntimeOperations.AdvanceAll`**. -- **`telegraph_fired`** at aggro→telegraph and recover→telegraph entries; **`npc_state_transition`** on every state commit including holder clear → idle. +- **`telegraph_fired`** at aggro→telegraph and recover→telegraph entries; **`npc_state_transition`** on every state commit during **`AdvanceAll`** (holder clear → idle). +- Dev fixture **`ResetAllPrototypeRows`** documented as bypassing **`AdvanceOne`** hooks; optional E9.M1 anchor on reset wrapper. - **`server/README.md`**, **E5_M2** module doc, **E5M2** backlog, and alignment register updated. - No new tests (comment-only); existing NPC runtime tests unchanged. @@ -84,7 +85,9 @@ Immediately after every **`WriteState`** / **`WriteIdle`** commit: | Windup complete | `telegraph_windup` → `recover` (logical attack; no visible `attack_execute` phase) | | Defensive `AttackExecute` branch | `attack_execute` → `recover` | | Recover cooldown complete | `recover` → `telegraph_windup` | -| Holder cleared / fixture reset | any → `idle` | +| Holder cleared during **`AdvanceAll`** | any → `idle` | + +**Not instrumented in NEO-96:** dev fixture **`ResetAllPrototypeRows`** writes idle directly (bypasses **`AdvanceOne`**); optional E9.M1 anchor noted on reset wrapper. Comment block names future E9.M1 event **`npc_state_transition`**. **`TODO(E9.M1): catalog emit`** — once per committed transition. diff --git a/docs/reviews/2026-05-29-NEO-96.md b/docs/reviews/2026-05-29-NEO-96.md index 59d3b0f..17ba9e9 100644 --- a/docs/reviews/2026-05-29-NEO-96.md +++ b/docs/reviews/2026-05-29-NEO-96.md @@ -4,6 +4,8 @@ **Scope:** Branch `NEO-96-e5m2-10-npc-telemetry-hooks` vs `origin/main` — commits `dedd9a9` … `5e1e374` **Base:** `origin/main` +**Follow-up:** Code review suggestions addressed (2026-05-29). + ## Verdict **Approve with nits** @@ -18,7 +20,7 @@ NEO-96 replaces NEO-93 telemetry stubs in **`NpcRuntimeOperations.AdvanceAll`** |------|--------| | `docs/plans/NEO-96-implementation-plan.md` | **Matches** — AC checked; kickoff decisions (comments-only, engine-only, telegraph at windup start, every state commit, no manual QA) reflected in code and docs; reconciliation section accurate. | | `docs/plans/E5M2-prototype-backlog.md` (E5M2-10) | **Matches** — AC checked, landed note present; no client counterpart (NEO-97 deferred). | -| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Partially matches** — Related implementation slices landed line present; Summary **Status** row still jumps **E5M2-09** → **E5M2-12** without **E5M2-10 / NEO-96 landed** (same gap NEO-95 review closed for E5M2-09). | +| `docs/decomposition/modules/E5_M2_NpcAiAndBehaviorProfiles.md` | **Matches** — Related implementation slices landed line present; Summary **Status** row includes **E5M2-10 / NEO-96 landed**. | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M2 row includes **NEO-96 landed** with README/plan links. | | `docs/decomposition/epics/epic_05_pve_combat.md` (Slice 2 telemetry) | **Matches** — hook vocabulary `telegraph_fired`, `npc_state_transition`. | | `docs/decomposition/modules/client_server_authority.md` | **N/A** — no authority change; server-only comment anchors. | @@ -30,13 +32,13 @@ None. ## Suggestions -1. **E5.M2 Summary status table** — Append **E5M2-10 / NEO-96 telemetry hooks landed** to the module Summary **Status** row (between E5M2-09 and E5M2-12), matching the pattern used when NEO-95 landed. +1. ~~**E5.M2 Summary status table** — Append **E5M2-10 / NEO-96 telemetry hooks landed** to the module Summary **Status** row (between E5M2-09 and E5M2-12), matching the pattern used when NEO-95 landed.~~ **Done.** Summary **Status** row updated. -2. **Fixture reset vs hook comments** — Plan transition table and the holder-clear hook comment cite “fixture reset,” but **`ResetAllPrototypeRows`** writes idle directly in the store (dev fixture API) without passing through **`AdvanceOne`** hook sites. Either add a comment-only anchor on the reset path when E9.M1 lands, or narrow comment/plan wording to “holder cleared during **`AdvanceAll`**” so implementers do not assume fixture reset is already instrumented. +2. ~~**Fixture reset vs hook comments** — Plan transition table and the holder-clear hook comment cite “fixture reset,” but **`ResetAllPrototypeRows`** writes idle directly in the store (dev fixture API) without passing through **`AdvanceOne`** hook sites. Either add a comment-only anchor on the reset path when E9.M1 lands, or narrow comment/plan wording to “holder cleared during **`AdvanceAll`**” so implementers do not assume fixture reset is already instrumented.~~ **Done.** Wording narrowed to “holder cleared during **`AdvanceAll`**”; reset wrapper documents bypass + optional E9.M1 anchor; plan and README updated. ## Nits -- Nit: Mid-cycle holder-clear path (inside the **`while`** loop) adds **`TryGet`** before **`WriteIdle`** — slightly exceeds “comments only” AC; **`snapshot`** from the top of the same iteration should already be valid, so the guard is redundant unless retained for future emit code that reads **`fromState`** from **`snapshot`**. +- ~~Nit: Mid-cycle holder-clear path (inside the **`while`** loop) adds **`TryGet`** before **`WriteIdle`** — slightly exceeds “comments only” AC; **`snapshot`** from the top of the same iteration should already be valid, so the guard is redundant unless retained for future emit code that reads **`fromState`** from **`snapshot`**.~~ **Done.** Redundant **`TryGet`** removed. - Nit: **`telegraph_fired`** payload comments list **`archetypeKind`** but engine scope has **`behaviorDefId`** only — fine for planned fields; E9.M1 implementer will map via behavior catalog (same as snapshot DTO projection). diff --git a/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs b/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs index c43ae3a..e138945 100644 --- a/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs +++ b/server/NeonSprawl.Server/Game/Npc/NpcRuntimeOperations.cs @@ -85,8 +85,12 @@ public static class NpcRuntimeOperations } /// Resets all prototype NPC runtime rows to idle (dev fixture). - public static void ResetAllPrototypeRows(INpcRuntimeStateStore runtimeStore) => + public static void ResetAllPrototypeRows(INpcRuntimeStateStore runtimeStore) + { + // NEO-96: dev fixture reset writes idle directly — bypasses AdvanceOne npc_state_transition hooks. + // TODO(E9.M1): optional catalog emit anchor here if fixture reset transitions must be tracked. runtimeStore.ResetAllPrototypeRows(); + } private static void AdvanceOne( string npcInstanceId, @@ -119,7 +123,7 @@ public static class NpcRuntimeOperations { WriteIdle(runtimeStore, npcInstanceId); // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- - // TODO(E9.M1): catalog emit — holder cleared or fixture reset (any → idle). + // TODO(E9.M1): catalog emit — holder cleared during AdvanceAll (any → idle). // Planned payload fields: npcInstanceId, behaviorDefId, fromState, toState (idle), // phaseStartedUtc, optional aggroHolderPlayerId (previous holder when known). } @@ -150,14 +154,9 @@ public static class NpcRuntimeOperations { if (!TryHasAggroHolder(npcInstanceId, threatStore, out aggroHolderPlayerId)) { - if (!runtimeStore.TryGet(npcInstanceId, out snapshot)) - { - return; - } - WriteIdle(runtimeStore, npcInstanceId); // --- Telemetry hook site (NEO-96): future E9.M1 catalog event `npc_state_transition` --- - // TODO(E9.M1): catalog emit — holder cleared mid-cycle (any → idle). + // TODO(E9.M1): catalog emit — holder cleared mid-cycle during AdvanceAll (any → idle). // Planned payload fields: npcInstanceId, behaviorDefId, fromState, toState (idle), // phaseStartedUtc, optional aggroHolderPlayerId (previous holder). return; diff --git a/server/README.md b/server/README.md index d5fb166..9fb9939 100644 --- a/server/README.md +++ b/server/README.md @@ -190,7 +190,7 @@ Plan: [NEO-93 implementation plan](../../docs/plans/NEO-93-implementation-plan.m ### NPC runtime telemetry hooks (NEO-96) -Comment-only hook sites in **`NpcRuntimeOperations.AdvanceAll`** for future E9.M1 catalog events — **`telegraph_fired`** at each **`telegraph_windup`** entry (aggro→telegraph and recover→telegraph); **`npc_state_transition`** on every committed state change (including holder clear → **`idle`**). **`TODO(E9.M1)`** — no production ingest. Snapshot GET invokes **`AdvanceAll`** before read (no duplicate API-layer hooks). Plan: [NEO-96 implementation plan](../../docs/plans/NEO-96-implementation-plan.md). +Comment-only hook sites in **`NpcRuntimeOperations.AdvanceAll`** for future E9.M1 catalog events — **`telegraph_fired`** at each **`telegraph_windup`** entry (aggro→telegraph and recover→telegraph); **`npc_state_transition`** on every committed state change during **`AdvanceAll`** (including holder clear → **`idle`**). Dev fixture **`ResetAllPrototypeRows`** writes idle directly and bypasses **`AdvanceOne`** hooks (optional E9.M1 anchor noted on reset path). **`TODO(E9.M1)`** — no production ingest. Snapshot GET invokes **`AdvanceAll`** before read (no duplicate API-layer hooks). Plan: [NEO-96 implementation plan](../../docs/plans/NEO-96-implementation-plan.md). ## NPC runtime snapshot (NEO-94)