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

102 lines
8.7 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters!

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

# NEO-59 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-59 |
| **Title** | E3.M1: Resource node definition registry + DI |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-59/e3m1-resource-node-definition-registry-di |
| **Module** | [E3.M1 — ResourceNodeAndGatherLoop](../decomposition/modules/E3_M1_ResourceNodeAndGatherLoop.md) · Epic 3 Slice 2 · backlog **E3M1-03** |
| **Branch** | `NEO-59-e3m1-resource-node-definition-registry-di` |
| **Precursor** | [NEO-58](https://linear.app/neon-sprawl/issue/NEO-58) — fail-fast `ResourceNodeCatalog` load (**merged** to `main` via PR #93) |
| **Pattern** | [NEO-52](https://linear.app/neon-sprawl/issue/NEO-52) — thin registry adapter over startup catalog + DI; strict split after NEO-58 loader |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **Enumeration API** | Include `GetDefinitionsInIdOrder()` now? | **Yes** — mirror `IItemDefinitionRegistry` / `ISkillDefinitionRegistry`; [NEO-60](https://linear.app/neon-sprawl/issue/NEO-60) `GET /game/world/resource-node-definitions` will need ordered defs without reaching into `ResourceNodeCatalog`. | **User:** include enumeration. |
| **Yield access** | How should yield rows be exposed? | **Separate `TryGetYield(nodeDefId)`** alongside `TryGetDefinition` — mirrors `ResourceNodeCatalog.TryGetNode` / `TryGetYield`; backlog E3M1-03 tests call for yield metadata + capacity (`maxGathers`) fields. | **User:** separate `TryGetYield`. |
| **Lookup naming** | `TryGetDefinition` vs `TryGetNode`? | **`TryGetDefinition(string? nodeDefId, …)`** — same naming as item/skill registries; catalog keeps `TryGetNode` for direct catalog access. | **Adopted** — NEO-52 / NEO-58 precedent. |
| **Program.cs eager resolve** | Eager-resolve `IResourceNodeDefinitionRegistry` at boot? | **Omit**`ResourceNodeCatalog` is already eager-resolved in `Program.cs` (NEO-58); registry is a thin adapter (NEO-52 default). | **Adopted** |
| **Runtime validation** | Re-validate nodes/yields in registry? | **No** — catalog load is fail-fast (NEO-58); registry delegates to loaded rows only. | **Adopted** |
## Goal, scope, and out-of-scope
**Goal:** Provide **`IResourceNodeDefinitionRegistry`** backed by the startup-loaded **`ResourceNodeCatalog`**: resolve node defs and yield rows by stable **`nodeDefId`**, enumerate node defs in id order, expose prototype metadata (`displayName`, `gatherLens`, `maxGathers`, yield `itemId` / `quantity`). Register in DI so **NEO-60+** (HTTP), **NEO-61** (depletion), **NEO-62** (gather engine), and interact wiring depend on the interface instead of the catalog type.
**In scope (from Linear + [E3M1-03](E3M1-prototype-backlog.md#e3m1-03--resource-node-definition-registry--di)):**
- `ResourceNodeDefinitionRegistry` thin adapter over `ResourceNodeCatalog`.
- DI registration alongside existing catalog singleton.
- Unit tests (AAA): known prototype id lookup + node metadata + yield metadata; unknown `nodeDefId` returns false without throwing; enumeration order; host resolves registry from DI.
**Out of scope (from Linear + backlog):**
- HTTP ([NEO-60](https://linear.app/neon-sprawl/issue/NEO-60)).
- Persistence of depletion state ([NEO-61](https://linear.app/neon-sprawl/issue/NEO-61)).
- Gather engine / inventory mutation ([NEO-62](https://linear.app/neon-sprawl/issue/NEO-62)).
- Changing loader, Slice 2 gate, or catalog load semantics (NEO-58).
## Acceptance criteria checklist
- [x] DI resolves `IResourceNodeDefinitionRegistry` at host startup (test via `InMemoryWebApplicationFactory`).
- [x] Unit tests (AAA): lookup for known prototype `nodeDefId` (e.g. `prototype_resource_node_alpha`) with expected node metadata (`gatherLens`, `maxGathers`) and yield metadata (`itemId`, `quantity`).
- [x] Unit tests (AAA): unknown `nodeDefId` returns false / absent without throwing (both node and yield lookups).
- [x] `GetDefinitionsInIdOrder` returns all loaded node defs ordered by `nodeDefId` (ordinal).
## Technical approach
1. **`IResourceNodeDefinitionRegistry`** — mirror [`IItemDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Items/IItemDefinitionRegistry.cs):
- `TryGetDefinition(string? nodeDefId, [NotNullWhen(true)] out ResourceNodeDefRow? definition)` — null and unknown ids return false without throwing.
- `TryGetYield(string? nodeDefId, [NotNullWhen(true)] out ResourceYieldRow? yield)` — same contract for yield rows.
- `GetDefinitionsInIdOrder()` — all node defs ordered by `ResourceNodeDefRow.NodeDefId` (ordinal).
- XML remarks: gather engine / depletion / interact callers (NEO-61+) should use this interface; HTTP read model (NEO-60) should not reach into `ResourceNodeCatalog`.
2. **`ResourceNodeDefinitionRegistry`** — primary-constructor adapter over `ResourceNodeCatalog` (same pattern as [`ItemDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Items/ItemDefinitionRegistry.cs)).
3. **DI** — extend [`AddResourceNodeCatalog`](../../server/NeonSprawl.Server/Game/Gathering/ResourceNodeCatalogServiceCollectionExtensions.cs) to register `IResourceNodeDefinitionRegistry``ResourceNodeDefinitionRegistry` after catalog registration. No change to `Program.cs` eager-resolve (catalog only).
4. **Comments** — update `ResourceNodeCatalog` summary to point at `IResourceNodeDefinitionRegistry`; update [server README](../../server/README.md) resource-node catalog section (currently says “lands in NEO-59”).
5. **Tests** — new `ResourceNodeDefinitionRegistryTests.cs` mirroring [`ItemDefinitionRegistryTests`](../../server/NeonSprawl.Server.Tests/Game/Items/ItemDefinitionRegistryTests.cs): in-memory catalog helper, loader-backed prototype fixture via `ResourceNodeCatalogTestPaths`, host DI test.
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Gathering/IResourceNodeDefinitionRegistry.cs` | Public contract: node lookup, yield lookup, enumerate, remarks for NEO-60/61/62 callers. |
| `server/NeonSprawl.Server/Game/Gathering/ResourceNodeDefinitionRegistry.cs` | Singleton adapter over `ResourceNodeCatalog`. |
| `server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeDefinitionRegistryTests.cs` | AAA unit + host DI tests (mirror `ItemDefinitionRegistryTests`). |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Gathering/ResourceNodeCatalogServiceCollectionExtensions.cs` | Register `IResourceNodeDefinitionRegistry``ResourceNodeDefinitionRegistry` after catalog singleton. |
| `server/NeonSprawl.Server/Game/Gathering/ResourceNodeCatalog.cs` | Tighten summary comment to reference `IResourceNodeDefinitionRegistry` (replace “Prefer registry in NEO-59” placeholder). |
| `server/README.md` | Resource-node catalog section: document `IResourceNodeDefinitionRegistry` as preferred lookup surface. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E3.M1 row — note NEO-59 registry when landed. |
## Tests
| Test file | What it covers |
|-----------|----------------|
| `server/NeonSprawl.Server.Tests/Game/Gathering/ResourceNodeDefinitionRegistryTests.cs` | **Unit:** `TryGetDefinition` for `prototype_resource_node_alpha` returns true with `GatherLens`, `MaxGathers`, `DisplayName`; `TryGetYield` for same id returns true with `ItemId` (`scrap_metal_bulk`) and `Quantity`; null and unknown id return false without throw for both methods; `GetDefinitionsInIdOrder` count (4) and ordinal order for multi-node fixture; loader-backed prototype fixture matches catalog. **Host:** `InMemoryWebApplicationFactory` resolves `IResourceNodeDefinitionRegistry` and finds `prototype_resource_node_alpha` with expected yield. |
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Catalog vs registry for game code** | New callers inject `IResourceNodeDefinitionRegistry`; leave `ResourceNodeCatalog` eager-resolve in `Program.cs` for fail-fast only. | **adopted** |
| **Duplicate API on catalog** | Keep `ResourceNodeCatalog.TryGetNode` / `TryGetYield` for loader/tests; registry is the game-facing surface (NEO-52 precedent). | **adopted** |
| **NEO-60 HTTP projection** | API layer joins node + yield via registrys two TryGet methods when building DTOs; no combined read model in NEO-59. | **adopted** |
## Decisions (kickoff)
- **`GetDefinitionsInIdOrder()` included** — enumeration for NEO-60 HTTP without catalog coupling.
- **Separate `TryGetYield`** — yield metadata available to gather engine / tests without a combined DTO.
- **`TryGetDefinition` naming** — consistent with item/skill registries; parameter is `nodeDefId`.
- **No `Program.cs` registry eager-resolve** — catalog fail-fast only (NEO-52 pattern).