don
/
neon-sprawl
mirror of https://github.com/ViPro-Technologies/neon-sprawl
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

NEO-115: address code review findings for quest definitions GET. 

Derive expected quest id order from PrototypeE7M1QuestCatalogRules,
assert refine craft_recipe objective in integration test, and update
module dependency register plus review doc strikethroughs.
pull/154/head
VinPropane 2026-06-03 22:22:44 -04:00
parent 31ffee1f7a
commit ab315f326b
3 changed files with 15 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
docs/decomposition/modules/module_dependency_register.md
View File

@ -109,7 +109,7 @@ Gameplay **content and curves** default to **boot load** with optional **dev-onl
|---|---|---|---|---|---|
| E7.M1 | QuestStateMachine | E3.M2, E5.M1 | QuestDef, QuestStepState, QuestStateTransition | Prototype | Planned |
**E7.M1 note:** Epic 7 **Slice 1** backlog in Linear ([Epic 7 — Questing, Narrative, and Faction Reputation](https://linear.app/neon-sprawl/project/epic-7-questing-narrative-and-faction-reputation-a9416783ceee)): [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) → [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123); label **`E7.M1`**. See [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md), [E7_M1_QuestStateMachine.md](E7_M1_QuestStateMachine.md). Upstream: E3.M1 gather **Ready**, E3.M2 craft **Ready**, E5.M3 encounter **Ready**. Client capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123). **E7M1-01 / NEO-112** catalog landed (schemas + CI); **E7M1-02 / NEO-113** server load landed; **E7M1-03 / NEO-114** registry landed ([NEO-114 plan](../../plans/NEO-114-implementation-plan.md)); register row stays **Planned** until quest runtime (E7M1-05+) lands.
**E7.M1 note:** Epic 7 **Slice 1** backlog in Linear ([Epic 7 — Questing, Narrative, and Faction Reputation](https://linear.app/neon-sprawl/project/epic-7-questing-narrative-and-faction-reputation-a9416783ceee)): [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) → [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123); label **`E7.M1`**. See [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md), [E7_M1_QuestStateMachine.md](E7_M1_QuestStateMachine.md). Upstream: E3.M1 gather **Ready**, E3.M2 craft **Ready**, E5.M3 encounter **Ready**. Client capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123). **E7M1-01 / NEO-112** catalog landed (schemas + CI); **E7M1-02 / NEO-113** server load landed; **E7M1-03 / NEO-114** registry landed ([NEO-114 plan](../../plans/NEO-114-implementation-plan.md)); **E7M1-04 / NEO-115** HTTP read landed ([NEO-115 plan](../../plans/NEO-115-implementation-plan.md)); register row stays **Planned** until quest runtime (E7M1-05+) lands.
| E7.M2 | RewardAndUnlockRouter | E2.M2, E3.M3, E7.M1 | QuestRewardBundle, UnlockGrant, RewardDeliveryEvent | Prototype | Planned |
| E7.M3 | FactionReputationLedger | E7.M1 | FactionStanding, ReputationDelta, FactionGateRule | Pre-production | Planned |
| E7.M4 | ContractMissionGenerator | E4.M1, E5.M3, E7.M3 | ContractTemplate, ContractSeed, ContractOutcome | Pre-production | Planned |

8
docs/reviews/2026-06-03-NEO-115.md
View File

@ -20,7 +20,7 @@ NEO-115 adds **`GET /game/world/quest-definitions`**, a versioned read-only proj
| `docs/plans/E7M1-prototype-backlog.md` (E7M1-04) | **Matches** — AC checked; in/out of scope aligned with shipped route. |
| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Matches** — HTTP read model bullet documents NEO-115 route + Bruno. |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7.M1 row notes NEO-115 HTTP projection; register stays **Planned** until E7M1-05+ runtime. |
| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — E7.M1 note still ends at NEO-114 registry; alignment doc covers NEO-115 (see Suggestion 2). |
| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E7.M1 note includes NEO-115 HTTP read (Suggestion 2 addressed). |
| `server/README.md` | **Matches** — quest definitions section with curl, registry note, plan/Bruno links. |
| Full-stack epic decomposition | **N/A** — server-only; client counterpart NEO-122 explicitly deferred. |
@ -30,13 +30,13 @@ NEO-115 adds **`GET /game/world/quest-definitions`**, a versioned read-only proj
## Suggestions
1. **Reuse frozen id ordering constant in tests** — Plan adopted reusing **`PrototypeE7M1QuestCatalogRules`** for ids. `QuestDefinitionsWorldApiTests` defines a parallel **`FrozenQuestIdsInOrdinalOrder`** array instead of deriving from **`ExpectedQuestIds.Order(StringComparer.Ordinal)`** (as **`QuestDefinitionRegistryTests`** already does). Consolidating avoids drift if the roster changes.
1. ~~**Reuse frozen id ordering constant in tests** — Plan adopted reusing **`PrototypeE7M1QuestCatalogRules`** for ids. `QuestDefinitionsWorldApiTests` defines a parallel **`FrozenQuestIdsInOrdinalOrder`** array instead of deriving from **`ExpectedQuestIds.Order(StringComparer.Ordinal)`** (as **`QuestDefinitionRegistryTests`** already does). Consolidating avoids drift if the roster changes.~~ **Done.** `QuestDefinitionsWorldApiTests` now asserts id order via **`ExpectedQuestIds.Order(StringComparer.Ordinal)`**.
2. **Optional: refresh `module_dependency_register` E7.M1 note** — E7.M1 module page and alignment register mention NEO-115; the register table note still stops at NEO-114. A one-line append (“**E7M1-04 / NEO-115** HTTP read landed”) would keep the dependency register consistent with NEO-114 follow-up style.
2. ~~**Optional: refresh `module_dependency_register` E7.M1 note** — E7.M1 module page and alignment register mention NEO-115; the register table note still stops at NEO-114. A one-line append (“**E7M1-04 / NEO-115** HTTP read landed”) would keep the dependency register consistent with NEO-114 follow-up style.~~ **Done.** E7.M1 note appended in **`module_dependency_register.md`**.
## Nits
- Nit: C# integration test asserts **`refine_intro`** prerequisite only, not the **`craft_recipe`** objective shape; Bruno likewise skips refine/combat spot-checks. Adequate for prototype freeze given gather + operator-chain + combat coverage, but a single **`craft_recipe`** assert would close the objective-kind loop in C#.
- ~~Nit: C# integration test asserts **`refine_intro`** prerequisite only, not the **`craft_recipe`** objective shape; Bruno likewise skips refine/combat spot-checks. Adequate for prototype freeze given gather + operator-chain + combat coverage, but a single **`craft_recipe`** assert would close the objective-kind loop in C#.~~ **Done.** `refine_intro` **`craft_recipe`** objective asserted in **`QuestDefinitionsWorldApiTests`**.
- Nit: **`MapObjective`** default switch arm copies all optional fields for unknown kinds — unreachable for validated catalog content but reasonable defensive fallback.

20
server/NeonSprawl.Server.Tests/Game/Quests/QuestDefinitionsWorldApiTests.cs
View File

@ -7,15 +7,6 @@ namespace NeonSprawl.Server.Tests.Game.Quests;
public class QuestDefinitionsWorldApiTests
{
/// <summary>Frozen prototype quest ids in ascending ordinal order. Keep in sync with Bruno.</summary>
public static readonly string[] FrozenQuestIdsInOrdinalOrder =
[
"prototype_quest_combat_intro",
"prototype_quest_gather_intro",
"prototype_quest_operator_chain",
"prototype_quest_refine_intro",
];
[Fact]
public async Task GetQuestDefinitions_ShouldReturnSchemaV1_WithFourFrozenQuestsInIdOrder()
{
@ -31,7 +22,9 @@ public class QuestDefinitionsWorldApiTests
Assert.Equal(QuestDefinitionsListResponse.CurrentSchemaVersion, body!.SchemaVersion);
Assert.NotNull(body.Quests);
Assert.Equal(4, body.Quests.Count);
Assert.Equal(FrozenQuestIdsInOrdinalOrder, body.Quests.Select(q => q.Id).ToArray());
Assert.Equal(
PrototypeE7M1QuestCatalogRules.ExpectedQuestIds.Order(StringComparer.Ordinal).ToArray(),
body.Quests.Select(q => q.Id).ToArray());
var gatherIntro = body.Quests.Single(q => q.Id == "prototype_quest_gather_intro");
Assert.Equal("Intro: Salvage Run", gatherIntro.DisplayName);
@ -51,6 +44,13 @@ public class QuestDefinitionsWorldApiTests
var refineIntro = body.Quests.Single(q => q.Id == "prototype_quest_refine_intro");
Assert.Equal(["prototype_quest_gather_intro"], refineIntro.PrerequisiteQuestIds);
var refineObjective = refineIntro.Steps[0].Objectives[0];
Assert.Equal("refine_intro_obj_recipe", refineObjective.Id);
Assert.Equal("craft_recipe", refineObjective.Kind);
Assert.Equal("refine_scrap_standard", refineObjective.RecipeId);
Assert.Equal(1, refineObjective.Quantity);
Assert.Null(refineObjective.ItemId);
Assert.Null(refineObjective.EncounterId);
var combatIntro = body.Quests.Single(q => q.Id == "prototype_quest_combat_intro");
Assert.Single(combatIntro.Steps);