diff --git a/docs/plans/E7M1-prototype-backlog.md b/docs/plans/E7M1-prototype-backlog.md index 85c4278..04aec38 100644 --- a/docs/plans/E7M1-prototype-backlog.md +++ b/docs/plans/E7M1-prototype-backlog.md @@ -128,13 +128,13 @@ Working backlog for **Epic 7 — Slice 1** ([quest core and persistence](../deco ### E7M1-03 — Injectable quest definition registry + DI -**Goal:** **`IQuestDefinitionRegistry`** with **`TryGet`**, startup registration from E7M1-02 catalog. +**Goal:** **`IQuestDefinitionRegistry`** with **`TryGetDefinition`**, **`TryNormalizeKnown`**, and **`GetDefinitionsInIdOrder`**, startup registration from E7M1-02 catalog. **In scope** -- Interface + in-memory implementation; **`AddQuestDefinitionRegistry`** DI extension. -- Unit tests: lookup known ids, unknown id returns false. -- Wire in host startup alongside other content registries. +- Interface + in-memory implementation; extend **`AddQuestDefinitionCatalog`** to register **`IQuestDefinitionRegistry`** after the catalog singleton. +- Unit tests: lookup known ids; unknown / null / empty id returns false; normalize trim/case; enumeration order; host DI resolve. +- Wire registry in **`AddQuestDefinitionCatalog`** alongside catalog load (catalog remains eager boot surface). **Out of scope** diff --git a/docs/reviews/2026-06-01-NEO-114.md b/docs/reviews/2026-06-01-NEO-114.md index 784cbe9..c4058cd 100644 --- a/docs/reviews/2026-06-01-NEO-114.md +++ b/docs/reviews/2026-06-01-NEO-114.md @@ -17,7 +17,7 @@ NEO-114 adds **`IQuestDefinitionRegistry`** and **`QuestDefinitionRegistry`**, a | Path | Result | |------|--------| | `docs/plans/NEO-114-implementation-plan.md` | **Matches** — kickoff decisions adopted; acceptance checklist checked; reconciliation accurate. | -| `docs/plans/E7M1-prototype-backlog.md` (E7M1-03) | **Partially matches** — AC checked; **In scope** bullets still say `TryGet` and `AddQuestDefinitionRegistry` (kickoff chose `TryGetDefinition` and extend `AddQuestDefinitionCatalog`). | +| `docs/plans/E7M1-prototype-backlog.md` (E7M1-03) | **Matches** — AC checked; **In scope** wording aligned with shipped API (NEO-114 review follow-up). | | `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Matches** — Status notes E7M1-03 / NEO-114 registry landed; runtime from E7M1-04. | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M1 row notes NEO-114 registry; register stays Planned until E7M1-05+ runtime. | | `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E7.M1 note updated for NEO-114 registry landed. | @@ -30,9 +30,9 @@ NEO-114 adds **`IQuestDefinitionRegistry`** and **`QuestDefinitionRegistry`**, a ## Suggestions -1. **Refresh E7M1-03 backlog wording** — Update `docs/plans/E7M1-prototype-backlog.md` **In scope** to match shipped API: `TryGetDefinition` (not `TryGet`), extend `AddQuestDefinitionCatalog` (not separate `AddQuestDefinitionRegistry`), and optional mention of `TryNormalizeKnown` / `GetDefinitionsInIdOrder` per the implementation plan. +1. ~~**Refresh E7M1-03 backlog wording** — Update `docs/plans/E7M1-prototype-backlog.md` **In scope** to match shipped API: `TryGetDefinition` (not `TryGet`), extend `AddQuestDefinitionCatalog` (not separate `AddQuestDefinitionRegistry`), and optional mention of `TryNormalizeKnown` / `GetDefinitionsInIdOrder` per the implementation plan.~~ **Done.** -2. **`TryGetDefinition` empty-string test** — Implementation plan test table lists false for null/unknown/**empty**; tests cover null and unknown but not `""`. Encounter registry has the same gap; adding one unit test would close the plan loop and guard accidental empty-key catalog entries. +2. ~~**`TryGetDefinition` empty-string test** — Implementation plan test table lists false for null/unknown/**empty**; tests cover null and unknown but not `""`. Encounter registry has the same gap; adding one unit test would close the plan loop and guard accidental empty-key catalog entries.~~ **Done.** — `TryGetDefinition_ShouldReturnFalse_WhenQuestIdIsEmpty`. ## Nits @@ -58,4 +58,4 @@ dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \ dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj ``` -**Reviewer note:** All 14 `QuestDefinitionRegistryTests` passed locally during review. +**Reviewer note:** All 14 `QuestDefinitionRegistryTests` passed locally during review. After follow-up: 15 tests (empty-string `TryGetDefinition` case). diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionRegistryTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionRegistryTests.cs index f67cf48..671f6f6 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionRegistryTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionRegistryTests.cs @@ -120,6 +120,22 @@ public class QuestDefinitionRegistryTests Assert.Null(def); } + [Fact] + public void TryGetDefinition_ShouldReturnFalse_WhenQuestIdIsEmpty() + { + // Arrange + var rows = new Dictionary(StringComparer.Ordinal) + { + [GatherIntroId] = CreateGatherIntroRow(), + }; + var registry = CreateRegistryFromRows(rows); + // Act + var found = registry.TryGetDefinition(string.Empty, out var def); + // Assert + Assert.False(found); + Assert.Null(def); + } + [Fact] public void TryGetDefinition_ShouldReturnFalse_WhenIdUnknown() {