neon-sprawl/docs/plans/NEO-119-implementation-plan.md

163 lines
12 KiB
Markdown

# NEO-119 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-119 |
| **Title** | E7M1-08: GET /game/players/{id}/quest-progress |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-119/e7m1-08-get-gameplayersidquest-progress |
| **Module** | [E7.M1 — QuestStateMachine](../decomposition/modules/E7_M1_QuestStateMachine.md) · Epic 7 Slice 1 · backlog **E7M1-08** |
| **Branch** | `NEO-119-get-quest-progress` |
| **Precursor** | [NEO-118](https://linear.app/neon-sprawl/issue/NEO-118) — `QuestObjectiveWiring` gather/craft/encounter + `inventory_has_item` (**landed on `main`**) |
| **Pattern** | [NEO-108](NEO-108-implementation-plan.md) — registry-backed player GET + position gate + Bruno; [NEO-115](NEO-115-implementation-plan.md) — quest registry ordering |
| **Blocks** | [NEO-122](https://linear.app/neon-sprawl/issue/NEO-122) — client quest progress HUD (E7M1-11) |
| **Client counterpart** | [NEO-122](https://linear.app/neon-sprawl/issue/NEO-122) — **`quest_progress_client.gd`** + HUD labels poll this GET after gather/craft/encounter mutations (E7M1-11). Bruno-only verification is **not** prototype-complete per [full-stack epic decomposition](../../.cursor/rules/full-stack-epic-decomposition.md). |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **GET-side `inventory_has_item` re-eval** | Should GET re-run inventory snapshot wiring before building the response? (NEO-118 deferred “GET polling” to NEO-119.) | **Re-eval on GET** — call wiring inventory refresh + step completion pass so HUD reflects pre-held tokens (e.g. chain terminal **`contract_handoff_token`** from combat intro) without another inventory mutation. | **Adopted** — user confirmed |
**Additional defaults (no kickoff question — settled by backlog / precedent):**
- **`status`** strings: `not_started` / `active` / `completed` (NEO-116 implicit missing row → `not_started` at HTTP layer).
- **`objectiveCounters`**: object map `{ objectiveId: count }` — mirrors store JSONB and current-step-only semantics.
- **Player gate:** `IPositionStateStore.TryGetPosition`**404** for unknown/blank ids (encounter-progress / gig-progression precedent).
- **Quest roster:** all catalog quests via **`GetDefinitionsInIdOrder()`**; ordinal `id` order matches NEO-115.
- **Manual QA doc:** skip `docs/manual-qa/NEO-119.md` — server-only; Bruno + API integration tests (NEO-108 / NEO-115 precedent). Accept flows in Bruno wait for [NEO-120](https://linear.app/neon-sprawl/issue/NEO-120).
## Goal, scope, and out-of-scope
**Goal:** Expose authoritative per-player quest progress over HTTP so clients and Bruno can read server-owned status, step index, objective counters, and completion timestamp without local objective math.
**In scope (from Linear + [E7M1-08](E7M1-prototype-backlog.md#e7m1-08--get-gameplayersidquest-progress)):**
- **`GET /game/players/{id}/quest-progress`** with versioned envelope (`schemaVersion` **1**, **`playerId`**, **`quests`** array).
- Per-quest row: **`questId`**, **`status`**, **`currentStepIndex`**, **`objectiveCounters`**, optional **`completedAt`** when **`completed`**.
- Projection from **`IQuestDefinitionRegistry`** + **`IPlayerQuestStateStore`** (missing row ⇒ **`not_started`**).
- **GET hook:** best-effort **`inventory_has_item`** refresh + step auto-advance/complete before snapshot (kickoff decision).
- Known-player gate (position store).
- Bruno `bruno/neon-sprawl-server/quest-progress/` — default read + shape assertions.
- Integration tests (AAA): unknown player 404; default all `not_started`; accept → partial counter → completed terminal step index.
- `server/README.md` route section.
**Out of scope (from Linear + backlog):**
- Quest definition catalog GET ([NEO-115](https://linear.app/neon-sprawl/issue/NEO-115) — already landed).
- **`POST …/quests/{questId}/accept`** ([NEO-120](https://linear.app/neon-sprawl/issue/NEO-120) / E7M1-09).
- Godot HUD ([NEO-122](https://linear.app/neon-sprawl/issue/NEO-122)).
- Telemetry hook comments ([NEO-121](https://linear.app/neon-sprawl/issue/NEO-121)).
- Dev fixture reset API for quest rows.
- `docs/manual-qa/NEO-119.md` (server-only per kickoff).
## Acceptance criteria checklist
- [x] GET returns stable v1 envelope for **`dev-local-1`** player (four quests, `schemaVersion` 1).
- [x] Completed quests show **`completed`** with **`currentStepIndex`** at terminal step (0-based; e.g. operator chain index **3**).
- [x] Bruno + integration tests pass in CI.
## Implementation reconciliation (shipped)
- **Route:** `GET /game/players/{id}/quest-progress``QuestProgressApi.BuildSnapshot`; 404 via `IPositionStateStore`.
- **GET hook:** `QuestObjectiveWiring.TryRefreshInventoryProgressForPlayer` — inventory refresh + step completion before snapshot.
- **DTOs:** `QuestProgressListResponse` (schema v1); row `status` not_started/active/completed; `objectiveCounters` map; `completedAt` when completed.
- **Tests:** seven AAA cases in `QuestProgressApiTests`.
- **Bruno:** `bruno/neon-sprawl-server/quest-progress/Get quest progress default.bru`.
- **Docs:** `server/README.md`; E7.M1 module snapshot + alignment register + backlog updated.
## Technical approach
1. **Route:** **`GET /game/players/{id}/quest-progress`** — inject **`IPositionStateStore`**, **`IQuestDefinitionRegistry`**, **`IPlayerQuestStateStore`**, **`IPlayerInventoryStore`**, **`IItemDefinitionRegistry`**, **`TimeProvider`**.
2. **Player gate:** Trim `id`; return **404** when empty or position missing (mirror **`EncounterProgressApi`**).
3. **GET-side wiring (kickoff):** Before building JSON, best-effort call a new public helper on **`QuestObjectiveWiring`** (e.g. **`TryRefreshInventoryProgressForPlayer`**) that:
- wraps existing **`RefreshInventoryHasItemCountersForPlayer`**
- then runs the same step-completion sweep as gather/craft paths (**`TryCompleteAllActiveQuests`** — extract from private to shared internal/public entry)
- swallows/logs at debug; never fails the HTTP response
4. **Status derivation** (per catalog quest id):
| Store row | **`status`** | **`currentStepIndex`** | **`objectiveCounters`** | **`completedAt`** |
|-----------|--------------|------------------------|-------------------------|-------------------|
| Missing | `not_started` | `0` | `{}` | omitted |
| Active | `active` | row index | row counters map | omitted |
| Completed | `completed` | terminal index (unchanged at complete) | last-step counters (may be empty after terminal satisfy) | ISO-8601 from row |
5. **Ordering:** Build rows from **`questRegistry.GetDefinitionsInIdOrder()`** — ids match **`PrototypeE7M1QuestCatalogRules.ExpectedQuestIds`** ordinal order.
6. **Implementation:** **`QuestProgressApi`** + **`QuestProgressListDtos.cs`** with internal **`BuildSnapshot(playerId, …)`** testable like **`EncounterProgressApi.BuildSnapshot`**. Register **`app.MapQuestProgressApi()`** in **`Program.cs`** after **`MapEncounterProgressApi()`**.
7. **Bruno:** `bruno/neon-sprawl-server/quest-progress/`
- **`Get quest progress default.bru`** — GET **`dev-local-1`** → 200, `schemaVersion` 1, `quests.length === 4`, all `not_started`, empty counters.
- Keep existing health smoke request.
- Accept / partial-progress Bruno flows deferred until NEO-120 POST lands.
8. **Docs:** Add **`server/README.md`** GET section (curl example, GET inventory refresh note). Update NEO-118 README deferral line. Module snapshot / alignment register / E7M1 backlog on story land.
### Expected v1 envelope (prototype default)
```json
{
"schemaVersion": 1,
"playerId": "dev-local-1",
"quests": [
{ "questId": "prototype_quest_combat_intro", "status": "not_started", "currentStepIndex": 0, "objectiveCounters": {} },
{ "questId": "prototype_quest_gather_intro", "status": "not_started", "currentStepIndex": 0, "objectiveCounters": {} },
{ "questId": "prototype_quest_operator_chain", "status": "not_started", "currentStepIndex": 0, "objectiveCounters": {} },
{ "questId": "prototype_quest_refine_intro", "status": "not_started", "currentStepIndex": 0, "objectiveCounters": {} }
]
}
```
### Wiring flow (GET)
```mermaid
flowchart TD
R[GET quest-progress] --> G{Known player?}
G -->|no| N404[404]
G -->|yes| W[QuestObjectiveWiring.TryRefreshInventoryProgressForPlayer]
W --> B[BuildSnapshot registry + store]
B --> J[JSON v1 response]
```
## Files to add
| Path | Purpose |
|------|---------|
| `server/NeonSprawl.Server/Game/Quests/QuestProgressApi.cs` | `Map*` extension; player gate; GET inventory refresh hook; `BuildSnapshot`. |
| `server/NeonSprawl.Server/Game/Quests/QuestProgressListDtos.cs` | Versioned list response + per-quest row DTOs (`status`, counters map). |
| `server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs` | AAA HTTP integration: 404, default not_started, active partial, completed terminal index, GET inventory refresh. |
| `bruno/neon-sprawl-server/quest-progress/Get quest progress default.bru` | Bruno default-state verification. |
| `docs/plans/NEO-119-implementation-plan.md` | This plan. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `server/NeonSprawl.Server/Game/Quests/QuestObjectiveWiring.cs` | Public GET-hook entry (`TryRefreshInventoryProgressForPlayer`) sharing refresh + step-completion sweep. |
| `server/NeonSprawl.Server/Program.cs` | Register `MapQuestProgressApi()` after encounter progress API. |
| `server/README.md` | Document GET route, response shape, GET-side `inventory_has_item` refresh; replace E7M1-08 deferral stubs. |
| `docs/decomposition/modules/E7_M1_QuestStateMachine.md` | HTTP quest-progress read bullet (on story complete). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M1 row — NEO-119 HTTP (on story complete). |
| `docs/decomposition/modules/module_dependency_register.md` | E7.M1 note — NEO-119 (on story complete). |
| `docs/plans/E7M1-prototype-backlog.md` | E7M1-08 acceptance checkboxes (on story complete). |
## Tests
| Test file | What it covers |
|-----------|----------------|
| `QuestProgressApiTests.cs` | **404:** unknown player + whitespace id. **Default:** GET `dev-local-1` → schema v1, four quests in ordinal id order, all `not_started`, `currentStepIndex` 0, empty `objectiveCounters`. **Active partial:** `QuestStateOperations.TryAccept` gather intro + `TryUpdateObjectiveCounter` → GET shows `active`, counter 2/3 on gather objective. **Completed:** accept + satisfy gather intro via operations/wiring helpers → GET shows `completed`, `currentStepIndex` 0, `completedAt` present. **Chain terminal:** operator chain completed → `currentStepIndex` 3. **GET refresh:** active chain on terminal `inventory_has_item` step with token pre-seeded in inventory (no new grant) → GET triggers complete (side-effecting read). |
| Existing quest test suite | Regression smoke — no behavior change to wiring stores when GET not called. |
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Side-effecting GET** | Adopted per kickoff — document in README; mirror deferred NEO-118 “polling hook” intent for client HUD | **adopted** |
| **Private `TryCompleteAllActiveQuests`** | Extract shared internal method invoked by GET hook and existing gather/craft/encounter paths | **adopted** |
| **Bruno without accept POST** | Default-state Bruno only this story; accept spine Bruno joins NEO-120 | **adopted** |
| **Completed row counters** | Project store counters as-is (terminal step may show satisfied counts or empty after advance logic) — assert `status` + `currentStepIndex` + `completedAt` in tests | **adopted** |