From 44ef5f24848b812729c95ff04bef5a098653d5f2 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 6 Jun 2026 19:23:55 -0400 Subject: [PATCH 1/5] NEO-117: Add kickoff implementation plan for QuestStateOperations. --- docs/plans/NEO-117-implementation-plan.md | 145 ++++++++++++++++++++++ 1 file changed, 145 insertions(+) create mode 100644 docs/plans/NEO-117-implementation-plan.md diff --git a/docs/plans/NEO-117-implementation-plan.md b/docs/plans/NEO-117-implementation-plan.md new file mode 100644 index 0000000..0606930 --- /dev/null +++ b/docs/plans/NEO-117-implementation-plan.md @@ -0,0 +1,145 @@ +# NEO-117 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-117 | +| **Title** | E7M1-06: QuestStateOperations (accept, step advance, complete) | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-117/e7m1-06-queststateoperations-accept-step-advance-complete | +| **Module** | [E7.M1 — QuestStateMachine](../decomposition/modules/E7_M1_QuestStateMachine.md) · Epic 7 Slice 1 · backlog **E7M1-06** | +| **Branch** | `NEO-117-quest-state-operations` | +| **Precursor** | [NEO-116](https://linear.app/neon-sprawl/issue/NEO-116) — `IPlayerQuestStateStore` + Postgres (**Done** on `main`) | +| **Pattern** | [NEO-104](NEO-104-implementation-plan.md) / [NEO-105](NEO-105-implementation-plan.md) — store mutations + static `*Operations` with `*ReasonCodes` + `*Result` envelope | +| **Blocks** | [NEO-118](https://linear.app/neon-sprawl/issue/NEO-118) — quest objective wiring; [NEO-120](https://linear.app/neon-sprawl/issue/NEO-120) — accept POST | +| **Client counterpart** | [NEO-120](https://linear.app/neon-sprawl/issue/NEO-120) — `POST /game/players/{id}/quests/{questId}/accept` (E7M1-09); player-visible accept feedback via [NEO-122](https://linear.app/neon-sprawl/issue/NEO-122). Bruno-only accept verification is **not** prototype-complete per [full-stack epic decomposition](../../.cursor/rules/full-stack-epic-decomposition.md). | + +## Kickoff clarifications + +**No clarifications needed.** [E7M1-06](E7M1-prototype-backlog.md#e7m1-06--queststateoperations-accept-step-advance-complete) backlog, Linear AC, and landed [NEO-116](NEO-116-implementation-plan.md) store contracts settle scope: + +- Accept denials use backlog reason codes (`unknown_quest`, `prerequisite_incomplete`, `already_completed`, `already_active`). +- Complete is **idempotent at the operations layer** (second call → `Success: true`, unchanged snapshot) — distinct from encounter completion which denies `already_completed`; matches E7M1 kickoff “once per player per quest id complete” and E7M1-06 backlog “second complete returns success without state change.” +- Store `TryMarkComplete` replay still returns `false`; operations short-circuit on existing `Completed` rows before calling the store. +- `TryAdvanceStep` validates quest id via `IQuestDefinitionRegistry` and step bounds against `QuestDefRow.Steps.Count`; no terminal-step gate on `TryMarkComplete` this story (E7M1-07 decides when to call complete). +- Static `QuestStateOperations` class (no new DI registration), mirroring `EncounterCompletionOperations`. + +## Goal, scope, and out-of-scope + +**Goal:** Server-authoritative quest lifecycle operations — **accept**, **step advance**, and **complete** — with structured **`reasonCode`** denials. Prerequisite enforcement from catalog **`prerequisiteQuestIds`**. Idempotent complete per `(playerId, questId)`. + +**In scope (from Linear + [E7M1-06](E7M1-prototype-backlog.md#e7m1-06--queststateoperations-accept-step-advance-complete)):** + +- **`QuestStateReasonCodes`** — stable string constants for accept, advance, and complete paths. +- **`QuestStateOperationResult`** — `Success`, optional `ReasonCode`, optional `QuestStepState` snapshot (when applicable). +- **`QuestStateOperations`** (static): + - **`TryAccept`** — registry validation, prerequisite gate, delegate to `IPlayerQuestStateStore.TryActivate`. + - **`TryAdvanceStep`** — registry validation, active-row + step-index bounds, delegate to store `TryAdvanceStep`. + - **`TryMarkComplete`** — registry validation, idempotent success when already completed, delegate to store `TryMarkComplete`. +- Unit tests (AAA): accept order, prerequisite deny, duplicate accept/active/completed denials, advance bounds, idempotent complete. +- `server/README.md` — quest operations + reason-code table (accept/advance/complete). + +**Out of scope (from Linear + backlog):** + +- Gather/craft/encounter objective wiring ([NEO-118](https://linear.app/neon-sprawl/issue/NEO-118) / E7M1-07). +- HTTP **`GET …/quest-progress`** ([NEO-119](https://linear.app/neon-sprawl/issue/NEO-119)) and accept POST ([NEO-120](https://linear.app/neon-sprawl/issue/NEO-120)). +- Telemetry hook comments ([NEO-121](https://linear.app/neon-sprawl/issue/NEO-121) / E7M1-10). +- Bruno, Godot, dev fixture quest reset API. +- Reward grants / E7.M2 bundles. +- Objective-satisfaction checks before advance/complete (E7M1-07). + +## Acceptance criteria checklist + +- [ ] Accept fails when prerequisite quest not **completed** (`prerequisite_incomplete`). +- [ ] Complete is idempotent per player+quest (`TryMarkComplete` second call → `Success: true`, same `CompletedAt`). +- [ ] Reason codes documented in `server/README.md`. + +## Technical approach + +1. **`QuestStateReasonCodes`** (`Game/Quests/`) + - **Accept:** `unknown_quest`, `prerequisite_incomplete`, `already_completed`, `already_active`, `unknown_player`. + - **Advance:** `unknown_quest`, `not_active`, `invalid_step_index`, `already_completed`, `unknown_player`. + - **Complete:** `unknown_quest`, `not_active`, `unknown_player`. + - (No `already_completed` deny on complete — idempotent success instead.) + +2. **`QuestStateOperationResult`** + - `readonly record struct` with `(bool Success, string? ReasonCode, QuestStepState? Snapshot)`. + - Deny helper returns `Success: false`, non-null `ReasonCode`, snapshot when useful (e.g. current row on `already_active`). + +3. **`QuestStateOperations.TryAccept`** + - `IQuestDefinitionRegistry.TryNormalizeKnown` + `TryGetDefinition` → else `unknown_quest`. + - `IPlayerQuestStateStore.TryGetProgress`: + - `Completed` → deny `already_completed` (return snapshot). + - `Active` → deny `already_active` (return snapshot). + - For each id in `definition.PrerequisiteQuestIds`: `TryGetProgress` must succeed with `Status == Completed`; else `prerequisite_incomplete`. + - `TryActivate` → on success return snapshot; on failure (unknown player bucket) → `unknown_player`. + +4. **`QuestStateOperations.TryAdvanceStep`** + - Registry gate → `unknown_quest`; load `QuestDefRow` for step count. + - Progress read: missing or not `Active` → `not_active`; `Completed` → `already_completed`. + - Reject when `newStepIndex <= currentStepIndex` or `newStepIndex >= definition.Steps.Count` → `invalid_step_index`. + - Store `TryAdvanceStep` → success or `unknown_player` on store denial. + +5. **`QuestStateOperations.TryMarkComplete`** + - Registry gate → `unknown_quest`. + - If progress exists and `Completed` → **`Success: true`**, return existing snapshot (idempotent; do not call store). + - If missing or not `Active` → `not_active`. + - `TryMarkComplete(playerId, questId, timeProvider.GetUtcNow(), …)` → success with snapshot; store replay handled by pre-check above. + +6. **Tests** + - Use `InMemoryWebApplicationFactory` to resolve real `IQuestDefinitionRegistry` + `IPlayerQuestStateStore` (dev player seeded). + - Frozen ids via `PrototypeE7M1QuestCatalogRules` (`GatherIntroQuestId`, `ChainQuestId`; inline `prototype_quest_refine_intro` / `prototype_quest_combat_intro` for prerequisite matrix). + - Prerequisite scenario: accept refine intro without gather complete → deny; complete gather via store + operations → accept refine succeeds. + - Idempotent complete: two `TryMarkComplete` calls → both `Success: true`, same `CompletedAt`. + +7. **Docs** + - Extend `server/README.md` quest section with **`QuestStateOperations`** summary and reason-code table. + - Module alignment register update deferred until story lands (same as NEO-116). + +### Operation flow (accept) + +```mermaid +flowchart TD + A[TryAccept] --> B{Known quest?} + B -->|no| UQ[deny unknown_quest] + B -->|yes| C{Progress row?} + C -->|completed| AC[deny already_completed] + C -->|active| AA[deny already_active] + C -->|missing| D{All prerequisites completed?} + D -->|no| PI[deny prerequisite_incomplete] + D -->|yes| E[TryActivate] + E -->|true| OK[Success + snapshot] + E -->|false| UP[deny unknown_player] +``` + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Quests/QuestStateReasonCodes.cs` | Stable deny reason strings for accept/advance/complete. | +| `server/NeonSprawl.Server/Game/Quests/QuestStateOperationResult.cs` | Operations result envelope (`Success`, `ReasonCode`, `Snapshot`). | +| `server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs` | Static `TryAccept`, `TryAdvanceStep`, `TryMarkComplete`. | +| `server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs` | AAA unit tests for accept prerequisites, denials, advance bounds, idempotent complete. | +| `docs/plans/NEO-117-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/README.md` | Document `QuestStateOperations`, reason codes, idempotent complete semantics; note HTTP deferred to E7M1-08/09. | +| `server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs` | Update XML `` now that the type will exist (optional nit from NEO-116 review). | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `QuestStateOperationsTests.cs` | **Accept:** gather intro happy path (`not_started` → active). **Accept:** refine intro denies `prerequisite_incomplete` before gather complete. **Accept:** refine succeeds after gather marked complete via operations. **Accept:** `unknown_quest`, `already_active`, `already_completed` denials. **Accept:** unknown player → `unknown_player`. **Advance:** chain quest step 0→1 clears counters path via operations. **Advance:** `invalid_step_index` (non-increasing, past last step). **Advance:** `not_active`, `already_completed`. **Complete:** first mark success; second mark idempotent success with unchanged `CompletedAt`. **Complete:** `not_active` when never accepted. **Host:** factory resolves registry + store for integration-style smoke. | + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|----------------------|--------| +| **Idempotent complete vs encounter deny pattern** | Operations return `Success: true` on replay (E7M1-06 backlog); store layer unchanged. | **adopted** | +| **Terminal step required before complete** | Defer to E7M1-07; operations do not validate objective satisfaction or terminal step index in NEO-117. | **adopted** | +| **Extra catalog test constants** | Optional `RefineIntroQuestId` / `CombatIntroQuestId` on `PrototypeE7M1QuestCatalogRules`; inline strings acceptable if only used in one test file. | **deferred** | +| **Prerequisite check strictness** | Require `TryGetProgress` + `Completed` for each prerequisite id (implicit `not_started` fails gate). | **adopted** | From f053ffbb398dc94d33a069ed3c2c1fae8d1e0434 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 6 Jun 2026 19:26:37 -0400 Subject: [PATCH 2/5] NEO-117: Add QuestStateOperations with reason codes and tests. Server-authoritative accept, step advance, and idempotent complete wrapping IPlayerQuestStateStore with prerequisite enforcement. --- docs/plans/NEO-117-implementation-plan.md | 12 +- .../Game/Quests/QuestStateOperationsTests.cs | 378 ++++++++++++++++++ .../Game/Quests/QuestStateOperationResult.cs | 10 + .../Game/Quests/QuestStateOperations.cs | 142 +++++++ .../Game/Quests/QuestStateReasonCodes.cs | 19 + server/README.md | 32 +- 6 files changed, 586 insertions(+), 7 deletions(-) create mode 100644 server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs create mode 100644 server/NeonSprawl.Server/Game/Quests/QuestStateOperationResult.cs create mode 100644 server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs create mode 100644 server/NeonSprawl.Server/Game/Quests/QuestStateReasonCodes.cs diff --git a/docs/plans/NEO-117-implementation-plan.md b/docs/plans/NEO-117-implementation-plan.md index 0606930..be4c898 100644 --- a/docs/plans/NEO-117-implementation-plan.md +++ b/docs/plans/NEO-117-implementation-plan.md @@ -50,9 +50,15 @@ ## Acceptance criteria checklist -- [ ] Accept fails when prerequisite quest not **completed** (`prerequisite_incomplete`). -- [ ] Complete is idempotent per player+quest (`TryMarkComplete` second call → `Success: true`, same `CompletedAt`). -- [ ] Reason codes documented in `server/README.md`. +- [x] Accept fails when prerequisite quest not **completed** (`prerequisite_incomplete`). +- [x] Complete is idempotent per player+quest (`TryMarkComplete` second call → `Success: true`, same `CompletedAt`). +- [x] Reason codes documented in `server/README.md`. + +## Implementation reconciliation (shipped) + +- **Types:** `QuestStateReasonCodes`, `QuestStateOperationResult`, `QuestStateOperations` (`TryAccept`, `TryAdvanceStep`, `TryMarkComplete`). +- **Tests:** `QuestStateOperationsTests` — 15 AAA cases (accept prerequisites, denials, advance bounds, idempotent complete, host DI smoke). +- **Docs:** `server/README.md` quest operations + reason-code table. ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs new file mode 100644 index 0000000..8ccc0f6 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs @@ -0,0 +1,378 @@ +using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Quests; +using NeonSprawl.Server.Tests; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Quests; + +public sealed class QuestStateOperationsTests +{ + private const string PlayerId = "dev-local-1"; + private const string UnknownPlayerId = "unknown-player-xyz"; + private const string GatherQuestId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; + private const string RefineQuestId = "prototype_quest_refine_intro"; + private const string ChainQuestId = PrototypeE7M1QuestCatalogRules.ChainQuestId; + private const string ChainObjectiveId = PrototypeE7M1QuestCatalogRules.ChainFirstObjectiveId; + + [Fact] + public async Task TryAccept_ShouldActivateGatherIntro_WhenNotStarted() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryAccept( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.True(result.Success); + Assert.Null(result.ReasonCode); + Assert.NotNull(result.Snapshot); + Assert.Equal(QuestProgressStatus.Active, result.Snapshot!.Status); + Assert.Equal(0, result.Snapshot.CurrentStepIndex); + } + + [Fact] + public async Task TryAccept_ShouldDenyPrerequisiteIncomplete_WhenRefineIntroBeforeGatherComplete() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryAccept( + PlayerId, + RefineQuestId, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.PrerequisiteIncomplete, result.ReasonCode); + Assert.Null(result.Snapshot); + } + + [Fact] + public async Task TryAccept_ShouldSucceedRefineIntro_WhenGatherIntroCompleted() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var timeProvider = TimeProvider.System; + Assert.True(QuestStateOperations.TryAccept(PlayerId, GatherQuestId, deps.QuestRegistry, deps.ProgressStore).Success); + Assert.True(QuestStateOperations.TryMarkComplete( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore, + timeProvider).Success); + + // Act + var result = QuestStateOperations.TryAccept( + PlayerId, + RefineQuestId, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.True(result.Success); + Assert.Null(result.ReasonCode); + Assert.Equal(QuestProgressStatus.Active, result.Snapshot!.Status); + } + + [Fact] + public async Task TryAccept_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryAccept( + PlayerId, + "prototype_quest_missing", + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.UnknownQuest, result.ReasonCode); + } + + [Fact] + public async Task TryAccept_ShouldDenyAlreadyActive_WhenQuestAlreadyAccepted() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + Assert.True(QuestStateOperations.TryAccept(PlayerId, GatherQuestId, deps.QuestRegistry, deps.ProgressStore).Success); + + // Act + var result = QuestStateOperations.TryAccept( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.AlreadyActive, result.ReasonCode); + Assert.NotNull(result.Snapshot); + Assert.Equal(QuestProgressStatus.Active, result.Snapshot!.Status); + } + + [Fact] + public async Task TryAccept_ShouldDenyAlreadyCompleted_WhenQuestAlreadyCompleted() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var timeProvider = TimeProvider.System; + Assert.True(QuestStateOperations.TryAccept(PlayerId, GatherQuestId, deps.QuestRegistry, deps.ProgressStore).Success); + Assert.True(QuestStateOperations.TryMarkComplete( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore, + timeProvider).Success); + + // Act + var result = QuestStateOperations.TryAccept( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.AlreadyCompleted, result.ReasonCode); + Assert.Equal(QuestProgressStatus.Completed, result.Snapshot!.Status); + } + + [Fact] + public async Task TryAccept_ShouldDenyUnknownPlayer_WhenPlayerNotInStore() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryAccept( + UnknownPlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.UnknownPlayer, result.ReasonCode); + } + + [Fact] + public async Task TryAdvanceStep_ShouldAdvanceChainQuestAndClearCounters() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var timeProvider = TimeProvider.System; + AcceptAndComplete(deps, GatherQuestId, timeProvider); + AcceptAndComplete(deps, RefineQuestId, timeProvider); + AcceptAndComplete(deps, "prototype_quest_combat_intro", timeProvider); + Assert.True(QuestStateOperations.TryAccept(PlayerId, ChainQuestId, deps.QuestRegistry, deps.ProgressStore).Success); + Assert.True(deps.ProgressStore.TryUpdateObjectiveCounter(PlayerId, ChainQuestId, ChainObjectiveId, 5, out _)); + + // Act + var result = QuestStateOperations.TryAdvanceStep( + PlayerId, + ChainQuestId, + 1, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.True(result.Success); + Assert.Null(result.ReasonCode); + Assert.Equal(1, result.Snapshot!.CurrentStepIndex); + Assert.Empty(result.Snapshot.ObjectiveCounters); + } + + [Theory] + [InlineData(0)] + [InlineData(4)] + public async Task TryAdvanceStep_ShouldDenyInvalidStepIndex_WhenIndexNotIncreasingOrPastLastStep(int newStepIndex) + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var timeProvider = TimeProvider.System; + AcceptAndComplete(deps, GatherQuestId, timeProvider); + AcceptAndComplete(deps, RefineQuestId, timeProvider); + AcceptAndComplete(deps, "prototype_quest_combat_intro", timeProvider); + Assert.True(QuestStateOperations.TryAccept(PlayerId, ChainQuestId, deps.QuestRegistry, deps.ProgressStore).Success); + + // Act + var result = QuestStateOperations.TryAdvanceStep( + PlayerId, + ChainQuestId, + newStepIndex, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.InvalidStepIndex, result.ReasonCode); + } + + [Fact] + public async Task TryAdvanceStep_ShouldDenyNotActive_WhenQuestNotAccepted() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryAdvanceStep( + PlayerId, + GatherQuestId, + 1, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.NotActive, result.ReasonCode); + } + + [Fact] + public async Task TryAdvanceStep_ShouldDenyAlreadyCompleted_WhenQuestCompleted() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var timeProvider = TimeProvider.System; + Assert.True(QuestStateOperations.TryAccept(PlayerId, GatherQuestId, deps.QuestRegistry, deps.ProgressStore).Success); + Assert.True(QuestStateOperations.TryMarkComplete( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore, + timeProvider).Success); + + // Act + var result = QuestStateOperations.TryAdvanceStep( + PlayerId, + GatherQuestId, + 1, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.AlreadyCompleted, result.ReasonCode); + } + + [Fact] + public async Task TryMarkComplete_ShouldBeIdempotentAtOperationsLayer() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var timeProvider = TimeProvider.System; + Assert.True(QuestStateOperations.TryAccept(PlayerId, GatherQuestId, deps.QuestRegistry, deps.ProgressStore).Success); + + // Act + var first = QuestStateOperations.TryMarkComplete( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore, + timeProvider); + var second = QuestStateOperations.TryMarkComplete( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore, + timeProvider); + + // Assert + Assert.True(first.Success); + Assert.Null(first.ReasonCode); + Assert.NotNull(first.Snapshot!.CompletedAt); + Assert.True(second.Success); + Assert.Null(second.ReasonCode); + Assert.Equal(first.Snapshot.CompletedAt, second.Snapshot!.CompletedAt); + Assert.Equal(QuestProgressStatus.Completed, second.Snapshot.Status); + } + + [Fact] + public async Task TryMarkComplete_ShouldDenyNotActive_WhenQuestNeverAccepted() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + var timeProvider = TimeProvider.System; + + // Act + var result = QuestStateOperations.TryMarkComplete( + PlayerId, + GatherQuestId, + deps.QuestRegistry, + deps.ProgressStore, + timeProvider); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.NotActive, result.ReasonCode); + } + + [Fact] + public async Task Host_ShouldResolveRegistryAndStore_ForQuestStateOperations() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + using var scope = factory.Services.CreateScope(); + var questRegistry = scope.ServiceProvider.GetRequiredService(); + var progressStore = scope.ServiceProvider.GetRequiredService(); + + // Act + var accept = QuestStateOperations.TryAccept(PlayerId, GatherQuestId, questRegistry, progressStore); + var complete = QuestStateOperations.TryMarkComplete( + PlayerId, + GatherQuestId, + questRegistry, + progressStore, + TimeProvider.System); + + // Assert + Assert.True(accept.Success); + Assert.True(complete.Success); + Assert.Equal(QuestProgressStatus.Completed, complete.Snapshot!.Status); + } + + private static void AcceptAndComplete( + (IQuestDefinitionRegistry QuestRegistry, IPlayerQuestStateStore ProgressStore) deps, + string questId, + TimeProvider timeProvider) + { + Assert.True(QuestStateOperations.TryAccept(PlayerId, questId, deps.QuestRegistry, deps.ProgressStore).Success); + Assert.True(QuestStateOperations.TryMarkComplete( + PlayerId, + questId, + deps.QuestRegistry, + deps.ProgressStore, + timeProvider).Success); + } + + private static (IQuestDefinitionRegistry QuestRegistry, IPlayerQuestStateStore ProgressStore) ResolveDependencies( + InMemoryWebApplicationFactory factory) + { + var scope = factory.Services.CreateScope(); + return ( + scope.ServiceProvider.GetRequiredService(), + scope.ServiceProvider.GetRequiredService()); + } +} diff --git a/server/NeonSprawl.Server/Game/Quests/QuestStateOperationResult.cs b/server/NeonSprawl.Server/Game/Quests/QuestStateOperationResult.cs new file mode 100644 index 0000000..ede7fad --- /dev/null +++ b/server/NeonSprawl.Server/Game/Quests/QuestStateOperationResult.cs @@ -0,0 +1,10 @@ +namespace NeonSprawl.Server.Game.Quests; + +/// +/// Server-internal quest state resolution envelope (NEO-117). +/// HTTP DTOs via E7M1-08/09 (NEO-119/NEO-120). +/// +public readonly record struct QuestStateOperationResult( + bool Success, + string? ReasonCode, + QuestStepState? Snapshot); diff --git a/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs new file mode 100644 index 0000000..ef23447 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs @@ -0,0 +1,142 @@ +namespace NeonSprawl.Server.Game.Quests; + +/// +/// Server-authoritative quest accept, step advance, and complete with reason codes (NEO-117). +/// Objective wiring: E7M1-07 (NEO-118). HTTP: E7M1-08/09 (NEO-119/NEO-120). +/// +public static class QuestStateOperations +{ + /// + /// Validates quest id and prerequisites, then activates at step 0 when not started. + /// + public static QuestStateOperationResult TryAccept( + string playerId, + string questId, + IQuestDefinitionRegistry questRegistry, + IPlayerQuestStateStore progressStore) + { + if (!questRegistry.TryNormalizeKnown(questId, out var normalizedQuestId) || + !questRegistry.TryGetDefinition(normalizedQuestId, out var definition)) + { + return Deny(QuestStateReasonCodes.UnknownQuest); + } + + if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var existing)) + { + return existing.Status switch + { + QuestProgressStatus.Completed => Deny(QuestStateReasonCodes.AlreadyCompleted, existing), + QuestProgressStatus.Active => Deny(QuestStateReasonCodes.AlreadyActive, existing), + _ => Deny(QuestStateReasonCodes.UnknownQuest), + }; + } + + foreach (var prerequisiteId in definition.PrerequisiteQuestIds) + { + if (!progressStore.TryGetProgress(playerId, prerequisiteId, out var prerequisite) || + prerequisite.Status != QuestProgressStatus.Completed) + { + return Deny(QuestStateReasonCodes.PrerequisiteIncomplete); + } + } + + if (progressStore.TryActivate(playerId, normalizedQuestId, out var snapshot)) + { + return Success(snapshot); + } + + return Deny(QuestStateReasonCodes.UnknownPlayer); + } + + /// + /// Advances an active quest to when within catalog bounds. + /// + public static QuestStateOperationResult TryAdvanceStep( + string playerId, + string questId, + int newStepIndex, + IQuestDefinitionRegistry questRegistry, + IPlayerQuestStateStore progressStore) + { + if (!questRegistry.TryNormalizeKnown(questId, out var normalizedQuestId) || + !questRegistry.TryGetDefinition(normalizedQuestId, out var definition)) + { + return Deny(QuestStateReasonCodes.UnknownQuest); + } + + if (!progressStore.TryGetProgress(playerId, normalizedQuestId, out var existing)) + { + return Deny(QuestStateReasonCodes.NotActive); + } + + if (existing.Status == QuestProgressStatus.Completed) + { + return Deny(QuestStateReasonCodes.AlreadyCompleted, existing); + } + + if (existing.Status != QuestProgressStatus.Active) + { + return Deny(QuestStateReasonCodes.NotActive); + } + + if (newStepIndex <= existing.CurrentStepIndex || newStepIndex >= definition.Steps.Count) + { + return Deny(QuestStateReasonCodes.InvalidStepIndex, existing); + } + + if (progressStore.TryAdvanceStep(playerId, normalizedQuestId, newStepIndex, out var snapshot)) + { + return Success(snapshot); + } + + return Deny(QuestStateReasonCodes.UnknownPlayer, existing); + } + + /// + /// Marks a quest complete. Idempotent: replays return success with the existing snapshot. + /// + public static QuestStateOperationResult TryMarkComplete( + string playerId, + string questId, + IQuestDefinitionRegistry questRegistry, + IPlayerQuestStateStore progressStore, + TimeProvider timeProvider) + { + if (!questRegistry.TryNormalizeKnown(questId, out var normalizedQuestId) || + !questRegistry.TryGetDefinition(normalizedQuestId, out _)) + { + return Deny(QuestStateReasonCodes.UnknownQuest); + } + + if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var existing) && + existing.Status == QuestProgressStatus.Completed) + { + return Success(existing); + } + + if (!progressStore.TryGetProgress(playerId, normalizedQuestId, out var active) || + active.Status != QuestProgressStatus.Active) + { + return Deny(QuestStateReasonCodes.NotActive); + } + + if (progressStore.TryMarkComplete(playerId, normalizedQuestId, timeProvider.GetUtcNow(), out var snapshot)) + { + return Success(snapshot); + } + + if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var afterAttempt) && + afterAttempt.Status == QuestProgressStatus.Completed) + { + return Success(afterAttempt); + } + + return Deny(QuestStateReasonCodes.UnknownPlayer); + } + + private static QuestStateOperationResult Success(QuestStepState snapshot) => + new(Success: true, ReasonCode: null, Snapshot: snapshot); + + private static QuestStateOperationResult Deny(string reasonCode, QuestStepState? snapshot = null) => + new(Success: false, ReasonCode: reasonCode, Snapshot: snapshot); +} diff --git a/server/NeonSprawl.Server/Game/Quests/QuestStateReasonCodes.cs b/server/NeonSprawl.Server/Game/Quests/QuestStateReasonCodes.cs new file mode 100644 index 0000000..44eb53f --- /dev/null +++ b/server/NeonSprawl.Server/Game/Quests/QuestStateReasonCodes.cs @@ -0,0 +1,19 @@ +namespace NeonSprawl.Server.Game.Quests; + +/// Stable deny reason codes for (NEO-117). +public static class QuestStateReasonCodes +{ + public const string UnknownQuest = "unknown_quest"; + + public const string PrerequisiteIncomplete = "prerequisite_incomplete"; + + public const string AlreadyCompleted = "already_completed"; + + public const string AlreadyActive = "already_active"; + + public const string UnknownPlayer = "unknown_player"; + + public const string NotActive = "not_active"; + + public const string InvalidStepIndex = "invalid_step_index"; +} diff --git a/server/README.md b/server/README.md index a4e4b07..c45c63c 100644 --- a/server/README.md +++ b/server/README.md @@ -158,20 +158,44 @@ curl -sS -i "http://localhost:5253/game/world/quest-definitions" ## Quest progress store (NEO-116) -Per-player quest runtime state lives in **`IPlayerQuestStateStore`**, keyed by **`(playerId, questId)`**. A missing row means **`not_started`**; **`TryActivate`** creates an **`active`** row at step index **0** with empty objective counters. Mutations are server-authoritative — HTTP read/accept routes land in E7M1-08/09 ([NEO-117](https://linear.app/neon-sprawl/issue/NEO-117)+ for **`QuestStateOperations`**). +Per-player quest runtime state lives in **`IPlayerQuestStateStore`**, keyed by **`(playerId, questId)`**. A missing row means **`not_started`**; **`TryActivate`** creates an **`active`** row at step index **0** with empty objective counters. Game code should call **`QuestStateOperations`** (NEO-117) for accept/advance/complete — not the store directly — so quest ids and prerequisites are validated. HTTP read/accept routes land in E7M1-08/09 (NEO-119/NEO-120). -**Interface methods:** +**Store interface methods:** - **`TryGetProgress`** — read one row; false when not started. - **`TryActivate`** — first accept (`not_started` → `active`). - **`TryAdvanceStep`** — bump step index and clear counters for the new step (denies when completed or index does not increase). - **`TryUpdateObjectiveCounter`** — set one objective counter on the current step (non-negative). -- **`TryMarkComplete`** — idempotent completion flag + **`completedAt`** timestamp. +- **`TryMarkComplete`** — first completion returns `true`; store replay returns `false` without changing **`completedAt`**. -Completed rows cannot regress to active without a reset API (none in prototype). Player ids are normalized (trim + lowercase). Quest ids should be validated via **`IQuestDefinitionRegistry.TryNormalizeKnown`** at operation/HTTP layers (NEO-117+). +Completed rows cannot regress to active without a reset API (none in prototype). Player ids are normalized (trim + lowercase). Quest ids are validated via **`IQuestDefinitionRegistry.TryNormalizeKnown`** in **`QuestStateOperations`**. **Storage:** in-memory singleton when **`ConnectionStrings:NeonSprawl`** is unset (seeds configured dev player only). When Postgres is configured, **`PostgresPlayerQuestStateStore`** persists to **`player_quest_progress`** ([`V008__player_quest_progress.sql`](../db/migrations/V008__player_quest_progress.sql)) with **`objective_counters`** JSONB for the current step. Plan: [NEO-116 implementation plan](../../docs/plans/NEO-116-implementation-plan.md). Bruno startup smoke: `bruno/neon-sprawl-server/quest-progress/` (health only until E7M1-08 HTTP). +### Quest state operations (NEO-117) + +**`QuestStateOperations`** (static) wraps the store with catalog validation and structured **`reasonCode`** denials. Returns **`QuestStateOperationResult`** (`success`, optional `reasonCode`, optional progress snapshot). + +| Method | Role | +|--------|------| +| **`TryAccept`** | Validates quest id + **`prerequisiteQuestIds`** (each prerequisite must be **completed**), then activates at step 0. | +| **`TryAdvanceStep`** | Requires active row; **`newStepIndex`** must be greater than current and less than step count from **`QuestDefRow`**. | +| **`TryMarkComplete`** | Marks active quest complete. **Idempotent:** second call returns **`success: true`** with unchanged snapshot (unlike encounter completion). | + +**Reason codes:** + +| Code | When | +|------|------| +| **`unknown_quest`** | Quest id not in catalog (accept, advance, complete). | +| **`prerequisite_incomplete`** | Accept when a listed prerequisite is not **completed** (implicit `not_started` fails). | +| **`already_completed`** | Accept or advance when row is already **completed**. | +| **`already_active`** | Accept when row is already **active**. | +| **`unknown_player`** | Player id cannot be written (no dev bucket / missing from Postgres `player_position`). | +| **`not_active`** | Advance or complete when quest was never accepted or is not **active**. | +| **`invalid_step_index`** | Advance when index does not increase or is past the last step. | + +Objective wiring (gather/craft/encounter hooks) is E7M1-07 ([NEO-118](https://linear.app/neon-sprawl/issue/NEO-118)). Plan: [NEO-117 implementation plan](../../docs/plans/NEO-117-implementation-plan.md). + ## Encounter definitions (NEO-103) **`GET /game/world/encounter-definitions`** returns a versioned JSON body (`schemaVersion` **1**, **`encounters`**) backed by **`IEncounterDefinitionRegistry`** and **`IRewardTableDefinitionRegistry`** — the same prototype rows loaded at startup (no second source of truth). Each row includes **`id`**, **`displayName`**, nested **`completionCriteria`** (`kind`), **`requiredNpcInstanceIds`**, and nested **`rewardTable`** (`id`, `displayName`, **`fixedGrants`** with `itemId` + `quantity`). Plan: [NEO-103 implementation plan](../../docs/plans/NEO-103-implementation-plan.md); Bruno: `bruno/neon-sprawl-server/encounter-definitions/`. From 84847e20f05ec1f2ef05e461bbbc668d06c793d4 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 6 Jun 2026 19:29:10 -0400 Subject: [PATCH 3/5] NEO-117: Add code review for QuestStateOperations branch. --- docs/reviews/2026-06-06-NEO-117.md | 67 ++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 docs/reviews/2026-06-06-NEO-117.md diff --git a/docs/reviews/2026-06-06-NEO-117.md b/docs/reviews/2026-06-06-NEO-117.md new file mode 100644 index 0000000..9e3f05c --- /dev/null +++ b/docs/reviews/2026-06-06-NEO-117.md @@ -0,0 +1,67 @@ +# Code review — NEO-117 (E7M1-06) + +**Date:** 2026-06-06 +**Scope:** Branch `NEO-117-quest-state-operations` vs `origin/main` — commits `44ef5f2` … `f053ffb` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-117 adds **`QuestStateOperations`** — static accept, step advance, and complete wrappers over **`IPlayerQuestStateStore`** with **`QuestStateReasonCodes`** and **`QuestStateOperationResult`**, mirroring the NEO-104/105 encounter store + operations pattern. Prerequisite enforcement reads catalog **`prerequisiteQuestIds`**; complete is **idempotent at the operations layer** (replay → `Success: true` with unchanged snapshot), including a defensive re-read after store denial for concurrent complete. Fifteen AAA tests cover accept prerequisites/denials, advance bounds, idempotent complete, and host DI smoke via **`InMemoryWebApplicationFactory`**. **`server/README.md`** documents operations and reason codes. Server-only infrastructure — correct scope; HTTP and objective wiring remain deferred. Risk is low; main follow-ups are test helper consistency, minor coverage gaps, and post-merge decomposition register updates. + +## Documentation checked + +| Path | Result | +|------|--------| +| `docs/plans/NEO-117-implementation-plan.md` | **Matches** — kickoff decisions adopted; acceptance checklist checked; reconciliation accurate. | +| `docs/plans/E7M1-prototype-backlog.md` (E7M1-06) | **Partially matches** — implementation satisfies AC; backlog checkboxes still `[ ]` (expected until merge/story end). | +| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Partially matches** — module page still lists **`QuestStateOperations`** as future E7M1-06; should note NEO-117 landed after merge. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E7.M1 row ends at E7M1-05 / NEO-116; plan defers register update until story lands — append E7M1-06 / NEO-117 on merge. | +| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — no NEO-117 note yet; same deferred update as NEO-116 review. | +| `server/README.md` | **Matches** — quest operations section, reason-code table, idempotent complete semantics, HTTP deferral to NEO-119/NEO-120. | +| Full-stack epic decomposition | **N/A** — server-only; client counterpart explicitly NEO-120/NEO-122; plan does not claim prototype slice complete. | + +**Register / tracking:** E7.M1 register row and module **Status** should gain an **E7M1-06 / NEO-117 landed** bullet after merge (register stays **Planned** until E7M1-07+ runtime and client capstone). + +## Blocking issues + +(none) + +## Suggestions + +1. **Test `ResolveDependencies` pattern** — `QuestStateOperationsTests.ResolveDependencies` creates an `IServiceScope` that is never disposed. **`EncounterCompletionOperationsTests`** resolves from `factory.Services` root (singleton stores). Align to avoid undisposed scopes and match the established encounter test pattern. + +2. **`unknown_quest` coverage on advance/complete** — Accept path has `TryAccept_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog`; advance and complete lack parity. Add two small `[Fact]` cases (mirror **`EncounterCompletionOperationsTests`** unknown-encounter test) for registry gate consistency. + +3. **Post-merge decomposition docs** — Plan defers register/module updates; on story end append **E7M1-06 / NEO-117** to `documentation_and_implementation_alignment.md`, `E7_M1_QuestStateMachine.md` (operations bullet + README link), and check E7M1-06 AC boxes in `E7M1-prototype-backlog.md`. + +## Nits + +- Nit: **`TryAccept`** progress switch default `_ => Deny(UnknownQuest)` is unreachable while **`QuestProgressStatus`** is only `Active`/`Completed`; if kept for exhaustiveness, a more accurate code (or `NotActive`) would aid future enum extensions. + +- Nit: Chain-quest setup tests hardcode **`prototype_quest_combat_intro`** and **`prototype_quest_refine_intro`**; plan deferred **`RefineIntroQuestId`** / **`CombatIntroQuestId`** constants — acceptable for now, but NEO-118 wiring will reuse these ids. + +- Nit: **`TryMarkComplete`** re-calls **`TryGetProgress`** after the active-row read (lines 117–118) where `active` could be reused on the `not_active` deny path — minor readability only. + +## Verification + +```bash +# Quest state operations (primary signal for this story) +cd server +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \ + --filter "FullyQualifiedName~QuestStateOperationsTests" + +# Quest store regression (precursor) +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \ + --filter "FullyQualifiedName~InMemoryPlayerQuestStateStoreTests|FullyQualifiedName~PlayerQuestProgressPersistence" + +# Full server suite (CI) +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj +``` + +**Reviewer note:** All 15 `QuestStateOperationsTests` and full server suite (663 tests) passed locally during review. + +**Manual:** None required — no HTTP or player-visible surface in this story. From 025c3250d248393e6230c5d1be57aadff47f0ae0 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 6 Jun 2026 19:30:21 -0400 Subject: [PATCH 4/5] NEO-117: Address code review findings for QuestStateOperations. Align test DI resolution with encounter pattern, add unknown_quest advance/complete tests, catalog quest id constants, refactor TryMarkComplete readability, and update decomposition docs. --- .../modules/E7_M1_QuestStateMachine.md | 6 +- ...umentation_and_implementation_alignment.md | 2 +- .../modules/module_dependency_register.md | 2 +- docs/plans/E7M1-prototype-backlog.md | 6 +- docs/plans/NEO-117-implementation-plan.md | 11 ++-- docs/reviews/2026-06-06-NEO-117.md | 20 +++--- .../Game/Quests/QuestStateOperationsTests.cs | 63 +++++++++++++++---- .../Quests/PrototypeE7M1QuestCatalogRules.cs | 6 ++ .../Game/Quests/QuestStateOperations.cs | 23 ++++--- 9 files changed, 96 insertions(+), 43 deletions(-) diff --git a/docs/decomposition/modules/E7_M1_QuestStateMachine.md b/docs/decomposition/modules/E7_M1_QuestStateMachine.md index be8a357..94f7392 100644 --- a/docs/decomposition/modules/E7_M1_QuestStateMachine.md +++ b/docs/decomposition/modules/E7_M1_QuestStateMachine.md @@ -7,7 +7,7 @@ | **Module ID** | E7.M1 | | **Epic** | [Epic 7 — Quest / Faction](../epics/epic_07_quest_faction.md) | | **Stage target** | Prototype | -| **Status** | Planned — Slice 1 backlog [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md): **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) catalog **landed** (schemas + CI); **E7M1-02** [NEO-113](https://linear.app/neon-sprawl/issue/NEO-113) server load **landed**; **E7M1-03** [NEO-114](https://linear.app/neon-sprawl/issue/NEO-114) registry **landed**; **E7M1-04** [NEO-115](https://linear.app/neon-sprawl/issue/NEO-115) HTTP read **landed**; **E7M1-05** [NEO-116](https://linear.app/neon-sprawl/issue/NEO-116) player quest state store **landed**; runtime from **E7M1-06** [NEO-117](https://linear.app/neon-sprawl/issue/NEO-117) → capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) | +| **Status** | Planned — Slice 1 backlog [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md): **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) catalog **landed** (schemas + CI); **E7M1-02** [NEO-113](https://linear.app/neon-sprawl/issue/NEO-113) server load **landed**; **E7M1-03** [NEO-114](https://linear.app/neon-sprawl/issue/NEO-114) registry **landed**; **E7M1-04** [NEO-115](https://linear.app/neon-sprawl/issue/NEO-115) HTTP read **landed**; **E7M1-05** [NEO-116](https://linear.app/neon-sprawl/issue/NEO-116) player quest state store **landed**; **E7M1-06** [NEO-117](https://linear.app/neon-sprawl/issue/NEO-117) **`QuestStateOperations`** **landed**; objective wiring from **E7M1-07** [NEO-118](https://linear.app/neon-sprawl/issue/NEO-118) → capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) | | **Linear** | Label **`E7.M1`** · [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md) | ## Purpose @@ -82,7 +82,9 @@ The **first shipped quest spine** is **frozen** for prototype tuning until a del **HTTP read model (NEO-115):** **`GET /game/world/quest-definitions`** — `QuestDefinitionsWorldApi` + DTOs in `Game/Quests/` ([NEO-115](../../plans/NEO-115-implementation-plan.md)); [server README — Quest definitions (NEO-115)](../../../server/README.md#quest-definitions-neo-115); Bruno `bruno/neon-sprawl-server/quest-definitions/`. -**Player quest state store (NEO-116):** **`IPlayerQuestStateStore`** — in-memory + Postgres (`V008__player_quest_progress.sql`) per-player rows keyed by `(playerId, questId)`; implicit `not_started` when missing ([NEO-116](../../plans/NEO-116-implementation-plan.md)); [server README — Quest progress store (NEO-116)](../../../server/README.md#quest-progress-store-neo-116). HTTP read/accept deferred to E7M1-08/09; **`QuestStateOperations`** in E7M1-06. +**Player quest state store (NEO-116):** **`IPlayerQuestStateStore`** — in-memory + Postgres (`V008__player_quest_progress.sql`) per-player rows keyed by `(playerId, questId)`; implicit `not_started` when missing ([NEO-116](../../plans/NEO-116-implementation-plan.md)); [server README — Quest progress store (NEO-116)](../../../server/README.md#quest-progress-store-neo-116). + +**Quest state operations (NEO-117):** **`QuestStateOperations`** — accept, step advance, complete with **`QuestStateReasonCodes`**; prerequisite gate on accept; idempotent complete at operations layer ([NEO-117](../../plans/NEO-117-implementation-plan.md)); [server README — Quest state operations (NEO-117)](../../../server/README.md#quest-state-operations-neo-117). HTTP read/accept deferred to E7M1-08/09; objective wiring in E7M1-07. **Reward policy (Slice 1):** Quest completion updates **`QuestStepState` only** — no duplicate item/XP grants. Gather/craft/encounter paths keep existing payouts; [E7.M2](E7_M2_RewardAndUnlockRouter.md) adds **`QuestRewardBundle`** apply in Slice 2. diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index f1e5b2a..5ce3c40 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -59,7 +59,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E5.M1 | Ready | **NEO-76 landed:** frozen prototype four-ability catalog in [`content/abilities/prototype_abilities.json`](../../../content/abilities/prototype_abilities.json); [`ability-def.schema.json`](../../../content/schemas/ability-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M1 four-id gate ([NEO-76](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` at startup — `server/NeonSprawl.Server/Game/Combat/` ([NEO-77](../../plans/NEO-77-implementation-plan.md)); [server README — Ability catalog](../../../server/README.md#ability-catalog-contentabilities-neo-77). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79](../../plans/NEO-79-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/hotbar-loadout/` + `ability-cast/` unknown-ability deny smokes. **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78](../../plans/NEO-78-implementation-plan.md)); [server README — Ability definitions (NEO-78)](../../../server/README.md#ability-definitions-neo-78); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **NEO-80 landed:** **`ICombatEntityHealthStore`** / **`InMemoryCombatEntityHealthStore`** — lazy init (Slice 1 flat 100 HP; **superseded by [NEO-91](../../plans/NEO-91-implementation-plan.md)** per-archetype catalog max HP + three NPC instance ids); DI via **`AddCombatEntityHealthStore()`** ([NEO-80](../../plans/NEO-80-implementation-plan.md)); [server README — Combat entity health (NEO-80, NEO-91)](../../../server/README.md#combat-entity-health-neo-80-neo-91). **NEO-81 landed:** **`CombatOperations.TryResolve`** + **`CombatResult`** + **`CombatReasonCodes`** — defeated pre-check deny, catalog damage ([NEO-81](../../plans/NEO-81-implementation-plan.md)); [server README — Combat engine (NEO-81)](../../../server/README.md#combat-engine-neo-81). **NEO-82 landed:** **`AbilityCastApi`** → **`CombatOperations.TryResolve`**; nested wire **`combatResolution`** on accept; per-ability catalog cooldown; **`target_defeated`** cast deny ([NEO-82](../../plans/NEO-82-implementation-plan.md)); [server README — Ability cast (NEO-82)](../../../server/README.md#ability-cast-neo-31-neo-82); Bruno `bruno/neon-sprawl-server/ability-cast/` defeat spine. **NEO-83 landed:** **`GET /game/world/combat-targets`** — `CombatTargetsWorldApi` + DTOs; authoritative HP snapshot from **`ICombatEntityHealthStore`** ([NEO-83](../../plans/NEO-83-implementation-plan.md)); [server README — Combat targets snapshot (NEO-83)](../../../server/README.md#combat-targets-snapshot-neo-83); Bruno `bruno/neon-sprawl-server/combat-targets/`. **NEO-44 landed:** **`CombatDefeatGigXpGrant`** on cast **`targetDefeated`** — **25** gig XP to **`breach`** via **`IPlayerGigProgressionStore`** (V007); **`GET …/gig-progression`** ([NEO-44](../../plans/NEO-44-implementation-plan.md), [`NEO-44` manual QA](../../manual-qa/NEO-44.md)); [server README — Gig progression (NEO-44)](../../../server/README.md#gig-progression-snapshot-neo-44); Bruno `bruno/neon-sprawl-server/gig-progression/`. **NEO-84 landed:** comment-only **`ability_used`** / **`enemy_defeat`** telemetry hook sites in **`CombatOperations.TryResolve`**; reserved **`encounter_start`** / **`player_death`** ([NEO-84](../../plans/NEO-84-implementation-plan.md)); [server README — Combat telemetry hooks (NEO-84)](../../../server/README.md#combat-telemetry-hooks-neo-84). **NEO-85 landed:** client combat HUD — **`ability_cast_client.gd`** parses **`combatResolution`**; **`combat_targets_client.gd`**, **`CastFeedbackLabel`** resolution copy, **`CombatTargetHpLabel`**; event-driven GET refresh ([NEO-85](../../plans/NEO-85-implementation-plan.md), [`NEO-85` manual QA](../../manual-qa/NEO-85.md)); `client/README.md` combat HUD section. **NEO-86 landed:** playable combat capstone — **`gig_progression_client.gd`**, **`GigXpLabel`**, defeat-triggered gig GET refresh; capstone manual QA [`NEO-86`](../../manual-qa/NEO-86.md); `client/README.md` end-to-end combat loop section ([NEO-86](../../plans/NEO-86-implementation-plan.md)). **Epic 5 Slice 1 client capstone complete.** **Backlog decomposed:** Epic 5 Slice 1 — **E5M1-01**–**E5M1-12** landed. **Precursor landed:** E1.M4 cast funnel (NEO-28/31/32). | [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1](E5_M1_CombatRulesEngine.md), [NEO-77](../../plans/NEO-77-implementation-plan.md), [NEO-79](../../plans/NEO-79-implementation-plan.md), [NEO-78](../../plans/NEO-78-implementation-plan.md), [NEO-80](../../plans/NEO-80-implementation-plan.md), [NEO-81](../../plans/NEO-81-implementation-plan.md), [NEO-82](../../plans/NEO-82-implementation-plan.md), [NEO-83](../../plans/NEO-83-implementation-plan.md), [NEO-44](../../plans/NEO-44-implementation-plan.md), [NEO-84](../../plans/NEO-84-implementation-plan.md), [NEO-85](../../plans/NEO-85-implementation-plan.md), [NEO-86](../../plans/NEO-86-implementation-plan.md), [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) | | E5.M2 | Ready | **NEO-87 landed:** frozen prototype three-behavior catalog in [`content/npc-behaviors/prototype_npc_behaviors.json`](../../../content/npc-behaviors/prototype_npc_behaviors.json); [`npc-behavior-def.schema.json`](../../../content/schemas/npc-behavior-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M2 three-id gate ([NEO-87](../../plans/NEO-87-implementation-plan.md)). **NEO-88 landed:** fail-fast server load of `content/npc-behaviors/*_npc_behaviors.json` at startup — `server/NeonSprawl.Server/Game/Npc/` ([NEO-88](../../plans/NEO-88-implementation-plan.md)); [server README — NPC behavior catalog](../../../server/README.md#npc-behavior-catalog-contentnpc-behaviors-neo-88). **NEO-89 landed:** injectable **`INpcBehaviorDefinitionRegistry`** + DI; **`PrototypeNpcBehaviorRegistry`** id constants ([NEO-89](../../plans/NEO-89-implementation-plan.md)). **NEO-90 landed:** **`GET /game/world/npc-behavior-definitions`** — `NpcBehaviorDefinitionsWorldApi` + DTOs in `Game/Npc/` ([NEO-90](../../plans/NEO-90-implementation-plan.md)); [server README — NPC behavior definitions (NEO-90)](../../../server/README.md#npc-behavior-definitions-neo-90); Bruno `bruno/neon-sprawl-server/npc-behavior-definitions/`. **NEO-91 landed:** **`PrototypeNpcRegistry`** + combat-target migration — three NPC instance ids replace alpha/beta; per-archetype max HP from behavior catalog ([NEO-91](../../plans/NEO-91-implementation-plan.md)); [server README — Combat entity health (NEO-91)](../../../server/README.md#combat-entity-health-neo-80-neo-91). **NEO-92 landed:** **`IThreatStateStore`** + **`AggroOperations`** — first-hit acquire on damaging cast, leash clear on move/cast; cast/move/fixture hooks ([NEO-92](../../plans/NEO-92-implementation-plan.md)); [server README — Threat / aggro state (NEO-92)](../../../server/README.md#threat--aggro-state-neo-92). **NEO-93 landed:** **`INpcRuntimeStateStore`** + **`NpcRuntimeOperations.AdvanceAll`** — catalog-driven behavior state machine, lazy tick advance with delta cap; dev fixture reset ([NEO-93](../../plans/NEO-93-implementation-plan.md)); [server README — NPC runtime behavior state (NEO-93)](../../../server/README.md#npc-runtime-behavior-state-neo-93). **NEO-94 landed:** **`GET /game/world/npc-runtime-snapshot`** — `NpcRuntimeSnapshotWorldApi` + DTOs; lazy **`AdvanceAll`** on poll; nested **`activeTelegraph`** with server-computed **`windupRemainingSeconds`** ([NEO-94](../../plans/NEO-94-implementation-plan.md)); [server README — NPC runtime snapshot (NEO-94)](../../../server/README.md#npc-runtime-snapshot-neo-94); Bruno `bruno/neon-sprawl-server/npc-runtime-snapshot/`. **NEO-95 landed:** **`IPlayerCombatHealthStore`** + **`NpcAttackOperations.TryResolveTelegraphComplete`** — telegraph resolve via behavior **`attackAbilityId`** + ability **`maxRange`** / **`baseDamage`** ([NEO-95](../../plans/NEO-95-implementation-plan.md), [NEO-98](../../plans/NEO-98-implementation-plan.md)); [server README — Session player combat HP (NEO-95)](../../../server/README.md#session-player-combat-hp-neo-95); Bruno `bruno/neon-sprawl-server/combat-health/`. **NEO-96 landed:** comment-only **`telegraph_fired`** + **`npc_state_transition`** hook sites in **`NpcRuntimeOperations.AdvanceAll`** ([NEO-96](../../plans/NEO-96-implementation-plan.md)); [server README — NPC runtime telemetry hooks (NEO-96)](../../../server/README.md#npc-runtime-telemetry-hooks-neo-96). **NEO-97 landed:** client telegraph HUD — **`npc_runtime_client.gd`**, **`player_combat_health_client.gd`**, **`npc_runtime_hud_state.gd`**, **`TelegraphLabel`** / **`NpcStateLabel`** / **`PlayerCombatHpLabel`**; combat-active ~1 Hz poll ([NEO-97](../../plans/NEO-97-implementation-plan.md), [`NEO-97` manual QA](../../manual-qa/NEO-97.md)); [client README — NPC runtime + telegraph HUD (NEO-97)](../../../client/README.md#npc-runtime--telegraph-hud-neo-97). **NEO-98 landed:** playable NPC telegraph combat capstone — [`NEO-98` manual QA](../../manual-qa/NEO-98.md); `client/README.md` end-to-end NPC telegraph combat loop section ([NEO-98](../../plans/NEO-98-implementation-plan.md)). **NEO-98 server integration:** defeat clears aggro/runtime (**`TryStopOnTargetDefeat`**); telegraph damage range-gated via [`prototype_npc_abilities.json`](../../../content/abilities/prototype_npc_abilities.json) + behavior **`attackAbilityId`**. **Epic 5 Slice 2 client capstone complete.** **Backlog decomposed:** Epic 5 Slice 2 — [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md) **E5M2-01** [NEO-87](https://linear.app/neon-sprawl/issue/NEO-87) → **E5M2-12** [NEO-98](https://linear.app/neon-sprawl/issue/NEO-98) **landed**. Three archetypes (`prototype_melee_pressure`, `prototype_ranged_control`, `prototype_elite_mini_boss`); replaces alpha/beta dummies with **`prototype_npc_melee` / `ranged` / `elite`**. Owns **`ThreatState`**, **`TelegraphEvent`**, **`GET …/npc-runtime-snapshot`**, session **`IPlayerCombatHealthStore`**. Upstream **E5.M1 Ready**. | [E5M2-prototype-backlog.md](../../plans/E5M2-prototype-backlog.md), [E5_M2](E5_M2_NpcAiAndBehaviorProfiles.md), [NEO-87](../../plans/NEO-87-implementation-plan.md), [NEO-88](../../plans/NEO-88-implementation-plan.md), [NEO-89](../../plans/NEO-89-implementation-plan.md), [NEO-90](../../plans/NEO-90-implementation-plan.md), [NEO-91](../../plans/NEO-91-implementation-plan.md), [NEO-92](../../plans/NEO-92-implementation-plan.md), [NEO-93](../../plans/NEO-93-implementation-plan.md), [NEO-94](../../plans/NEO-94-implementation-plan.md), [NEO-95](../../plans/NEO-95-implementation-plan.md), [NEO-96](../../plans/NEO-96-implementation-plan.md), [NEO-97](../../plans/NEO-97-implementation-plan.md), [NEO-98](../../plans/NEO-98-implementation-plan.md), label **`E5.M2`** on NEO-87–NEO-98 | | E5.M3 | Ready | **E5M3-01 catalog landed ([NEO-100](https://linear.app/neon-sprawl/issue/NEO-100)):** encounter + reward-table schemas, `prototype_encounters.json`, `prototype_reward_tables.json`, CI gates. **E5M3-02 server load ([NEO-101](https://linear.app/neon-sprawl/issue/NEO-101)):** fail-fast startup loaders for encounter + reward-table catalogs (CI parity). **E5M3-03 registries ([NEO-102](https://linear.app/neon-sprawl/issue/NEO-102)):** `IEncounterDefinitionRegistry` + `IRewardTableDefinitionRegistry` + DI. **E5M3-04 HTTP read ([NEO-103](https://linear.app/neon-sprawl/issue/NEO-103)):** **`GET /game/world/encounter-definitions`** — `EncounterDefinitionsWorldApi` + nested **`rewardTable`** summary ([NEO-103](../../plans/NEO-103-implementation-plan.md)); [server README — Encounter definitions (NEO-103)](../../../server/README.md#encounter-definitions-neo-103); Bruno `bruno/neon-sprawl-server/encounter-definitions/`. **E5M3-05 stores ([NEO-104](https://linear.app/neon-sprawl/issue/NEO-104)):** **`IEncounterProgressStore`** + **`IEncounterCompletionStore`** + **`EncounterProgressOperations`** ([NEO-104](../../plans/NEO-104-implementation-plan.md)); [server README — Encounter progress (NEO-104)](../../../server/README.md#encounter-progress--completion-stores-neo-104). **E5M3-06 completion grants ([NEO-105](https://linear.app/neon-sprawl/issue/NEO-105)):** **`EncounterCompletionOperations`** + **`EncounterCompleteEvent`** result payload ([NEO-105](../../plans/NEO-105-implementation-plan.md)); [server README — Encounter completion (NEO-105)](../../../server/README.md#encounter-completion--inventory-grants-neo-105). **E5M3-07 combat wiring ([NEO-106](https://linear.app/neon-sprawl/issue/NEO-106)):** **`EncounterCombatWiring`** on **`AbilityCastApi`** ([NEO-106](../../plans/NEO-106-implementation-plan.md)); [server README — Encounter combat wiring (NEO-106)](../../../server/README.md#encounter-combat-wiring-neo-106). **E5M3-09 event record ([NEO-107](https://linear.app/neon-sprawl/issue/NEO-107)):** **`IEncounterCompleteEventStore`** + E7.M2 hook stub ([NEO-107](../../plans/NEO-107-implementation-plan.md)); [server README — Encounter complete event (NEO-107)](../../../server/README.md#encounter-complete-event-record-neo-107). **E5M3-08 per-player GET ([NEO-108](https://linear.app/neon-sprawl/issue/NEO-108)):** **`GET /game/players/{id}/encounter-progress`** — `EncounterProgressApi` + DTOs ([NEO-108](../../plans/NEO-108-implementation-plan.md)); [server README — Per-player encounter progress (NEO-108)](../../../server/README.md#per-player-encounter-progress-neo-108); Bruno `bruno/neon-sprawl-server/encounter-progress/`. **E5M3-10 telemetry ([NEO-109](https://linear.app/neon-sprawl/issue/NEO-109)):** comment-only **`encounter_start`**, **`encounter_complete`**, **`reward_attribution`**, **`encounter_complete_denied`** in **`EncounterProgressOperations`** / **`EncounterCompletionOperations`** ([NEO-109](../../plans/NEO-109-implementation-plan.md)); [server README — Encounter telemetry hooks (NEO-109)](../../../server/README.md#encounter-telemetry-hooks-neo-109). **E5M3-11 client HUD ([NEO-110](https://linear.app/neon-sprawl/issue/NEO-110)):** **`encounter_progress_client.gd`**, **`EncounterProgressLabel`** / **`EncounterCompleteLabel`**; boot + defeat-triggered GET; inventory refresh on **`completed`** ([NEO-110](../../plans/NEO-110-implementation-plan.md), [`NEO-110` manual QA](../../manual-qa/NEO-110.md)); [client README — Encounter progress + loot HUD (NEO-110)](../../../client/README.md#encounter-progress--loot-hud-neo-110). **E5M3-12 client capstone ([NEO-111](https://linear.app/neon-sprawl/issue/NEO-111)):** playable encounter clear loop — [`NEO-111` manual QA](../../manual-qa/NEO-111.md); [client README — End-to-end encounter clear loop (NEO-111)](../../../client/README.md#end-to-end-encounter-clear-loop-neo-111); plan [NEO-111](../../plans/NEO-111-implementation-plan.md). **Epic 5 Slice 3 client capstone complete.** Epic 5 Slice 3 backlog [E5M3-prototype-backlog.md](../../plans/E5M3-prototype-backlog.md) **E5M3-01** [NEO-100](https://linear.app/neon-sprawl/issue/NEO-100) → **E5M3-12** [NEO-111](https://linear.app/neon-sprawl/issue/NEO-111) **landed**. Prototype spine: **`prototype_combat_pocket`** → **`prototype_combat_pocket_clear`**; idempotent completion; per-defeat gig XP unchanged ([NEO-44](../../plans/NEO-44-implementation-plan.md)). Upstream **E5.M2 Ready**, **E3.M3** inventory landed. | [NEO-111 plan](../../plans/NEO-111-implementation-plan.md), [NEO-110 plan](../../plans/NEO-110-implementation-plan.md), [NEO-109 plan](../../plans/NEO-109-implementation-plan.md), [NEO-108 plan](../../plans/NEO-108-implementation-plan.md), [NEO-107 plan](../../plans/NEO-107-implementation-plan.md), [NEO-106 plan](../../plans/NEO-106-implementation-plan.md), [NEO-105 plan](../../plans/NEO-105-implementation-plan.md), [NEO-104 plan](../../plans/NEO-104-implementation-plan.md), [NEO-103 plan](../../plans/NEO-103-implementation-plan.md), [NEO-102 plan](../../plans/NEO-102-implementation-plan.md), [E5_M3](E5_M3_EncounterAndRewardTables.md), label **`E5.M3`** on NEO-100–NEO-111 | -| E7.M1 | Planned | **E7M1-01 catalog landed ([NEO-112](https://linear.app/neon-sprawl/issue/NEO-112)):** quest schemas, `prototype_quests.json`, CI gates (four frozen quest ids, objective cross-refs, acyclic prerequisites, chain terminal token). **E7M1-02 server load ([NEO-113](https://linear.app/neon-sprawl/issue/NEO-113)):** fail-fast startup load of `content/quests/*_quests.json` — `server/NeonSprawl.Server/Game/Quests/` ([NEO-113](../../plans/NEO-113-implementation-plan.md)); [server README — Quest catalog](../../../server/README.md#quest-catalog-contentquests-neo-113). **E7M1-03 registry ([NEO-114](https://linear.app/neon-sprawl/issue/NEO-114)):** **`IQuestDefinitionRegistry`** + DI ([NEO-114](../../plans/NEO-114-implementation-plan.md)). **E7M1-04 HTTP read ([NEO-115](https://linear.app/neon-sprawl/issue/NEO-115)):** **`GET /game/world/quest-definitions`** — `QuestDefinitionsWorldApi` + DTOs ([NEO-115](../../plans/NEO-115-implementation-plan.md)); [server README — Quest definitions (NEO-115)](../../../server/README.md#quest-definitions-neo-115); Bruno `bruno/neon-sprawl-server/quest-definitions/`. **E7M1-05 store ([NEO-116](https://linear.app/neon-sprawl/issue/NEO-116)):** **`IPlayerQuestStateStore`** + in-memory/Postgres persistence ([NEO-116](../../plans/NEO-116-implementation-plan.md)); [server README — Quest progress store (NEO-116)](../../../server/README.md#quest-progress-store-neo-116). **Backlog decomposed (runtime from E7M1-06+):** Epic 7 Slice 1 — [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md) **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) → **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123). Four frozen quests (3 onboarding + 1 gather→craft→combat chain); solo only; state-only completion in Slice 1 (rewards deferred to E7.M2). Client capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123). Upstream: E3.M1 gather **Ready**, E3.M2 craft **Ready**, E5.M3 encounter **`contract_handoff_token`** + **`EncounterCompleteEvent`** **Ready**. Register row stays **Planned** until quest runtime (E7M1-06+) lands. | [NEO-116 plan](../../plans/NEO-116-implementation-plan.md), [NEO-115 plan](../../plans/NEO-115-implementation-plan.md), [NEO-114 plan](../../plans/NEO-114-implementation-plan.md), [NEO-113 plan](../../plans/NEO-113-implementation-plan.md), [NEO-112 plan](../../plans/NEO-112-implementation-plan.md), [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md), [E7_M1](E7_M1_QuestStateMachine.md), label **`E7.M1`** on NEO-112–NEO-123 | +| E7.M1 | Planned | **E7M1-01 catalog landed ([NEO-112](https://linear.app/neon-sprawl/issue/NEO-112)):** quest schemas, `prototype_quests.json`, CI gates (four frozen quest ids, objective cross-refs, acyclic prerequisites, chain terminal token). **E7M1-02 server load ([NEO-113](https://linear.app/neon-sprawl/issue/NEO-113)):** fail-fast startup load of `content/quests/*_quests.json` — `server/NeonSprawl.Server/Game/Quests/` ([NEO-113](../../plans/NEO-113-implementation-plan.md)); [server README — Quest catalog](../../../server/README.md#quest-catalog-contentquests-neo-113). **E7M1-03 registry ([NEO-114](https://linear.app/neon-sprawl/issue/NEO-114)):** **`IQuestDefinitionRegistry`** + DI ([NEO-114](../../plans/NEO-114-implementation-plan.md)). **E7M1-04 HTTP read ([NEO-115](https://linear.app/neon-sprawl/issue/NEO-115)):** **`GET /game/world/quest-definitions`** — `QuestDefinitionsWorldApi` + DTOs ([NEO-115](../../plans/NEO-115-implementation-plan.md)); [server README — Quest definitions (NEO-115)](../../../server/README.md#quest-definitions-neo-115); Bruno `bruno/neon-sprawl-server/quest-definitions/`. **E7M1-05 store ([NEO-116](https://linear.app/neon-sprawl/issue/NEO-116)):** **`IPlayerQuestStateStore`** + in-memory/Postgres persistence ([NEO-116](../../plans/NEO-116-implementation-plan.md)); [server README — Quest progress store (NEO-116)](../../../server/README.md#quest-progress-store-neo-116). **E7M1-06 operations ([NEO-117](https://linear.app/neon-sprawl/issue/NEO-117)):** **`QuestStateOperations`** + reason codes ([NEO-117](../../plans/NEO-117-implementation-plan.md)); [server README — Quest state operations (NEO-117)](../../../server/README.md#quest-state-operations-neo-117). **Backlog decomposed (objective wiring from E7M1-07+):** Epic 7 Slice 1 — [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md) **E7M1-01** [NEO-112](https://linear.app/neon-sprawl/issue/NEO-112) → **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123). Four frozen quests (3 onboarding + 1 gather→craft→combat chain); solo only; state-only completion in Slice 1 (rewards deferred to E7.M2). Client capstone **E7M1-12** [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123). Upstream: E3.M1 gather **Ready**, E3.M2 craft **Ready**, E5.M3 encounter **`contract_handoff_token`** + **`EncounterCompleteEvent`** **Ready**. Register row stays **Planned** until E7M1-07+ objective wiring and client capstone. | [NEO-117 plan](../../plans/NEO-117-implementation-plan.md), [NEO-116 plan](../../plans/NEO-116-implementation-plan.md), [NEO-115 plan](../../plans/NEO-115-implementation-plan.md), [NEO-114 plan](../../plans/NEO-114-implementation-plan.md), [NEO-113 plan](../../plans/NEO-113-implementation-plan.md), [NEO-112 plan](../../plans/NEO-112-implementation-plan.md), [E7M1-prototype-backlog.md](../../plans/E7M1-prototype-backlog.md), [E7_M1](E7_M1_QuestStateMachine.md), label **`E7.M1`** on NEO-112–NEO-123 | --- diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index e118a21..c19f60f 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -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)); **E7M1-04 / NEO-115** HTTP read landed ([NEO-115 plan](../../plans/NEO-115-implementation-plan.md)); **E7M1-05 / NEO-116** player quest state store landed ([NEO-116 plan](../../plans/NEO-116-implementation-plan.md)); register row stays **Planned** until quest runtime (E7M1-06+) 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)); **E7M1-05 / NEO-116** player quest state store landed ([NEO-116 plan](../../plans/NEO-116-implementation-plan.md)); **E7M1-06 / NEO-117** **`QuestStateOperations`** landed ([NEO-117 plan](../../plans/NEO-117-implementation-plan.md)); register row stays **Planned** until E7M1-07+ objective wiring and client capstone. | 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 | diff --git a/docs/plans/E7M1-prototype-backlog.md b/docs/plans/E7M1-prototype-backlog.md index bb0fbfd..1956779 100644 --- a/docs/plans/E7M1-prototype-backlog.md +++ b/docs/plans/E7M1-prototype-backlog.md @@ -214,9 +214,9 @@ Working backlog for **Epic 7 — Slice 1** ([quest core and persistence](../deco **Acceptance criteria** -- [ ] Accept fails when prerequisite quest not **completed**. -- [ ] Complete is idempotent per player+quest. -- [ ] Reason codes documented in `server/README.md`. +- [x] Accept fails when prerequisite quest not **completed**. +- [x] Complete is idempotent per player+quest. +- [x] Reason codes documented in `server/README.md`. **Client counterpart:** [NEO-120](https://linear.app/neon-sprawl/issue/NEO-120) exposes accept POST to Godot. diff --git a/docs/plans/NEO-117-implementation-plan.md b/docs/plans/NEO-117-implementation-plan.md index be4c898..10ede08 100644 --- a/docs/plans/NEO-117-implementation-plan.md +++ b/docs/plans/NEO-117-implementation-plan.md @@ -58,7 +58,7 @@ - **Types:** `QuestStateReasonCodes`, `QuestStateOperationResult`, `QuestStateOperations` (`TryAccept`, `TryAdvanceStep`, `TryMarkComplete`). - **Tests:** `QuestStateOperationsTests` — 15 AAA cases (accept prerequisites, denials, advance bounds, idempotent complete, host DI smoke). -- **Docs:** `server/README.md` quest operations + reason-code table. +- **Docs:** `server/README.md` quest operations + reason-code table; E7.M1 alignment register, module doc, dependency register note, E7M1-06 backlog checkboxes. ## Technical approach @@ -132,8 +132,11 @@ flowchart TD | Path | Rationale | |------|-----------| -| `server/README.md` | Document `QuestStateOperations`, reason codes, idempotent complete semantics; note HTTP deferred to E7M1-08/09. | -| `server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs` | Update XML `` now that the type will exist (optional nit from NEO-116 review). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M1 row — E7M1-06 / NEO-117 operations landed. | +| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | NEO-117 operations bullet + README link. | +| `docs/decomposition/modules/module_dependency_register.md` | E7.M1 note — NEO-117 landed. | +| `docs/plans/E7M1-prototype-backlog.md` | E7M1-06 acceptance checkboxes checked. | +| `server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs` | Add `RefineIntroQuestId` and `CombatIntroQuestId` test constants. | ## Tests @@ -147,5 +150,5 @@ flowchart TD |-----------------|----------------------|--------| | **Idempotent complete vs encounter deny pattern** | Operations return `Success: true` on replay (E7M1-06 backlog); store layer unchanged. | **adopted** | | **Terminal step required before complete** | Defer to E7M1-07; operations do not validate objective satisfaction or terminal step index in NEO-117. | **adopted** | -| **Extra catalog test constants** | Optional `RefineIntroQuestId` / `CombatIntroQuestId` on `PrototypeE7M1QuestCatalogRules`; inline strings acceptable if only used in one test file. | **deferred** | +| **Extra catalog test constants** | Optional `RefineIntroQuestId` / `CombatIntroQuestId` on `PrototypeE7M1QuestCatalogRules`; inline strings acceptable if only used in one test file. | **adopted** | | **Prerequisite check strictness** | Require `TryGetProgress` + `Completed` for each prerequisite id (implicit `not_started` fails gate). | **adopted** | diff --git a/docs/reviews/2026-06-06-NEO-117.md b/docs/reviews/2026-06-06-NEO-117.md index 9e3f05c..04d6c6a 100644 --- a/docs/reviews/2026-06-06-NEO-117.md +++ b/docs/reviews/2026-06-06-NEO-117.md @@ -17,10 +17,10 @@ NEO-117 adds **`QuestStateOperations`** — static accept, step advance, and com | Path | Result | |------|--------| | `docs/plans/NEO-117-implementation-plan.md` | **Matches** — kickoff decisions adopted; acceptance checklist checked; reconciliation accurate. | -| `docs/plans/E7M1-prototype-backlog.md` (E7M1-06) | **Partially matches** — implementation satisfies AC; backlog checkboxes still `[ ]` (expected until merge/story end). | -| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Partially matches** — module page still lists **`QuestStateOperations`** as future E7M1-06; should note NEO-117 landed after merge. | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E7.M1 row ends at E7M1-05 / NEO-116; plan defers register update until story lands — append E7M1-06 / NEO-117 on merge. | -| `docs/decomposition/modules/module_dependency_register.md` | **Partially matches** — no NEO-117 note yet; same deferred update as NEO-116 review. | +| `docs/plans/E7M1-prototype-backlog.md` (E7M1-06) | **Matches** — E7M1-06 acceptance checkboxes checked. | +| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | **Matches** — NEO-117 operations bullet + README link added. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E7M1-06 / NEO-117 landed note appended. | +| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — NEO-117 note appended. | | `server/README.md` | **Matches** — quest operations section, reason-code table, idempotent complete semantics, HTTP deferral to NEO-119/NEO-120. | | Full-stack epic decomposition | **N/A** — server-only; client counterpart explicitly NEO-120/NEO-122; plan does not claim prototype slice complete. | @@ -32,19 +32,19 @@ NEO-117 adds **`QuestStateOperations`** — static accept, step advance, and com ## Suggestions -1. **Test `ResolveDependencies` pattern** — `QuestStateOperationsTests.ResolveDependencies` creates an `IServiceScope` that is never disposed. **`EncounterCompletionOperationsTests`** resolves from `factory.Services` root (singleton stores). Align to avoid undisposed scopes and match the established encounter test pattern. +1. ~~**Test `ResolveDependencies` pattern** — `QuestStateOperationsTests.ResolveDependencies` creates an `IServiceScope` that is never disposed. **`EncounterCompletionOperationsTests`** resolves from `factory.Services` root (singleton stores). Align to avoid undisposed scopes and match the established encounter test pattern.~~ **Done.** `ResolveDependencies` and host smoke test resolve from `factory.Services`. -2. **`unknown_quest` coverage on advance/complete** — Accept path has `TryAccept_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog`; advance and complete lack parity. Add two small `[Fact]` cases (mirror **`EncounterCompletionOperationsTests`** unknown-encounter test) for registry gate consistency. +2. ~~**`unknown_quest` coverage on advance/complete** — Accept path has `TryAccept_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog`; advance and complete lack parity. Add two small `[Fact]` cases (mirror **`EncounterCompletionOperationsTests`** unknown-encounter test) for registry gate consistency.~~ **Done.** Added `TryAdvanceStep_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog` and `TryMarkComplete_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog`. -3. **Post-merge decomposition docs** — Plan defers register/module updates; on story end append **E7M1-06 / NEO-117** to `documentation_and_implementation_alignment.md`, `E7_M1_QuestStateMachine.md` (operations bullet + README link), and check E7M1-06 AC boxes in `E7M1-prototype-backlog.md`. +3. ~~**Post-merge decomposition docs** — Plan defers register/module updates; on story end append **E7M1-06 / NEO-117** to `documentation_and_implementation_alignment.md`, `E7_M1_QuestStateMachine.md` (operations bullet + README link), and check E7M1-06 AC boxes in `E7M1-prototype-backlog.md`.~~ **Done.** Alignment register, module doc, dependency register note, and E7M1-06 backlog checkboxes updated. ## Nits -- Nit: **`TryAccept`** progress switch default `_ => Deny(UnknownQuest)` is unreachable while **`QuestProgressStatus`** is only `Active`/`Completed`; if kept for exhaustiveness, a more accurate code (or `NotActive`) would aid future enum extensions. +- ~~Nit: **`TryAccept`** progress switch default `_ => Deny(UnknownQuest)` is unreachable while **`QuestProgressStatus`** is only `Active`/`Completed`; if kept for exhaustiveness, a more accurate code (or `NotActive`) would aid future enum extensions.~~ **Done.** Replaced switch with explicit `Completed` / `AlreadyActive` branches. -- Nit: Chain-quest setup tests hardcode **`prototype_quest_combat_intro`** and **`prototype_quest_refine_intro`**; plan deferred **`RefineIntroQuestId`** / **`CombatIntroQuestId`** constants — acceptable for now, but NEO-118 wiring will reuse these ids. +- ~~Nit: Chain-quest setup tests hardcode **`prototype_quest_combat_intro`** and **`prototype_quest_refine_intro`**; plan deferred **`RefineIntroQuestId`** / **`CombatIntroQuestId`** constants — acceptable for now, but NEO-118 wiring will reuse these ids.~~ **Done.** Added **`RefineIntroQuestId`** and **`CombatIntroQuestId`** to **`PrototypeE7M1QuestCatalogRules`**; tests updated. -- Nit: **`TryMarkComplete`** re-calls **`TryGetProgress`** after the active-row read (lines 117–118) where `active` could be reused on the `not_active` deny path — minor readability only. +- ~~Nit: **`TryMarkComplete`** re-calls **`TryGetProgress`** after the active-row read (lines 117–118) where `active` could be reused on the `not_active` deny path — minor readability only.~~ **Done.** Single **`TryGetProgress`** read at start of **`TryMarkComplete`**. ## Verification diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs index 8ccc0f6..2ee2d1e 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs @@ -10,7 +10,8 @@ public sealed class QuestStateOperationsTests private const string PlayerId = "dev-local-1"; private const string UnknownPlayerId = "unknown-player-xyz"; private const string GatherQuestId = PrototypeE7M1QuestCatalogRules.GatherIntroQuestId; - private const string RefineQuestId = "prototype_quest_refine_intro"; + private const string RefineQuestId = PrototypeE7M1QuestCatalogRules.RefineIntroQuestId; + private const string CombatQuestId = PrototypeE7M1QuestCatalogRules.CombatIntroQuestId; private const string ChainQuestId = PrototypeE7M1QuestCatalogRules.ChainQuestId; private const string ChainObjectiveId = PrototypeE7M1QuestCatalogRules.ChainFirstObjectiveId; @@ -181,7 +182,7 @@ public sealed class QuestStateOperationsTests var timeProvider = TimeProvider.System; AcceptAndComplete(deps, GatherQuestId, timeProvider); AcceptAndComplete(deps, RefineQuestId, timeProvider); - AcceptAndComplete(deps, "prototype_quest_combat_intro", timeProvider); + AcceptAndComplete(deps, CombatQuestId, timeProvider); Assert.True(QuestStateOperations.TryAccept(PlayerId, ChainQuestId, deps.QuestRegistry, deps.ProgressStore).Success); Assert.True(deps.ProgressStore.TryUpdateObjectiveCounter(PlayerId, ChainQuestId, ChainObjectiveId, 5, out _)); @@ -211,7 +212,7 @@ public sealed class QuestStateOperationsTests var timeProvider = TimeProvider.System; AcceptAndComplete(deps, GatherQuestId, timeProvider); AcceptAndComplete(deps, RefineQuestId, timeProvider); - AcceptAndComplete(deps, "prototype_quest_combat_intro", timeProvider); + AcceptAndComplete(deps, CombatQuestId, timeProvider); Assert.True(QuestStateOperations.TryAccept(PlayerId, ChainQuestId, deps.QuestRegistry, deps.ProgressStore).Success); // Act @@ -227,6 +228,46 @@ public sealed class QuestStateOperationsTests Assert.Equal(QuestStateReasonCodes.InvalidStepIndex, result.ReasonCode); } + [Fact] + public async Task TryAdvanceStep_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryAdvanceStep( + PlayerId, + "prototype_quest_missing", + 1, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.UnknownQuest, result.ReasonCode); + } + + [Fact] + public async Task TryMarkComplete_ShouldDenyUnknownQuest_WhenQuestIdNotInCatalog() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryMarkComplete( + PlayerId, + "prototype_quest_missing", + deps.QuestRegistry, + deps.ProgressStore, + TimeProvider.System); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.UnknownQuest, result.ReasonCode); + } + [Fact] public async Task TryAdvanceStep_ShouldDenyNotActive_WhenQuestNotAccepted() { @@ -334,17 +375,15 @@ public sealed class QuestStateOperationsTests { // Arrange await using var factory = new InMemoryWebApplicationFactory(); - using var scope = factory.Services.CreateScope(); - var questRegistry = scope.ServiceProvider.GetRequiredService(); - var progressStore = scope.ServiceProvider.GetRequiredService(); + var deps = ResolveDependencies(factory); // Act - var accept = QuestStateOperations.TryAccept(PlayerId, GatherQuestId, questRegistry, progressStore); + var accept = QuestStateOperations.TryAccept(PlayerId, GatherQuestId, deps.QuestRegistry, deps.ProgressStore); var complete = QuestStateOperations.TryMarkComplete( PlayerId, GatherQuestId, - questRegistry, - progressStore, + deps.QuestRegistry, + deps.ProgressStore, TimeProvider.System); // Assert @@ -370,9 +409,9 @@ public sealed class QuestStateOperationsTests private static (IQuestDefinitionRegistry QuestRegistry, IPlayerQuestStateStore ProgressStore) ResolveDependencies( InMemoryWebApplicationFactory factory) { - var scope = factory.Services.CreateScope(); + var services = factory.Services; return ( - scope.ServiceProvider.GetRequiredService(), - scope.ServiceProvider.GetRequiredService()); + services.GetRequiredService(), + services.GetRequiredService()); } } diff --git a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs index 1de299f..0c168e0 100644 --- a/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs +++ b/server/NeonSprawl.Server/Game/Quests/PrototypeE7M1QuestCatalogRules.cs @@ -26,6 +26,12 @@ public static class PrototypeE7M1QuestCatalogRules /// First frozen onboarding quest id (gather intro). public const string GatherIntroQuestId = "prototype_quest_gather_intro"; + /// Refine onboarding quest id (requires gather intro complete). + public const string RefineIntroQuestId = "prototype_quest_refine_intro"; + + /// Combat onboarding quest id (no prerequisites). + public const string CombatIntroQuestId = "prototype_quest_combat_intro"; + /// First objective id on gather intro step (frozen catalog). public const string GatherIntroFirstObjectiveId = "gather_intro_obj_scrap"; diff --git a/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs index ef23447..1ca6717 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs @@ -23,12 +23,12 @@ public static class QuestStateOperations if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var existing)) { - return existing.Status switch + if (existing.Status == QuestProgressStatus.Completed) { - QuestProgressStatus.Completed => Deny(QuestStateReasonCodes.AlreadyCompleted, existing), - QuestProgressStatus.Active => Deny(QuestStateReasonCodes.AlreadyActive, existing), - _ => Deny(QuestStateReasonCodes.UnknownQuest), - }; + return Deny(QuestStateReasonCodes.AlreadyCompleted, existing); + } + + return Deny(QuestStateReasonCodes.AlreadyActive, existing); } foreach (var prerequisiteId in definition.PrerequisiteQuestIds) @@ -108,14 +108,17 @@ public static class QuestStateOperations return Deny(QuestStateReasonCodes.UnknownQuest); } - if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var existing) && - existing.Status == QuestProgressStatus.Completed) + if (!progressStore.TryGetProgress(playerId, normalizedQuestId, out var progress)) { - return Success(existing); + return Deny(QuestStateReasonCodes.NotActive); } - if (!progressStore.TryGetProgress(playerId, normalizedQuestId, out var active) || - active.Status != QuestProgressStatus.Active) + if (progress.Status == QuestProgressStatus.Completed) + { + return Success(progress); + } + + if (progress.Status != QuestProgressStatus.Active) { return Deny(QuestStateReasonCodes.NotActive); } From 540257d12d97c331a7dc5bb9fdd787cffb285133 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 6 Jun 2026 19:36:41 -0400 Subject: [PATCH 5/5] NEO-117: Fix Bugbot deny-code misclassification on quest accept. Add CanWritePlayer to IPlayerQuestStateStore and check before prerequisite gate; map TryActivate/TryAdvanceStep store failures from progress snapshots instead of always unknown_player. --- docs/reviews/2026-06-06-NEO-117.md | 6 ++ .../Game/Quests/QuestStateOperationsTests.cs | 44 ++++++++++++ .../Game/Quests/IPlayerQuestStateStore.cs | 3 + .../Quests/InMemoryPlayerQuestStateStore.cs | 7 ++ .../Quests/PostgresPlayerQuestStateStore.cs | 14 ++++ .../Game/Quests/QuestStateOperations.cs | 67 ++++++++++++++++++- server/README.md | 1 + 7 files changed, 139 insertions(+), 3 deletions(-) diff --git a/docs/reviews/2026-06-06-NEO-117.md b/docs/reviews/2026-06-06-NEO-117.md index 04d6c6a..62d6fe9 100644 --- a/docs/reviews/2026-06-06-NEO-117.md +++ b/docs/reviews/2026-06-06-NEO-117.md @@ -30,6 +30,12 @@ NEO-117 adds **`QuestStateOperations`** — static accept, step advance, and com (none) +## Bugbot follow-up (2026-06-06) + +1. ~~**TryActivate failure misclassified** — Map store mutation failures from returned snapshots / re-read progress instead of always `unknown_player`.~~ **Done.** `DenyActivateFailure` / `DenyAdvanceFailure` helpers; store replay on `TryMarkComplete` returns idempotent success. + +2. ~~**Unknown player wrong deny code** — Check `CanWritePlayer` before prerequisite gate so unknown players get `unknown_player`, not `prerequisite_incomplete`.~~ **Done.** Added `IPlayerQuestStateStore.CanWritePlayer` (in-memory + Postgres); `TryAccept` calls it before prerequisites. + ## Suggestions 1. ~~**Test `ResolveDependencies` pattern** — `QuestStateOperationsTests.ResolveDependencies` creates an `IServiceScope` that is never disposed. **`EncounterCompletionOperationsTests`** resolves from `factory.Services` root (singleton stores). Align to avoid undisposed scopes and match the established encounter test pattern.~~ **Done.** `ResolveDependencies` and host smoke test resolve from `factory.Services`. diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs index 2ee2d1e..d4e66a9 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestStateOperationsTests.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.PositionState; using NeonSprawl.Server.Game.Quests; using NeonSprawl.Server.Tests; using Xunit; @@ -154,6 +155,25 @@ public sealed class QuestStateOperationsTests Assert.Equal(QuestProgressStatus.Completed, result.Snapshot!.Status); } + [Fact] + public async Task TryAccept_ShouldDenyUnknownPlayer_WhenPrerequisiteQuestNotWritable() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var deps = ResolveDependencies(factory); + + // Act + var result = QuestStateOperations.TryAccept( + UnknownPlayerId, + RefineQuestId, + deps.QuestRegistry, + deps.ProgressStore); + + // Assert + Assert.False(result.Success); + Assert.Equal(QuestStateReasonCodes.UnknownPlayer, result.ReasonCode); + } + [Fact] public async Task TryAccept_ShouldDenyUnknownPlayer_WhenPlayerNotInStore() { @@ -392,6 +412,30 @@ public sealed class QuestStateOperationsTests Assert.Equal(QuestProgressStatus.Completed, complete.Snapshot!.Status); } + [Fact] + public void CanWritePlayer_ShouldReturnFalse_ForUnknownPlayer() + { + // Arrange + var store = new InMemoryPlayerQuestStateStore( + Microsoft.Extensions.Options.Options.Create(new GamePositionOptions { DevPlayerId = PlayerId })); + // Act + var canWrite = store.CanWritePlayer(UnknownPlayerId); + // Assert + Assert.False(canWrite); + } + + [Fact] + public void CanWritePlayer_ShouldReturnTrue_ForDevPlayer() + { + // Arrange + var store = new InMemoryPlayerQuestStateStore( + Microsoft.Extensions.Options.Options.Create(new GamePositionOptions { DevPlayerId = PlayerId })); + // Act + var canWrite = store.CanWritePlayer(PlayerId); + // Assert + Assert.True(canWrite); + } + private static void AcceptAndComplete( (IQuestDefinitionRegistry QuestRegistry, IPlayerQuestStateStore ProgressStore) deps, string questId, diff --git a/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs b/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs index 53342d4..1b48cd7 100644 --- a/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs +++ b/server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs @@ -6,6 +6,9 @@ namespace NeonSprawl.Server.Game.Quests; /// public interface IPlayerQuestStateStore { + /// True when mutations for are allowed (dev bucket or Postgres player_position row). + bool CanWritePlayer(string playerId); + /// Missing row ⇒ false (callers treat as not_started). bool TryGetProgress(string playerId, string questId, out QuestStepState snapshot); diff --git a/server/NeonSprawl.Server/Game/Quests/InMemoryPlayerQuestStateStore.cs b/server/NeonSprawl.Server/Game/Quests/InMemoryPlayerQuestStateStore.cs index c32e320..487b2e8 100644 --- a/server/NeonSprawl.Server/Game/Quests/InMemoryPlayerQuestStateStore.cs +++ b/server/NeonSprawl.Server/Game/Quests/InMemoryPlayerQuestStateStore.cs @@ -66,6 +66,13 @@ public sealed class InMemoryPlayerQuestStateStore(IOptions return new HashSet(StringComparer.OrdinalIgnoreCase) { id }; } + /// + public bool CanWritePlayer(string playerId) + { + var player = QuestProgressIds.NormalizePlayerId(playerId); + return player.Length > 0 && knownPlayers.Contains(player); + } + /// public bool TryGetProgress(string playerId, string questId, out QuestStepState snapshot) { diff --git a/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs b/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs index 2a60b37..b8f4631 100644 --- a/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs +++ b/server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs @@ -10,6 +10,20 @@ public sealed class PostgresPlayerQuestStateStore(Npgsql.NpgsqlDataSource dataSo PropertyNamingPolicy = JsonNamingPolicy.CamelCase, }; + /// + public bool CanWritePlayer(string playerId) + { + var player = QuestProgressIds.NormalizePlayerId(playerId); + if (player.Length == 0) + { + return false; + } + + PostgresPlayerQuestProgressBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + return PlayerExists(conn, player); + } + /// public bool TryGetProgress(string playerId, string questId, out QuestStepState snapshot) { diff --git a/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs index 1ca6717..4fd4bf3 100644 --- a/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs +++ b/server/NeonSprawl.Server/Game/Quests/QuestStateOperations.cs @@ -21,6 +21,11 @@ public static class QuestStateOperations return Deny(QuestStateReasonCodes.UnknownQuest); } + if (!progressStore.CanWritePlayer(playerId)) + { + return Deny(QuestStateReasonCodes.UnknownPlayer); + } + if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var existing)) { if (existing.Status == QuestProgressStatus.Completed) @@ -45,7 +50,7 @@ public static class QuestStateOperations return Success(snapshot); } - return Deny(QuestStateReasonCodes.UnknownPlayer); + return DenyActivateFailure(progressStore, playerId, normalizedQuestId, snapshot); } /// @@ -89,7 +94,7 @@ public static class QuestStateOperations return Success(snapshot); } - return Deny(QuestStateReasonCodes.UnknownPlayer, existing); + return DenyAdvanceFailure(progressStore, playerId, normalizedQuestId, snapshot, existing); } /// @@ -128,13 +133,69 @@ public static class QuestStateOperations return Success(snapshot); } + if (snapshot?.Status == QuestProgressStatus.Completed) + { + return Success(snapshot); + } + if (progressStore.TryGetProgress(playerId, normalizedQuestId, out var afterAttempt) && afterAttempt.Status == QuestProgressStatus.Completed) { return Success(afterAttempt); } - return Deny(QuestStateReasonCodes.UnknownPlayer); + if (!progressStore.CanWritePlayer(playerId)) + { + return Deny(QuestStateReasonCodes.UnknownPlayer); + } + + return DenyFromProgressSnapshot(snapshot, QuestStateReasonCodes.NotActive) + ?? Deny(QuestStateReasonCodes.UnknownPlayer, progress); + } + + private static QuestStateOperationResult DenyActivateFailure( + IPlayerQuestStateStore progressStore, + string playerId, + string questId, + QuestStepState attemptSnapshot) => + DenyFromProgressSnapshot(attemptSnapshot, QuestStateReasonCodes.AlreadyActive) + ?? (progressStore.TryGetProgress(playerId, questId, out var reread) + ? DenyFromProgressSnapshot(reread, QuestStateReasonCodes.AlreadyActive) + : null) + ?? Deny(QuestStateReasonCodes.UnknownPlayer); + + private static QuestStateOperationResult DenyAdvanceFailure( + IPlayerQuestStateStore progressStore, + string playerId, + string questId, + QuestStepState attemptSnapshot, + QuestStepState priorRead) => + DenyFromProgressSnapshot(attemptSnapshot, QuestStateReasonCodes.InvalidStepIndex) + ?? (progressStore.TryGetProgress(playerId, questId, out var reread) + ? DenyFromProgressSnapshot(reread, QuestStateReasonCodes.InvalidStepIndex) + : null) + ?? Deny(QuestStateReasonCodes.UnknownPlayer, priorRead); + + private static QuestStateOperationResult? DenyFromProgressSnapshot( + QuestStepState? snapshot, + string activeOrIntermediateReasonCode) + { + if (snapshot is null) + { + return null; + } + + if (snapshot.Status == QuestProgressStatus.Completed) + { + return Deny(QuestStateReasonCodes.AlreadyCompleted, snapshot); + } + + if (snapshot.Status == QuestProgressStatus.Active) + { + return Deny(activeOrIntermediateReasonCode, snapshot); + } + + return null; } private static QuestStateOperationResult Success(QuestStepState snapshot) => diff --git a/server/README.md b/server/README.md index c45c63c..b658051 100644 --- a/server/README.md +++ b/server/README.md @@ -162,6 +162,7 @@ Per-player quest runtime state lives in **`IPlayerQuestStateStore`**, keyed by * **Store interface methods:** +- **`CanWritePlayer`** — whether mutations are allowed for the player (dev bucket / Postgres `player_position`). - **`TryGetProgress`** — read one row; false when not started. - **`TryActivate`** — first accept (`not_started` → `active`). - **`TryAdvanceStep`** — bump step index and clear counters for the new step (denies when completed or index does not increase).