diff --git a/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md b/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md index 38cde7c..2d26fef 100644 --- a/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md +++ b/docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md @@ -7,7 +7,7 @@ | **Module ID** | E7.M2 | | **Epic** | [Epic 7 — Quest / Faction](../epics/epic_07_quest_faction.md) | | **Stage target** | Prototype | -| **Status** | Planned (see [dependency register](module_dependency_register.md)) | +| **Status** | Planned (see [dependency register](module_dependency_register.md)). **NEO-43 prep:** server `MissionRewardSkillXpGrant` for scripted skill XP from bundles (`mission_reward`); full router + `QuestRewardBundle` apply still TBD. | ## Purpose @@ -44,3 +44,7 @@ Epic 7 **Slice 2** — `reward_delivery`, `unlock_granted`; no double-claim on r - Master plan: [`neon_sprawl_vision.plan.md`](../../../neon_sprawl_vision.plan.md) — Epic 7. - [Module dependency register](module_dependency_register.md) + +## Implementation anchor (server) + +**NEO-43:** When applying scripted **skill XP** from a mission/quest reward bundle, call **`MissionRewardSkillXpGrant.GrantFromMissionReward`** (`server/NeonSprawl.Server/Game/Skills/`) — fixed **`sourceKind: mission_reward`**, **`skillId`** and **`amount`** from reward data — [NEO-43 implementation plan](../../plans/NEO-43-implementation-plan.md). diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index c391377..1f2bed5 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -51,7 +51,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E1.M3 | In Progress | **NEO-23 landed:** JSON v1 targeting read + select (`GET`/`POST …/target`, `Game/Targeting/`, [NEO-23](../../plans/NEO-23-implementation-plan.md)) — `PlayerTargetStateResponse` / soft lock / `reasonCode` denials; wire name differs from illustrative **`TargetState`** in module doc (called out in plan + README). **NEO-24 landed:** client tab-target + lock HUD synced to server ([NEO-24](../../plans/NEO-24-implementation-plan.md)) — `TargetSelectionClient` ([`client/scripts/target_selection_client.gd`](../../../client/scripts/target_selection_client.gd)), Tab / Esc bindings, `TargetLockLabel` HUD, hybrid movement-triggered refresh via new `PositionAuthorityClient.authoritative_ack` signal (fires on every server-confirmed position, boot + `move-stream` 200) with a 250 ms cooldown; manual QA in [`docs/manual-qa/NEO-24.md`](../../manual-qa/NEO-24.md). **NEO-25 landed:** versioned **`GET /game/world/interactables`** ([NEO-25](../../plans/NEO-25-implementation-plan.md)) — `InteractablesListResponse` / `InteractablesWorldApi` in `server/NeonSprawl.Server/Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` (fetch-driven props + glow); [server README — Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25). **NEO-26 landed:** prototype **`SelectionEvent`** — **`TargetSelectionClient.selection_event`** ([NEO-26](../../plans/NEO-26-implementation-plan.md)) when **`lockedTargetId`** changes; optional **`log_selection_events`**. **NEO-27 landed:** Slice 3 telemetry hook sites documented — `selection_event` maps to product `target_changed` in `target_selection_client.gd`; server lock transition hook comments in `InMemoryPlayerTargetLockStore`; reserved `ability_cast_requested` / `ability_cast_denied` comments in `client/scripts/main.gd` (all TODO(E9.M1)); see [NEO-27](../../plans/NEO-27-implementation-plan.md) and [`docs/manual-qa/NEO-27.md`](../../manual-qa/NEO-27.md). **Follow-on / still open:** richer **`InteractableDescriptor`** consumers beyond list projection + existing `POST …/interact`; production telemetry ingest/catalog after E9.M1. **Precursor (E1.M1):** [NEO-9](../../plans/NEO-9-implementation-plan.md) `POST …/interact`. | Linear [NEO-24](https://linear.app/neon-sprawl/issue/NEO-24)–[NEO-27](https://linear.app/neon-sprawl/issue/NEO-27); [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md); [E1M3 prototype backlog](../../plans/E1M3-prototype-backlog.md); [server README — Targeting](../../../server/README.md#targeting-neo-23), [Interactable descriptors (NEO-25)](../../../server/README.md#interactable-descriptors-neo-25), [Interaction](../../../server/README.md#interaction-neo-9) | | E1.M4 | In Progress | **NEO-29 landed:** prototype `HotbarLoadout` v1 server contract (`GET`/`POST /game/players/{id}/hotbar-loadout`) with stable deny reason codes, per-player scope, and persistence policy matching NEO-8/NS-17 (Postgres when configured, in-memory fallback otherwise). Client boot hydration is wired via `hotbar_loadout_client.gd` + `hotbar_state.gd` from `main.gd`; manual QA checklist created. **NEO-31 landed:** prototype **`POST /game/players/{id}/ability-cast`**, digit-key cast wiring from `main.gd`, `ability_cast_client.gd`, and `hotbar_cast_slot_resolver.gd` (target id from `lockedTargetId` in cached target state); GdUnit covers resolver + HTTP client; manual QA [NEO-31](../../manual-qa/NEO-31.md). **NEO-28 landed:** cast POST validates **lock + registry + range** (`invalid_target`, `out_of_range`); **`AbilityCastClient.cast_result_received`** + **`CastFeedbackLabel`** HUD line; manual QA [NEO-28](../../manual-qa/NEO-28.md). **NEO-30 landed:** Slice 3 cast funnel telemetry **hook-site comments** in [`AbilityCastApi.cs`](../../../server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs) (`ability_cast_requested` on authoritative accept, `ability_cast_denied` + non-empty `reasonCode` on each JSON deny branch); client dev hooks unchanged; manual QA [NEO-30](../../manual-qa/NEO-30.md); plan [NEO-30](../../plans/NEO-30-implementation-plan.md). **NEO-32 landed:** `GET /game/players/{id}/cooldown-snapshot` + `on_cooldown` cast deny + prototype global cooldown commit; [`cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`cooldown_state.gd`](../../../client/scripts/cooldown_state.gd), **`CooldownSlotsLabel`** in [`main.gd`](../../../client/scripts/main.gd); manual QA [NEO-32](../../manual-qa/NEO-32.md); plan [NEO-32](../../plans/NEO-32-implementation-plan.md). | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-implementation-plan.md), [NEO-28](../../plans/NEO-28-implementation-plan.md), [NEO-30](../../plans/NEO-30-implementation-plan.md), [NEO-32](../../plans/NEO-32-implementation-plan.md); [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md); [E1M4 prototype backlog](../../plans/E1M4-prototype-backlog.md); `server/NeonSprawl.Server/Game/AbilityInput/`; [`client/scripts/hotbar_loadout_client.gd`](../../../client/scripts/hotbar_loadout_client.gd), [`client/scripts/hotbar_state.gd`](../../../client/scripts/hotbar_state.gd), [`client/scripts/ability_cast_client.gd`](../../../client/scripts/ability_cast_client.gd), [`client/scripts/hotbar_cast_slot_resolver.gd`](../../../client/scripts/hotbar_cast_slot_resolver.gd), [`client/scripts/cooldown_snapshot_client.gd`](../../../client/scripts/cooldown_snapshot_client.gd), [`client/scripts/cooldown_state.gd`](../../../client/scripts/cooldown_state.gd); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29), [Ability cast (NEO-31)](../../../server/README.md#ability-cast-neo-31), [Cooldown snapshot (NEO-32)](../../../server/README.md#cooldown-snapshot-neo-32) | | E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **NEO-34 landed:** fail-fast server load of `content/skills/*.json` at startup — `server/NeonSprawl.Server/Game/Skills/` ([NEO-34](../../plans/NEO-34-implementation-plan.md)); config + discovery in [server README — Skill catalog](../../../server/README.md#skill-catalog-contentskills-neo-34). **NEO-35 landed:** `ISkillDefinitionRegistry` / `SkillDefinitionRegistry` + DI ([NEO-35](../../plans/NEO-35-implementation-plan.md)). **NEO-36 landed:** versioned **`GET /game/world/skill-definitions`** ([NEO-36](../../plans/NEO-36-implementation-plan.md)) — `SkillDefinitionsWorldApi` + `SkillDefinitionsListDtos` in `server/NeonSprawl.Server/Game/Skills/`; Bruno `bruno/neon-sprawl-server/skill-definitions/`; manual QA [`NEO-36`](../../manual-qa/NEO-36.md). **E2.M2 consumer (NEO-38 landed):** grant path validates `skillId` + `sourceKind` vs catalog **`allowedXpSourceKinds`** on **`POST …/skill-progression`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); see [server README — Skill progression grant](../../../server/README.md#skill-progression-grant-neo-38) and [E2_M2](E2_M2_XpAwardAndLevelEngine.md). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-implementation-plan.md), [NEO-36](../../plans/NEO-36-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/`; [`docs/manual-qa/NEO-36.md`](../../manual-qa/NEO-36.md); [server README — Skill definitions (NEO-36)](../../../server/README.md#skill-definitions-neo-36) | -| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), structured denies + **`levelUps`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); manual QA [`NEO-38`](../../manual-qa/NEO-38.md); Bruno `bruno/neon-sprawl-server/skill-progression/`; [server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38). **NEO-39 landed:** data-driven `LevelCurve` content + schema + CI validation (`*_level_curve.json`) with fail-fast startup schema checks; progression GET/POST level resolution now uses `ISkillLevelCurve` content-backed thresholds ([NEO-39](../../plans/NEO-39-implementation-plan.md)); manual QA [`NEO-39`](../../manual-qa/NEO-39.md). **NEO-40 landed:** comment-only telemetry hook sites in [`SkillProgressionSnapshotApi.cs`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs) on **`POST …/skill-progression`** for future **`xp_grant`** / **`level_up`** ([NEO-40](../../plans/NEO-40-implementation-plan.md), [`NEO-40` manual QA](../../manual-qa/NEO-40.md)). **NEO-41 landed:** gather prototype — **`POST …/interact`** with **`kind: resource_node`** grants **`salvage`** + **`activity`** (10 XP) via [`SkillProgressionGrantOperations`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs) ([NEO-41](../../plans/NEO-41-implementation-plan.md), [`NEO-41` manual QA](../../manual-qa/NEO-41.md)); [server README — Interaction](../../../server/README.md#interaction-neo-9). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack; **E3.M2** must invoke on craft/refine success ([NEO-42](../../plans/NEO-42-implementation-plan.md), [`NEO-42` manual QA](../../manual-qa/NEO-42.md)); [server README — Craft / refine hook (NEO-42)](../../../server/README.md#craft--refine-hook--skill-xp-neo-42). **Slice 3 still open:** [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43), [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [NEO-39](../../plans/NEO-39-implementation-plan.md), [NEO-40](../../plans/NEO-40-implementation-plan.md), [NEO-41](../../plans/NEO-41-implementation-plan.md), [NEO-42](../../plans/NEO-42-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-41, NEO-42–NEO-43 | +| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), structured denies + **`levelUps`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); manual QA [`NEO-38`](../../manual-qa/NEO-38.md); Bruno `bruno/neon-sprawl-server/skill-progression/`; [server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38). **NEO-39 landed:** data-driven `LevelCurve` content + schema + CI validation (`*_level_curve.json`) with fail-fast startup schema checks; progression GET/POST level resolution now uses `ISkillLevelCurve` content-backed thresholds ([NEO-39](../../plans/NEO-39-implementation-plan.md)); manual QA [`NEO-39`](../../manual-qa/NEO-39.md). **NEO-40 landed:** comment-only telemetry hook sites in [`SkillProgressionSnapshotApi.cs`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs) on **`POST …/skill-progression`** for future **`xp_grant`** / **`level_up`** ([NEO-40](../../plans/NEO-40-implementation-plan.md), [`NEO-40` manual QA](../../manual-qa/NEO-40.md)). **NEO-41 landed:** gather prototype — **`POST …/interact`** with **`kind: resource_node`** grants **`salvage`** + **`activity`** (10 XP) via [`SkillProgressionGrantOperations`](../../../server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantOperations.cs) ([NEO-41](../../plans/NEO-41-implementation-plan.md), [`NEO-41` manual QA](../../manual-qa/NEO-41.md)); [server README — Interaction](../../../server/README.md#interaction-neo-9). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack; **E3.M2** must invoke on craft/refine success ([NEO-42](../../plans/NEO-42-implementation-plan.md), [`NEO-42` manual QA](../../manual-qa/NEO-42.md)); [server README — Craft / refine hook (NEO-42)](../../../server/README.md#craft--refine-hook--skill-xp-neo-42). **NEO-43 landed (prep):** **`MissionRewardSkillXpGrant`** / **`MissionRewardSkillXpConstants`** + deny-registry factory + tests — same NEO-38 grant stack with fixed **`sourceKind: mission_reward`**; **E7.M2** must invoke from quest hand-in ([NEO-43](../../plans/NEO-43-implementation-plan.md), [`NEO-43` manual QA](../../manual-qa/NEO-43.md)); [server README — Mission / quest reward (NEO-43)](../../../server/README.md#mission--quest-reward--skill-xp-neo-43). **Slice 3 still open:** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [NEO-39](../../plans/NEO-39-implementation-plan.md), [NEO-40](../../plans/NEO-40-implementation-plan.md), [NEO-41](../../plans/NEO-41-implementation-plan.md), [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-43](../../plans/NEO-43-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-41, NEO-42–NEO-44 | | E3.M1 | In Progress | **NEO-41 landed (prototype):** `POST …/interact` success on **`resource_node`** (`prototype_resource_node_alpha`) applies **`salvage`** skill XP (**`sourceKind: activity`**, 10 XP) via shared NEO-38 grant operations. **Still planned:** `GatherResult`, yields, inventory per [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md). | [NEO-41](../../plans/NEO-41-implementation-plan.md), [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md); `server/NeonSprawl.Server/Game/Interaction/`, `Game/Skills/` | --- diff --git a/docs/manual-qa/NEO-43.md b/docs/manual-qa/NEO-43.md new file mode 100644 index 0000000..29aa6c0 --- /dev/null +++ b/docs/manual-qa/NEO-43.md @@ -0,0 +1,63 @@ +# Manual QA — NEO-43 (mission / quest → skill XP, `mission_reward`) + +Reference: [implementation plan](../plans/NEO-43-implementation-plan.md), [NEO-38](./NEO-38.md) (skill progression grant), [NEO-42](./NEO-42.md) (prep-hook analogue). + +## Preconditions + +- Run `NeonSprawl.Server` from `server/NeonSprawl.Server` (`dotnet run`; default `http://localhost:5253`). +- Player **`dev-local-1`** (default seed). + +```bash +BASE=http://localhost:5253 +ID=dev-local-1 +``` + +## Automated coverage + +`MissionRewardSkillXpGrantTests` exercises the **NEO-38** grant path with disk **`prototype_skills.json`** (`salvage` allows **`mission_reward`**) and a stub registry where **`salvage`** omits **`mission_reward`** from **`allowedXpSourceKinds`**. + +## Control: direct POST grant (same stack as E7.M2 hook) + +Move once so **`GET …/skill-progression`** is not **404**: + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/move" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"target":{"x":0.5,"y":0.9,"z":0.5}}' +``` + +Baseline: + +```bash +curl -sS "${BASE}/game/players/${ID}/skill-progression" +``` + +Apply **`salvage`** + **`mission_reward`** (mirrors what **`MissionRewardSkillXpGrant.GrantFromMissionReward`** does internally for that skill/amount): + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":25,"sourceKind":"mission_reward"}' +``` + +Expect **HTTP 200**, **`granted": true`**, and **`salvage`** **`xp`** increased (repeat **POST** to see cumulative XP). + +## Deny path (catalog guard) + +With default catalog, **`trainer`** is **not** allowed for **`salvage`**: + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":5,"sourceKind":"trainer"}' +``` + +Expect **HTTP 200**, **`granted": false`**, **`reasonCode":"source_kind_not_allowed"`**. + +## Combat vs mission XP + +**Skill XP** from missions uses **`mission_reward`** (this story). **Combat encounters** must not grant skill XP by default — they award **gig XP** separately; see [progression.md](../game-design/progression.md) (*Mission rewards vs combat XP*). There is no encounter reward implementation under `server/` yet; when **E5 / E7** adds encounter payouts, keep skill progression grants off the default encounter pipe unless a product exception is explicitly modeled. + +## E7.M2 follow-up + +When quest hand-in / **`QuestRewardBundle`** apply exists, confirm **one** server-side call to **`MissionRewardSkillXpGrant.GrantFromMissionReward`** per scripted skill XP line item (with content-driven **`skillId`** and **`amount`**), then **`GET …/skill-progression`** shows expected totals. diff --git a/docs/plans/NEO-43-implementation-plan.md b/docs/plans/NEO-43-implementation-plan.md new file mode 100644 index 0000000..7b7edf3 --- /dev/null +++ b/docs/plans/NEO-43-implementation-plan.md @@ -0,0 +1,88 @@ +# NEO-43 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-43 | +| **Title** | Mission / quest rewards grant skill XP (sourceKind: mission_reward) | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-43/mission-quest-rewards-grant-skill-xp-sourcekind-mission-reward | +| **Labels** | E7.M2, E2.M2, server, Story | +| **Blocked by (Linear text)** | [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) — **done** on `main` (grant stack + `allowedXpSourceKinds`). **E7.M2** [RewardAndUnlockRouter](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) — **design only** at kickoff; no quest hand-in / `QuestRewardBundle` code in `server/` yet. | + +## Kickoff clarifications + +| Topic | Question / note | Resolution | +|--------|-----------------|------------| +| **Delivery before E7.M2 router** | E7.M2 router does not exist in repo; how to ship NEO-43? | **User (implementation):** accepted agent recommendation — **NEO-42-style prep:** **`MissionRewardSkillXpGrant`** + tests + README + manual QA; **E7.M2** wires the real quest hand-in later. | +| **Reward shape** | Fixed prototype vs generic skill + amount? | **User (implementation):** accepted agent recommendation — **generic** **`skillId`** + **`amount`**; helper fixes **`sourceKind: mission_reward`**. | +| **Combat encounters ≠ skill XP** | No encounter/gig XP code under `server/` today. | **User (implementation):** accepted agent recommendation — **README + manual QA + plan** document the invariant and current call sites; no synthetic encounter guard test until encounter code exists. | + +## Goal, scope, and out-of-scope + +**Goal:** When **E7.M2** (or any scripted mission/quest hand-in) applies **explicit** skill XP from a **mission reward** channel, route the grant through **E2.M2** / **NEO-38** using **`sourceKind: mission_reward`**, validated against the target skill’s **`allowedXpSourceKinds`**. **Not** the combat encounter pipe ([progression.md](../game-design/progression.md) **Mission rewards vs combat XP**). + +**In scope (from Linear + defaults above):** + +- **Preparatory (this slice):** one **server-side** helper that delegates to **`SkillProgressionGrantOperations.TryApplyGrant`** with caller-supplied **`skillId`** / **`amount`** and fixed **`mission_reward`** `sourceKind`. +- **Tests:** happy path (grant applies when allowlist permits); deny path when **`mission_reward`** ∉ **`allowedXpSourceKinds`** for the chosen skill (stub registry pattern aligned with NEO-41/NEO-42 denied factories). +- **Docs:** README subsection + **`docs/manual-qa/NEO-43.md`** (verify via **`POST …/skill-progression`** with **`mission_reward`** and via helper in a test host, same spirit as NEO-42). + +**Out of scope:** + +- Full **`QuestRewardBundle`** schema, idempotent **`RewardDeliveryEvent`**, inventory/unlocks ([E7_M2_RewardAndUnlockRouter.md](../decomposition/modules/E7_M2_RewardAndUnlockRouter.md) backlog). +- **Combat encounter** implementation or gig XP awards. +- **Batch** mission grants in one HTTP call (can follow once router batches rewards). + +## Acceptance criteria checklist + +- [x] **Scripted mission rewards can include skill XP;** validated against **`allowedXpSourceKinds`** (helper + tests; manual QA documents **`POST …/skill-progression`** with **`mission_reward`** for skills that allow it). +- [x] **Combat encounters do not grant skill XP by default** — documented invariant + **current codebase** has no encounter grant path; gig XP remains separate (design reference: [progression.md](../game-design/progression.md), [E2_M1](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md)). + +## Decisions (implementation) + +- **`MissionRewardSkillXpConstants.MissionRewardSourceKind`** — single canonical string for the helper and docs. +- **`MissionRewardDeniedRegistryWebApplicationFactory`** — **`salvage`** stub omits **`mission_reward`** (deny coverage); happy paths use disk catalog via **`InMemoryWebApplicationFactory`**. +- **Code review follow-up (2026-05-15):** **`documentation_and_implementation_alignment.md`** E2.M2 row lists **NEO-43 landed** and **Slice 3 still open** → **NEO-44** only; **`E7_M2_RewardAndUnlockRouter.md`** Summary **Status** notes NEO-43 prep; **`NEO-43-implementation-plan.md`** technical approach uses **`MissionRewardSkillXpConstants.MissionRewardSourceKind`** (matches code). + +## Technical approach + +1. Add **`MissionRewardSkillXpConstants`** (or equivalent single type) holding the canonical **`mission_reward`** string used by the helper (avoid string drift; match **`prototype_skills.json`** / NEO-38 docs). + +2. Add **`MissionRewardSkillXpGrant`** (static class, parallel to **`RefineActivitySkillXpGrant`**): + - Method e.g. **`GrantFromMissionReward(playerId, skillId, amount, registry, xpStore, levelCurve)`** calling **`TryApplyGrant`** with **`MissionRewardSkillXpConstants.MissionRewardSourceKind`**. + - **Caller contract:** `playerId` normalized like other grant hooks; **`amount`** > 0 (delegate invalid amount to existing deny behavior); outcome may be ignored on “fire and forget” success paths or inspected when E7.M2 needs telemetry/deny handling. + +3. **Tests** — factory with registry where a prototype skill **excludes** **`mission_reward`** for deny coverage; in-memory progression store; assert snapshot / store deltas consistent with **`RefineActivitySkillXpGrantTests`** style. + +4. **Documentation** — **`server/README.md`**: short “Mission / quest skill XP (NEO-43)” subsection: E7.M2 should call **`MissionRewardSkillXpGrant`**; until then verify **`mission_reward`** via **`POST …/skill-progression`**; link **`docs/manual-qa/NEO-43.md`**. + +5. **Decomposition anchor** — one sentence in **`docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md`** (or **`documentation_and_implementation_alignment.md`** if E7.M2 file should stay design-pure): server prep hook name + NEO-43 plan link. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpConstants.cs` | Canonical `mission_reward` source kind string for mission/quest grant path. | +| `server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpGrant.cs` | Static helper delegating to `SkillProgressionGrantOperations.TryApplyGrant` with fixed `mission_reward`. | +| `server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardSkillXpGrantTests.cs` | Happy path + GET snapshot alignment (optional mirror NEO-42); deny when `mission_reward` not allowed. | +| `server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs` | WebApplicationFactory with stub `ISkillDefinitionRegistry` where target skill omits `mission_reward` from `allowedXpSourceKinds`. | +| `docs/manual-qa/NEO-43.md` | curl/Bruno order: grant via `POST …/skill-progression` with `mission_reward` + note future E7.M2 calls helper. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/README.md` | Document NEO-43 mission-reward skill XP hook and verification until E7.M2 wires quest hand-in. | +| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | Implementation anchor: which server helper to call when applying skill XP from `QuestRewardBundle` / mission payout. | + +## Tests + +| Path | Coverage | +|------|----------| +| `server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardSkillXpGrantTests.cs` | **Granted:** helper applies XP when `mission_reward` ∈ `allowedXpSourceKinds` for chosen `skillId`; optional round-trip **`GET …/skill-progression`**. **Denied:** stub registry without `mission_reward` on target skill → no XP delta; no throw. | +| Existing `SkillProgressionGrantApiTests.cs` | Regression only if POST path or shared types change during implementation (likely unchanged). | + +## Open questions / risks + +None. **Risk:** Low — additive surface; helper **unreferenced** until E7.M2 (same pattern as NEO-42). diff --git a/docs/reviews/2026-05-15-NEO-43.md b/docs/reviews/2026-05-15-NEO-43.md new file mode 100644 index 0000000..036f88c --- /dev/null +++ b/docs/reviews/2026-05-15-NEO-43.md @@ -0,0 +1,52 @@ +# Code review — NEO-43 (mission / quest reward skill XP prep) + +**Date:** 2026-05-15 + +**Scope:** Branch `NEO-43-mission-quest-rewards-skill-xp` (commits on top of `origin/main` through `2af9b1c`), working tree clean at review time. Issue **NEO-43**. + +**Base:** `origin/main` + +## Verdict + +**Approve** + +## Summary + +The branch adds a small **NEO-42-style** server prep slice: `MissionRewardSkillXpConstants`, `MissionRewardSkillXpGrant.GrantFromMissionReward` delegating to `SkillProgressionGrantOperations.TryApplyGrant` with fixed `sourceKind: mission_reward`, a denied-registry `WebApplicationFactory` plus three tests, manual QA, README, implementation plan, and an **E7.M2** decomposition anchor. Behavior matches the NEO-38 grant stack and prototype catalog (`salvage` allows `mission_reward`). Risk is low: additive API, no new HTTP routes, helper unused until E7.M2 wires quest payout. + +## Documentation checked + +| Document | Assessment | +|----------|------------| +| `docs/plans/NEO-43-implementation-plan.md` | **Matches** — helper, constants, factory pattern, tests (happy + GET alignment + deny), README, manual QA, E7 anchor align with plan and kickoff decisions. | +| `docs/decomposition/modules/E7_M2_RewardAndUnlockRouter.md` | **Matches** — new **Implementation anchor (server)** points to `MissionRewardSkillXpGrant.GrantFromMissionReward` and the NEO-43 plan. | +| `docs/decomposition/modules/module_dependency_register.md` | **N/A for code conflict** — E7.M2 remains **Planned**; prep hook does not require register churn. Optional follow-up: if the team treats any server hook toward E7.M2 as “started,” consider **In Progress** + Summary sync (not required for correctness). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E2.M2 row updated: **NEO-43 landed** (prep) + **Slice 3 still open** lists **NEO-44** only. | +| `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` | **N/A** — not edited; NEO-43 correctly extends the existing POST/grant story via shared operations. | +| `docs/game-design/progression.md` (referenced) | **Matches** — README / manual QA echo mission vs combat XP separation. | + +~~**Register / tracking table:** Updating **`documentation_and_implementation_alignment.md`** (E2.M2 snapshot) is the main doc hygiene gap; see suggestions.~~ **Done.** (E2.M2 row updated; see **Suggestions** §1.) + +## Blocking issues + +_None._ + +## Suggestions + +1. ~~**Implementation tracking table** — In `docs/decomposition/modules/documentation_and_implementation_alignment.md`, the **E2.M2** row still says **Slice 3 still open** and links NEO-43 as future work. After this story merges, add a **NEO-43 landed** line (mirror NEO-42: `MissionRewardSkillXpGrant`, tests, manual QA, README) and adjust the “still open” list so the inventory matches the repo (per **Status column** rule in that file).~~ **Done.** E2.M2 row: NEO-43 landed (prep) + Slice 3 still open → NEO-44 only; References column includes [NEO-43 plan](../plans/NEO-43-implementation-plan.md). + +2. ~~**E7.M2 Summary table (optional)** — `E7_M2_RewardAndUnlockRouter.md` **Status** remains **Planned** in the Summary table, consistent with the register. No change required unless you want a one-line note under Summary that server-side **skill XP from quest bundles** has a named hook (the new anchor already covers this).~~ **Done.** Status cell notes NEO-43 prep hook; module remains Planned. + +## Nits + +- ~~**Nit:** In `NEO-43-implementation-plan.md` **Technical approach** §1, the constant is named `MissionRewardSourceKind` in code, not `SourceKind` as in one plan sentence — trivial naming drift inside the plan only.~~ **Done.** Plan now references `MissionRewardSkillXpConstants.MissionRewardSourceKind` (matches `MissionRewardSkillXpConstants.cs`). + +## Verification + +Already run during review: + +```bash +cd server && dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~MissionRewardSkillXpGrantTests" +``` + +Author should run full `dotnet test` on `NeonSprawl.Server.Tests` before merge if not already part of CI. Manual spot-check: follow `docs/manual-qa/NEO-43.md` (move + POST `mission_reward` + deny control with `trainer`). diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs new file mode 100644 index 0000000..3d6d35d --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardDeniedRegistryWebApplicationFactory.cs @@ -0,0 +1,112 @@ +using System.Diagnostics.CodeAnalysis; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Time.Testing; +using NeonSprawl.Server.Game.AbilityInput; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Skills; +using Npgsql; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +/// +/// Same in-memory overrides as , plus a stub +/// where salvage does not allow mission_reward — NEO-43 defensive tests without mutating disk catalog. +/// +public sealed class MissionRewardDeniedRegistryWebApplicationFactory : WebApplicationFactory +{ + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment("Testing"); + + var skillsDir = SkillCatalogPathResolution.TryDiscoverSkillsDirectory(AppContext.BaseDirectory) + ?? throw new InvalidOperationException( + "Could not discover repo content/skills from AppContext.BaseDirectory; run tests from the neon-sprawl clone."); + builder.UseSetting("Content:SkillsDirectory", skillsDir); + + builder.ConfigureTestServices(services => + { + for (var i = services.Count - 1; i >= 0; i--) + { + var d = services[i]; + if (d.ServiceType == typeof(IPositionStateStore) || + d.ServiceType == typeof(IPlayerHotbarLoadoutStore) || + d.ServiceType == typeof(IPlayerSkillProgressionStore) || + d.ServiceType == typeof(TimeProvider) || + d.ServiceType == typeof(IPlayerAbilityCooldownStore) || + d.ServiceType == typeof(ISkillDefinitionRegistry) || + d.ServiceType == typeof(NpgsqlDataSource) || + (d.ServiceType == typeof(IHostedService) && + (d.ImplementationType == typeof(PostgresDevPlayerSeedHostedService) || + d.ImplementationType == typeof(NpgsqlDataSourceShutdownHostedService)))) + { + services.RemoveAt(i); + } + } + + var fakeTime = new FakeTimeProvider(new DateTimeOffset(2026, 4, 30, 12, 0, 0, TimeSpan.Zero)); + services.AddSingleton(fakeTime); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + }); + } +} + +/// Prototype trio ids; salvage excludes mission_reward so mission grant path denies without throwing. +internal sealed class MissionRewardDeniedSkillRegistry : ISkillDefinitionRegistry +{ + private static readonly SkillDefRow SalvageNoMissionReward = new( + "salvage", + "gather", + "Salvage", + new[] { "activity" }); + + private static readonly SkillDefRow Refine = new( + "refine", + "process", + "Refine", + new[] { "activity", "mission_reward", "trainer" }); + + private static readonly SkillDefRow Intrusion = new( + "intrusion", + "tech", + "Intrusion", + new[] { "activity", "mission_reward", "book_or_item" }); + + private static readonly IReadOnlyList Ordered = + [ + Intrusion, + Refine, + SalvageNoMissionReward, + ]; + + public bool TryGetDefinition(string? skillId, [NotNullWhen(true)] out SkillDefRow? definition) + { + if (skillId is null) + { + definition = null; + return false; + } + + var k = skillId.Trim(); + foreach (var row in Ordered) + { + if (string.Equals(row.Id, k, StringComparison.OrdinalIgnoreCase)) + { + definition = row; + return true; + } + } + + definition = null; + return false; + } + + public IReadOnlyList GetDefinitionsInIdOrder() => Ordered; +} diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardSkillXpGrantTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardSkillXpGrantTests.cs new file mode 100644 index 0000000..06c6dc5 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/MissionRewardSkillXpGrantTests.cs @@ -0,0 +1,101 @@ +using System.Linq; +using System.Net; +using System.Net.Http.Json; +using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Skills; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +/// NEO-43: mission/quest reward hook → skill XP via shared NEO-38 grant path (sourceKind: mission_reward). +public sealed class MissionRewardSkillXpGrantTests +{ + private const int TestMissionRewardXp = 25; + + [Fact] + public async Task GrantFromMissionReward_WhenCatalogAllowsMissionReward_ShouldIncreaseSalvageXp() + { + // Arrange — dev player is seeded on the in-memory progression store (NEO-38). + await using var factory = new InMemoryWebApplicationFactory(); + _ = factory.CreateClient(); + using var scope = factory.Services.CreateScope(); + var registry = scope.ServiceProvider.GetRequiredService(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var levelCurve = scope.ServiceProvider.GetRequiredService(); + + // Act + MissionRewardSkillXpGrant.GrantFromMissionReward( + "dev-local-1", + "salvage", + TestMissionRewardXp, + registry, + xpStore, + levelCurve); + + // Assert + var totals = xpStore.GetXpTotals("dev-local-1"); + Assert.True(totals.TryGetValue("salvage", out var xp)); + Assert.Equal(TestMissionRewardXp, xp); + } + + [Fact] + public async Task GrantFromMissionReward_WhenCatalogAllowsMissionReward_ShouldMatchSkillProgressionGet() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var move = new MoveCommandRequest + { + SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, + Target = new PositionVector { X = 0.5, Y = 0.9, Z = 0.5 }, + }; + await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); + using var scope = factory.Services.CreateScope(); + var registry = scope.ServiceProvider.GetRequiredService(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var levelCurve = scope.ServiceProvider.GetRequiredService(); + + // Act + MissionRewardSkillXpGrant.GrantFromMissionReward( + "dev-local-1", + "salvage", + TestMissionRewardXp, + registry, + xpStore, + levelCurve); + var snapshot = await client.GetAsync("/game/players/dev-local-1/skill-progression"); + + // Assert + Assert.Equal(HttpStatusCode.OK, snapshot.StatusCode); + var body = await snapshot.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + var salvage = body!.Skills!.Single(static s => s.Id == "salvage"); + Assert.Equal(TestMissionRewardXp, salvage.Xp); + } + + [Fact] + public async Task GrantFromMissionReward_WhenSalvageDisallowsMissionReward_ShouldLeaveSalvageXpZero() + { + // Arrange + await using var factory = new MissionRewardDeniedRegistryWebApplicationFactory(); + _ = factory.CreateClient(); + using var scope = factory.Services.CreateScope(); + var registry = scope.ServiceProvider.GetRequiredService(); + var xpStore = scope.ServiceProvider.GetRequiredService(); + var levelCurve = scope.ServiceProvider.GetRequiredService(); + + // Act + MissionRewardSkillXpGrant.GrantFromMissionReward( + "dev-local-1", + "salvage", + TestMissionRewardXp, + registry, + xpStore, + levelCurve); + + // Assert — deny returns before TryApplyXpDelta; salvage row must not appear in stored XP map. + var totals = xpStore.GetXpTotals("dev-local-1"); + Assert.False(totals.ContainsKey("salvage")); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpConstants.cs b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpConstants.cs new file mode 100644 index 0000000..5401a0e --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpConstants.cs @@ -0,0 +1,7 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// NEO-43: quest/mission scripted payouts use sourceKind: mission_reward (catalog + NEO-38). +public static class MissionRewardSkillXpConstants +{ + public const string MissionRewardSourceKind = "mission_reward"; +} diff --git a/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpGrant.cs b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpGrant.cs new file mode 100644 index 0000000..31f024c --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/MissionRewardSkillXpGrant.cs @@ -0,0 +1,29 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// +/// NEO-43: E7.M2 mission/quest reward apply should call so XP uses the same path as +/// POST …/skill-progression. Outcome is ignored on success paths (mirror NEO-42); use +/// directly when callers need deny envelopes. +/// +public static class MissionRewardSkillXpGrant +{ + /// Applies one grant with sourceKind: mission_reward if the catalog allowlist permits it. + /// Pass a normalized (e.g. route-trimmed), matching other grant hook callers. + public static void GrantFromMissionReward( + string playerId, + string skillId, + int amount, + ISkillDefinitionRegistry registry, + IPlayerSkillProgressionStore xpStore, + ISkillLevelCurve levelCurve) + { + _ = SkillProgressionGrantOperations.TryApplyGrant( + playerId, + skillId, + amount, + MissionRewardSkillXpConstants.MissionRewardSourceKind, + registry, + xpStore, + levelCurve); + } +} diff --git a/server/README.md b/server/README.md index f78509e..631e4b2 100644 --- a/server/README.md +++ b/server/README.md @@ -182,6 +182,10 @@ When **`allowed` is `true`** and the interactable’s **`kind`** is **`resource_ **E3.M2** recipe/refine success paths should call **`RefineActivitySkillXpGrant.GrantOnSuccessfulCraftOrRefine`**, which applies **10** **`refine`** skill XP with **`sourceKind: activity`** through **`SkillProgressionGrantOperations.TryApplyGrant`** (same validation, **`allowedXpSourceKinds`**, and persistence as **[NEO-38](#skill-progression-grant-neo-38)**). There is no separate craft HTTP in NEO-42; verify behavior with **`POST …/skill-progression`** for **`refine`**/**`activity`** until the craft module wires the helper. Plan: [NEO-42 implementation plan](../../docs/plans/NEO-42-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-42.md`](../../docs/manual-qa/NEO-42.md). +### Mission / quest reward → skill XP (NEO-43) + +**E7.M2** quest/mission payout paths should call **`MissionRewardSkillXpGrant.GrantFromMissionReward`** with content-driven **`skillId`** and **`amount`**; the helper always uses **`sourceKind: mission_reward`** through **`SkillProgressionGrantOperations.TryApplyGrant`** (same validation, **`allowedXpSourceKinds`**, and persistence as **[NEO-38](#skill-progression-grant-neo-38)**). **Combat encounter** rewards must **not** use this path for default clears — gig XP stays separate ([`docs/game-design/progression.md`](../docs/game-design/progression.md)). Until the router exists, verify **`mission_reward`** with **`POST …/skill-progression`**. Plan: [NEO-43 implementation plan](../../docs/plans/NEO-43-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-43.md`](../../docs/manual-qa/NEO-43.md). + Contract details and PR blurb: [NEO-9 implementation plan](../../docs/plans/NEO-9-implementation-plan.md). ## Interactable descriptors (NEO-25)