don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity
neon-sprawl/docs/plans/NEO-71-implementation-plan.md

116 lines
9.1 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-71 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-71 |
| **Title** | E3.M2: item_crafted / craft_failed telemetry hook sites |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-71/e3m2-item-crafted-craft-failed-telemetry-hook-sites |
| **Module** | [E3.M2 — RefinementAndRecipeExecution](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) · Epic 3 Slice 3 · backlog **E3M2-07** |
| **Branch** | `NEO-71-item-crafted-craft-failed-telemetry-hooks` |
| **Blocked by** | [NEO-69](https://linear.app/neon-sprawl/issue/NEO-69) — `CraftOperations.TryCraft` + `CraftResult` (**Done** on `main`); [NEO-70](https://linear.app/neon-sprawl/issue/NEO-70) — craft HTTP + Bruno spine (**Done** on `main`; may parallel). |
| **Pattern** | [NEO-64](https://linear.app/neon-sprawl/issue/NEO-64) — gather telemetry hooks in engine; [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56) — centralized **`Deny`** hook for structured denies |
| **Client counterpart** | None — comment-only server engine story; craft UI is [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74) (Slice 5). |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Manual QA doc** | Add `docs/manual-qa/NEO-71.md`? User asked whether anything is testable in the UI. | **No** — no Godot/UI surface in this story (comment-only server hooks; craft UI is NEO-74). Verify via code review + `dotnet test` + optional Bruno craft regression (NEO-70 waiver for E3.M2 server-only). | **User:** No UI to test; **no manual QA doc** (NEO-70 pattern). |
| **Runtime behavior** | Comments-only vs dev `ILogger`? | **Comments-only** — `TODO(E9.M1)`; no production logging (NEO-49/56/64 precedent). | **Adopted** (repo precedent). |
| **Hook anchor** | Engine only vs engine + `PlayerCraftApi`? | **Engine only** in **`CraftOperations.TryCraft`** — HTTP delegates to engine; no duplicate API-layer hooks (NEO-56/64 precedent; Linear out-of-scope). | **Adopted** (repo precedent). |
| **`craft_failed` placement** | Central **`Deny`** vs inline at each early return? | **Central `Deny`** — one hook site covers all **`CraftReasonCodes`** (NEO-56 **`PlayerInventoryOperations.Deny`** pattern). | **Adopted** (repo precedent). |
| **`craft_failed` scope** | All structured denies vs pre-commit rule denies only? | **All** **`Deny`** outcomes including store-missing and post-mutation rollback paths — stable **`reasonCode`** on every craft failure (NEO-56 all-deny precedent). | **Adopted** (repo precedent). |
| **`time-to-first-craft`** | Separate hook in prototype? | **No separate hook** — derived metric from **`item_crafted`** + session context at E9.M1 (E3M2-07 backlog note). | **Adopted** (backlog default). |
## Goal, scope, and out-of-scope
**Goal:** Document and place **comment-only hook sites** on the server-authoritative craft path for future E9.M1 catalog events **`item_crafted`** and **`craft_failed`**. Align with Epic 3 Slice 3 telemetry vocabulary ([epic_03 — Slice 3](../decomposition/epics/epic_03_crafting_economy.md#epic-3-slice-3)).
**In scope (from Linear + [E3M2-07](E3M2-prototype-backlog.md#e3m2-07--item_crafted--craft_failed-telemetry-hook-sites)):**
- Hook placement in **`CraftOperations.TryCraft`** (success + structured deny via **`Deny`**).
- **`TODO(E9.M1)`** comments; no production logging.
- **`server/README.md`** craft telemetry subsection; [E3_M2 module](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) hook note.
- E3M2 backlog + alignment register updates when implementation completes.
**Out of scope (from Linear):**
- Telemetry ingest, dashboards (**E9.M1**).
- `ILogger` / metrics / dev-only log lines.
- Duplicate hook comments in **`PlayerCraftApi`** (engine-only precedent).
- Godot craft UI — **[NEO-74](https://linear.app/neon-sprawl/issue/NEO-74)**.
- **`docs/manual-qa/NEO-71.md`** (kickoff decision — no player-visible or UI verification).
## Acceptance criteria checklist
- [x] Hook sites documented in code and `server/README.md`.
- [x] Matches Epic 3 Slice 3 telemetry vocabulary (`item_crafted`, `craft_failed` with reason; **`time-to-first-craft`** noted as derived, no separate hook).
## Technical approach
1. **Authoritative surface:** **`CraftOperations.TryCraft`** — all craft side effects (recipe resolve → inventory pre-flight → input removal → output grant → refine XP) funnel here; **`PlayerCraftApi`** delegates without duplicating hooks.
2. **Hook site A — `item_crafted`:** Immediately before the successful **`CraftResult`** return (after refine XP grant succeeds):
- Comment block names future E9.M1 event **`item_crafted`**.
- **`TODO(E9.M1): catalog emit`** (NEO-64 / NEO-56 style).
- Planned payload fields: `playerId`, `recipeId`, `quantity` (batch), `inputsConsumed`, `outputsGranted`, `xpGrantSummary` (`skillId`, `amount`, `sourceKind`).
3. **Hook site B — `craft_failed`:** In private **`Deny(string reasonCode)`** (centralized, NEO-56 pattern):
- Comment block names future E9.M1 event **`craft_failed`**.
- **`TODO(E9.M1): catalog emit`** — on every structured craft deny with stable **`reasonCode`** from **`CraftReasonCodes`**.
- Planned payload fields: `playerId`, `recipeId`, `quantity`, `reasonCode`; note **`playerId` / `recipeId` / `quantity`** are in enclosing **`TryCraft`** scope (same pattern as NEO-56 **`Deny`** noting fields at call sites).
4. **Cross-references (minimal):**
- Update **`CraftOperations`** class summary → NEO-71 telemetry hook anchor.
- **`CraftResult.cs`** summary already points to **`TryCraft`** — confirm wording after hooks land.
- **`server/README.md`:** new **NEO-71 craft telemetry hooks** subsection under craft engine (replace “Telemetry hook sites are **NEO-71**” deferral with landed doc).
- **`E3_M2_RefinementAndRecipeExecution.md`:** NEO-71 landed line; remove “Telemetry hooks deferred to NEO-71”.
- **`documentation_and_implementation_alignment.md`** + **`module_dependency_register.md`:** E3.M2 row — E3M2-07 complete; module **Ready** when all Slice 3 backlog lands.
- **`E3M2-prototype-backlog.md`:** E3M2-07 checkboxes + landed note.
5. **NEO-56 interaction:** Successful craft output grants already trigger **`item_created`** inside **`PlayerInventoryOperations.TryAddStack`**; **`item_crafted`** is the craft-funnel catalog event at the orchestration layer (analogous to **`resource_gathered`** + **`item_created`** on gather).
6. **`time-to-first-craft`:** Document in README that E9.M1 derives this from first **`item_crafted`** per player/session — no prototype hook.
## Files to add
| Path | Purpose |
|------|---------|
| `docs/plans/NEO-71-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Crafting/CraftOperations.cs` | **NEO-71:** comment-only **`item_crafted`** hook on success path + **`craft_failed`** hook in **`Deny`**. |
| `server/NeonSprawl.Server/Game/Crafting/CraftResult.cs` | Confirm type summary cross-references NEO-71 hook anchor (wording tweak if needed). |
| `server/README.md` | Document craft telemetry hook placement (NEO-71); remove deferral note. |
| `docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md` | Implementation snapshot: NEO-71 telemetry hooks landed; Slice 3 telemetry vocabulary. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M2 row: NEO-71 hook sites. |
| `docs/decomposition/modules/module_dependency_register.md` | E3.M2 note — E3M2-07 complete; module status **Ready**. |
| `docs/plans/E3M2-prototype-backlog.md` | E3M2-07 checkboxes + landed note; Slice 3 server spine complete. |
## Tests
| Test file | What it covers |
|-----------|----------------|
| *(none added or changed)* | Comments-only change; no behavior or wire contract change. Regression: `dotnet test NeonSprawl.sln` (existing **`CraftOperationsTests`** + **`PlayerCraftApiTests`** unchanged). Optional Bruno: `bruno/neon-sprawl-server/craft/` spine against running dev server (NEO-70; behavior unchanged). |
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Overlap with NEO-56 `item_created`** | Keep both — **`item_created`** at inventory layer; **`item_crafted`** at craft orchestration (E9.M1 can correlate). | **adopted** |
| **No UI / manual QA doc** | User confirmed no Godot verification; README + code review + existing test suite sufficient (NEO-70 E3.M2 server-only pattern). | **adopted** (kickoff) |
| **Slice 3 module closure** | Landing NEO-71 completes E3.M2 prototype **server** backlog; client loop remains NEO-72–NEO-75 (Slice 5). | **adopted** |
## Decisions (kickoff)
- **No `docs/manual-qa/NEO-71.md`** — no UI surface; server comment-only (user confirmed via UI question).
- **Engine-only hooks** in **`CraftOperations`**; no **`PlayerCraftApi`** duplicates.
- **Central `Deny` for `craft_failed`** — all **`CraftReasonCodes`** paths.
- **Comments + `TODO(E9.M1)` only** — no ingest or logging.
- **`time-to-first-craft`** — derived at E9.M1; no separate prototype hook.
Reference in New Issue View Git Blame Copy Permalink