From bba1415d54b482ea53bc921f8c465d43f2c794c1 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 17:14:44 -0400 Subject: [PATCH] NEO-69: Add craft engine implementation plan. Kickoff plan for CraftOperations/CraftResult with pre-flight output checks, refine XP grant, and unit test coverage before NEO-70 HTTP. --- docs/plans/NEO-69-implementation-plan.md | 138 +++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 docs/plans/NEO-69-implementation-plan.md diff --git a/docs/plans/NEO-69-implementation-plan.md b/docs/plans/NEO-69-implementation-plan.md new file mode 100644 index 0000000..a9e2a15 --- /dev/null +++ b/docs/plans/NEO-69-implementation-plan.md @@ -0,0 +1,138 @@ +# NEO-69 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-69 | +| **Title** | E3.M2: CraftResult engine (inputs, outputs, refine XP) | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-69/e3m2-craftresult-engine-inputs-outputs-refine-xp | +| **Module** | [E3.M2 — RefinementAndRecipeExecution](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) · Epic 3 Slice 3 · backlog **E3M2-05** | +| **Branch** | `NEO-69-craftresult-engine-inputs-outputs-refine-xp` | +| **Precursors** | [NEO-67](https://linear.app/neon-sprawl/issue/NEO-67) — `IRecipeDefinitionRegistry` + DI (**Done** on `main`); [NEO-54](https://linear.app/neon-sprawl/issue/NEO-54) — `PlayerInventoryOperations` (**Done**); [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42) — `RefineActivitySkillXpGrant` (**Done**) | +| **Pattern** | [NEO-62](https://linear.app/neon-sprawl/issue/NEO-62) — static `*Operations` orchestrator + subsystem reason codes; [NEO-64](https://linear.app/neon-sprawl/issue/NEO-64) — engine-only scope (HTTP in follow-up) | +| **Client counterpart** | [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74) — E3S5-03 craft UI + recipe list (blocked by [NEO-70](https://linear.app/neon-sprawl/issue/NEO-70) craft HTTP; server-only in this story) | +| **Blocks** | [NEO-70](https://linear.app/neon-sprawl/issue/NEO-70) — craft POST + Bruno spine; [NEO-71](https://linear.app/neon-sprawl/issue/NEO-71) — telemetry hook sites | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Output-full deny strategy** | Pre-flight output capacity vs consume inputs then rollback? | **Pre-flight** — simulate all scaled output adds on a cloned snapshot before any input removal; deny **`inventory_full`** with zero mutations (cleaner for multi-input recipes than compensating rollback). | **User:** pre-flight. | +| **Quantity in engine API** | Include `quantity` in `TryCraft` in NEO-69? | **Yes** — E3M2-05 lists **`invalid_quantity`**; [NEO-70](https://linear.app/neon-sprawl/issue/NEO-70) POST will pass `recipeId` + optional quantity default **1**; engine owns validation now. | **Adopted** — backlog + NEO-70 wiring. | +| **Craft vs inventory reason codes** | Surface `insufficient_quantity` or craft-layer `insufficient_materials`? | **Craft-layer `insufficient_materials`** — E3M2 backlog stable deny vocabulary; map from inventory checks at orchestration boundary (mirror gather passthrough for `inventory_full` only). | **Adopted** — backlog. | +| **XP failure after commit** | Roll back inventory on XP deny? | **Yes** — compensating remove of outputs + re-add inputs (mirror NEO-62 gather rollback on XP failure). | **Adopted** — NEO-62 precedent. | +| **Manual QA doc** | Add `docs/manual-qa/NEO-69.md`? | **No** — engine-only like NEO-62; automated tests + [NEO-70](https://linear.app/neon-sprawl/issue/NEO-70) manual QA cover player-visible verification. | **Adopted** — NEO-62 pattern. | + +## Goal, scope, and out-of-scope + +**Goal:** Single server operation resolving **`CraftResult`**: validate recipe + inventory, atomically remove inputs and add outputs via **`PlayerInventoryOperations`**, award **`refine`** XP via **`RefineActivitySkillXpGrant`** on success — engine-only, ready for craft HTTP (NEO-70). + +**In scope (from Linear + [E3M2-05](E3M2-prototype-backlog.md#e3m2-05--craftresult-engine-consume-inputs-grant-outputs-refine-xp)):** + +- **`CraftOperations`** / **`CraftResult`** types in `server/NeonSprawl.Server/Game/Crafting/`. +- Stable deny reason codes: **`unknown_recipe`**, **`insufficient_materials`**, **`inventory_full`**, **`invalid_quantity`** (+ store-missing craft-layer codes as needed). +- Pre-flight output capacity before input removal (kickoff decision). +- Compensating inventory rollback when XP grant fails after successful input/output mutations (NEO-62 pattern). +- Unit tests (AAA) via **`InMemoryWebApplicationFactory`** with real catalogs and stores; **`RefineActivitySkillXpGrant`** invoked on success path. +- Document **`CraftResult`** shape in module doc (server-internal until promoted to wire in NEO-70). + +**Out of scope (from Linear + backlog):** + +- Craft HTTP, Bruno, `docs/manual-qa/` ([NEO-70](https://linear.app/neon-sprawl/issue/NEO-70)). +- Client HUD ([NEO-74](https://linear.app/neon-sprawl/issue/NEO-74)). +- Skill level gates, station checks, craft failure RNG. +- Telemetry hook sites ([NEO-71](https://linear.app/neon-sprawl/issue/NEO-71)). + +## Acceptance criteria checklist + +- [ ] Successful craft removes configured inputs and adds outputs deterministically. +- [ ] Insufficient materials returns stable **`reasonCode`** with **no** inventory change. +- [ ] Full bag on output returns stable **`reasonCode`**; inputs are **not** consumed. +- [ ] **`refine`** skill XP increases by **10** on success (NEO-42). +- [ ] **`make_prototype_armor`** respects equipment slot rules from E3.M3 when bag is full but equip slot empty. + +## Technical approach + +1. **Entry point:** **`CraftOperations.TryCraft(...)`** — static orchestrator accepting `playerId`, `recipeId`, `quantity`, and dependencies: + - **`IRecipeDefinitionRegistry`** + - **`IItemDefinitionRegistry`** + **`IPlayerInventoryStore`** + - **`ISkillDefinitionRegistry`**, **`IPlayerSkillProgressionStore`**, **`ISkillLevelCurve`**, **`PerkUnlockEngine`** + +2. **Validate quantity:** `quantity <= 0` → **`Denied`** + **`CraftReasonCodes.InvalidQuantity`** (no writes). + +3. **Resolve recipe:** **`registry.TryGetDefinition(recipeId.Trim(), out def)`** — miss → **`unknown_recipe`**. + +4. **Pre-flight inputs (read-only):** For each input row, required amount = **`row.Quantity * quantity`**. Sum held quantity in the correct container (bag vs equipment via catalog **`inventorySlotKind`**) from current snapshot. Any shortfall → **`insufficient_materials`** without mutations. + +5. **Pre-flight outputs (read-only, kickoff decision):** Clone current snapshot; for each output row with scaled quantity, simulate **`TryAddStack`** placement rules (merge + fill empty slots, catalog **`stackMax`**, equipment routing) on the clone. If any output cannot be fully placed → **`inventory_full`** without mutations. Implementation: add **`PlayerInventoryOperations.CanPlaceStack(snapshot, itemId, quantity, registry)`** (or equivalent simulate helper) to avoid duplicating merge/fill logic. + +6. **Commit mutations (only after steps 4–5 pass):** + - **Remove inputs:** foreach input row, **`TryRemoveStack`** scaled quantity. (Should succeed given pre-flight; defensive deny **`insufficient_materials`** if not.) + - **Add outputs:** foreach output row, **`TryAddStack`** scaled quantity. (Should succeed given pre-flight; defensive deny + compensating re-add inputs if not.) + - **XP:** **`RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine`**. On failure (store missing / skill deny): compensating rollback — remove granted outputs, re-add removed inputs; return **`Denied`** + **`progression_store_missing`** or passthrough skill **`reasonCode`**. + +7. **Success envelope — `CraftResult`:** + - **`Success = true`**, **`ReasonCode = null`** + - **`InputsConsumed`**: list of `{ itemId, quantity }` applied (scaled). + - **`OutputsGranted`**: list of `{ itemId, quantity }` applied (scaled). + - **`XpGrantSummary`**: compact summary (`refine`, **10**, `activity`) for tests / future wire. + +8. **Reason codes (`CraftReasonCodes`):** + - **`unknown_recipe`**, **`insufficient_materials`**, **`invalid_quantity`** — craft-layer. + - **`inventory_full`** — passthrough literal (same string as NEO-54). + - **`progression_store_missing`**, **`inventory_store_missing`** — craft-layer store-missing (mirror gather). + +9. **Equipment slot AC (`make_prototype_armor`):** No special-case in craft engine — **`prototype_armor_shell`** routes to equipment via catalog metadata in pre-flight + **`TryAddStack`**. Test: fill all **24** bag slots, leave equipment empty, craft succeeds and places armor in **`equipmentSlots[0]`**. + +10. **Primary demo recipe for happy path tests:** **`refine_scrap_standard`** (5× **`scrap_metal_bulk`** → 1× **`refined_plate_stock`**) and/or **`make_field_stim_mk0`**. + +11. **Docs (on land):** Update [E3_M2](E3_M2_RefinementAndRecipeExecution.md) **Implementation snapshot** + **`CraftResult`** contract note; [documentation_and_implementation_alignment.md](documentation_and_implementation_alignment.md) E3.M2 row; E3M2-05 checkboxes in [E3M2-prototype-backlog.md](E3M2-prototype-backlog.md); brief **`server/README.md`** craft-engine subsection (engine-only; HTTP in NEO-70). Cross-link **NEO-74** / full-stack note per [full-stack epic decomposition](../../.cursor/rules/full-stack-epic-decomposition.md). + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Crafting/CraftReasonCodes.cs` | Stable craft deny codes (craft-layer + passthrough). | +| `server/NeonSprawl.Server/Game/Crafting/CraftIoApplied.cs` | One consumed input or granted output row in success envelope. | +| `server/NeonSprawl.Server/Game/Crafting/CraftXpGrantSummary.cs` | Compact refine XP summary on successful craft. | +| `server/NeonSprawl.Server/Game/Crafting/CraftResult.cs` | Success/deny envelope: inputs consumed, outputs granted, reasonCode, xp summary. | +| `server/NeonSprawl.Server/Game/Crafting/CraftOperations.cs` | Orchestrator: validate → pre-flight → remove inputs → add outputs → XP; rollback on XP failure. | +| `server/NeonSprawl.Server.Tests/Game/Crafting/CraftOperationsTests.cs` | AAA: success, insufficient_materials, inventory_full (bag), make_prototype_armor + full bag, unknown_recipe, invalid_quantity, XP grant + rollback path. | +| `docs/plans/NEO-69-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs` | Add snapshot-based **`CanPlaceStack`** (or simulate helper) for craft output pre-flight without mutation. | +| `server/README.md` | Document craft engine (`CraftOperations`, reason codes, refine XP hook); note HTTP deferred to NEO-70. | +| `docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md` | **Implementation snapshot** — craft engine + **`CraftResult`** field table. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M2 row — note NEO-69 craft engine when landed. | +| `docs/plans/E3M2-prototype-backlog.md` | E3M2-05 acceptance checkboxes + landed note when complete. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `server/NeonSprawl.Server.Tests/Game/Crafting/CraftOperationsTests.cs` | **Success:** seed **`scrap_metal_bulk`**, craft **`refine_scrap_standard`**, assert inputs removed, **`refined_plate_stock`** granted, **`refine`** XP +10, envelope fields. **Insufficient materials:** empty inventory → **`insufficient_materials`**, snapshot unchanged. **Inventory full:** fill bag so output cannot fit → **`inventory_full`**, inputs unchanged. **Equipment routing:** full bag + empty equip slot → **`make_prototype_armor`** succeeds, armor in equipment slot. **Unknown recipe** → **`unknown_recipe`**. **Invalid quantity** (0 or negative) → **`invalid_quantity`**. **XP path:** assert **`RefineActivitySkillXpGrant`** effect via progression store (mirror **`GatherOperationsTests`** / **`RefineActivitySkillXpGrantTests`**). **AAA** per [csharp-style](../../.cursor/rules/csharp-style.md). | +| `server/NeonSprawl.Server.Tests/Game/Items/PlayerInventoryOperationsTests.cs` | **Optional:** one test for new **`CanPlaceStack`** helper if added as public surface (bag full vs equipment route). | + +No Bruno or manual QA in this story — [NEO-70](https://linear.app/neon-sprawl/issue/NEO-70) owns player-visible HTTP verification. + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Pre-flight simulate vs rollback** | User chose **pre-flight** at kickoff. | **adopted** | +| **Multi-output pre-flight order** | Simulate adds in recipe **`outputs`** order on one cloned snapshot (order should not matter for prototype fixed outputs; document if later recipes introduce interactions). | **deferred** | +| **NEO-71 telemetry hooks** | Do not add `item_crafted` / `craft_failed` comments in NEO-69 — NEO-71 owns hook placement. | **adopted** | +| **Client not in this story** | Cross-link **NEO-74**; Bruno-only verification is not prototype-complete. | **adopted** | + +## Decisions (kickoff) + +- **Pre-flight output capacity** before any input removal (user confirmed). +- **`quantity` parameter** on **`TryCraft`** with **`invalid_quantity`** deny in this story. +- **Craft-layer `insufficient_materials`** at orchestration boundary (not raw **`insufficient_quantity`** on craft deny). +- **XP failure rollback** — remove outputs, restore inputs (NEO-62 gather precedent). +- **No `docs/manual-qa/NEO-69.md`** — engine-only; NEO-70 manual QA covers HTTP.