155 lines
11 KiB
Markdown
155 lines
11 KiB
Markdown
# 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
|
|
|
|
- [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; E7.M1 alignment register, module doc, dependency register note, E7M1-06 backlog checkboxes.
|
|
|
|
## 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 |
|
|
|------|-----------|
|
|
| `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
|
|
|
|
| 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. | **adopted** |
|
|
| **Prerequisite check strictness** | Require `TryGetProgress` + `Completed` for each prerequisite id (implicit `not_started` fails gate). | **adopted** |
|