From 9056a9b0051e7ec71946988323cf85ce65bbbfe8 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 1 Jun 2026 21:36:07 -0400 Subject: [PATCH] NEO-114: Add kickoff implementation plan for quest definition registry. --- docs/plans/NEO-114-implementation-plan.md | 102 ++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 docs/plans/NEO-114-implementation-plan.md diff --git a/docs/plans/NEO-114-implementation-plan.md b/docs/plans/NEO-114-implementation-plan.md new file mode 100644 index 0000000..516d673 --- /dev/null +++ b/docs/plans/NEO-114-implementation-plan.md @@ -0,0 +1,102 @@ +# NEO-114 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-114 | +| **Title** | E7M1-03: Injectable quest definition registry + DI | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-114/e7m1-03-injectable-quest-definition-registry-di | +| **Module** | [E7.M1 — QuestStateMachine](../decomposition/modules/E7_M1_QuestStateMachine.md) · Epic 7 Slice 1 · backlog **E7M1-03** | +| **Branch** | `NEO-114-e7m1-injectable-quest-definition-registry-di` | +| **Precursor** | [NEO-113](https://linear.app/neon-sprawl/issue/NEO-113) — fail-fast quest catalog load (**landed on `main`**) | +| **Pattern** | [NEO-102](NEO-102-implementation-plan.md) / [NEO-52](NEO-52-implementation-plan.md) — thin registry adapter over startup catalog + DI; strict split after NEO-113 loader | +| **Blocks** | [NEO-115](https://linear.app/neon-sprawl/issue/NEO-115) — GET `/game/world/quest-definitions`; E7M1-05+ quest runtime | +| **Client counterpart** | None — server-only registry surface; player-visible work starts at [NEO-122](https://linear.app/neon-sprawl/issue/NEO-122) / [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Registry API surface** | Include `TryNormalizeKnown` + `GetDefinitionsInIdOrder` on `IQuestDefinitionRegistry`? | **Yes** — mirror [NEO-102](NEO-102-implementation-plan.md) / `IEncounterDefinitionRegistry`; [NEO-115](https://linear.app/neon-sprawl/issue/NEO-115) (E7M1-04 HTTP) needs enumeration; accept flow will need fail-closed id normalization. | **Adopted** — user chose full surface. | +| **Lookup naming** | `TryGetDefinition` vs `TryGetQuest`? | **`TryGetDefinition`** — item/skill/ability/recipe/encounter precedent; catalog keeps `TryGetQuest`. | **Adopted** | +| **DI wiring** | Separate `AddQuestDefinitionRegistry` extension vs extend catalog registration? | **Extend `AddQuestDefinitionCatalog`** — register `IQuestDefinitionRegistry` after `QuestDefinitionCatalog` singleton (same file as NEO-113). | **Adopted** | +| **Test scope** | Registry unit tests only, or also host DI resolve? | **Both** — mirror NEO-102; NEO-113 already covers catalog boot. | **Adopted** | +| **Program.cs eager resolve** | Eager-resolve registry at boot? | **Omit** — catalog singleton already eager-resolved (NEO-113); registry is thin adapter. | **Adopted** | +| **Call site migration** | Migrate existing consumers off `QuestDefinitionCatalog`? | **None this story** — no game code injects quest catalog yet; HTTP is NEO-115. | **Adopted** | + +## Goal, scope, and out-of-scope + +**Goal:** Provide **`IQuestDefinitionRegistry`** backed by the startup-loaded **`QuestDefinitionCatalog`**: resolve by stable quest id, enumerate definitions in id order, and expose **`TryNormalizeKnown`** for fail-closed id validation. Register in DI so **NEO-115+** (HTTP, player quest state, accept/advance engines) depend on the interface instead of the catalog type. + +**In scope (from Linear + [E7M1-03](E7M1-prototype-backlog.md#e7m1-03--injectable-quest-definition-registry--di)):** + +- `IQuestDefinitionRegistry` + `QuestDefinitionRegistry` thin adapter over NEO-113 catalog. +- DI registration in `AddQuestDefinitionCatalog`. +- Unit tests (AAA): all four frozen quest ids resolve with expected metadata; unknown id / null miss without throwing; `TryNormalizeKnown` trim/case/whitespace behavior; enumeration order; host resolves registry from DI. + +**Out of scope (from Linear + backlog):** + +- HTTP routes ([NEO-115](https://linear.app/neon-sprawl/issue/NEO-115) / E7M1-04). +- Player quest state, accept/advance hooks (E7M1-05+). +- Changing loader, E7M1 gates, or catalog load semantics (NEO-113). +- Godot / client changes. + +## Acceptance criteria checklist + +- [ ] Registry resolves all four frozen quest ids at runtime (`PrototypeE7M1QuestCatalogRules.ExpectedQuestIds`). +- [ ] Unknown quest id fails closed (`TryGetDefinition` / `TryNormalizeKnown` return false; no throw). + +## Technical approach + +1. **`IQuestDefinitionRegistry`** — mirror [`IEncounterDefinitionRegistry`](../../server/NeonSprawl.Server/Game/Encounters/IEncounterDefinitionRegistry.cs): + - `TryGetDefinition(string? questId, [NotNullWhen(true)] out QuestDefRow? definition)` — null and unknown ids return false without throwing. + - `TryNormalizeKnown(string? rawQuestId, [NotNullWhen(true)] out string normalized)` — trim + lowercase + catalog lookup; null/empty/whitespace return false. + - `GetDefinitionsInIdOrder()` — all rows ordered by `QuestDefRow.Id` (ordinal). + - XML remarks: quest runtime / accept callers (NEO-116+) and HTTP read model (NEO-115) should use this interface, not `QuestDefinitionCatalog`. + +2. **`QuestDefinitionRegistry`** — primary-constructor adapter delegating to `QuestDefinitionCatalog.TryGetQuest`; cache `GetDefinitionsInIdOrder` at construction (NEO-102 / NEO-79 pattern). + +3. **DI** — extend [`QuestCatalogServiceCollectionExtensions.AddQuestDefinitionCatalog`](../../server/NeonSprawl.Server/Game/Quests/QuestCatalogServiceCollectionExtensions.cs): + - `services.AddSingleton(sp => new QuestDefinitionRegistry(sp.GetRequiredService()));` + - Update extension XML summary to document both catalog + registry registration. + - No change to `Program.cs` eager-resolve (catalog only). + +4. **Comments / docs** — `QuestDefinitionCatalog` summary already references NEO-114; update [server README](../../server/README.md) quest catalog section to document registry as preferred lookup surface (replace “will be” placeholder with concrete method list, mirror encounter section). + +5. **Tests** — new `QuestDefinitionRegistryTests.cs` mirroring [`EncounterDefinitionRegistryTests`](../../server/NeonSprawl.Server.Tests/Game/Encounters/EncounterDefinitionRegistryTests.cs): + - **Unit (in-memory catalog fixture):** each of four `ExpectedQuestIds` resolves via `TryGetDefinition` with non-empty `DisplayName` and step count smoke; null/unknown/empty `TryGetDefinition` false; `TryNormalizeKnown` success for trimmed mixed-case known id; false for null/whitespace/unknown; `GetDefinitionsInIdOrder` length 4 and ordinal id order. + - **Host:** `InMemoryWebApplicationFactory` + `/health` + DI resolve `IQuestDefinitionRegistry`; `TryGetDefinition("prototype_quest_gather_intro")` true; unknown id false; `TryNormalizeKnown` on a frozen id. + +6. **Alignment register** — when complete, note NEO-114 registry in `documentation_and_implementation_alignment.md` E7.M1 row (registry landed; runtime still Planned until E7M1-05+). + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Quests/IQuestDefinitionRegistry.cs` | Public contract: lookup, normalize, enumerate. | +| `server/NeonSprawl.Server/Game/Quests/QuestDefinitionRegistry.cs` | Thin adapter over `QuestDefinitionCatalog`. | +| `server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionRegistryTests.cs` | AAA unit + host DI tests for registry. | +| `docs/plans/NEO-114-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Quests/QuestCatalogServiceCollectionExtensions.cs` | Register `IQuestDefinitionRegistry` after catalog singleton; update doc comment. | +| `server/README.md` | Quest catalog section — registry is preferred lookup surface (methods + normalization note). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M1 row — note NEO-114 registry when complete. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionRegistryTests.cs` | **Unit:** `TryGetDefinition` true + metadata for each `PrototypeE7M1QuestCatalogRules.ExpectedQuestIds` member (at least `displayName` + `Steps.Count` smoke on gather intro and operator chain); false for null/unknown. **Unit:** `TryNormalizeKnown` trim/case success on known id; false for null/whitespace/unknown. **Unit:** `GetDefinitionsInIdOrder` returns 4 rows in ordinal id order. **Host:** `InMemoryWebApplicationFactory` resolves `IQuestDefinitionRegistry`; frozen id lookup; unknown + normalize failures. | + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Case sensitivity on raw lookup** | `TryGetDefinition` is case-sensitive on catalog keys; wire validation uses `TryNormalizeKnown` (document in README like encounters). | **adopted** | +| **Constants for tests** | Reuse `PrototypeE7M1QuestCatalogRules.ExpectedQuestIds` / named ids in tests; avoid duplicating id strings. | **adopted** | +| **Registry vs catalog injection in new code** | No new `QuestDefinitionCatalog` injections this repo pass; enforcement via XML + README only until analyzers exist. | **adopted** |