160 lines
14 KiB
Markdown
160 lines
14 KiB
Markdown
# NEO-116 — Implementation plan
|
|
|
|
## Story reference
|
|
|
|
| Field | Value |
|
|
|--------|--------|
|
|
| **Key** | NEO-116 |
|
|
| **Title** | E7M1-05: Player quest state store + QuestStepState persistence |
|
|
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-116/e7m1-05-player-quest-state-store-queststepstate-persistence |
|
|
| **Module** | [E7.M1 — QuestStateMachine](../decomposition/modules/E7_M1_QuestStateMachine.md) · Epic 7 Slice 1 · backlog **E7M1-05** |
|
|
| **Branch** | `NEO-116-player-quest-state-store` |
|
|
| **Precursor** | [NEO-114](https://linear.app/neon-sprawl/issue/NEO-114) — quest definition registry (**Done**); [NEO-115](https://linear.app/neon-sprawl/issue/NEO-115) — GET `/game/world/quest-definitions` (**landed on `main`**) |
|
|
| **Pattern** | [NEO-104](NEO-104-implementation-plan.md) — keyed in-memory store + atomic mutations; [NEO-44](NEO-44-implementation-plan.md) — Postgres when configured + persistence integration test |
|
|
| **Blocks** | [NEO-117](https://linear.app/neon-sprawl/issue/NEO-117) — `QuestStateOperations` (accept, step advance, complete); E7M1-07+ objective wiring |
|
|
| **Client counterpart** | None — server-only store; player-visible progress starts at [NEO-122](https://linear.app/neon-sprawl/issue/NEO-122) / [NEO-123](https://linear.app/neon-sprawl/issue/NEO-123) |
|
|
|
|
## Kickoff clarifications
|
|
|
|
| Topic | Question | Agent recommendation | Answer |
|
|
|--------|----------|----------------------|--------|
|
|
| **Postgres persistence** | Include `V008` migration + `PostgresPlayerQuestStateStore` in NEO-116 vs defer? | **Include when configured** — E7M1 backlog default is in-memory **+ optional Postgres**; mirror `AddGigProgressionStore` / `PostgresPlayerGigProgressionStore`. | **Adopted** — user chose in-memory + Postgres when configured. |
|
|
| **Store API shape** | Mutation methods on store vs minimal `TryGet` + `TryReplace` only? | **Mutation methods on store** — `TryActivate`, `TryAdvanceStep`, `TryUpdateObjectiveCounter`, `TryMarkComplete`; E7M1-06 `QuestStateOperations` adds prerequisite/registry validation on top (NEO-104 encounter store precedent). | **Adopted** (agent default; no objection). |
|
|
| **`not_started` representation** | Missing row vs explicit row with `not_started` status? | **Implicit missing row** — encounter progress pattern; E7M1-08 HTTP merges registry + store. | **Adopted** (agent default). |
|
|
| **DI registration** | Extend `AddQuestDefinitionCatalog` vs separate extension? | **Separate `AddPlayerQuestStateStore`** — persistence separate from catalog load (NEO-113/114), like `AddGigProgressionStore`. | **Adopted** (agent default). |
|
|
| **Objective counters scope** | Counters for current step only vs full quest history? | **Current step only** — reset counters on `TryAdvanceStep`; sufficient for E7M1-07 quantity objectives. | **Adopted** (agent default). |
|
|
|
|
## Goal, scope, and out-of-scope
|
|
|
|
**Goal:** Durable per-player quest progress rows keyed by **`(playerId, questId)`**: status (`not_started` implicit / `active` / `completed`), current step index, per-objective counters for the active step, and completion timestamp. Provide **`IPlayerQuestStateStore`** with in-memory and Postgres implementations registered in DI.
|
|
|
|
**In scope (from Linear + [E7M1-05](E7M1-prototype-backlog.md#e7m1-05--player-quest-state-store--queststepstate-persistence)):**
|
|
|
|
- **`QuestStepState`** snapshot type and **`PlayerQuestProgressRow`** (or equivalent immutable read model).
|
|
- **`QuestProgressStatus`** enum: `Active`, `Completed` (missing row ⇒ `not_started` at call sites).
|
|
- **`IPlayerQuestStateStore`** + **`InMemoryPlayerQuestStateStore`** (dev player bucket seeded; thread-safe).
|
|
- **`PostgresPlayerQuestStateStore`** + **`PostgresPlayerQuestProgressBootstrap`** + **`V008__player_quest_progress.sql`** when `ConnectionStrings:NeonSprawl` is set.
|
|
- **`QuestProgressIds`** — player/quest id normalization + composite key helper (mirror `EncounterProgressIds`).
|
|
- **`QuestStateServiceCollectionExtensions.AddPlayerQuestStateStore`** wired from **`Program.cs`**.
|
|
- Unit tests (AAA): accept (`not_started` → `active`), step advance + counter updates, idempotent complete, completed row cannot regress to active.
|
|
- Postgres persistence integration test (`RequirePostgresFact`): write via store on first factory, read on second factory.
|
|
- `server/README.md` quest progress store section (brief).
|
|
- Update alignment register E7.M1 row when complete.
|
|
|
|
**Out of scope (from Linear + backlog):**
|
|
|
|
- **`QuestStateOperations`** prerequisite enforcement and reason codes ([NEO-117](https://linear.app/neon-sprawl/issue/NEO-117) / E7M1-06).
|
|
- Gather/craft/encounter objective wiring (E7M1-07).
|
|
- HTTP **`GET /game/players/{id}/quest-progress`** (E7M1-08) and accept POST (E7M1-09).
|
|
- Bruno, Godot, dev fixture reset API for quest rows (may add in NEO-108-style story if needed).
|
|
- Reward grants / E7.M2 bundles.
|
|
|
|
## Acceptance criteria checklist
|
|
|
|
- [x] Store supports read/update per `(playerId, questId)`.
|
|
- [x] Completed quest cannot regress to active without explicit reset API (none in prototype — store denies `TryActivate` / `TryAdvanceStep` / counter updates when completed).
|
|
- [x] Idempotent complete: second `TryMarkComplete` returns `false` without changing `CompletedAt` (Linear AC).
|
|
|
|
## Implementation reconciliation (shipped)
|
|
|
|
- **Types:** `QuestProgressStatus`, `QuestStepState`, `QuestProgressIds`.
|
|
- **Store:** `IPlayerQuestStateStore`; `InMemoryPlayerQuestStateStore` (dev player seeded); `PostgresPlayerQuestStateStore` + `V008__player_quest_progress.sql` when Postgres configured.
|
|
- **DI:** `AddPlayerQuestStateStore` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`.
|
|
- **Tests:** `InMemoryPlayerQuestStateStoreTests` (10 AAA cases + host DI); `PlayerQuestProgressPersistenceIntegrationTests` (`RequirePostgresFact`).
|
|
- **Docs:** `server/README.md` quest progress store section; alignment register E7.M1 row updated.
|
|
|
|
## Technical approach
|
|
|
|
1. **Types (`Game/Quests/`)**
|
|
- **`QuestProgressStatus`**: `Active`, `Completed`.
|
|
- **`QuestStepState`**: immutable snapshot for one quest row — `PlayerId`, `QuestId`, `Status`, `CurrentStepIndex` (0-based), `IReadOnlyDictionary<string, int> ObjectiveCounters` (objective id → accumulated count for **current step only**), optional `CompletedAt`.
|
|
- **`QuestProgressIds`**: `NormalizePlayerId`, `NormalizeQuestId`, `MakeProgressKey` (trim + lowercase; empty guard).
|
|
|
|
2. **`IPlayerQuestStateStore`**
|
|
- `bool TryGetProgress(string playerId, string questId, out QuestStepState snapshot)` — missing row ⇒ `false` (callers treat as `not_started`).
|
|
- `bool TryActivate(string playerId, string questId, out QuestStepState snapshot)` — creates row at step 0, empty counters; `true` on first activation; `false` if already active or completed or unknown player (in-memory: no dev bucket).
|
|
- `bool TryAdvanceStep(string playerId, string questId, int newStepIndex, out QuestStepState snapshot)` — requires active row; sets index, **clears** objective counters; denies when completed or `newStepIndex` ≤ current.
|
|
- `bool TryUpdateObjectiveCounter(string playerId, string questId, string objectiveId, int newCount, out QuestStepState snapshot)` — requires active row; sets counter for objective id (non-negative); denies when completed.
|
|
- `bool TryMarkComplete(string playerId, string questId, DateTimeOffset completedAt, out QuestStepState snapshot)` — first mark `true`; replay `false`; preserves first `CompletedAt`.
|
|
- XML remarks: game code should inject this interface; **`QuestStateOperations`** (NEO-117) and HTTP (E7M1-08) consume it — not the catalog.
|
|
|
|
3. **`InMemoryPlayerQuestStateStore`**
|
|
- `ConcurrentDictionary<string, ProgressRow>` keyed by `MakeProgressKey`; per-key locks.
|
|
- Seed configured dev player bucket on construction (`GamePositionOptions.DevPlayerId`) — empty inner map (all quests implicit `not_started`).
|
|
- `CanWritePlayer` semantics: returns false when player has no bucket (mirror gig/inventory stores).
|
|
|
|
4. **Postgres (`V008__player_quest_progress.sql`)**
|
|
- Table **`player_quest_progress`**: `player_id` FK → `player_position`, `quest_id`, `status` (`active` / `completed`), `current_step_index`, `objective_counters` JSONB (objective id → int map), `completed_at` TIMESTAMPTZ NULL, `updated_at`, PK `(player_id, quest_id)`.
|
|
- **`PostgresPlayerQuestProgressBootstrap.EnsureSchema`** — idempotent DDL from migration file (NEO-44 pattern).
|
|
- **`PostgresPlayerQuestStateStore`** — same interface; deny writes when player missing from `player_position`; use transactions + `FOR UPDATE` on mutations.
|
|
|
|
5. **DI**
|
|
- **`QuestStateServiceCollectionExtensions.AddPlayerQuestStateStore(configuration)`**:
|
|
- When connection string set → `PostgresPlayerQuestStateStore`.
|
|
- Else → `InMemoryPlayerQuestStateStore`.
|
|
- Call from **`Program.cs`** after quest catalog registration.
|
|
- Extend **`InMemoryWebApplicationFactory`** — strip Postgres quest store + register in-memory implementation (same list as gig progression).
|
|
|
|
6. **Tests**
|
|
- **`InMemoryPlayerQuestStateStoreTests`**: AAA coverage for activate, advance (counter reset), counter update, idempotent complete, completed-row denial on activate/advance/counter, unknown player false, host DI resolve.
|
|
- **`PlayerQuestProgressPersistenceIntegrationTests`**: direct store mutation on `PostgresWebApplicationFactory`, second factory reads same row (no HTTP — store-only story).
|
|
|
|
7. **Docs**
|
|
- `server/README.md` — quest progress store section (interface methods, implicit `not_started`, Postgres table, note HTTP deferred to E7M1-08).
|
|
- `documentation_and_implementation_alignment.md` E7.M1 row — note NEO-116 store landed; register stays **Planned** until E7M1-06+ runtime.
|
|
|
|
### Prototype store behavior (frozen for tests)
|
|
|
|
| Step | Expected |
|
|
|------|----------|
|
|
| Missing row | `TryGetProgress` false; `TryActivate` → active, step 0, empty counters |
|
|
| Re-activate | `TryActivate` false (already active) |
|
|
| Counter | `TryUpdateObjectiveCounter(..., "gather_scrap", 3)` → counter map updated |
|
|
| Advance | `TryAdvanceStep(..., 1)` → step 1, counters cleared |
|
|
| Complete | `TryMarkComplete` once → true, status completed; second → false |
|
|
| Regression guard | After complete, `TryActivate` / `TryAdvanceStep` / `TryUpdateObjectiveCounter` all false |
|
|
|
|
Use frozen quest id **`prototype_quest_gather_intro`** and objective id from catalog in tests (load from `PrototypeE7M1QuestCatalogRules` / first step objective where practical).
|
|
|
|
## Files to add
|
|
|
|
| Path | Purpose |
|
|
|------|---------|
|
|
| `server/NeonSprawl.Server/Game/Quests/QuestProgressStatus.cs` | Status enum for active/completed rows. |
|
|
| `server/NeonSprawl.Server/Game/Quests/QuestStepState.cs` | Immutable progress snapshot (module contract). |
|
|
| `server/NeonSprawl.Server/Game/Quests/QuestProgressIds.cs` | Player/quest id normalization and composite key. |
|
|
| `server/NeonSprawl.Server/Game/Quests/IPlayerQuestStateStore.cs` | Store contract. |
|
|
| `server/NeonSprawl.Server/Game/Quests/InMemoryPlayerQuestStateStore.cs` | Thread-safe in-memory implementation. |
|
|
| `server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestProgressBootstrap.cs` | Applies V008 DDL once per process. |
|
|
| `server/NeonSprawl.Server/Game/Quests/PostgresPlayerQuestStateStore.cs` | PostgreSQL-backed implementation. |
|
|
| `server/NeonSprawl.Server/Game/Quests/QuestStateServiceCollectionExtensions.cs` | DI: Postgres when configured, else in-memory. |
|
|
| `server/db/migrations/V008__player_quest_progress.sql` | Quest progress table DDL. |
|
|
| `server/NeonSprawl.Server.Tests/Game/Quests/InMemoryPlayerQuestStateStoreTests.cs` | AAA unit + host DI tests. |
|
|
| `server/NeonSprawl.Server.Tests/Game/Quests/PlayerQuestProgressPersistenceIntegrationTests.cs` | Postgres cross-factory persistence test. |
|
|
| `docs/plans/NEO-116-implementation-plan.md` | This plan. |
|
|
|
|
## Files to modify
|
|
|
|
| Path | Rationale |
|
|
|------|-----------|
|
|
| `server/NeonSprawl.Server/Program.cs` | Register `AddPlayerQuestStateStore` after quest catalog. |
|
|
| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Force in-memory quest store in unit/integration tests (strip Postgres registration). |
|
|
| `server/README.md` | Document quest progress store, V008 table, implicit `not_started`, deferred HTTP. |
|
|
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M1 row — note NEO-116 store when complete. |
|
|
|
|
## Tests
|
|
|
|
| Test file | What it covers |
|
|
|-----------|----------------|
|
|
| `InMemoryPlayerQuestStateStoreTests.cs` | **Unit:** `TryActivate` creates active row at step 0; second activate false. **Unit:** `TryUpdateObjectiveCounter` updates map; denied when completed. **Unit:** `TryAdvanceStep` bumps index and clears counters; denied when completed or non-increasing index. **Unit:** `TryMarkComplete` idempotent (second false, `CompletedAt` unchanged). **Unit:** completed row denies activate/advance/counter. **Unit:** unknown player / empty ids false. **Host:** `InMemoryWebApplicationFactory` resolves `IPlayerQuestStateStore`; smoke activate + get on dev player + frozen quest id. |
|
|
| `PlayerQuestProgressPersistenceIntegrationTests.cs` | **Postgres:** `TryActivate` + `TryUpdateObjectiveCounter` + `TryMarkComplete` on first factory; second `PostgresWebApplicationFactory` `TryGetProgress` returns completed row with same counters-at-complete semantics and `CompletedAt`. |
|
|
|
|
## Open questions / risks
|
|
|
|
| Question / risk | Agent recommendation | Status |
|
|
|-----------------|----------------------|--------|
|
|
| **JSONB vs normalized objective counter table** | Single JSONB column on `player_quest_progress` — flexible for variable objectives per step; prototype four quests only. | **adopted** |
|
|
| **Quest id normalization** | Lowercase + trim via `QuestProgressIds`; store keys use normalized quest id; align with `IQuestDefinitionRegistry.TryNormalizeKnown` at operation layer (NEO-117). | **adopted** |
|
|
| **Dev fixture quest reset** | Omit this story; Bruno quest flows may need clear hook in E7M1-08/09 or follow NEO-108 encounter reset pattern. | **deferred** |
|
|
| **Constants for tests** | Reuse `PrototypeE7M1QuestCatalogRules.ExpectedQuestIds` / gather intro id; avoid duplicating quest id strings. | **adopted** |
|