NEO-114: Add kickoff implementation plan for quest definition registry.
parent
d236aee261
commit
9056a9b005
|
|
@ -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<IQuestDefinitionRegistry>(sp => new QuestDefinitionRegistry(sp.GetRequiredService<QuestDefinitionCatalog>()));`
|
||||
- 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** |
|
||||
Loading…
Reference in New Issue