From 7188c5b55888fd9c04ce1477cf694a2764aa686e Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 16:56:06 -0400 Subject: [PATCH 1/4] NEO-67: add implementation plan for recipe definition registry --- docs/plans/NEO-67-implementation-plan.md | 104 +++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 docs/plans/NEO-67-implementation-plan.md diff --git a/docs/plans/NEO-67-implementation-plan.md b/docs/plans/NEO-67-implementation-plan.md new file mode 100644 index 0000000..c0a139f --- /dev/null +++ b/docs/plans/NEO-67-implementation-plan.md @@ -0,0 +1,104 @@ +# NEO-67 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-67 | +| **Title** | E3.M2: Recipe definition registry + DI | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-67/e3m2-recipe-definition-registry-di | +| **Module** | [E3.M2 — RefinementAndRecipeExecution](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) · Epic 3 Slice 3 · backlog **E3M2-03** | +| **Branch** | `NEO-67-recipe-definition-registry-di` | +| **Precursor** | [NEO-66](https://linear.app/neon-sprawl/issue/NEO-66) — fail-fast `RecipeDefinitionCatalog` load (**Done** on `main`) | +| **Pattern** | [NEO-52](https://linear.app/neon-sprawl/issue/NEO-52) / [NEO-59](https://linear.app/neon-sprawl/issue/NEO-59) — thin registry adapter over startup catalog + DI; strict split after NEO-66 loader | +| **Blocks** | [NEO-68](https://linear.app/neon-sprawl/issue/NEO-68) — GET world recipe-definitions; [NEO-69](https://linear.app/neon-sprawl/issue/NEO-69) — CraftResult engine | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Enumeration API** | Include `GetDefinitionsInIdOrder()` now? | **Yes** — mirror `IItemDefinitionRegistry` / `ISkillDefinitionRegistry`; [NEO-68](https://linear.app/neon-sprawl/issue/NEO-68) `GET /game/world/recipe-definitions` will need ordered defs without reaching into `RecipeDefinitionCatalog`. | **User:** include enumeration. | +| **Lookup naming** | `TryGetDefinition` vs `TryGetRecipe`? | **`TryGetDefinition(string? recipeId, …)`** — same naming as item/skill registries; catalog keeps `TryGetRecipe` for direct catalog access. | **User:** `TryGetDefinition`. | +| **NEO-66 vs NEO-67 split** | Combine registry with catalog load? | **Strict split** (decided on NEO-66 kickoff) — NEO-66 ships loader + catalog; this story adds injectable registry only. | **Adopted** — NEO-66 plan. | +| **Program.cs eager resolve** | Eager-resolve `IRecipeDefinitionRegistry` at boot? | **Omit** — `RecipeDefinitionCatalog` is already eager-resolved in `Program.cs` (NEO-66); registry is a thin adapter (NEO-52 default). | **Adopted** | +| **I/O + `recipeKind` exposure** | Separate TryGet methods for inputs/outputs? | **No** — embedded on `RecipeDefRow` returned by `TryGetDefinition`; backlog tests cover metadata + `recipeKind` via one lookup (unlike node+yield on NEO-59). | **Adopted** | +| **Runtime validation** | Re-validate recipes in registry? | **No** — catalog load is fail-fast (NEO-66); registry delegates to loaded rows only. | **Adopted** | + +## Goal, scope, and out-of-scope + +**Goal:** Provide **`IRecipeDefinitionRegistry`** backed by the startup-loaded **`RecipeDefinitionCatalog`**: resolve by stable **`recipeId`**, enumerate definitions in id order, expose prototype metadata (`displayName`, `recipeKind`, `requiredSkillId`, embedded `inputs` / `outputs`). Register in DI so **NEO-68+** (HTTP), **NEO-69** (craft engine), and future callers depend on the interface instead of the catalog type. + +**In scope (from Linear + [E3M2-03](E3M2-prototype-backlog.md#e3m2-03--recipe-definition-registry--di)):** + +- `RecipeDefinitionRegistry` thin adapter over `RecipeDefinitionCatalog`. +- DI registration alongside existing catalog singleton. +- Unit tests (AAA): known prototype id lookup + `recipeKind` + I/O metadata; unknown `recipeId` returns false without throwing; enumeration order; host resolves registry from DI. + +**Out of scope (from Linear + backlog):** + +- HTTP ([NEO-68](https://linear.app/neon-sprawl/issue/NEO-68)). +- Persistence, per-player craft state, inventory mutation ([NEO-69+](E3M2-prototype-backlog.md)). +- Changing loader, Slice 3 gate, or catalog load semantics (NEO-66). + +## Acceptance criteria checklist + +- [ ] DI resolves `IRecipeDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`). +- [ ] Unit tests (AAA): lookup for known prototype `recipeId` (e.g. `refine_scrap_standard`, `make_field_stim_mk0`) with expected `recipeKind`, `requiredSkillId`, and input/output rows. +- [ ] Unit tests (AAA): unknown `recipeId` returns false / absent without throwing. +- [ ] `GetDefinitionsInIdOrder` returns all eight loaded rows ordered by `id` (ordinal). + +## Technical approach + +1. **`IRecipeDefinitionRegistry`** — mirror [`IItemDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Items/IItemDefinitionRegistry.cs): + - `TryGetDefinition(string? recipeId, [NotNullWhen(true)] out RecipeDefRow? definition)` — null and unknown ids return false without throwing. + - `GetDefinitionsInIdOrder()` — all rows ordered by `RecipeDefRow.Id` (ordinal). + - XML remarks: craft engine / inventory callers (NEO-69+) should use this interface; HTTP read model (NEO-68) should not reach into `RecipeDefinitionCatalog`. + +2. **`RecipeDefinitionRegistry`** — primary-constructor adapter over `RecipeDefinitionCatalog` (same pattern as [`ItemDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs)). + +3. **DI** — extend [`AddRecipeDefinitionCatalog`](../../server/NeonSprawl.Server/Game/Crafting/RecipeCatalogServiceCollectionExtensions.cs) to register `IRecipeDefinitionRegistry` → `RecipeDefinitionRegistry` after catalog registration. No change to `Program.cs` eager-resolve (catalog only). + +4. **Comments** — update `RecipeDefinitionCatalog` summary to point at `IRecipeDefinitionRegistry`; update [server README](../../server/README.md) recipe-catalog section (currently says “until NEO-67”). + +5. **Tests** — new `RecipeDefinitionRegistryTests.cs` mirroring [`ItemDefinitionRegistryTests`](../../server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs): in-memory catalog helper, loader-backed prototype fixture via `RecipeCatalogTestPaths` + item/skill id stubs, host DI test. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Crafting/IRecipeDefinitionRegistry.cs` | Public contract: lookup, enumerate, remarks for NEO-68/69 callers. | +| `server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs` | Singleton adapter over `RecipeDefinitionCatalog`. | +| `server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs` | AAA unit + host DI tests (mirror `ItemDefinitionRegistryTests`). | +| `docs/plans/NEO-67-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Crafting/RecipeCatalogServiceCollectionExtensions.cs` | Register `IRecipeDefinitionRegistry` → `RecipeDefinitionRegistry` after catalog singleton. | +| `server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalog.cs` | Tighten summary comment to reference `IRecipeDefinitionRegistry` (replace “NEO-67 adds…” placeholder). | +| `server/README.md` | Recipe catalog section: document `IRecipeDefinitionRegistry` as preferred lookup surface. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M2 row — note NEO-67 registry when landed. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs` | **Unit:** `TryGetDefinition` for `refine_scrap_standard` returns true with `RecipeKind` `process`, `RequiredSkillId` `refine`, expected input/output `ItemId`/`Quantity`; `make_field_stim_mk0` returns `make` with multi-input rows; null and unknown id return false without throw; `GetDefinitionsInIdOrder` count (8) and ordinal order for prototype fixture; loader-backed prototype fixture matches catalog. **Host:** `InMemoryWebApplicationFactory` resolves `IRecipeDefinitionRegistry` and finds `refine_scrap_standard`. | + +**Manual QA:** Skipped — no player-visible HTTP; acceptance is fully covered by AAA unit/host tests (same as NEO-52 / NEO-59). + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Catalog vs registry for game code** | New callers inject `IRecipeDefinitionRegistry`; leave `RecipeDefinitionCatalog` eager-resolve in `Program.cs` for fail-fast only. | **adopted** | +| **Duplicate API on catalog** | Keep `RecipeDefinitionCatalog.TryGetRecipe` for loader/tests; registry is the game-facing surface (NEO-52 precedent). | **adopted** | +| **NEO-68 HTTP projection** | API layer maps `RecipeDefRow` fields to DTOs via registry enumeration + lookup; no combined read model in NEO-67. | **adopted** | + +## Decisions (kickoff) + +- **`GetDefinitionsInIdOrder()` included** — enumeration for NEO-68 HTTP without catalog coupling (user confirmed). +- **`TryGetDefinition` naming** — consistent with item/skill registries; parameter is `recipeId` (user confirmed). +- **No `Program.cs` registry eager-resolve** — catalog fail-fast only (NEO-52 pattern). +- **I/O and `recipeKind` on `RecipeDefRow`** — single lookup surface; no separate TryGet methods. From 81b0e3fc4b078d67720b20d2496240268068653e Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 16:57:17 -0400 Subject: [PATCH 2/4] NEO-67: add IRecipeDefinitionRegistry and DI adapter Thin registry over RecipeDefinitionCatalog mirrors NEO-52/59: TryGetDefinition, GetDefinitionsInIdOrder, AAA unit/host tests, README and alignment doc updates. --- .../E3_M2_RefinementAndRecipeExecution.md | 2 +- ...umentation_and_implementation_alignment.md | 2 +- docs/plans/E3M2-prototype-backlog.md | 4 +- docs/plans/NEO-67-implementation-plan.md | 8 +- .../Crafting/RecipeDefinitionRegistryTests.cs | 254 ++++++++++++++++++ .../Crafting/IRecipeDefinitionRegistry.cs | 20 ++ ...ecipeCatalogServiceCollectionExtensions.cs | 7 +- .../Game/Crafting/RecipeDefinitionCatalog.cs | 2 +- .../Game/Crafting/RecipeDefinitionRegistry.cs | 39 +++ server/README.md | 2 +- 10 files changed, 329 insertions(+), 11 deletions(-) create mode 100644 server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs create mode 100644 server/NeonSprawl.Server/Game/Crafting/IRecipeDefinitionRegistry.cs create mode 100644 server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs diff --git a/docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md b/docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md index 27463f9..100e260 100644 --- a/docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md +++ b/docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md @@ -13,7 +13,7 @@ **Prep (NEO-42):** Server ships **`RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine`** + **`RefineSkillXpConstants`** under `server/NeonSprawl.Server/Game/Skills/` — delegates to **`SkillProgressionGrantOperations.TryApplyGrant`** with **`refine`** + **`activity`** (prototype **10** XP), same persistence and **`allowedXpSourceKinds`** rules as **`POST …/skill-progression`**. **No** `CraftRequest` / **`CraftResult`** route yet; when this module adds authoritative craft/refine success, call that helper once on success ([NEO-42 implementation plan](../../plans/NEO-42-implementation-plan.md); [server README — Craft / refine hook (NEO-42)](../../../server/README.md#craft--refine-hook--skill-xp-neo-42)). -**E3M2-01 (NEO-65):** Frozen 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 plan](../../plans/NEO-65-implementation-plan.md)). **E3M2-02 (NEO-66):** fail-fast server load under `server/NeonSprawl.Server/Game/Crafting/` — `RecipeDefinitionCatalogLoader`, `RecipeDefinitionCatalog`, cross-check vs item + skill catalogs, Slice 3 gate; [NEO-66 plan](../../plans/NEO-66-implementation-plan.md); [server README — Recipe catalog](../../../server/README.md#recipe-catalog-contentrecipes-neo-66). +**E3M2-01 (NEO-65):** Frozen 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 plan](../../plans/NEO-65-implementation-plan.md)). **E3M2-02 (NEO-66):** fail-fast server load under `server/NeonSprawl.Server/Game/Crafting/` — `RecipeDefinitionCatalogLoader`, `RecipeDefinitionCatalog`, cross-check vs item + skill catalogs, Slice 3 gate; [NEO-66 plan](../../plans/NEO-66-implementation-plan.md); [server README — Recipe catalog](../../../server/README.md#recipe-catalog-contentrecipes-neo-66). **E3M2-03 (NEO-67):** injectable **`IRecipeDefinitionRegistry`** + DI; [NEO-67 plan](../../plans/NEO-67-implementation-plan.md). ## Purpose diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index ea6acaa..d0af413 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -55,7 +55,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), structured denies + **`levelUps`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); manual QA [`NEO-38`](../../manual-qa/NEO-38.md); Bruno `bruno/neon-sprawl-server/skill-progression/`; [server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38). **NEO-39 landed:** data-driven `LevelCurve` content + schema + CI validation (`*_level_curve.json`) with fail-fast startup schema checks; progression GET/POST level resolution now uses `ISkillLevelCurve` content-backed thresholds ([NEO-39](../../plans/NEO-39-implementation-plan.md)); manual QA [`NEO-39`](../../manual-qa/NEO-39.md). **NEO-40 landed:** comment-only telemetry hook sites in [`SkillProgressionSnapshotApi.cs`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs) on **`POST …/skill-progression`** for future **`xp_grant`** / **`level_up`** ([NEO-40](../../plans/NEO-40-implementation-plan.md), [`NEO-40` manual QA](../../manual-qa/NEO-40.md)). **NEO-41 landed:** gather prototype — **`POST …/interact`** with **`kind: resource_node`** grants **`salvage`** + **`activity`** (10 XP) via [`SkillProgressionGrantOperations`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs) ([NEO-41](../../plans/NEO-41-implementation-plan.md), [`NEO-41` manual QA](../../manual-qa/NEO-41.md)); [server README — Interaction](../../../server/README.md#interaction-neo-9). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack; **E3.M2** must invoke on craft/refine success ([NEO-42](../../plans/NEO-42-implementation-plan.md), [`NEO-42` manual QA](../../manual-qa/NEO-42.md)); [server README — Craft / refine hook (NEO-42)](../../../server/README.md#craft--refine-hook--skill-xp-neo-42). **NEO-43 landed (prep):** **`MissionRewardSkillXpGrant`** / **`MissionRewardSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack with fixed **`sourceKind: mission_reward`**; **E7.M2** must invoke from quest hand-in ([NEO-43](../../plans/NEO-43-implementation-plan.md), [`NEO-43` manual QA](../../manual-qa/NEO-43.md)); [server README — Mission / quest reward (NEO-43)](../../../server/README.md#mission--quest-reward--skill-xp-neo-43). **Slice 3 still open:** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [NEO-39](../../plans/NEO-39-implementation-plan.md), [NEO-40](../../plans/NEO-40-implementation-plan.md), [NEO-41](../../plans/NEO-41-implementation-plan.md), [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-43](../../plans/NEO-43-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-41, NEO-42–NEO-44 | | E3.M1 | Ready | **NEO-41 landed (prototype, superseded on interact by NEO-63):** inline XP on **`resource_node`** interact. **NEO-57–NEO-61** — content, registry, HTTP defs, depletion store (see prior alignment). **NEO-62 landed:** **`GatherOperations`** + **`GatherResult`**. **NEO-63 landed:** **`POST …/interact`** **`resource_node`** → gather engine; **four** **`PrototypeInteractableRegistry`** anchors; Bruno gather → inventory + depletion deny ([NEO-63](../../plans/NEO-63-implementation-plan.md), [`NEO-63` manual QA](../../manual-qa/NEO-63.md)); [server README — Gather via interact (NEO-63)](../../../server/README.md#gather-via-interact-neo-63). **NEO-64 landed:** comment-only **`resource_gathered`** / **`gather_node_depleted`** hook sites in **`GatherOperations.TryGather`** ([NEO-64](../../plans/NEO-64-implementation-plan.md), [`NEO-64` manual QA](../../manual-qa/NEO-64.md)); Epic 3 Slice 2 backlog **E3M1-01**–**E3M1-08** complete. **Slice 5 client (planned):** gather feedback HUD [NEO-73](https://linear.app/neon-sprawl/issue/NEO-73) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [NEO-41](../../plans/NEO-41-implementation-plan.md) … [NEO-64](../../plans/NEO-64-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md); `server/NeonSprawl.Server/Game/Gathering/`, `Game/Interaction/` | | 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. **Slice 5 client (planned):** inventory HUD [NEO-72](https://linear.app/neon-sprawl/issue/NEO-72) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [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), [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 | In Progress | **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-42 landed (prep):** **`RefineActivitySkillXpGrant`** — craft/refine success should award **`refine`** XP via shared NEO-38 grant path; no craft route yet. **Slice 3 backlog in Linear:** [NEO-67](https://linear.app/neon-sprawl/issue/NEO-67) → [NEO-71](https://linear.app/neon-sprawl/issue/NEO-71); [E3M2-prototype-backlog.md](../../plans/E3M2-prototype-backlog.md) — **E3M2-03**–**E3M2-07** open. Upstream: E3.M1 **Ready**, E3.M3 inventory landed. **Slice 5 client (planned):** craft UI [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74); capstone playable loop [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [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](https://linear.app/neon-sprawl/issue/NEO-67) … [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75), [E3M2-prototype-backlog](../../plans/E3M2-prototype-backlog.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M2](E3_M2_RefinementAndRecipeExecution.md) | +| E3.M2 | In Progress | **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-42 landed (prep):** **`RefineActivitySkillXpGrant`** — craft/refine success should award **`refine`** XP via shared NEO-38 grant path; no craft route yet. **Slice 3 backlog in Linear:** [NEO-68](https://linear.app/neon-sprawl/issue/NEO-68) → [NEO-71](https://linear.app/neon-sprawl/issue/NEO-71); [E3M2-prototype-backlog.md](../../plans/E3M2-prototype-backlog.md) — **E3M2-04**–**E3M2-07** open. Upstream: E3.M1 **Ready**, E3.M3 inventory landed. **Slice 5 client (planned):** craft UI [NEO-74](https://linear.app/neon-sprawl/issue/NEO-74); capstone playable loop [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75) — [E3S5 backlog](../../plans/E3S5-client-prototype-backlog.md). | [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](https://linear.app/neon-sprawl/issue/NEO-67) … [NEO-75](https://linear.app/neon-sprawl/issue/NEO-75), [E3M2-prototype-backlog](../../plans/E3M2-prototype-backlog.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M2](E3_M2_RefinementAndRecipeExecution.md) | --- diff --git a/docs/plans/E3M2-prototype-backlog.md b/docs/plans/E3M2-prototype-backlog.md index 4de6720..dfecd2a 100644 --- a/docs/plans/E3M2-prototype-backlog.md +++ b/docs/plans/E3M2-prototype-backlog.md @@ -127,7 +127,9 @@ Working backlog for **Epic 3 — Slice 3** ([recipes and crafting pipeline](../d **Acceptance criteria** -- [ ] Host resolves registry from DI; unknown `recipeId` distinguishable from valid ids in tests. +- [x] Host resolves registry from DI; unknown `recipeId` distinguishable from valid ids in tests. + +**Landed ([NEO-67](https://linear.app/neon-sprawl/issue/NEO-67)):** `IRecipeDefinitionRegistry` + `RecipeDefinitionRegistry` in `server/NeonSprawl.Server/Game/Crafting/`; plan [NEO-67-implementation-plan.md](NEO-67-implementation-plan.md). --- diff --git a/docs/plans/NEO-67-implementation-plan.md b/docs/plans/NEO-67-implementation-plan.md index c0a139f..344df2e 100644 --- a/docs/plans/NEO-67-implementation-plan.md +++ b/docs/plans/NEO-67-implementation-plan.md @@ -42,10 +42,10 @@ ## Acceptance criteria checklist -- [ ] DI resolves `IRecipeDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`). -- [ ] Unit tests (AAA): lookup for known prototype `recipeId` (e.g. `refine_scrap_standard`, `make_field_stim_mk0`) with expected `recipeKind`, `requiredSkillId`, and input/output rows. -- [ ] Unit tests (AAA): unknown `recipeId` returns false / absent without throwing. -- [ ] `GetDefinitionsInIdOrder` returns all eight loaded rows ordered by `id` (ordinal). +- [x] DI resolves `IRecipeDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`). +- [x] Unit tests (AAA): lookup for known prototype `recipeId` (e.g. `refine_scrap_standard`, `make_field_stim_mk0`) with expected `recipeKind`, `requiredSkillId`, and input/output rows. +- [x] Unit tests (AAA): unknown `recipeId` returns false / absent without throwing. +- [x] `GetDefinitionsInIdOrder` returns all eight loaded rows ordered by `id` (ordinal). ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs b/server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs new file mode 100644 index 0000000..8a204d6 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs @@ -0,0 +1,254 @@ +using System.Collections.Frozen; +using System.IO; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NeonSprawl.Server.Game.Crafting; +using NeonSprawl.Server.Game.Items; +using NeonSprawl.Server.Tests; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Crafting; + +public class RecipeDefinitionRegistryTests +{ + private static readonly FrozenSet KnownItemIds = PrototypeSlice1ItemCatalogRules.ExpectedItemIds; + + private static readonly FrozenSet KnownSkillIds = FrozenSet.ToFrozenSet( + ["salvage", "refine", "intrusion"], + StringComparer.Ordinal); + + private static RecipeDefinitionRegistry CreateRegistryFromRows(IReadOnlyDictionary byId) + { + var catalog = new RecipeDefinitionCatalog("/tmp/recipes", byId, catalogJsonFileCount: 1); + return new RecipeDefinitionRegistry(catalog); + } + + [Fact] + public void TryGetDefinition_ShouldReturnTrueAndExpectedMetadata_WhenRefineRecipeExists() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["refine_scrap_standard"] = new RecipeDefRow( + "refine_scrap_standard", + "Refine Scrap (Standard)", + "process", + "refine", + [new RecipeIoRow("scrap_metal_bulk", 5)], + [new RecipeIoRow("refined_plate_stock", 1)], + RequiredSkillLevel: null, + StationTag: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition("refine_scrap_standard", out var def); + // Assert + Assert.True(found); + Assert.NotNull(def); + Assert.Equal("process", def.RecipeKind); + Assert.Equal("refine", def.RequiredSkillId); + Assert.Equal("Refine Scrap (Standard)", def.DisplayName); + Assert.Single(def.Inputs); + Assert.Equal("scrap_metal_bulk", def.Inputs[0].ItemId); + Assert.Equal(5, def.Inputs[0].Quantity); + Assert.Single(def.Outputs); + Assert.Equal("refined_plate_stock", def.Outputs[0].ItemId); + Assert.Equal(1, def.Outputs[0].Quantity); + } + + [Fact] + public void TryGetDefinition_ShouldReturnTrueAndExpectedMetadata_WhenMakeRecipeExists() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["make_field_stim_mk0"] = new RecipeDefRow( + "make_field_stim_mk0", + "Make Field Stim Mk0", + "make", + "refine", + [ + new RecipeIoRow("refined_plate_stock", 2), + new RecipeIoRow("scrap_metal_bulk", 1), + ], + [new RecipeIoRow("field_stim_mk0", 1)], + RequiredSkillLevel: null, + StationTag: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition("make_field_stim_mk0", out var def); + // Assert + Assert.True(found); + Assert.NotNull(def); + Assert.Equal("make", def.RecipeKind); + Assert.Equal(2, def.Inputs.Count); + Assert.Equal("refined_plate_stock", def.Inputs[0].ItemId); + Assert.Equal("scrap_metal_bulk", def.Inputs[1].ItemId); + Assert.Single(def.Outputs); + Assert.Equal("field_stim_mk0", def.Outputs[0].ItemId); + } + + [Fact] + public void TryGetDefinition_ShouldReturnFalse_WhenRecipeIdIsNull() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["refine_scrap_standard"] = new RecipeDefRow( + "refine_scrap_standard", + "Refine Scrap (Standard)", + "process", + "refine", + [new RecipeIoRow("scrap_metal_bulk", 5)], + [new RecipeIoRow("refined_plate_stock", 1)], + RequiredSkillLevel: null, + StationTag: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition(null, out var def); + // Assert + Assert.False(found); + Assert.Null(def); + } + + [Fact] + public void TryGetDefinition_ShouldReturnFalse_WhenIdUnknown() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["refine_scrap_standard"] = new RecipeDefRow( + "refine_scrap_standard", + "Refine Scrap (Standard)", + "process", + "refine", + [new RecipeIoRow("scrap_metal_bulk", 5)], + [new RecipeIoRow("refined_plate_stock", 1)], + RequiredSkillLevel: null, + StationTag: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition("not_a_real_recipe", out var def); + // Assert + Assert.False(found); + Assert.Null(def); + } + + [Fact] + public void GetDefinitionsInIdOrder_ShouldListAllRowsOrderedById_WhenMultipleRecipes() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["make_field_stim_mk0"] = new RecipeDefRow( + "make_field_stim_mk0", + "Make Field Stim Mk0", + "make", + "refine", + [new RecipeIoRow("refined_plate_stock", 2)], + [new RecipeIoRow("field_stim_mk0", 1)], + RequiredSkillLevel: null, + StationTag: null), + ["refine_scrap_efficient"] = new RecipeDefRow( + "refine_scrap_efficient", + "Refine Scrap (Efficient)", + "process", + "refine", + [new RecipeIoRow("scrap_metal_bulk", 10)], + [new RecipeIoRow("refined_plate_stock", 2)], + RequiredSkillLevel: null, + StationTag: null), + ["refine_scrap_standard"] = new RecipeDefRow( + "refine_scrap_standard", + "Refine Scrap (Standard)", + "process", + "refine", + [new RecipeIoRow("scrap_metal_bulk", 5)], + [new RecipeIoRow("refined_plate_stock", 1)], + RequiredSkillLevel: null, + StationTag: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var list = registry.GetDefinitionsInIdOrder(); + // Assert + Assert.Equal(3, list.Count); + Assert.Equal("make_field_stim_mk0", list[0].Id); + Assert.Equal("refine_scrap_efficient", list[1].Id); + Assert.Equal("refine_scrap_standard", list[2].Id); + } + + [Fact] + public void TryGetDefinition_ShouldMatchLoaderCatalog_WhenUsingPrototypeFixture() + { + // Arrange + var root = Directory.CreateTempSubdirectory("neon-sprawl-recipe-registry-loader-"); + try + { + var recipesDir = Path.Combine(root.FullName, "content", "recipes"); + var schemaDir = Path.Combine(root.FullName, "content", "schemas"); + Directory.CreateDirectory(recipesDir); + Directory.CreateDirectory(schemaDir); + var recipeDefSchemaPath = Path.Combine(schemaDir, "recipe-def.schema.json"); + var recipeIoRowSchemaPath = Path.Combine(schemaDir, "recipe-io-row.schema.json"); + File.Copy(RecipeCatalogTestPaths.DiscoverRepoRecipeDefSchemaPath(), recipeDefSchemaPath, overwrite: true); + File.Copy(RecipeCatalogTestPaths.DiscoverRepoRecipeIoRowSchemaPath(), recipeIoRowSchemaPath, overwrite: true); + File.Copy( + Path.Combine(RecipeCatalogTestPaths.DiscoverRepoRecipesDirectory(), "prototype_recipes.json"), + Path.Combine(recipesDir, "prototype_recipes.json"), + overwrite: true); + var loaded = RecipeDefinitionCatalogLoader.Load( + recipesDir, + recipeDefSchemaPath, + recipeIoRowSchemaPath, + KnownItemIds, + KnownSkillIds, + NullLogger.Instance); + var registry = new RecipeDefinitionRegistry(loaded); + // Act + var ok = registry.TryGetDefinition("refine_scrap_standard", out var refine); + var list = registry.GetDefinitionsInIdOrder(); + // Assert + Assert.True(ok); + Assert.NotNull(refine); + Assert.Equal("process", refine.RecipeKind); + Assert.Equal("refine", refine.RequiredSkillId); + Assert.Equal(8, list.Count); + Assert.Equal("make_armor_quick", list[0].Id); + Assert.Equal("refine_scrap_standard", list[^1].Id); + } + finally + { + try + { + Directory.Delete(root.FullName, recursive: true); + } + catch (IOException) + { + // Best-effort: transient lock or race on some hosts; temp dir is unique per run. + } + } + } + + [Fact] + public async Task Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + using var client = factory.CreateClient(); + _ = await client.GetAsync("/health"); + // Act + var registry = factory.Services.GetRequiredService(); + var found = registry.TryGetDefinition("refine_scrap_standard", out var refine); + var list = registry.GetDefinitionsInIdOrder(); + // Assert + Assert.True(found); + Assert.NotNull(refine); + Assert.Equal("process", refine.RecipeKind); + Assert.Equal(8, list.Count); + Assert.Contains(list, r => r.Id == "make_field_stim_mk0" && r.RecipeKind == "make"); + } +} diff --git a/server/NeonSprawl.Server/Game/Crafting/IRecipeDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Crafting/IRecipeDefinitionRegistry.cs new file mode 100644 index 0000000..48a5e47 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Crafting/IRecipeDefinitionRegistry.cs @@ -0,0 +1,20 @@ +using System.Diagnostics.CodeAnalysis; + +namespace NeonSprawl.Server.Game.Crafting; + +/// +/// Read-only access to validated entries loaded at startup (). +/// +/// +/// E3.M2 (craft / refine): callers validating CraftRequest and resolving I/O rows should depend on this interface +/// rather than so recipe metadata stays centralized. +/// NEO-68: HTTP/read-model projections should depend on this interface rather than reaching into the catalog. +/// +public interface IRecipeDefinitionRegistry +{ + /// Attempts to resolve a recipe by stable id (see recipe-def.schema.json). Unknown ids and null return false without throwing. + bool TryGetDefinition(string? recipeId, [NotNullWhen(true)] out RecipeDefRow? definition); + + /// Every loaded definition, ordered by (ordinal). + IReadOnlyList GetDefinitionsInIdOrder(); +} diff --git a/server/NeonSprawl.Server/Game/Crafting/RecipeCatalogServiceCollectionExtensions.cs b/server/NeonSprawl.Server/Game/Crafting/RecipeCatalogServiceCollectionExtensions.cs index f7c1097..763dc46 100644 --- a/server/NeonSprawl.Server/Game/Crafting/RecipeCatalogServiceCollectionExtensions.cs +++ b/server/NeonSprawl.Server/Game/Crafting/RecipeCatalogServiceCollectionExtensions.cs @@ -6,10 +6,10 @@ using NeonSprawl.Server.Game.Skills; namespace NeonSprawl.Server.Game.Crafting; -/// DI registration for the fail-fast recipe catalog (NEO-66). +/// DI registration for the fail-fast recipe catalog (NEO-66) and recipe definition registry (NEO-67). public static class RecipeCatalogServiceCollectionExtensions { - /// Binds and registers as a singleton. + /// Binds and registers and as singletons. public static IServiceCollection AddRecipeDefinitionCatalog(this IServiceCollection services, IConfiguration configuration) { services.AddOptions() @@ -48,6 +48,9 @@ public static class RecipeCatalogServiceCollectionExtensions logger); }); + services.AddSingleton(sp => + new RecipeDefinitionRegistry(sp.GetRequiredService())); + return services; } } diff --git a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalog.cs b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalog.cs index fee0958..daba3c2 100644 --- a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalog.cs +++ b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionCatalog.cs @@ -2,7 +2,7 @@ using System.Collections.ObjectModel; namespace NeonSprawl.Server.Game.Crafting; -/// In-memory recipe catalog loaded at startup (NEO-66). NEO-67 adds injectable recipe definition registry for game callers. +/// In-memory recipe catalog loaded at startup (NEO-66). Game callers should use (NEO-67). public sealed class RecipeDefinitionCatalog( string recipesDirectory, IReadOnlyDictionary byId, diff --git a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs new file mode 100644 index 0000000..441a0f5 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs @@ -0,0 +1,39 @@ +using System.Diagnostics.CodeAnalysis; + +namespace NeonSprawl.Server.Game.Crafting; + +/// Adapter over (NEO-67). +public sealed class RecipeDefinitionRegistry(RecipeDefinitionCatalog catalog) : IRecipeDefinitionRegistry +{ + /// + public bool TryGetDefinition(string? recipeId, [NotNullWhen(true)] out RecipeDefRow? definition) + { + if (recipeId is null) + { + definition = null; + return false; + } + + if (catalog.ById.TryGetValue(recipeId, out var row)) + { + definition = row; + return true; + } + + definition = null; + return false; + } + + /// + public IReadOnlyList GetDefinitionsInIdOrder() + { + var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); + var list = new List(ids.Length); + foreach (var id in ids) + { + list.Add(catalog.ById[id]); + } + + return list; + } +} diff --git a/server/README.md b/server/README.md index 973f3c7..554459e 100644 --- a/server/README.md +++ b/server/README.md @@ -78,7 +78,7 @@ On startup the host loads every **`*_recipes.json`** under the recipes directory **Docker / CI:** include **`content/recipes`**, **`content/items`**, **`content/skills`**, and the two recipe schemas in the mounted **`content/`** tree; set **`Content__RecipesDirectory`** when layout differs. -On success, **Information** logs include the resolved recipes directory path, distinct recipe count, and catalog file count. NEO-67 adds **`IRecipeDefinitionRegistry`** for game callers; until then, **`RecipeDefinitionCatalog`** is the startup-loaded snapshot. +On success, **Information** logs include the resolved recipes directory path, distinct recipe count, and catalog file count. Game code should use **`IRecipeDefinitionRegistry`** for lookups (NEO-67). The catalog singleton remains for fail-fast startup only; do not inject **`RecipeDefinitionCatalog`** in new game code. ## Resource node definitions (NEO-60) From 2fc3b8f7508717a1e721d0ab464a05cb58ff2555 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 16:58:00 -0400 Subject: [PATCH 3/4] NEO-67: Add code review for recipe definition registry + DI. --- docs/reviews/2026-05-24-NEO-67.md | 56 +++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 docs/reviews/2026-05-24-NEO-67.md diff --git a/docs/reviews/2026-05-24-NEO-67.md b/docs/reviews/2026-05-24-NEO-67.md new file mode 100644 index 0000000..57ab41b --- /dev/null +++ b/docs/reviews/2026-05-24-NEO-67.md @@ -0,0 +1,56 @@ +# Code review — NEO-67 recipe definition registry + DI + +**Date:** 2026-05-24 +**Scope:** Branch `NEO-67-recipe-definition-registry-di` · commits `7188c5b`–`81b0e3f` vs `origin/main` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-67 delivers **E3M2-03**: a thin **`IRecipeDefinitionRegistry`** / **`RecipeDefinitionRegistry`** adapter over the NEO-66 startup-loaded **`RecipeDefinitionCatalog`**, registered in DI alongside the catalog singleton. The interface mirrors **`IItemDefinitionRegistry`** (NEO-52): `TryGetDefinition(string? recipeId, …)` and `GetDefinitionsInIdOrder()` with ordinal id ordering. Seven AAA unit/host tests pass; docs (plan, E3M2 backlog, E3.M2 snapshot, alignment register, `server/README.md`) are updated. No HTTP, craft engine, or client work — correctly deferred to NEO-68+. Risk is very low: pure adapter + DI wiring over immutable catalog data; no behavior change at boot beyond an extra singleton registration. + +## Documentation checked + +| Document | Result | +|----------|--------| +| [`docs/plans/NEO-67-implementation-plan.md`](../plans/NEO-67-implementation-plan.md) | **Matches** — interface, adapter, DI extension, comment/README touches, acceptance checklist complete; HTTP/craft/loader changes correctly out of scope. | +| [`docs/plans/E3M2-prototype-backlog.md`](../plans/E3M2-prototype-backlog.md) | **Matches** — E3M2-03 acceptance checkbox + landed note. | +| [`docs/decomposition/modules/E3_M2_RefinementAndRecipeExecution.md`](../decomposition/modules/E3_M2_RefinementAndRecipeExecution.md) | **Matches** — implementation snapshot references NEO-67 registry. | +| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M2 row updated with NEO-67 landed note; next backlog item NEO-68. | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — table **Status** still **Planned** for E3.M2 (same pattern as post–NEO-66); optional bump when Slice 3 server spine completes. | +| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **N/A** — server DI/read surface only; no client mutation. | +| [`server/README.md`](../../server/README.md) | **Matches** — recipe catalog section documents `IRecipeDefinitionRegistry` as preferred lookup surface. | +| Manual QA | **N/A** — plan documents skip rationale: automated unit/host tests (same as NEO-52 / NEO-59); no player-visible HTTP. | + +Register/tracking: alignment table E3.M2 **In Progress** with NEO-67 note is correct. + +## Blocking issues + +None. + +## Suggestions + +None. + +## Nits + +- Nit: `RecipeDefinitionRegistry.TryGetDefinition` reads `catalog.ById` directly instead of delegating to `catalog.TryGetRecipe` — consistent with `ItemDefinitionRegistry` (NEO-52), but both bypass the catalog’s public lookup helper; harmless for immutable data. + +- Nit: `GetDefinitionsInIdOrder()` allocates a new sorted list on every call — acceptable for prototype HTTP/craft volume; optional future optimization is to cache the ordered list at catalog/registry construction time if hot-path profiling warrants it (same as item registry). + +- Nit: `Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds` asserts a known id and enumeration count but not an unknown id in the host context — unit tests cover unknown/null; host test could add one line for symmetry with E3M2-03 acceptance wording (“distinguishable from valid ids in tests”). + +## Verification + +```bash +cd /home/don/neon-sprawl/server +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~RecipeDefinitionRegistryTests" +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj +``` + +Manual (optional): + +- Start server from repo root; confirm existing NEO-66 Information log (recipe count **8**) unchanged — registry does not alter boot semantics. From 95d42803e1b25787d304ecca7e1bed52125c2dc6 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 16:58:38 -0400 Subject: [PATCH 4/4] NEO-67: address code review nits on recipe registry Delegate TryGetDefinition to catalog.TryGetRecipe, cache ordered defs at construction, and assert unknown recipeId in host DI test. --- docs/reviews/2026-05-24-NEO-67.md | 6 +++--- .../Crafting/RecipeDefinitionRegistryTests.cs | 3 +++ .../Game/Crafting/RecipeDefinitionRegistry.cs | 15 ++++++--------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/reviews/2026-05-24-NEO-67.md b/docs/reviews/2026-05-24-NEO-67.md index 57ab41b..966e272 100644 --- a/docs/reviews/2026-05-24-NEO-67.md +++ b/docs/reviews/2026-05-24-NEO-67.md @@ -37,11 +37,11 @@ None. ## Nits -- Nit: `RecipeDefinitionRegistry.TryGetDefinition` reads `catalog.ById` directly instead of delegating to `catalog.TryGetRecipe` — consistent with `ItemDefinitionRegistry` (NEO-52), but both bypass the catalog’s public lookup helper; harmless for immutable data. +- ~~Nit: `RecipeDefinitionRegistry.TryGetDefinition` reads `catalog.ById` directly instead of delegating to `catalog.TryGetRecipe` — consistent with `ItemDefinitionRegistry` (NEO-52), but both bypass the catalog’s public lookup helper; harmless for immutable data.~~ **Done.** Registry now delegates to `catalog.TryGetRecipe`. -- Nit: `GetDefinitionsInIdOrder()` allocates a new sorted list on every call — acceptable for prototype HTTP/craft volume; optional future optimization is to cache the ordered list at catalog/registry construction time if hot-path profiling warrants it (same as item registry). +- ~~Nit: `GetDefinitionsInIdOrder()` allocates a new sorted list on every call — acceptable for prototype HTTP/craft volume; optional future optimization is to cache the ordered list at catalog/registry construction time if hot-path profiling warrants it (same as item registry).~~ **Done.** Ordered list built once in `RecipeDefinitionRegistry` constructor. -- Nit: `Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds` asserts a known id and enumeration count but not an unknown id in the host context — unit tests cover unknown/null; host test could add one line for symmetry with E3M2-03 acceptance wording (“distinguishable from valid ids in tests”). +- ~~Nit: `Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds` asserts a known id and enumeration count but not an unknown id in the host context — unit tests cover unknown/null; host test could add one line for symmetry with E3M2-03 acceptance wording (“distinguishable from valid ids in tests”).~~ **Done.** Host test asserts unknown `recipeId` returns false without throw. ## Verification diff --git a/server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs b/server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs index 8a204d6..4a20715 100644 --- a/server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Crafting/RecipeDefinitionRegistryTests.cs @@ -243,11 +243,14 @@ public class RecipeDefinitionRegistryTests // Act var registry = factory.Services.GetRequiredService(); var found = registry.TryGetDefinition("refine_scrap_standard", out var refine); + var unknown = registry.TryGetDefinition("not_a_real_recipe", out var missing); var list = registry.GetDefinitionsInIdOrder(); // Assert Assert.True(found); Assert.NotNull(refine); Assert.Equal("process", refine.RecipeKind); + Assert.False(unknown); + Assert.Null(missing); Assert.Equal(8, list.Count); Assert.Contains(list, r => r.Id == "make_field_stim_mk0" && r.RecipeKind == "make"); } diff --git a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs index 441a0f5..7b84018 100644 --- a/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs +++ b/server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionRegistry.cs @@ -5,6 +5,8 @@ namespace NeonSprawl.Server.Game.Crafting; /// Adapter over (NEO-67). public sealed class RecipeDefinitionRegistry(RecipeDefinitionCatalog catalog) : IRecipeDefinitionRegistry { + private readonly IReadOnlyList _definitionsInIdOrder = BuildDefinitionsInIdOrder(catalog); + /// public bool TryGetDefinition(string? recipeId, [NotNullWhen(true)] out RecipeDefRow? definition) { @@ -14,18 +16,13 @@ public sealed class RecipeDefinitionRegistry(RecipeDefinitionCatalog catalog) : return false; } - if (catalog.ById.TryGetValue(recipeId, out var row)) - { - definition = row; - return true; - } - - definition = null; - return false; + return catalog.TryGetRecipe(recipeId, out definition); } /// - public IReadOnlyList GetDefinitionsInIdOrder() + public IReadOnlyList GetDefinitionsInIdOrder() => _definitionsInIdOrder; + + private static IReadOnlyList BuildDefinitionsInIdOrder(RecipeDefinitionCatalog catalog) { var ids = catalog.ById.Keys.OrderBy(k => k, StringComparer.Ordinal).ToArray(); var list = new List(ids.Length);