From 1e9f1e9a3a68afb964454190c05a7852fd68b8cb Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 23 May 2026 18:09:02 -0400 Subject: [PATCH 1/4] NEO-52: add implementation plan for item definition registry + DI Kickoff plan mirrors NEO-35 skill registry pattern on NEO-51 catalog; includes GetDefinitionsInIdOrder and TryGetDefinition per kickoff answers. --- docs/plans/NEO-52-implementation-plan.md | 92 ++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 docs/plans/NEO-52-implementation-plan.md diff --git a/docs/plans/NEO-52-implementation-plan.md b/docs/plans/NEO-52-implementation-plan.md new file mode 100644 index 0000000..47b561a --- /dev/null +++ b/docs/plans/NEO-52-implementation-plan.md @@ -0,0 +1,92 @@ +# NEO-52 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-52 | +| **Title** | E3.M3: Item definition registry + DI | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-52/e3m3-item-definition-registry-di | +| **Module** | [E3.M3 — ItemizationAndInventorySchema](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) · Epic 3 Slice 1 (E3M3-03) | +| **Branch** | `NEO-52-item-definition-registry-di` | +| **Precursor** | [NEO-51](https://linear.app/neon-sprawl/issue/NEO-51) — fail-fast `ItemDefinitionCatalog` load (**Done** on `main`) | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Registry API surface** | Include `GetDefinitionsInIdOrder()` now or TryGet-only? | **Include enumeration** — mirror [NEO-35](NEO-35-implementation-plan.md) / `ISkillDefinitionRegistry`; [NEO-53](https://linear.app/neon-sprawl/issue/NEO-53) `GET /game/world/item-definitions` will need ordered defs without reaching into `ItemDefinitionCatalog`. | **User:** include `GetDefinitionsInIdOrder`. | +| **Lookup method name** | `TryGetDefinition` vs `TryGetItem`? | **`TryGetDefinition(string? itemId, …)`** — same naming as `ISkillDefinitionRegistry.TryGetDefinition`; catalog keeps `TryGetItem` for direct catalog access. | **User:** `TryGetDefinition`. | +| **NEO-51 vs NEO-52 split** | Combine registry with catalog load? | **Strict split** (already decided on NEO-51 kickoff) — NEO-51 ships loader + catalog; this story adds injectable registry only. | **Adopted** — NEO-51 plan + user answer on NEO-51 kickoff. | +| **Program.cs eager resolve** | Eager-resolve `IItemDefinitionRegistry` at boot? | **Omit** — `ItemDefinitionCatalog` is already eager-resolved in `Program.cs` (NEO-51); registry is a thin adapter (NEO-35 default). | **Adopted** | +| **Runtime validation** | Re-validate items in registry? | **No** — catalog load is fail-fast (NEO-51); registry delegates to loaded `ItemDefRow` only. | **Adopted** | + +## Goal, scope, and out-of-scope + +**Goal:** Provide **`IItemDefinitionRegistry`** backed by the startup-loaded **`ItemDefinitionCatalog`**: resolve by stable **`itemId`**, enumerate definitions in id order, expose prototype metadata (`displayName`, `prototypeRole`, `stackMax`, `inventorySlotKind`). Register in DI so **NEO-53+** (HTTP), **NEO-54** (inventory engine), and future craft/gather callers depend on the interface instead of the catalog type. + +**In scope (from Linear + E3M3-03):** + +- `ItemDefinitionRegistry` thin adapter over `ItemDefinitionCatalog`. +- DI registration alongside existing catalog singleton. +- Unit tests (AAA): known prototype id lookup + metadata; unknown `itemId` returns false without throwing; enumeration order; host resolves registry from DI. + +**Out of scope (from Linear):** + +- HTTP ([NEO-53](https://linear.app/neon-sprawl/issue/NEO-53)). +- Persistence, per-player inventory, stack mutation ([NEO-54+](E3M3-prototype-backlog.md)). +- Changing loader, Slice 1 gate, or catalog load semantics (NEO-51). + +## Acceptance criteria checklist + +- [ ] DI resolves `IItemDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`). +- [ ] Unit tests (AAA): lookup for known prototype `itemId` (e.g. `scrap_metal_bulk`, `prototype_armor_shell`) with expected metadata. +- [ ] Unit tests (AAA): unknown `itemId` returns false / absent without throwing. +- [ ] `GetDefinitionsInIdOrder` returns all loaded rows ordered by `id` (ordinal). + +## Technical approach + +1. **`IItemDefinitionRegistry`** — mirror [`ISkillDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Skills/ISkillDefinitionRegistry.cs): + - `TryGetDefinition(string? itemId, [NotNullWhen(true)] out ItemDefRow? definition)` — null and unknown ids return false without throwing. + - `GetDefinitionsInIdOrder()` — all rows ordered by `ItemDefRow.Id` (ordinal). + - XML remarks: inventory/craft/gather callers (NEO-54+) should use this interface; HTTP read model (NEO-53) should not reach into `ItemDefinitionCatalog`. + +2. **`ItemDefinitionRegistry`** — primary-constructor adapter over `ItemDefinitionCatalog` (same pattern as [`SkillDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Skills/SkillDefinitionRegistry.cs)). + +3. **DI** — extend [`AddItemDefinitionCatalog`](../../server/NeonSprawl.Server/Game/Items/ItemCatalogServiceCollectionExtensions.cs) to register `IItemDefinitionRegistry` → `ItemDefinitionRegistry` after catalog registration. No change to `Program.cs` eager-resolve (catalog only). + +4. **Comments** — update `ItemDefinitionCatalog` summary if needed to point at `IItemDefinitionRegistry`; update [server README](../../server/README.md) item-catalog section (currently says “until NEO-52”). + +5. **Tests** — new `ItemDefinitionRegistryTests.cs` mirroring [`SkillDefinitionRegistryTests`](../../server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionRegistryTests.cs): in-memory catalog helper, prototype fixture via loader + `ItemCatalogTestPaths`, host DI test. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Items/IItemDefinitionRegistry.cs` | Public contract: lookup, enumerate, remarks for NEO-53/NEO-54 callers. | +| `server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs` | Singleton adapter over `ItemDefinitionCatalog`. | +| `server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs` | AAA unit + host DI tests (mirror `SkillDefinitionRegistryTests`). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Items/ItemCatalogServiceCollectionExtensions.cs` | Register `IItemDefinitionRegistry` → `ItemDefinitionRegistry` after catalog singleton. | +| `server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalog.cs` | Tighten summary comment to reference `IItemDefinitionRegistry` (replace “NEO-52” placeholder). | +| `server/README.md` | Item catalog section: document `IItemDefinitionRegistry` as preferred lookup surface. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M3 row — note NEO-52 registry when landed. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs` | **Unit:** `TryGetDefinition` for `scrap_metal_bulk` (or equip stub) returns true with `StackMax`, `PrototypeRole`, `InventorySlotKind`; null and unknown id return false without throw; `GetDefinitionsInIdOrder` count and ordinal order for multi-row fixture; loader-backed prototype fixture matches catalog. **Host:** `InMemoryWebApplicationFactory` resolves `IItemDefinitionRegistry` and finds `scrap_metal_bulk`. | + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Catalog vs registry for game code** | New callers inject `IItemDefinitionRegistry`; leave `ItemDefinitionCatalog` eager-resolve in `Program.cs` for fail-fast only. | `adopted` | +| **Duplicate API on catalog** | Keep `ItemDefinitionCatalog.TryGetItem` for loader/tests; registry is the game-facing surface (NEO-35 precedent with catalog `ById`). | `adopted` | + +None blocking beyond the above. From 792b38d8a9c77541fee5bd743820560159651d68 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 23 May 2026 18:11:33 -0400 Subject: [PATCH 2/4] NEO-52: add IItemDefinitionRegistry with DI and lookup tests Thin adapter over ItemDefinitionCatalog mirroring NEO-35 skill registry; registers singleton in AddItemDefinitionCatalog with TryGetDefinition, GetDefinitionsInIdOrder, and AAA unit + host DI tests. --- ...umentation_and_implementation_alignment.md | 2 +- docs/plans/NEO-52-implementation-plan.md | 8 +- .../Game/Items/ItemDefinitionRegistryTests.cs | 195 ++++++++++++++++++ .../Game/Items/IItemDefinitionRegistry.cs | 20 ++ .../ItemCatalogServiceCollectionExtensions.cs | 5 +- .../Game/Items/ItemDefinitionCatalog.cs | 2 +- .../Game/Items/ItemDefinitionRegistry.cs | 39 ++++ server/README.md | 2 +- 8 files changed, 265 insertions(+), 8 deletions(-) create mode 100644 server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs create mode 100644 server/NeonSprawl.Server/Game/Items/IItemDefinitionRegistry.cs create mode 100644 server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 6477757..3e13f4f 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -54,7 +54,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E2.M3 | In Progress | **NEO-45 landed:** prototype **`salvage`** mastery catalog + CI gates (see [NEO-45 plan](../../plans/NEO-45-implementation-plan.md)). **NEO-46 landed:** fail-fast server load under `server/NeonSprawl.Server/Game/Mastery/` — `MasteryCatalogLoader`, `IMasteryCatalogRegistry`, cross-check vs `ISkillDefinitionRegistry`, Slice 4 + **`tierIndex`** 1..N gate; see [NEO-46 plan](../../plans/NEO-46-implementation-plan.md). **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants; see [NEO-47 plan](../../plans/NEO-47-implementation-plan.md). **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook site in [`PerkUnlockEngine.TryUnlockPerks`](../../../server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs) ([NEO-49 plan](../../plans/NEO-49-implementation-plan.md), [`NEO-49` manual QA](../../manual-qa/NEO-49.md)); [server README — Perk unlock telemetry (NEO-49)](../../../server/README.md#perk-unlock-engine-and-telemetry-hooks-neo-47-neo-49). **NEO-48 landed:** **`GET`/`POST /game/players/{id}/perk-state`** — `PerkStateApi` + DTOs in `Game/Mastery/` ([NEO-48](../../plans/NEO-48-implementation-plan.md), [`NEO-48` manual QA](../../manual-qa/NEO-48.md)); [server README — Perk state (NEO-48)](../../../server/README.md#perk-state-neo-48); Bruno `bruno/neon-sprawl-server/perk-state/`. | [NEO-45](../../plans/NEO-45-implementation-plan.md), [NEO-46](../../plans/NEO-46-implementation-plan.md), [NEO-47](../../plans/NEO-47-implementation-plan.md), [NEO-48](../../plans/NEO-48-implementation-plan.md), [NEO-49](../../plans/NEO-49-implementation-plan.md), [E2M3-pre-production-backlog](../../plans/E2M3-pre-production-backlog.md), [E2_M3](E2_M3_MasteryAndPerkUnlocks.md) | | 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 | In Progress | **NEO-41 landed (prototype):** `POST …/interact` success on **`resource_node`** (`prototype_resource_node_alpha`) applies **`salvage`** skill XP (**`sourceKind: activity`**, 10 XP) via shared NEO-38 grant operations. **Still planned:** `GatherResult`, yields, inventory per [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md). | [NEO-41](../../plans/NEO-41-implementation-plan.md), [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md); `server/NeonSprawl.Server/Game/Interaction/`, `Game/Skills/` | -| 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). **Still planned:** `IItemDefinitionRegistry` (NEO-52+), inventory store, HTTP. | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) | +| 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)). **Still planned:** inventory store, HTTP. | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) | --- diff --git a/docs/plans/NEO-52-implementation-plan.md b/docs/plans/NEO-52-implementation-plan.md index 47b561a..7de2ed4 100644 --- a/docs/plans/NEO-52-implementation-plan.md +++ b/docs/plans/NEO-52-implementation-plan.md @@ -39,10 +39,10 @@ ## Acceptance criteria checklist -- [ ] DI resolves `IItemDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`). -- [ ] Unit tests (AAA): lookup for known prototype `itemId` (e.g. `scrap_metal_bulk`, `prototype_armor_shell`) with expected metadata. -- [ ] Unit tests (AAA): unknown `itemId` returns false / absent without throwing. -- [ ] `GetDefinitionsInIdOrder` returns all loaded rows ordered by `id` (ordinal). +- [x] DI resolves `IItemDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`). +- [x] Unit tests (AAA): lookup for known prototype `itemId` (e.g. `scrap_metal_bulk`, `prototype_armor_shell`) with expected metadata. +- [x] Unit tests (AAA): unknown `itemId` returns false / absent without throwing. +- [x] `GetDefinitionsInIdOrder` returns all loaded rows ordered by `id` (ordinal). ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs b/server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs new file mode 100644 index 0000000..bd1d70a --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs @@ -0,0 +1,195 @@ +using System.IO; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NeonSprawl.Server.Game.Items; +using NeonSprawl.Server.Tests; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Items; + +public class ItemDefinitionRegistryTests +{ + private static ItemDefinitionRegistry CreateRegistryFromRows(IReadOnlyDictionary byId) + { + var catalog = new ItemDefinitionCatalog("/tmp/catalog", byId, catalogJsonFileCount: 1); + return new ItemDefinitionRegistry(catalog); + } + + [Fact] + public void TryGetDefinition_ShouldReturnTrueAndExpectedMetadata_WhenIdExists() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["scrap_metal_bulk"] = new ItemDefRow( + "scrap_metal_bulk", + "Scrap Metal (Bulk)", + "material", + StackMax: 999, + InventorySlotKind: "bag", + Rarity: null, + BindPolicy: null, + DurabilityMax: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition("scrap_metal_bulk", out var def); + // Assert + Assert.True(found); + Assert.NotNull(def); + Assert.Equal("material", def.PrototypeRole); + Assert.Equal("Scrap Metal (Bulk)", def.DisplayName); + Assert.Equal(999, def.StackMax); + Assert.Equal("bag", def.InventorySlotKind); + } + + [Fact] + public void TryGetDefinition_ShouldReturnFalse_WhenItemIdIsNull() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["scrap_metal_bulk"] = new ItemDefRow( + "scrap_metal_bulk", + "Scrap Metal (Bulk)", + "material", + StackMax: 999, + InventorySlotKind: "bag", + Rarity: null, + BindPolicy: null, + DurabilityMax: 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) + { + ["scrap_metal_bulk"] = new ItemDefRow( + "scrap_metal_bulk", + "Scrap Metal (Bulk)", + "material", + StackMax: 999, + InventorySlotKind: "bag", + Rarity: null, + BindPolicy: null, + DurabilityMax: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition("not_a_real_item", out var def); + // Assert + Assert.False(found); + Assert.Null(def); + } + + [Fact] + public void GetDefinitionsInIdOrder_ShouldListAllRowsOrderedById_WhenMultipleItems() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + ["survey_drone_kit"] = new ItemDefRow( + "survey_drone_kit", + "Survey Drone Kit", + "utility", + StackMax: 1, + InventorySlotKind: "bag", + Rarity: null, + BindPolicy: null, + DurabilityMax: null), + ["scrap_metal_bulk"] = new ItemDefRow( + "scrap_metal_bulk", + "Scrap Metal (Bulk)", + "material", + StackMax: 999, + InventorySlotKind: "bag", + Rarity: null, + BindPolicy: null, + DurabilityMax: null), + ["prototype_armor_shell"] = new ItemDefRow( + "prototype_armor_shell", + "Prototype Armor Shell", + "equip_stub", + StackMax: 1, + InventorySlotKind: "equipment", + Rarity: null, + BindPolicy: null, + DurabilityMax: null), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var list = registry.GetDefinitionsInIdOrder(); + // Assert + Assert.Equal(3, list.Count); + Assert.Equal("prototype_armor_shell", list[0].Id); + Assert.Equal("scrap_metal_bulk", list[1].Id); + Assert.Equal("survey_drone_kit", list[2].Id); + } + + [Fact] + public void TryGetDefinition_ShouldMatchLoaderCatalog_WhenUsingPrototypeFixture() + { + // Arrange + var root = Directory.CreateTempSubdirectory("neon-sprawl-item-registry-loader-"); + try + { + var itemsDir = Path.Combine(root.FullName, "content", "items"); + var schemaDir = Path.Combine(root.FullName, "content", "schemas"); + Directory.CreateDirectory(itemsDir); + Directory.CreateDirectory(schemaDir); + var schemaPath = Path.Combine(schemaDir, "item-def.schema.json"); + File.Copy(ItemCatalogTestPaths.DiscoverRepoItemDefSchemaPath(), schemaPath, overwrite: true); + File.Copy( + Path.Combine(ItemCatalogTestPaths.DiscoverRepoItemsDirectory(), "prototype_items.json"), + Path.Combine(itemsDir, "prototype_items.json"), + overwrite: true); + var loaded = ItemDefinitionCatalogLoader.Load(itemsDir, schemaPath, NullLogger.Instance); + var registry = new ItemDefinitionRegistry(loaded); + // Act + var ok = registry.TryGetDefinition("prototype_armor_shell", out var equipStub); + // Assert + Assert.True(ok); + Assert.NotNull(equipStub); + Assert.Equal("equip_stub", equipStub.PrototypeRole); + Assert.Equal("equipment", equipStub.InventorySlotKind); + Assert.Equal(1, equipStub.StackMax); + } + 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("scrap_metal_bulk", out var scrap); + // Assert + Assert.True(found); + Assert.NotNull(scrap); + Assert.Equal("material", scrap.PrototypeRole); + Assert.Equal(999, scrap.StackMax); + } +} diff --git a/server/NeonSprawl.Server/Game/Items/IItemDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Items/IItemDefinitionRegistry.cs new file mode 100644 index 0000000..a843612 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Items/IItemDefinitionRegistry.cs @@ -0,0 +1,20 @@ +using System.Diagnostics.CodeAnalysis; + +namespace NeonSprawl.Server.Game.Items; + +/// +/// Read-only access to validated entries loaded at startup (). +/// +/// +/// E3.M3 (inventory / craft / gather): callers granting or validating items should depend on this interface +/// rather than so stack limits and slot kinds stay centralized. +/// NEO-53: HTTP/read-model projections should depend on this interface rather than reaching into the catalog. +/// +public interface IItemDefinitionRegistry +{ + /// Attempts to resolve an item by stable id (see item-def.schema.json). Unknown ids and null return false without throwing. + bool TryGetDefinition(string? itemId, [NotNullWhen(true)] out ItemDefRow? definition); + + /// Every loaded definition, ordered by (ordinal). + IReadOnlyList GetDefinitionsInIdOrder(); +} diff --git a/server/NeonSprawl.Server/Game/Items/ItemCatalogServiceCollectionExtensions.cs b/server/NeonSprawl.Server/Game/Items/ItemCatalogServiceCollectionExtensions.cs index 3a0c731..2b8aab5 100644 --- a/server/NeonSprawl.Server/Game/Items/ItemCatalogServiceCollectionExtensions.cs +++ b/server/NeonSprawl.Server/Game/Items/ItemCatalogServiceCollectionExtensions.cs @@ -8,7 +8,7 @@ namespace NeonSprawl.Server.Game.Items; /// DI registration for the fail-fast item catalog (NEO-51). public static class ItemCatalogServiceCollectionExtensions { - /// Binds and registers as a singleton. + /// Binds and registers and as singletons. public static IServiceCollection AddItemDefinitionCatalog(this IServiceCollection services, IConfiguration configuration) { services.AddOptions() @@ -30,6 +30,9 @@ public static class ItemCatalogServiceCollectionExtensions return ItemDefinitionCatalogLoader.Load(itemsDir, schemaPath, logger); }); + services.AddSingleton(sp => + new ItemDefinitionRegistry(sp.GetRequiredService())); + return services; } } diff --git a/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalog.cs b/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalog.cs index 1ee21ea..d3fe3fb 100644 --- a/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalog.cs +++ b/server/NeonSprawl.Server/Game/Items/ItemDefinitionCatalog.cs @@ -2,7 +2,7 @@ using System.Collections.ObjectModel; namespace NeonSprawl.Server.Game.Items; -/// In-memory item catalog loaded at startup (NEO-51). Game code should prefer injectable item registry for lookups (NEO-52). +/// In-memory item catalog loaded at startup (NEO-51). Game code should prefer for lookups (NEO-52). public sealed class ItemDefinitionCatalog( string itemsDirectory, IReadOnlyDictionary byId, diff --git a/server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs b/server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs new file mode 100644 index 0000000..c6af36a --- /dev/null +++ b/server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs @@ -0,0 +1,39 @@ +using System.Diagnostics.CodeAnalysis; + +namespace NeonSprawl.Server.Game.Items; + +/// Adapter over (NEO-52). +public sealed class ItemDefinitionRegistry(ItemDefinitionCatalog catalog) : IItemDefinitionRegistry +{ + /// + public bool TryGetDefinition(string? itemId, [NotNullWhen(true)] out ItemDefRow? definition) + { + if (itemId is null) + { + definition = null; + return false; + } + + if (catalog.ById.TryGetValue(itemId, 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 78b7002..85112fc 100644 --- a/server/README.md +++ b/server/README.md @@ -48,7 +48,7 @@ On startup the host loads every **`*_items.json`** under the items directory, va **Docker / CI:** include **`content/items`** and **`content/schemas/item-def.schema.json`** in the mounted **`content/`** tree; set **`Content__ItemsDirectory`** when layout differs. -On success, **Information** logs include the resolved items directory path, distinct item count, and catalog file count. Game code should use **`IItemDefinitionRegistry`** for lookups after **NEO-52**; until then use **`ItemDefinitionCatalog.TryGetItem`**. +On success, **Information** logs include the resolved items directory path, distinct item count, and catalog file count. Game code should use **`IItemDefinitionRegistry`** for lookups (NEO-52). ## Mastery catalog (`content/mastery`, NEO-46) From 7bd4064b93630b706a25fb31e49a677d1a73f70d Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 23 May 2026 18:16:17 -0400 Subject: [PATCH 3/4] NEO-52: Add code review for item definition registry + DI. --- docs/reviews/2026-05-23-NEO-52.md | 54 +++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 docs/reviews/2026-05-23-NEO-52.md diff --git a/docs/reviews/2026-05-23-NEO-52.md b/docs/reviews/2026-05-23-NEO-52.md new file mode 100644 index 0000000..50eb8e0 --- /dev/null +++ b/docs/reviews/2026-05-23-NEO-52.md @@ -0,0 +1,54 @@ +# Code review — NEO-52 item definition registry + DI + +**Date:** 2026-05-23 +**Scope:** Branch `NEO-52-item-definition-registry-di` · commits `1e9f1e9`–`792b38d` vs `origin/main` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-52 adds **`IItemDefinitionRegistry`** and **`ItemDefinitionRegistry`** as a thin adapter over the NEO-51 **`ItemDefinitionCatalog`**, registers both in **`AddItemDefinitionCatalog`**, and ships six AAA unit tests plus a host DI smoke test mirroring **`SkillDefinitionRegistry`** (NEO-35). Kickoff decisions (`TryGetDefinition`, `GetDefinitionsInIdOrder`, no extra eager resolve in `Program.cs`, no re-validation) are implemented as specified. Risk is low: no HTTP, persistence, or gameplay callers yet; enumeration allocates per call at prototype scale (same as skills). Tests pass locally. + +## Documentation checked + +| Document | Result | +|----------|--------| +| [`docs/plans/NEO-52-implementation-plan.md`](../plans/NEO-52-implementation-plan.md) | **Matches** — interface surface, adapter, DI, README, alignment table, test matrix; acceptance checklist complete. | +| [`docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) | **Partially matches** — frozen roster and NEO-51 server load documented; **no NEO-52 registry bullet** yet (see suggestion). | +| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M3 row notes NEO-52 landed with plan link. | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E3.M3 **In Progress** is correct; footnote names NEO-50/NEO-51 only (optional NEO-52 mention). | +| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **N/A** — read-only definition registry; no client trust boundary change. | +| [`server/README.md`](../../server/README.md) | **Matches** — item catalog section points game code at **`IItemDefinitionRegistry`**. | + +Register/tracking: **E3.M3** row update in alignment doc is appropriate; no module **Status** promotion required until HTTP/inventory slices land. + +## Blocking issues + +None. + +## Suggestions + +1. **E3.M3 module page — NEO-52 registry** — Under **Related implementation slices**, add a short bullet parallel to **Server load (NEO-51)** documenting **`IItemDefinitionRegistry`** in `Game/Items/` and that NEO-53+ / inventory callers should inject the interface (link [NEO-52 plan](../plans/NEO-52-implementation-plan.md)). Readers who open the module doc before the alignment register will otherwise only see catalog load. + +2. **Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-52** alongside NEO-50/NEO-51 so the register matches the alignment table. + +## Nits + +- Nit: **`Host_ShouldResolveRegistryFromDi_WhenStartupSucceeds`** places **`TryGetDefinition`** in **Act** (metadata assertions in **Assert**). Same pattern as **`SkillDefinitionRegistryTests`**; strictly, only **`GetRequiredService()`** belongs in Act — optional tidy for both registries later, not blocking. + +- Nit: **`GetDefinitionsInIdOrder`** rebuilds a sorted list on every call (keys sort + list materialization). Acceptable for six prototype rows and consistent with **`SkillDefinitionRegistry`**; revisit only if callers poll in hot paths. + +- Nit: **`ItemDefinitionRegistry.TryGetDefinition`** duplicates **`ItemDefinitionCatalog.ById`** lookup rather than delegating to **`TryGetItem`** — intentional per plan (game surface vs catalog helper); **`TryGetItem`** does not guard **`null`** ids while the registry does (good for public API). + +## Verification + +```bash +cd server +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~ItemDefinitionRegistryTests" +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj +``` + +Manual QA: not required for this story (no user-visible API); NEO-53 will exercise enumeration via HTTP. From 0d904527ac4cc37e1140007f6b500eccfabe8902 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 23 May 2026 18:17:27 -0400 Subject: [PATCH 4/4] NEO-52: address code review doc suggestions for registry Add NEO-52 registry bullet to E3.M3 module doc, extend dependency register footnote, and mark review suggestions done. --- .../modules/E3_M3_ItemizationAndInventorySchema.md | 2 ++ docs/decomposition/modules/module_dependency_register.md | 2 +- docs/reviews/2026-05-23-NEO-52.md | 8 ++++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md b/docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md index f502ff6..985b0ca 100644 --- a/docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md +++ b/docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md @@ -64,6 +64,8 @@ Epic 3 **Slice 1** — MVP inventory; `item_created`, transfer failures. **Server load (NEO-51):** On host startup, `server/NeonSprawl.Server/Game/Items/` loads `content/items/*_items.json` with the same validation gates as CI (`scripts/validate_content.py`) and **refuses to listen** when the catalog is invalid. Config and discovery: [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). Plan: [NEO-51 implementation plan](../../plans/NEO-51-implementation-plan.md). +**Item definition registry (NEO-52):** **`IItemDefinitionRegistry`** in `server/NeonSprawl.Server/Game/Items/` wraps the startup-loaded catalog for read-only lookup by stable `itemId` and ordered enumeration. **NEO-53+** (HTTP read model), **NEO-54+** (inventory engine), and future craft/gather callers should inject the interface rather than `ItemDefinitionCatalog`. Plan: [NEO-52 implementation plan](../../plans/NEO-52-implementation-plan.md). + **Linear backlog (decomposed):** [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md) — **E3M3-01** [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) (content + CI) through **E3M3-07** [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56) (telemetry hooks). ## Source anchors diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 6138a68..72f6c5c 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -48,7 +48,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen | E3.M4 | SinkAndDurabilityLifecycle | E3.M3, E8.M3 | DurabilityState, ItemSinkEvent, RepairCostRule | Pre-production | Planned | | E3.M5 | EconomyBalancePolicy | E3.M4, E9.M2 | EconomyPolicy, PriceBandRule, FaucetSinkRatio | Pre-production | Planned | -**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI) and **NEO-51** (server fail-fast load) moved the register row to **In Progress**; later slices update the alignment table as they land. +**E3.M3 note:** Epic 3 **Slice 1** backlog in Linear ([Epic 3 — Crafting, Gathering, and Itemization Economy](https://linear.app/neon-sprawl/project/epic-3-crafting-gathering-and-itemization-economy-65785ed05bc2)): [NEO-50](https://linear.app/neon-sprawl/issue/NEO-50) → [NEO-56](https://linear.app/neon-sprawl/issue/NEO-56); label **`E3.M3`**. See [E3M3-prototype-backlog.md](../../plans/E3M3-prototype-backlog.md), [E3_M3_ItemizationAndInventorySchema.md](E3_M3_ItemizationAndInventorySchema.md). **NEO-50** (content + CI), **NEO-51** (server fail-fast load), and **NEO-52** (`IItemDefinitionRegistry` + DI) moved the register row to **In Progress**; later slices update the alignment table as they land. ### Epic 4 — World Topology diff --git a/docs/reviews/2026-05-23-NEO-52.md b/docs/reviews/2026-05-23-NEO-52.md index 50eb8e0..50fe97d 100644 --- a/docs/reviews/2026-05-23-NEO-52.md +++ b/docs/reviews/2026-05-23-NEO-52.md @@ -17,9 +17,9 @@ NEO-52 adds **`IItemDefinitionRegistry`** and **`ItemDefinitionRegistry`** as a | Document | Result | |----------|--------| | [`docs/plans/NEO-52-implementation-plan.md`](../plans/NEO-52-implementation-plan.md) | **Matches** — interface surface, adapter, DI, README, alignment table, test matrix; acceptance checklist complete. | -| [`docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) | **Partially matches** — frozen roster and NEO-51 server load documented; **no NEO-52 registry bullet** yet (see suggestion). | +| [`docs/decomposition/modules/E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md) | **Matches** — frozen roster, NEO-51 server load, and NEO-52 registry bullet under **Related implementation slices**. | | [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E3.M3 row notes NEO-52 landed with plan link. | -| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E3.M3 **In Progress** is correct; footnote names NEO-50/NEO-51 only (optional NEO-52 mention). | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E3.M3 **In Progress**; footnote names NEO-50/NEO-51/NEO-52. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **N/A** — read-only definition registry; no client trust boundary change. | | [`server/README.md`](../../server/README.md) | **Matches** — item catalog section points game code at **`IItemDefinitionRegistry`**. | @@ -31,9 +31,9 @@ None. ## Suggestions -1. **E3.M3 module page — NEO-52 registry** — Under **Related implementation slices**, add a short bullet parallel to **Server load (NEO-51)** documenting **`IItemDefinitionRegistry`** in `Game/Items/` and that NEO-53+ / inventory callers should inject the interface (link [NEO-52 plan](../plans/NEO-52-implementation-plan.md)). Readers who open the module doc before the alignment register will otherwise only see catalog load. +1. ~~**E3.M3 module page — NEO-52 registry** — Under **Related implementation slices**, add a short bullet parallel to **Server load (NEO-51)** documenting **`IItemDefinitionRegistry`** in `Game/Items/` and that NEO-53+ / inventory callers should inject the interface (link [NEO-52 plan](../plans/NEO-52-implementation-plan.md)). Readers who open the module doc before the alignment register will otherwise only see catalog load.~~ **Done.** — bullet added to [`E3_M3_ItemizationAndInventorySchema.md`](../decomposition/modules/E3_M3_ItemizationAndInventorySchema.md). -2. **Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-52** alongside NEO-50/NEO-51 so the register matches the alignment table. +2. ~~**Dependency register footnote (optional)** — Extend the **E3.M3 note** in [`module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) to mention **NEO-52** alongside NEO-50/NEO-51 so the register matches the alignment table.~~ **Done.** ## Nits