From 44ef5f24848b812729c95ff04bef5a098653d5f2 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sat, 6 Jun 2026 19:23:55 -0400 Subject: [PATCH] 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** |