Merge pull request #148 from ViPro-Technologies/NEO-109-slice-3-encounter-telemetry-hook-sites
NEO-109: Slice 3 encounter telemetry hook sitespull/150/head
commit
c3022201fe
|
|
@ -96,6 +96,8 @@ The **first shipped encounter + reward spine** is **frozen** for prototype tunin
|
|||
|
||||
**Per-player progress HTTP landed ([NEO-108](https://linear.app/neon-sprawl/issue/NEO-108)):** **`GET /game/players/{id}/encounter-progress`** — **`EncounterProgressApi`** + DTOs; projects progress, completion, and event stores; **`rewardGrantSummary`** from commit-time **`GrantedItems`**. Plan: [NEO-108 implementation plan](../../plans/NEO-108-implementation-plan.md); [server README — Per-player encounter progress (NEO-108)](../../../server/README.md#per-player-encounter-progress-neo-108); Bruno `bruno/neon-sprawl-server/encounter-progress/`. Client counterpart: [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110).
|
||||
|
||||
**Slice 3 telemetry hooks landed ([NEO-109](https://linear.app/neon-sprawl/issue/NEO-109)):** comment-only **`encounter_start`**, **`encounter_complete`**, **`reward_attribution`**, and **`encounter_complete_denied`** in **`EncounterProgressOperations`** / **`EncounterCompletionOperations`**; **`EncounterCombatWiring`** delegates without duplicate hooks. Plan: [NEO-109 implementation plan](../../plans/NEO-109-implementation-plan.md); [server README — Encounter telemetry hooks (NEO-109)](../../../server/README.md#encounter-telemetry-hooks-neo-109).
|
||||
|
||||
## Source anchors
|
||||
|
||||
- Master plan: [`neon_sprawl_vision.plan.md`](../../../neon_sprawl_vision.plan.md) — Epic 5.
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
|
|
@ -80,7 +80,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen
|
|||
|
||||
**E5.M2 note:** Epic 5 **Slice 2** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98); label **`E5.M2`**. Client capstone **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98). See [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2_NpcAiAndBehaviorProfiles.md](E5_M2_NpcAiAndBehaviorProfiles.md). Upstream **E5.M1 Ready**. **NEO-87 landed:** frozen three-behavior catalog + CI ([NEO-87 plan](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` ([NEO-88 plan](../../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 ([NEO-89 plan](../../plans/NEO-89-implementation-plan.md)). **NEO-90 landed:** **`GET /game/world/npc-behavior-definitions`** — `NpcBehaviorDefinitionsWorldApi` + DTOs in `Game/Npc/` ([NEO-90 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/`. **NEO-91 landed:** **`PrototypeNpcRegistry`** + combat-target migration — three NPC instance ids, per-archetype catalog max HP ([NEO-91 plan](../../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–NEO-96 landed:** aggro/threat, runtime state machine, **`GET …/npc-runtime-snapshot`**, player combat HP + NPC attack resolve, telemetry hooks ([NEO-92](../../plans/NEO-92-implementation-plan.md)–[NEO-96](../../plans/NEO-96-implementation-plan.md)). **NEO-97 landed:** client telegraph HUD ([NEO-97 plan](../../plans/NEO-97-implementation-plan.md), [`NEO-97` manual QA](../../manual-qa/NEO-97.md)). **NEO-98 landed:** playable NPC telegraph combat capstone ([NEO-98 plan](../../plans/NEO-98-implementation-plan.md), [`NEO-98` manual QA](../../manual-qa/NEO-98.md)); [client README — End-to-end NPC telegraph combat loop (NEO-98)](../../../client/README.md#end-to-end-npc-telegraph-combat-loop-neo-98). **Epic 5 Slice 2 complete — register row Ready.** Replaces **`prototype_target_alpha` / `beta`** with three NPC archetype instances; owns **`ThreatState`**, **`TelegraphEvent`**, session **`IPlayerCombatHealthStore`**.
|
||||
|
||||
**E5.M3 note:** Epic 5 **Slice 3** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-100](https://linear.app/neon-sprawl/issue/NEO-100) → [NEO-111](https://linear.app/neon-sprawl/issue/NEO-111); label **`E5.M3`**. Client capstone **E5M3-12** [NEO-111](https://linear.app/neon-sprawl/issue/NEO-111). See [E5M3-prototype-backlog.md](../../plans/E5M3-prototype-backlog.md), [E5_M3_EncounterAndRewardTables.md](E5_M3_EncounterAndRewardTables.md). Upstream **E5.M2 Ready**, **E3.M3** inventory landed. Prototype spine: one encounter **`prototype_combat_pocket`** (defeat all three E5.M2 NPC ids) → reward table **`prototype_combat_pocket_clear`** ( **`scrap_metal_bulk` ×10**, **`contract_handoff_token` ×1** ); idempotent completion per player; per-defeat gig XP unchanged ([NEO-44](../../plans/NEO-44-implementation-plan.md)). **NEO-100 landed:** encounter + reward-table schemas, prototype catalogs, CI gates ([NEO-100 plan](../../plans/NEO-100-implementation-plan.md)). **NEO-101 landed:** fail-fast server load of `content/encounters/*_encounters.json` + `content/reward-tables/*_reward_tables.json` ([NEO-101 plan](../../plans/NEO-101-implementation-plan.md)); [server README — Encounter + reward-table catalogs (NEO-101)](../../../server/README.md#reward-table-catalog-contentreward-tables-neo-101). **NEO-102 landed:** injectable **`IEncounterDefinitionRegistry`** + **`IRewardTableDefinitionRegistry`** + DI ([NEO-102 plan](../../plans/NEO-102-implementation-plan.md)); [server README — Encounter + reward-table registries (NEO-102)](../../../server/README.md#reward-table-catalog-contentreward-tables-neo-101). **NEO-103 landed:** **`GET /game/world/encounter-definitions`** — `EncounterDefinitionsWorldApi` + nested **`rewardTable`** summary ([NEO-103 plan](../../plans/NEO-103-implementation-plan.md)); [server README — Encounter definitions (NEO-103)](../../../server/README.md#encounter-definitions-neo-103); Bruno `bruno/neon-sprawl-server/encounter-definitions/`. **NEO-104 landed:** **`IEncounterProgressStore`** + **`IEncounterCompletionStore`** + **`EncounterProgressOperations`** ([NEO-104 plan](../../plans/NEO-104-implementation-plan.md)); [server README — Encounter progress (NEO-104)](../../../server/README.md#encounter-progress--completion-stores-neo-104). **NEO-105 landed:** **`EncounterCompletionOperations`** + **`EncounterCompleteEvent`** result payload ([NEO-105 plan](../../plans/NEO-105-implementation-plan.md)); [server README — Encounter completion (NEO-105)](../../../server/README.md#encounter-completion--inventory-grants-neo-105). **NEO-106 landed:** **`EncounterCombatWiring`** on **`AbilityCastApi`** ([NEO-106 plan](../../plans/NEO-106-implementation-plan.md)); [server README — Encounter combat wiring (NEO-106)](../../../server/README.md#encounter-combat-wiring-neo-106). **NEO-107 landed:** **`IEncounterCompleteEventStore`** + E7.M2 hook stub ([NEO-107 plan](../../plans/NEO-107-implementation-plan.md)); [server README — Encounter complete event (NEO-107)](../../../server/README.md#encounter-complete-event-record-neo-107). **NEO-108 landed:** **`GET /game/players/{id}/encounter-progress`** — `EncounterProgressApi` + DTOs ([NEO-108 plan](../../plans/NEO-108-implementation-plan.md)); [server README — Per-player encounter progress (NEO-108)](../../../server/README.md#per-player-encounter-progress-neo-108); Bruno `bruno/neon-sprawl-server/encounter-progress/`. **E5M3-10+ outstanding.**
|
||||
**E5.M3 note:** Epic 5 **Slice 3** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-100](https://linear.app/neon-sprawl/issue/NEO-100) → [NEO-111](https://linear.app/neon-sprawl/issue/NEO-111); label **`E5.M3`**. Client capstone **E5M3-12** [NEO-111](https://linear.app/neon-sprawl/issue/NEO-111). See [E5M3-prototype-backlog.md](../../plans/E5M3-prototype-backlog.md), [E5_M3_EncounterAndRewardTables.md](E5_M3_EncounterAndRewardTables.md). Upstream **E5.M2 Ready**, **E3.M3** inventory landed. Prototype spine: one encounter **`prototype_combat_pocket`** (defeat all three E5.M2 NPC ids) → reward table **`prototype_combat_pocket_clear`** ( **`scrap_metal_bulk` ×10**, **`contract_handoff_token` ×1** ); idempotent completion per player; per-defeat gig XP unchanged ([NEO-44](../../plans/NEO-44-implementation-plan.md)). **NEO-100 landed:** encounter + reward-table schemas, prototype catalogs, CI gates ([NEO-100 plan](../../plans/NEO-100-implementation-plan.md)). **NEO-101 landed:** fail-fast server load of `content/encounters/*_encounters.json` + `content/reward-tables/*_reward_tables.json` ([NEO-101 plan](../../plans/NEO-101-implementation-plan.md)); [server README — Encounter + reward-table catalogs (NEO-101)](../../../server/README.md#reward-table-catalog-contentreward-tables-neo-101). **NEO-102 landed:** injectable **`IEncounterDefinitionRegistry`** + **`IRewardTableDefinitionRegistry`** + DI ([NEO-102 plan](../../plans/NEO-102-implementation-plan.md)); [server README — Encounter + reward-table registries (NEO-102)](../../../server/README.md#reward-table-catalog-contentreward-tables-neo-101). **NEO-103 landed:** **`GET /game/world/encounter-definitions`** — `EncounterDefinitionsWorldApi` + nested **`rewardTable`** summary ([NEO-103 plan](../../plans/NEO-103-implementation-plan.md)); [server README — Encounter definitions (NEO-103)](../../../server/README.md#encounter-definitions-neo-103); Bruno `bruno/neon-sprawl-server/encounter-definitions/`. **NEO-104 landed:** **`IEncounterProgressStore`** + **`IEncounterCompletionStore`** + **`EncounterProgressOperations`** ([NEO-104 plan](../../plans/NEO-104-implementation-plan.md)); [server README — Encounter progress (NEO-104)](../../../server/README.md#encounter-progress--completion-stores-neo-104). **NEO-105 landed:** **`EncounterCompletionOperations`** + **`EncounterCompleteEvent`** result payload ([NEO-105 plan](../../plans/NEO-105-implementation-plan.md)); [server README — Encounter completion (NEO-105)](../../../server/README.md#encounter-completion--inventory-grants-neo-105). **NEO-106 landed:** **`EncounterCombatWiring`** on **`AbilityCastApi`** ([NEO-106 plan](../../plans/NEO-106-implementation-plan.md)); [server README — Encounter combat wiring (NEO-106)](../../../server/README.md#encounter-combat-wiring-neo-106). **NEO-107 landed:** **`IEncounterCompleteEventStore`** + E7.M2 hook stub ([NEO-107 plan](../../plans/NEO-107-implementation-plan.md)); [server README — Encounter complete event (NEO-107)](../../../server/README.md#encounter-complete-event-record-neo-107). **NEO-108 landed:** **`GET /game/players/{id}/encounter-progress`** — `EncounterProgressApi` + DTOs ([NEO-108 plan](../../plans/NEO-108-implementation-plan.md)); [server README — Per-player encounter progress (NEO-108)](../../../server/README.md#per-player-encounter-progress-neo-108); Bruno `bruno/neon-sprawl-server/encounter-progress/`. **NEO-109 landed:** comment-only **`encounter_start`**, **`encounter_complete`**, **`reward_attribution`**, **`encounter_complete_denied`** hook sites ([NEO-109 plan](../../plans/NEO-109-implementation-plan.md)); [server README — Encounter telemetry hooks (NEO-109)](../../../server/README.md#encounter-telemetry-hooks-neo-109). **E5M3-11+ client HUD outstanding.**
|
||||
|
||||
### Epic 6 — PvP Security
|
||||
|
||||
|
|
|
|||
|
|
@ -312,8 +312,10 @@ Working backlog for **Epic 5 — Slice 3** ([Epic 5 · Slice 3 — encounters an
|
|||
|
||||
**Acceptance criteria**
|
||||
|
||||
- [ ] Hook names match [epic_05 Slice 3](../decomposition/epics/epic_05_pve_combat.md#epic-5-slice-3) vocabulary.
|
||||
- [ ] No runtime behavior change beyond comments.
|
||||
- [x] Hook names match [epic_05 Slice 3](../decomposition/epics/epic_05_pve_combat.md#epic-5-slice-3) vocabulary.
|
||||
- [x] No runtime behavior change beyond comments.
|
||||
|
||||
**Landed ([NEO-109](https://linear.app/neon-sprawl/issue/NEO-109)):** comment-only **`encounter_start`**, **`encounter_complete`**, **`reward_attribution`** (once per completion), **`encounter_complete_denied`**; [server README — Encounter telemetry hooks (NEO-109)](../../server/README.md#encounter-telemetry-hooks-neo-109). Plan: [NEO-109-implementation-plan.md](NEO-109-implementation-plan.md).
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,143 @@
|
|||
# NEO-109 — Implementation plan
|
||||
|
||||
## Story reference
|
||||
|
||||
| Field | Value |
|
||||
|--------|--------|
|
||||
| **Key** | NEO-109 |
|
||||
| **Title** | E5M3-10: Slice 3 encounter telemetry hook sites |
|
||||
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-109/e5m3-10-slice-3-encounter-telemetry-hook-sites |
|
||||
| **Module** | [E5.M3 — EncounterAndRewardTables](../decomposition/modules/E5_M3_EncounterAndRewardTables.md) · Epic 5 Slice 3 · backlog **E5M3-10** |
|
||||
| **Branch** | `NEO-109-slice-3-encounter-telemetry-hook-sites` |
|
||||
| **Blocked by** | [NEO-106](https://linear.app/neon-sprawl/issue/NEO-106) — combat wiring (**Done** on `main`); [NEO-107](https://linear.app/neon-sprawl/issue/NEO-107) — event record (**Done** on `main`) |
|
||||
| **Pattern** | [NEO-49](NEO-49-implementation-plan.md) / [NEO-64](NEO-64-implementation-plan.md) — comment-only engine anchors; [NEO-84](NEO-84-implementation-plan.md) — reserved `encounter_start` in Slice 1 combat |
|
||||
| **Blocks** | None — client capstone [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) / [NEO-111](https://linear.app/neon-sprawl/issue/NEO-111) do not depend on telemetry |
|
||||
| **Client counterpart** | None — server-only comments; player-visible HUD is [NEO-110](https://linear.app/neon-sprawl/issue/NEO-110) |
|
||||
|
||||
## Kickoff clarifications
|
||||
|
||||
| Topic | Question | Agent recommendation | Answer |
|
||||
|--------|----------|----------------------|--------|
|
||||
| **`reward_attribution` shape** | Once per completion vs per grant vs skip? | **Once per successful completion** — batch payload after all grants + completion mark commit; `item_created` remains at inventory layer (**NEO-56**). | **Adopted** — once per completion |
|
||||
| **`encounter_complete` anchor** | Single engine anchor vs dual wiring + ops? | **`EncounterCompletionOperations.TryCompleteAndGrant` only** — remove duplicate in **`EncounterCombatWiring`** (**NEO-49** / **NEO-64** precedent). | **Adopted** — completion ops only |
|
||||
| **Manual QA doc** | `docs/manual-qa/NEO-109.md`? | **Yes** (NEO-64 pattern). | **Adopted** — **skip** (NEO-108 server-only precedent per user) |
|
||||
| **`encounter_complete_denied`** | Keep deny hook on `Deny`? | **Keep** — already landed from NEO-105 review; **NEO-56** deny precedent; not in epic vocabulary but comments-only. | **Adopted** (implicit — no change) |
|
||||
| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only** — `TODO(E9.M1)`; no production logging (**NEO-49** precedent). | **Adopted** (repo precedent; not asked) |
|
||||
|
||||
## Goal, scope, and out-of-scope
|
||||
|
||||
**Goal:** Document and place **comment-only hook sites** on the server-authoritative encounter funnel for Epic 5 Slice 3 product telemetry **`encounter_start`**, **`encounter_complete`**, and **`reward_attribution`**, plus consolidate anchors so E9.M1 wiring has one documented surface per event.
|
||||
|
||||
**In scope (from Linear + [E5M3-10](E5M3-prototype-backlog.md#e5m3-10--slice-3-encounter-telemetry-hook-sites)):**
|
||||
|
||||
- **`TODO(E9.M1)`** markers at encounter start, completion commit, and **once** reward attribution on successful grant batch.
|
||||
- **`server/README.md`** encounter telemetry subsection; epic Slice 3 vocabulary cross-link.
|
||||
- Normalize partial NEO-109 stubs from NEO-104/105/106 into full **NEO-64**-style comment blocks (event name, payload fields, no ingest).
|
||||
- Update **NEO-84** reserved **`encounter_start`** pointer in **`CombatOperations`** to E5.M3 anchor (comments only).
|
||||
- E5M3 backlog + alignment register + module snapshot when implementation completes.
|
||||
|
||||
**Out of scope (from Linear + backlog):**
|
||||
|
||||
- Production ingest, dashboards (**E9.M1**).
|
||||
- `ILogger` / metrics / dev-only log lines.
|
||||
- Godot client, Bruno changes, API/wire contract changes.
|
||||
- `docs/manual-qa/NEO-109.md` (kickoff: skip).
|
||||
- Duplicate hook comments in **`AbilityCastApi`** or **`EncounterCombatWiring`** for **`encounter_complete`** / **`reward_attribution`**.
|
||||
|
||||
## Acceptance criteria checklist
|
||||
|
||||
- [x] Hook names match [epic_05 Slice 3](../decomposition/epics/epic_05_pve_combat.md#epic-5-slice-3) vocabulary (`encounter_complete`, reward attribution / **`reward_attribution`**).
|
||||
- [x] No runtime behavior change beyond comments.
|
||||
|
||||
## Implementation reconciliation (shipped)
|
||||
|
||||
- **`encounter_start`:** full comment block in **`EncounterProgressOperations.TryActivateOnFirstEngagement`** after successful **`TryActivate`** (anchor matches once-per-activation emit); NEO-84 **`CombatOperations`** reserved pointer updated.
|
||||
- **`encounter_complete`** + **`reward_attribution`:** full comment blocks in **`EncounterCompletionOperations.TryCompleteAndGrant`** after event record; duplicate removed from **`EncounterCombatWiring`** (delegate pointer only).
|
||||
- **`encounter_complete_denied`:** retained on **`Deny`**.
|
||||
- **Docs:** `server/README.md` encounter telemetry subsection; E5.M3 module snapshot; alignment register; module dependency register; E5M3-10 backlog landed note.
|
||||
|
||||
## Technical approach
|
||||
|
||||
### Current state (on `main`)
|
||||
|
||||
Partial **NEO-109** tags already exist from precursor stories:
|
||||
|
||||
| Event | File | Status |
|
||||
|-------|------|--------|
|
||||
| **`encounter_start`** | `EncounterProgressOperations.TryActivateOnFirstEngagement` | Stub present — expand to full block |
|
||||
| **`encounter_complete`** | `EncounterCompletionOperations` success path | Thin `TODO` — expand; **remove** `EncounterCombatWiring` duplicate |
|
||||
| **`encounter_complete_denied`** | `EncounterCompletionOperations.Deny` | Block present — keep |
|
||||
| **`reward_attribution`** | — | **Missing** — add on completion success path |
|
||||
|
||||
### Hook sites (after implementation)
|
||||
|
||||
1. **`encounter_start`** — **`EncounterProgressOperations.TryActivateOnFirstEngagement`**, immediately before **`progressStore.TryActivate`** when activation will proceed (not when already started/completed/no encounter):
|
||||
- Comment block: future E9.M1 event **`encounter_start`**.
|
||||
- **`TODO(E9.M1): catalog emit`** — once per player+encounter on first activation.
|
||||
- Payload: `playerId`, `encounterId`, `npcInstanceId` (engagement target), optional `requiredNpcInstanceIds` count.
|
||||
- **NEO-84:** update class-level reserved comment in **`CombatOperations`** to point here (Slice 3 landed).
|
||||
|
||||
2. **`encounter_complete`** — **`EncounterCompletionOperations.TryCompleteAndGrant`**, after **`TryMarkCompleted`** + **`TryRecord`** succeed (authoritative commit):
|
||||
- Replace thin TODO with full block naming **`encounter_complete`**.
|
||||
- Payload: `playerId`, `encounterId`, `rewardTableId`, `completedAt`, `idempotencyKey`, `grantedItems` summary from **`EncounterCompleteEvent`**.
|
||||
- **Remove** duplicate hook block at end of **`EncounterCombatWiring.TryProcessCastOutcome`** (wiring only calls completion ops).
|
||||
|
||||
3. **`reward_attribution`** — same success path, **once per completion** (kickoff decision):
|
||||
- Comment block naming **`reward_attribution`** (snake_case catalog name; epic prose “reward attribution”).
|
||||
- **`TODO(E9.M1): catalog emit`** — once when reward table grants commit for encounter completion.
|
||||
- Payload: `playerId`, `encounterId`, `rewardTableId`, `grantedItems[]` (batch from **`EncounterGrantApplied`** / event store snapshot).
|
||||
- Note in comment: inventory layer still emits **`item_created`** per stack via **`PlayerInventoryOperations`** (**NEO-56**); **`reward_attribution`** is encounter-scoped attribution for E7.M2 / loot analytics.
|
||||
|
||||
4. **`encounter_complete_denied`** — keep existing **`Deny`** block ( **`reasonCode`** from **`EncounterCompletionReasonCodes`** ).
|
||||
|
||||
5. **Cross-references:**
|
||||
- **`EncounterCompletionResult`** / **`EncounterCompleteEvent`** xml summaries → telemetry anchors in **`EncounterCompletionOperations`**.
|
||||
- **`EncounterProgressOperations`** class summary → **`encounter_start`** anchor.
|
||||
- **`server/README.md`:** new **Encounter telemetry hooks (NEO-109)** subsection (after NEO-108 or NEO-106 block) listing all three epic events + deny + **`CombatOperations`** reserved pointer.
|
||||
- **`E5_M3_EncounterAndRewardTables.md`**, **`documentation_and_implementation_alignment.md`**, **`module_dependency_register.md`**, **`E5M3-prototype-backlog.md`:** E5M3-10 landed note when complete.
|
||||
|
||||
### NEO-56 / NEO-64 analogy
|
||||
|
||||
| Layer | Inventory / gather | Encounter Slice 3 |
|
||||
|-------|-------------------|-------------------|
|
||||
| Low-level item mutation | **`item_created`** (**NEO-56**) | Same on each **`TryAddStack`** during grants |
|
||||
| Orchestration / funnel | **`resource_gathered`** (**NEO-64**) | **`encounter_complete`** + **`reward_attribution`** (**NEO-109**) at completion commit |
|
||||
| Engagement | — | **`encounter_start`** at first activation |
|
||||
|
||||
## Files to add
|
||||
|
||||
None.
|
||||
|
||||
## Files to modify
|
||||
|
||||
| Path | Rationale |
|
||||
|------|-----------|
|
||||
| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionOperations.cs` | Expand **`encounter_complete`** block; add **`reward_attribution`** once on success; keep **`encounter_complete_denied`** on **`Deny`**. |
|
||||
| `server/NeonSprawl.Server/Game/Encounters/EncounterCombatWiring.cs` | Remove duplicate **`encounter_complete`** hook; one-line pointer to **`EncounterCompletionOperations`**. |
|
||||
| `server/NeonSprawl.Server/Game/Encounters/EncounterProgressOperations.cs` | Expand **`encounter_start`** block to NEO-64 style; class summary cross-ref. |
|
||||
| `server/NeonSprawl.Server/Game/Encounters/EncounterCompletionResult.cs` | Xml summary → telemetry hook anchor (no behavior change). |
|
||||
| `server/NeonSprawl.Server/Game/Encounters/EncounterCompleteEvent.cs` | Xml summary → completion + attribution hook anchor (no behavior change). |
|
||||
| `server/NeonSprawl.Server/Game/Combat/CombatOperations.cs` | Update reserved **`encounter_start`** comment → E5.M3 **`EncounterProgressOperations`** anchor. |
|
||||
| `server/README.md` | **NEO-109** encounter telemetry hooks subsection; fix NEO-104 vs NEO-109 label on **`encounter_start`** in NEO-106 table if needed. |
|
||||
| `docs/decomposition/modules/E5_M3_EncounterAndRewardTables.md` | NEO-109 telemetry hooks landed line. |
|
||||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M3 row: NEO-109 hook sites. |
|
||||
| `docs/decomposition/modules/module_dependency_register.md` | E5.M3 note — E5M3-10 complete when landed. |
|
||||
| `docs/plans/E5M3-prototype-backlog.md` | E5M3-10 acceptance checkboxes + landed note when complete. |
|
||||
|
||||
## Tests
|
||||
|
||||
| Test file | What it covers |
|
||||
|-----------|----------------|
|
||||
| *(none added or changed)* | Comments-only; no behavior or wire contract change. Regression: `dotnet test NeonSprawl.sln` (existing **`EncounterCompletionOperationsTests`**, **`EncounterProgressOperationsTests`**, cast integration tests unchanged). |
|
||||
|
||||
**Manual verification (no checklist doc):** grep `NEO-109` / `encounter_complete` / `reward_attribution` / `encounter_start` under `server/NeonSprawl.Server/Game/Encounters/`; confirm **`EncounterCombatWiring`** has no **`encounter_complete`** emit comment; confirm no new **`ILogger`** or ingest calls.
|
||||
|
||||
## Open questions / risks
|
||||
|
||||
| Question / risk | Agent recommendation | Status |
|
||||
|-----------------|----------------------|--------|
|
||||
| **Overlap with NEO-56 `item_created`** | Keep both — **`item_created`** per stack; **`reward_attribution`** once per encounter completion for attribution context. | **adopted** (kickoff) |
|
||||
| **`encounter_complete_denied` not in epic list** | Keep deny hook — **NEO-56** precedent; zero runtime cost. | **adopted** |
|
||||
| **Precursor partial stubs** | Treat as draft; NEO-109 normalizes style and removes wiring duplicate. | **adopted** |
|
||||
|
||||
None blocking.
|
||||
|
|
@ -0,0 +1,59 @@
|
|||
# Code review — NEO-109 (E5M3-10)
|
||||
|
||||
**Date:** 2026-05-31
|
||||
**Scope:** Branch `NEO-109-slice-3-encounter-telemetry-hook-sites` vs `4aa7665` (merge-base on `main`) — commits `e6debc2` … `8cda0d7`
|
||||
**Base:** `4aa7665` (main at branch point, post NEO-108 merge)
|
||||
|
||||
## Verdict
|
||||
|
||||
**Approve with nits**
|
||||
|
||||
## Summary
|
||||
|
||||
NEO-109 lands **comment-only** E9.M1 telemetry hook sites for Epic 5 Slice 3 encounter funnel events: **`encounter_start`**, **`encounter_complete`**, **`reward_attribution`**, plus retained **`encounter_complete_denied`**. Hooks follow the **NEO-64** / **NEO-84** pattern (`TODO(E9.M1)`, planned payload fields, no ingest or `ILogger`). Duplicate **`encounter_complete`** stub in **`EncounterCombatWiring`** is removed in favor of a delegate pointer to **`EncounterCompletionOperations.TryCompleteAndGrant`**; NEO-84 reserved **`encounter_start`** in **`CombatOperations`** now points at **`EncounterProgressOperations`**. Docs (implementation plan, backlog E5M3-10, E5.M3 module snapshot, alignment register, module register, `server/README.md`) are updated consistently. No tests changed — appropriate for comments-only work. Godot client correctly remains **NEO-110** / **NEO-111**. Overall risk is negligible.
|
||||
|
||||
## Documentation checked
|
||||
|
||||
| Path | Result |
|
||||
|------|--------|
|
||||
| `docs/plans/NEO-109-implementation-plan.md` | **Matches** — kickoff decisions adopted; acceptance checklist checked; reconciliation section accurate. |
|
||||
| `docs/plans/E5M3-prototype-backlog.md` (E5M3-10) | **Matches** — acceptance criteria checked; landed note cites README + plan. |
|
||||
| `docs/decomposition/modules/E5_M3_EncounterAndRewardTables.md` | **Matches** — NEO-109 telemetry hooks snapshot bullet added. |
|
||||
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E5.M3 row notes **NEO-109**; outstanding work updated to **E5M3-11+ client HUD**. |
|
||||
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E5.M3 note cites **NEO-109** landed; **E5M3-11+** outstanding. |
|
||||
| `docs/decomposition/epics/epic_05_pve_combat.md` (Slice 3) | **Matches** — Slice 3 vocabulary **`encounter_complete`** + reward attribution reflected; **`encounter_start`** is Slice 1 reserved hook now anchored per NEO-84 → NEO-109 (plan documents this). |
|
||||
| Full-stack epic decomposition | **N/A** — plan explicitly states no client counterpart; server-only comments. |
|
||||
| `server/README.md` | **Matches** — new **Encounter telemetry hooks (NEO-109)** section; NEO-106 table and NEO-84 combat hooks cross-link correctly. |
|
||||
|
||||
## Blocking issues
|
||||
|
||||
None.
|
||||
|
||||
## Suggestions
|
||||
|
||||
1. ~~**`encounter_start` hook anchor vs emit condition** — The comment block sits **before** `progressStore.TryActivate`, but the TODO correctly says emit **when `TryActivate` returns true**. On re-engagement after activation, execution still passes the comment then `TryActivate` returns `false`. For E9.M1 wiring clarity, consider moving the block to immediately **after** a successful `TryActivate` (or wrapping with `if (progressStore.TryActivate(...)) { /* hook */ return true; }`) so the anchor matches the once-per-activation contract. Comments-only today; non-blocking.~~ **Done.** Hook block moved after successful **`TryActivate`**; method returns **`true`** on first activation (same outward behavior).
|
||||
|
||||
2. ~~**`encounter_complete_denied` payload scope** — The **`Deny`** hook comment lists `playerId`, `encounterId`, `reasonCode`, but **`Deny(string reasonCode)`** has no player/encounter context. Pre-existing from NEO-105; E9.M1 will need call-site context or a signature change. Worth a one-line note in the comment (“wire at caller with playerId + encounterId”) to avoid implementer confusion.~~ **Done.** Comment notes wire at each **`TryCompleteAndGrant`** **`Deny()`** call site.
|
||||
|
||||
## Nits
|
||||
|
||||
- ~~Nit: Plan lists optional **`requiredNpcInstanceIds` count** in **`encounter_start`** payload; the comment block omits it. Fine for prototype; add when E9.M1 shapes the catalog schema.~~ **Done.** Payload comment includes **`requiredNpcInstanceIdsCount`**.
|
||||
|
||||
- ~~Nit: **`EncounterCombatWiring`** removed `return;` after the deny log block — behavior unchanged (method ends immediately after the delegate comment). Harmless; no action needed.~~ **Done (Bugbot PR #148).** Restored early **`return;`** after failure log to match guard-clause pattern in **`TryProcessCastOutcome`**.
|
||||
|
||||
- Nit: **`encounter_complete_denied`** is not in the Epic 5 Slice 3 telemetry list — plan documents the **NEO-56** deny precedent; acceptable.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
# Grep hook consolidation (manual plan check)
|
||||
rg 'NEO-109|encounter_complete|reward_attribution|encounter_start' \
|
||||
server/NeonSprawl.Server/Game/Encounters/
|
||||
# Confirm EncounterCombatWiring has delegate pointer only, no encounter_complete emit block
|
||||
|
||||
# Regression (encounters module)
|
||||
cd server && dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \
|
||||
--filter "FullyQualifiedName~Encounters"
|
||||
```
|
||||
|
||||
**Local run:** 81/82 Encounters-filter tests passed; 1 failure in `RewardTableDefinitionRegistryTests.Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds` (host DI at `Program.cs:39`) — appears **environment/pre-existing**, unrelated to NEO-109 comment changes. Re-run full `dotnet test NeonSprawl.sln` on CI before merge.
|
||||
|
|
@ -8,7 +8,7 @@ namespace NeonSprawl.Server.Game.Combat;
|
|||
public static class CombatOperations
|
||||
{
|
||||
// --- Reserved telemetry hook sites (NEO-84): no emit until future modules ---
|
||||
// encounter_start — future E5.M3 encounter spawn/engage path (not wired in Slice 1).
|
||||
// encounter_start — E5.M3 anchor: EncounterProgressOperations.TryActivateOnFirstEngagement (NEO-109).
|
||||
// player_death — future player HP / death resolution (player HP out of scope in Slice 1).
|
||||
/// <summary>
|
||||
/// Resolves one ability against one prototype combat target using catalog damage and the health store.
|
||||
|
|
|
|||
|
|
@ -91,7 +91,6 @@ public static class EncounterCombatWiring
|
|||
return;
|
||||
}
|
||||
|
||||
// --- Telemetry hook site (NEO-109): future E9.M1 catalog event `encounter_complete` ---
|
||||
// TODO(E9.M1): catalog emit — once per player+encounter when grants commit (NEO-106 cast path).
|
||||
// encounter_complete / reward_attribution hooks: EncounterCompletionOperations.TryCompleteAndGrant (NEO-109).
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ namespace NeonSprawl.Server.Game.Encounters;
|
|||
/// Internal completion payload returned on successful encounter grant (NEO-105).
|
||||
/// Persisted via <see cref="IEncounterCompleteEventStore"/> on success (NEO-107).
|
||||
/// E7.M2 quest-credit routing consumes this record (hook stub on NEO-107).
|
||||
/// NEO-109 telemetry: <c>encounter_complete</c> / <c>reward_attribution</c> hook sites in
|
||||
/// <see cref="EncounterCompletionOperations.TryCompleteAndGrant"/>.
|
||||
/// </summary>
|
||||
public readonly record struct EncounterCompleteEvent(
|
||||
string PlayerId,
|
||||
|
|
|
|||
|
|
@ -5,6 +5,8 @@ namespace NeonSprawl.Server.Game.Encounters;
|
|||
/// <summary>
|
||||
/// Applies encounter reward-table grants and marks completion idempotently (NEO-105).
|
||||
/// Combat defeat wiring: <see cref="AbilityInput.AbilityCastApi"/> (NEO-106).
|
||||
/// NEO-109 telemetry hook sites: <see cref="TryCompleteAndGrant"/> (<c>encounter_complete</c>, <c>reward_attribution</c>),
|
||||
/// <see cref="Deny"/> (<c>encounter_complete_denied</c>).
|
||||
/// </summary>
|
||||
public static class EncounterCompletionOperations
|
||||
{
|
||||
|
|
@ -122,7 +124,16 @@ public static class EncounterCompletionOperations
|
|||
// TODO(E7.M2): reward_delivery — QuestRewardBundle router consumes EncounterCompleteEvent
|
||||
// (playerId, encounterId, idempotencyKey, grantedItemSummary) → idempotent RewardDeliveryEvent.
|
||||
// Prototype quest credit: contract_handoff_token item loot + this event record; full router: E7.M2.
|
||||
// TODO(E9.M1): catalog emit — encounter_complete on successful grant commit (NEO-109).
|
||||
|
||||
// --- Telemetry hook site (NEO-109): future E9.M1 catalog event `encounter_complete` ---
|
||||
// TODO(E9.M1): catalog emit — once per player+encounter on successful grant + completion mark commit.
|
||||
// Planned payload: playerId, encounterId, rewardTableId, completedAt, idempotencyKey, grantedItems summary
|
||||
// (EncounterCompleteEvent). No ingest or ILogger here (comments-only). EncounterCombatWiring delegates here.
|
||||
|
||||
// --- Telemetry hook site (NEO-109): future E9.M1 catalog event `reward_attribution` ---
|
||||
// TODO(E9.M1): catalog emit — once per successful completion (batch attribution, not per TryAddStack).
|
||||
// Planned payload: playerId, encounterId, rewardTableId, grantedItems[] (EncounterGrantApplied batch).
|
||||
// Inventory layer still emits item_created per stack via PlayerInventoryOperations (NEO-56).
|
||||
|
||||
return new EncounterCompletionResult(
|
||||
Success: true,
|
||||
|
|
@ -167,7 +178,8 @@ public static class EncounterCompletionOperations
|
|||
{
|
||||
// --- Telemetry hook site (NEO-109): future E9.M1 catalog event `encounter_complete_denied` ---
|
||||
// TODO(E9.M1): catalog emit — structured deny with reasonCode (EncounterCompletionReasonCodes).
|
||||
// Planned payload: playerId, encounterId, reasonCode. No ingest or ILogger here (comments-only).
|
||||
// Planned payload: playerId, encounterId, reasonCode — wire at each TryCompleteAndGrant Deny() call site
|
||||
// (this helper has reasonCode only; callers hold playerId + encounterId). No ingest or ILogger here.
|
||||
|
||||
return new EncounterCompletionResult(
|
||||
Success: false,
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ namespace NeonSprawl.Server.Game.Encounters;
|
|||
/// <summary>
|
||||
/// Server-internal encounter completion resolution envelope (NEO-105).
|
||||
/// Combat wiring promotes via NEO-106; HTTP DTOs via NEO-108.
|
||||
/// NEO-109 telemetry hook sites live in <see cref="EncounterCompletionOperations.TryCompleteAndGrant"/>.
|
||||
/// </summary>
|
||||
/// <param name="GrantsApplied">Item grants on success; empty when denied.</param>
|
||||
/// <param name="CompleteEvent">Completion payload on success; null when denied.</param>
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ namespace NeonSprawl.Server.Game.Encounters;
|
|||
/// <summary>
|
||||
/// Encounter activation and defeat-all progress orchestration (NEO-104).
|
||||
/// Combat cast wiring: <see cref="AbilityInput.AbilityCastApi"/> (NEO-106).
|
||||
/// NEO-109 telemetry hook site: <see cref="TryActivateOnFirstEngagement"/> (<c>encounter_start</c>).
|
||||
/// </summary>
|
||||
public static class EncounterProgressOperations
|
||||
{
|
||||
|
|
@ -62,9 +63,18 @@ public static class EncounterProgressOperations
|
|||
return false;
|
||||
}
|
||||
|
||||
if (!progressStore.TryActivate(playerId, encounterId))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
// --- Telemetry hook site (NEO-109): future E9.M1 catalog event `encounter_start` ---
|
||||
// TODO(E9.M1): catalog emit — on first activation for player+encounter (NEO-106 wires cast path).
|
||||
return progressStore.TryActivate(playerId, encounterId);
|
||||
// TODO(E9.M1): catalog emit — once per player+encounter on first successful TryActivate (anchor matches emit).
|
||||
// Planned payload: playerId, encounterId, npcInstanceId (engagement target),
|
||||
// requiredNpcInstanceIdsCount (catalog requiredNpcInstanceIds for encounterId). No ingest or ILogger here
|
||||
// (comments-only). EncounterCombatWiring calls this on first damaging hit vs a required NPC (NEO-106).
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
|
|
|||
|
|
@ -206,13 +206,26 @@ Prototype: defeat all three E5.M2 NPCs via cast → row **`prototype_combat_pock
|
|||
curl -sS -i "http://localhost:5253/game/players/dev-local-1/encounter-progress"
|
||||
```
|
||||
|
||||
## Encounter telemetry hooks (NEO-109)
|
||||
|
||||
Comment-only hook sites for Epic 5 **Slice 3** catalog events ([epic_05 Slice 3](../../docs/decomposition/epics/epic_05_pve_combat.md#epic-5-slice-3)). **`TODO(E9.M1)`** — no production ingest. **`POST …/ability-cast`** and **`EncounterCombatWiring`** delegate to the engines below (no duplicate API-layer hooks).
|
||||
|
||||
| Event | Anchor |
|
||||
|-------|--------|
|
||||
| **`encounter_start`** | **`EncounterProgressOperations.TryActivateOnFirstEngagement`** — hook anchor after successful **`TryActivate`** (first engagement only). Activates NEO-84 reserved **`encounter_start`** pointer in **`CombatOperations`**. |
|
||||
| **`encounter_complete`** | **`EncounterCompletionOperations.TryCompleteAndGrant`** — once per successful grant + completion mark commit. |
|
||||
| **`reward_attribution`** | Same success path — once per completion (batch **`grantedItems`**); per-stack **`item_created`** remains in **`PlayerInventoryOperations`** ([NEO-56](#player-inventory-neo-54-store-neo-55-http)). |
|
||||
| **`encounter_complete_denied`** | **`EncounterCompletionOperations.Deny`** — structured deny + **`EncounterCompletionReasonCodes`**. |
|
||||
|
||||
Plan: [NEO-109 implementation plan](../../docs/plans/NEO-109-implementation-plan.md).
|
||||
|
||||
## Encounter combat wiring (NEO-106)
|
||||
|
||||
On every successful **`POST …/ability-cast`** accept, **`EncounterCombatWiring.TryProcessCastOutcome`** (best-effort; does not change cast JSON):
|
||||
|
||||
| Cast outcome | Encounter effect |
|
||||
|--------------|------------------|
|
||||
| **`damageDealt > 0`** vs a required NPC | **`EncounterProgressOperations.TryActivateOnFirstEngagement`** — starts **`prototype_combat_pocket`** progress (`encounter_start` hook site in NEO-104). |
|
||||
| **`damageDealt > 0`** vs a required NPC | **`EncounterProgressOperations.TryActivateOnFirstEngagement`** — starts **`prototype_combat_pocket`** progress (**`encounter_start`** hook site, [NEO-109](#encounter-telemetry-hooks-neo-109)). |
|
||||
| **`targetDefeated: true`** | **`TryMarkTargetDefeated`**; when all three E5.M2 NPC ids defeated, **`EncounterCompletionOperations.TryCompleteAndGrant`**. |
|
||||
| Inventory full on completion | Defeat progress retained; completion/grants fail closed (retry when bag has space). |
|
||||
|
||||
|
|
@ -363,7 +376,7 @@ Comment-only hook sites in **`CombatOperations.TryResolve`** for future E9.M1 ca
|
|||
|
||||
- **`ability_used`** — on every successful resolve (zero-damage abilities included).
|
||||
- **`enemy_defeat`** — only when lethal damage drives **`after.Defeated`** to **`true`** (re-hit on defeated targets denies at **`target_defeated`** before damage).
|
||||
- **Reserved (comments only):** **`encounter_start`** (E5.M3 encounters).
|
||||
- **Reserved (comments only):** **`encounter_start`** — E5.M3 anchor: [Encounter telemetry hooks (NEO-109)](#encounter-telemetry-hooks-neo-109).
|
||||
- **`player_death`** — only when lethal NPC damage drives player **`currentHp`** to **0** (comment hook in NEO-95; full event deferred).
|
||||
|
||||
E1.M4 cast funnel events **`ability_cast_requested`** / **`ability_cast_denied`** remain in **`AbilityCastApi`** ([NEO-30](#ability-cast-neo-31-neo-82)). E9.M1 ingest should correlate route **`playerId`** with engine payload fields at accept. Plan: [NEO-84 implementation plan](../../docs/plans/NEO-84-implementation-plan.md).
|
||||
|
|
|
|||
Loading…
Reference in New Issue