92 lines
6.1 KiB
Markdown
92 lines
6.1 KiB
Markdown
# NEO-49 — Implementation plan
|
|
|
|
## Story reference
|
|
|
|
| Field | Value |
|
|
|--------|--------|
|
|
| **Key** | NEO-49 |
|
|
| **Title** | E2.M3: perk_unlock telemetry hook sites |
|
|
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-49/e2m3-perk-unlock-telemetry-hook-sites |
|
|
| **Module** | [E2.M3 — MasteryAndPerkUnlocks](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) · Epic 2 Slice 4 |
|
|
| **Branch** | `NEO-49-perk-unlock-telemetry-hook-sites` |
|
|
| **Blocked by** | [NEO-47](https://linear.app/neon-sprawl/issue/NEO-47) — **Done** on `main` (`PerkUnlockEngine`, `PerkUnlockEvent`). May land in parallel with [NEO-48](https://linear.app/neon-sprawl/issue/NEO-48) (perk HTTP). |
|
|
|
|
## Kickoff clarifications
|
|
|
|
| Topic | Question / note | Resolution |
|
|
|--------|-----------------|------------|
|
|
| **Hook runtime behavior** | Comments-only vs dev `ILogger` (same as NEO-40)? | **User:** **comments-only** — no runtime emit, no production logging. |
|
|
| **Hook anchor** | Single site in `TryUnlockPerks` vs duplicate markers at callers? | User asked for recommendation → **Agent recommendation (adopted):** **single anchor** in `PerkUnlockEngine.TryUnlockPerks` immediately after `PerkUnlockEvent` instances are built (covers branch-pick unlocks, path-auto on pick, and level-up reevaluation). **Do not** duplicate at `SkillProgressionGrantOperations` or future NEO-48 HTTP — those call the engine and already receive `Events`; E9.M1 wiring can subscribe at the engine or via a future `IPerkUnlockEventSink` (NEO-47 plan note). |
|
|
| **Branch-select without unlock** | Hook on successful `TrySelectBranch` when `Events` is empty? | **No** — catalog event is **`perk_unlock`** only when a perk id is newly persisted; tier-1 pick-without-perk is not an unlock event. |
|
|
| **Deny paths** | `perk_branch_select_denied` hooks? | **Out of scope** (Linear); only **`perk_unlock`**. |
|
|
|
|
## Goal, scope, and out-of-scope
|
|
|
|
**Goal:** Document and place **comment-only hook sites** on the server-authoritative perk unlock path for future E9.M1 catalog event **`perk_unlock`**. Align with Epic 2 Slice 4 telemetry vocabulary ([epic_02 — Slice 4](../decomposition/epics/epic_02_skills_and_progression.md)).
|
|
|
|
**In scope (from Linear + E2M3-05):**
|
|
|
|
- Hook location(s) in unlock / branch-select path documented in **code comments** with **`TODO(E9.M1)`**.
|
|
- **`server/README.md`** + [E2_M3 module](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) note hook placement.
|
|
- Manual QA checklist per repo convention.
|
|
|
|
**Out of scope (from Linear):**
|
|
|
|
- E9.M1 catalog, ingest, dashboards.
|
|
- `ILogger` / metrics / dev-only log lines (rejected at kickoff).
|
|
- Optional `IPerkUnlockEventSink` implementation (deferred; comments-only this story).
|
|
|
|
## Acceptance criteria checklist
|
|
|
|
- [ ] Hook location(s) in unlock / branch-select path documented in code comments.
|
|
- [ ] `TODO(E9.M1)` alignment; no production logging calls.
|
|
- [ ] Module doc + server README note hook placement.
|
|
|
|
## Technical approach
|
|
|
|
1. **Authoritative unlock surface:** `PerkUnlockEngine.TryUnlockPerks` — the only place new perk ids are persisted and `PerkUnlockEvent` records are created. All paths funnel here:
|
|
- **`TrySelectBranch`** → explicit pick tier perks + `ApplyPathAutoTiersAtOrBelowLevel` (may emit zero events on tier-1-only pick).
|
|
- **`ReevaluateAfterLevelUp`** → path-auto tiers after skill level-up (called from `SkillProgressionGrantOperations` and any future callers).
|
|
|
|
2. **Hook site — `perk_unlock`:** After building the `events` list (one `PerkUnlockEvent` per newly unlocked `perkId`), add a **comment block** before `return true`:
|
|
- Name future E9.M1 event **`perk_unlock`**.
|
|
- **`TODO(E9.M1): catalog emit`** (match NEO-30 / NEO-40 style).
|
|
- Document payload fields from `PerkUnlockEvent`: `playerId`, `perkId`, `skillId`, `tierIndex`, `branchId`, `source` (`BranchPick` | `LevelUp`).
|
|
- Note: emit **once per unlocked perk** in the batch (idempotent reevaluation produces no events when already unlocked).
|
|
|
|
3. **Cross-references (minimal):**
|
|
- One-line pointer in `PerkUnlockEvent.cs` xml summary → hook site in `TryUnlockPerks`.
|
|
- **`server/README.md`:** new short subsection under mastery (after NEO-46 block) for perk unlock engine + **NEO-49 telemetry hook** anchor.
|
|
- **`E2_M3_MasteryAndPerkUnlocks.md`:** replace “telemetry in NEO-49” backlog wording with **NEO-49 landed** line when implementation completes.
|
|
- **`documentation_and_implementation_alignment.md`:** E2.M3 row cites NEO-49 hook sites.
|
|
|
|
4. **NEO-48 readiness:** When perk HTTP lands, it will call `TrySelectBranch` and surface `Events`; no extra hook comments required at the API layer if the engine anchor exists.
|
|
|
|
## Files to add
|
|
|
|
| Path | Purpose |
|
|
|------|---------|
|
|
| `docs/manual-qa/NEO-49.md` | Manual QA: verify comment blocks + no behavior/API change. |
|
|
|
|
## Files to modify
|
|
|
|
| Path | Rationale |
|
|
|------|-----------|
|
|
| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs` | **NEO-49:** comment-only `perk_unlock` hook site in `TryUnlockPerks` when events are created. |
|
|
| `server/NeonSprawl.Server/Game/Mastery/PerkUnlockEvent.cs` | Cross-reference hook anchor in type summary (no behavior change). |
|
|
| `server/README.md` | Document perk unlock telemetry hook placement (NEO-49). |
|
|
| `docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md` | Implementation snapshot: NEO-49 telemetry hooks landed. |
|
|
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E2.M3 row: NEO-49 hook sites (still no E9.M1 ingest). |
|
|
|
|
## Tests
|
|
|
|
| Path | Change |
|
|
|------|--------|
|
|
| **None required** | Comments-only; existing `PerkUnlockEngineTests` and grant integration tests already cover unlock paths. |
|
|
|
|
**Manual verification:** `docs/manual-qa/NEO-49.md` — open `PerkUnlockEngine.cs`, confirm hook cites **`perk_unlock`**, **`TODO(E9.M1)`**, payload fields, and no new log statements; run `dotnet test` on server solution for regression sanity.
|
|
|
|
## Open questions / risks
|
|
|
|
**None.** NEO-47 blocker is complete; comments-only avoids production logging risk. If E9.M1 later needs caller context (HTTP request id), prefer `IPerkUnlockEventSink` at call sites without moving the primary hook documentation away from `TryUnlockPerks`.
|