neon-sprawl/docs/plans/NEO-52-implementation-plan.md

93 lines
7.1 KiB
Markdown

# 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
- [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
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.