NEO-116: Add kickoff implementation plan for player quest state store.
Documents store contract, Postgres V008, tests, and kickoff decisions (in-memory + Postgres when configured) before E7M1-06 operations land.pull/155/head
parent
91d6cba34c
commit
065872405c
|
|
@ -0,0 +1,151 @@
|
|||
# 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
|
||||
|
||||
- [ ] Store supports read/update per `(playerId, questId)`.
|
||||
- [ ] Completed quest cannot regress to active without explicit reset API (none in prototype — store denies `TryActivate` / `TryAdvanceStep` / counter updates when completed).
|
||||
- [ ] Idempotent complete: second `TryMarkComplete` returns `false` without changing `CompletedAt` (Linear AC).
|
||||
|
||||
## 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** |
|
||||
Loading…
Reference in New Issue