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

180 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# NEO-131 — Implementation plan
## Story reference
| Field | Value |
|--------|--------|
| **Key** | NEO-131 |
| **Title** | E7M2-08: Client quest completion reward HUD (Godot) |
| **Linear** | https://linear.app/neon-sprawl/issue/NEO-131/e7m2-08-client-quest-completion-reward-hud-godot |
| **Module** | [E7.M2 — RewardAndUnlockRouter](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) · Epic 7 Slice 2 · backlog **E7M2-08** |
| **Branch** | `NEO-131-client-quest-completion-reward-hud` |
| **Server deps** | [NEO-129](https://linear.app/neon-sprawl/issue/NEO-129) **`Done`** on `main``completionRewardSummary` on completed quest-progress rows |
| **Pattern** | [NEO-110](NEO-110-implementation-plan.md) — separate completion grant label + item-def display names; [NEO-122](NEO-122-implementation-plan.md) — `quest_hud_controller.gd` + event-driven GET refresh |
| **Blocks** | [NEO-132](https://linear.app/neon-sprawl/issue/NEO-132) — playable quest reward delivery capstone (E7M2-09) |
| **Server counterpart** | [NEO-129](https://linear.app/neon-sprawl/issue/NEO-129) — authoritative GET; Bruno is **not** prototype-complete per [full-stack epic decomposition](../../.cursor/rules/full-stack-epic-decomposition.md) |
## Kickoff clarifications
| Topic | Question | Agent recommendation | Answer |
|--------|----------|----------------------|--------|
| **HUD component** | Separate reward label vs inline on progress list? | **Separate `QuestRewardDeliveryLabel`** after **`QuestAcceptFeedbackLabel`** — NEO-110 **`EncounterCompleteLabel`** precedent; keeps four-quest progress compact. | **Adopted** — separate label |
| **Reward surfacing** | When to paint grant copy? | **Transition-only** — detect quest status → **`completed`** on refresh; matches Linear AC (“completing a quest shows…”); avoids replaying all summaries on boot/idempotency (NEO-123). | **Adopted** — transition-only |
| **Skill XP copy** | How to label skill lines without a skill-definitions client? | **Title-case `skillId` + amount** (e.g. **`Salvage +25 XP`**) — no skill-def client in repo; item lines use **`ItemDefinitionsClient`** like encounter loot. | **Adopted** (no kickoff question — settled by precedent) |
**Additional defaults (no kickoff question — settled by backlog / landed code):**
- **JSON shape:** nested **`completionRewardSummary`** with **`itemGrants: [{ itemId, quantity }]`** + **`skillXpGrants: [{ skillId, amount }]`** per NEO-129; empty **`itemGrants: []`** included when summary present.
- **Missing summary:** when **`status: completed`** but **`completionRewardSummary`** omitted (no delivery record), reward label stays **`—`** on transition — server omits field per NEO-129 kickoff.
- **Refresh strategy:** reuse NEO-122 triggers (boot hydrate + gather/craft/**`targetDefeated`**/encounter **`completed`**/accept POST) — no new poll or key.
- **Error surfacing:** failed GET continues **`sync error — {reason}`** on **`QuestProgressLabel`** (NEO-122); reward label shows **`—`** while sync error set (NEO-110 encounter loot precedent).
- **Server changes:** none — client-only story.
## Goal, scope, and out-of-scope
**Goal:** Godot shows **server-owned** quest completion reward grants using **`completionRewardSummary`** on **`GET /game/players/{id}/quest-progress`** — player-visible readable copy when a quest newly completes, without Bruno.
**In scope (from Linear + [E7M2-08](E7M2-prototype-backlog.md#e7m2-08--client-quest-completion-reward-hud-godot)):**
- Parse and retain **`completionRewardSummary`** on quest-progress rows (existing top-level parse passes through; add tests).
- **`QuestRewardDeliveryLabel`** under **`HudRoot`** after **`QuestAcceptFeedbackLabel`** — compact grant lines on in-session completion transition.
- Extend **`quest_hud_controller.gd`**: transition detection, reward label render, optional **`ItemDefinitionsClient`** for item display names.
- **`main.gd` / `main.tscn`:** wire reward label + item-defs client into controller **`setup`**.
- GdUnit parse + HUD render tests ([testing-expectations.md](../../.cursor/rules/testing-expectations.md)).
- **`docs/manual-qa/NEO-131.md`** — component checklist (gather intro skill XP; operator chain item + XP).
- **`client/README.md`** quest reward HUD subsection.
**Out of scope (from Linear + backlog):**
- Final quest journal UI; reward VFX.
- Skill-definitions client; server route/DTO changes.
- Full Slice 2 capstone script (**[NEO-132](https://linear.app/neon-sprawl/issue/NEO-132)**).
- Inventory/skills auto-refresh on quest complete (optional follow-up; NEO-132 capstone may assert economy HUD separately).
## Acceptance criteria checklist
- [x] Completing a quest shows readable reward copy on HUD.
- [x] Failed GET surfaces visible error (NEO-122 pattern).
## Implementation reconciliation (shipped)
- **`quest_progress_client.gd`:** **`completion_reward_summary`** static helper; parse preserves NEO-129 nested summary on completed rows.
- **`quest_hud_controller.gd`:** transition-only detection (**`_previous_status_by_quest`**); **`QuestRewardDeliveryLabel`** render with item-def display names + title-case skill XP lines; sync error idle state.
- **`main.gd` / `main.tscn`:** **`QuestRewardDeliveryLabel`** after accept feedback; **`ItemDefinitionsClient`** threaded into controller **`setup`**.
- **Tests:** `quest_progress_client_test.gd` (summary parse); `quest_hud_controller_test.gd` (transition, boot idempotency, first-success-after-active, sync error, item grant display name, quest/item `definitions_ready` reward repaint).
- **Docs:** `client/README.md`; `docs/manual-qa/NEO-131.md`; E7.M2 module + backlog alignment.
## Technical approach
### Server contract (landed — NEO-129)
Per completed quest row when delivery recorded:
```json
{
"questId": "prototype_quest_gather_intro",
"status": "completed",
"completionRewardSummary": {
"itemGrants": [],
"skillXpGrants": [{ "skillId": "salvage", "amount": 25 }]
}
}
```
Operator chain adds item line: **`survey_drone_kit` ×1** + **`salvage` 50** XP.
### 1. Parse layer (`quest_progress_client.gd`)
- No parser change required — **`parse_quest_progress_json`** already accepts arbitrary quest row fields.
- Optional static helper **`completion_reward_summary(quest_id, snapshot) -> Dictionary`** returning **`{}`** when absent — keeps HUD code readable (mirror **`encounter_row`** usage).
- GdUnit: completed row JSON with nested summary preserves **`itemGrants`** / **`skillXpGrants`** through parse + **`quest_row`**.
### 2. Transition detection (`quest_hud_controller.gd`)
- Maintain **`_previous_status_by_quest: Dictionary`** (questId → last known status string).
- On **`_on_progress_received`**, before overwriting snapshot:
- For each quest row in new snapshot, compare **`status`** to previous.
- When previous ∉ **`{completed}`** and new **`status == "completed"`** with non-empty **`completionRewardSummary`**, set **`_last_reward_quest_id`** + **`_last_reward_summary`**.
- On boot first snapshot: seed **`_previous_status_by_quest`** without firing transition unless the quest was **`active` in-session** (accept or prior active row) — covers failed GET then first success already **`completed`**; Godot restart idempotency unchanged.
- On sync failure: clear **`_last_reward_summary`** paint to **`—`**; preserve NEO-122 progress error lines.
### 3. Reward label render
**Header:** **`Quest rewards:`** (idle **`Quest rewards:\n—`**).
When **`_last_reward_quest_id`** set and summary present:
```
Quest rewards:
Intro: Salvage Run
Salvage +25 XP
```
Item lines: **`{displayName} ×{quantity}`** via **`ItemDefinitionsClient.display_name_for`**, fallback **`itemId`**.
Skill lines: **`{_title_case_skill_id(skillId)} +{amount} XP`**.
When **`completed`** transition but summary omitted: show quest display name + **`(rewards pending)`** or **`—`** — prefer **`—`** only (no delivery record = no copy; keeps label honest).
### 4. Scene + wiring
- **`main.tscn`:** add **`QuestRewardDeliveryLabel`** after **`QuestAcceptFeedbackLabel`** (same font/theme as quest labels).
- **`main.gd`:** `@onready` reward label; extend **`_setup_quest_progress_sync()`** to pass reward label + **`_item_defs_client`** into controller **`setup`** (new optional params at end for backward-compatible test calls).
### 5. Display copy reference (freeze table)
| Quest id | Expected reward lines |
|----------|----------------------|
| `prototype_quest_gather_intro` | Salvage +25 XP |
| `prototype_quest_refine_intro` | Refine +25 XP |
| `prototype_quest_combat_intro` | Salvage +25 XP |
| `prototype_quest_operator_chain` | Survey Drone Kit ×1; Salvage +50 XP |
### 6. Relationship to adjacent HUD
| HUD element | Story | Role |
|-------------|-------|------|
| **`QuestProgressLabel`** | NEO-122 | Status/step/counters for all four quests |
| **`QuestRewardDeliveryLabel`** | NEO-131 | Grant copy on completion transition |
| **`EncounterCompleteLabel`** | NEO-110 | Encounter loot (separate from quest completion bundles) |
Combat intro completion is **skill XP only** in quest bundle — encounter token loot remains on encounter label (E7M2 freeze).
## Files to add
| Path | Purpose |
|------|---------|
| `docs/plans/NEO-131-implementation-plan.md` | This plan. |
| `docs/manual-qa/NEO-131.md` | Manual QA: gather complete → skill XP line; operator chain item + XP; sync error visible. |
## Files to modify
| Path | Rationale |
|------|-----------|
| `client/scripts/quest_hud_controller.gd` | Transition detection, reward label render, item-def display names, sync-error idle state. |
| `client/scripts/quest_progress_client.gd` | Optional **`completion_reward_summary`** helper; class doc cites NEO-131 field. |
| `client/scripts/main.gd` | Wire **`QuestRewardDeliveryLabel`** + **`ItemDefinitionsClient`** into quest HUD **`setup`**. |
| `client/scenes/main.tscn` | Add **`QuestRewardDeliveryLabel`** node after accept feedback label. |
| `client/README.md` | Quest completion reward HUD subsection; cross-link NEO-129/131. |
| `docs/plans/E7M2-prototype-backlog.md` | E7M2-08 checkboxes + landed note when complete. |
| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | Client implementation anchor (NEO-131). |
| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E7.M2 row: NEO-131 client HUD. |
## Tests
| Test file | What it covers |
|-----------|----------------|
| `client/test/quest_progress_client_test.gd` | **Add:** parse completed row with **`completionRewardSummary`** (empty **`itemGrants`**, skill XP row); helper returns summary dict. |
| `client/test/quest_hud_controller_test.gd` | **Add:** transition **`active``completed`** paints reward label with skill XP line; boot with already-**`completed`** does not paint; sync failure leaves progress error + reward **`—`**; operator-chain item grant + display name; **`definitions_ready`** reward label repaint. |
**Regression:** existing NEO-122 GdUnit suites unchanged; `dotnet test` N/A (client-only).
## Open questions / risks
| Question / risk | Agent recommendation | Status |
|-----------------|----------------------|--------|
| **Multiple quests complete in one GET** | Paint **last catalog-order** transition in snapshot iteration — rare in prototype; document in manual QA. | **adopted** |
| **Inventory deny on complete (NEO-128)** | Quest may stay **`active`** when router denies — no **`completed`** transition, no reward label; acceptable for prototype. | **adopted** |
| **Skill display without defs client** | Title-case **`skillId`**; defer skill-def client to pre-production. | **adopted** |
| **NEO-132 capstone overlap** | NEO-131 proves component HUD; NEO-132 owns end-to-end reward idempotency manual QA. | **deferred** |