From 52900448459094cd562a568776672ee8ce1dbdc8 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:06:52 -0400 Subject: [PATCH 01/14] NEO-48: add implementation plan for perk-state HTTP Kickoff plan for GET/POST /game/players/{id}/perk-state, Bruno, and API tests; records user choices on route path and unlockedEvents on success. --- docs/plans/NEO-48-implementation-plan.md | 124 +++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 docs/plans/NEO-48-implementation-plan.md diff --git a/docs/plans/NEO-48-implementation-plan.md b/docs/plans/NEO-48-implementation-plan.md new file mode 100644 index 0000000..6be9695 --- /dev/null +++ b/docs/plans/NEO-48-implementation-plan.md @@ -0,0 +1,124 @@ +# NEO-48 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-48 | +| **Title** | E2.M3: Perk state GET + branch selection POST + Bruno | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-48/e2m3-perk-state-get-branch-selection-post-bruno | +| **Module** | [E2.M3 — MasteryAndPerkUnlocks](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) · Epic 2 Slice 4 | +| **Branch** | `NEO-48-perk-state-get-branch-selection-post-bruno` | +| **Blocked by** | [NEO-47](https://linear.app/neon-sprawl/issue/NEO-47) — **Done** on `main` (`PerkUnlockEngine`, `IPlayerPerkStateStore`, `PerkUnlockReasonCodes`). | + +## Kickoff clarifications + +| Topic | Question | Resolution | +|--------|----------|------------| +| **POST URL** | Same resource path as GET vs dedicated sub-path? | **User:** **`POST /game/players/{id}/perk-state`** (same as GET; mirrors skill-progression / hotbar-loadout). | +| **POST success body** | Include unlock events from `PerkUnlockEngine`? | **User:** **Yes** — `selected: true` + full **`perkState`** snapshot + **`unlockedEvents[]`** (mirrors NEO-38 `levelUps` on grant). | +| **GET JSON shape** | Nested maps vs arrays? | **Agent (repo pattern):** serialize branch picks as a **`branchPicks`** array of `{ skillId, picks: [{ tierIndex, branchId }] }` (ordinal sort by `skillId`; picks sorted by `tierIndex`). **`unlockedPerkIds`** as a sorted string array. Internal `PerkStateSnapshot` mapping only — no rename of persisted fields. | +| **Deny envelope** | Field names for success/deny? | **Agent (NEO-38 mirror):** HTTP **200** with **`selected`** (`true`/`false`), optional **`reasonCode`** (stable codes from `PerkUnlockReasonCodes`), and authoritative **`perkState`** on both paths. **400** for missing/wrong `schemaVersion`; **404** when player absent from position state (same gate as skill-progression GET). | +| **Telemetry at HTTP** | Duplicate NEO-49 hook comments on POST? | **No** — [NEO-49 plan](NEO-49-implementation-plan.md): engine `TryUnlockPerks` is the authoritative hook; HTTP calls `TrySelectBranch` and surfaces returned `Events` only in JSON. | + +## Goal, scope, and out-of-scope + +**Goal:** Versioned HTTP **GET** read model and **POST** branch selection for per-player perk state, delegating all rules to **`PerkUnlockEngine`** from NEO-47. Match NEO-37/NEO-38 versioning, player gate, and structured deny patterns. + +**In scope (from Linear):** + +- **`GET /game/players/{id}/perk-state`** — `schemaVersion` **1** JSON: branch picks + unlocked perk ids. +- **`POST /game/players/{id}/perk-state`** — commit one branch pick; structured success/deny; stable **`reasonCode`** values from engine. +- Bruno under **`bruno/neon-sprawl-server/perk-state/`** (GET + POST happy + deny). +- **`server/README.md`** section; API integration tests (AAA). +- **`docs/manual-qa/NEO-48.md`** during implementation (repo convention for HTTP stories). + +**Out of scope (from Linear):** + +- Godot client HUD. +- Applying perk `effectKind` in gather/craft. +- New telemetry hooks (NEO-49 complete). +- Respec; gig perks. + +## Acceptance criteria checklist + +- [ ] `GET /game/players/{id}/perk-state` returns versioned JSON v1. +- [ ] `POST` branch selection applies NEO-47 unlock rules; structured denies on invalid tier/branch. +- [ ] Bruno collection `bruno/neon-sprawl-server/perk-state/` (GET + POST + deny). +- [ ] `server/README.md` section for perk-state endpoints. +- [ ] Unit/integration tests (AAA) for API happy/deny paths. + +## Technical approach + +1. **Routes (`Game/Mastery/PerkStateApi.cs`):** + - **`GET /game/players/{id}/perk-state`** — trim id; **404** if `!IPositionStateStore.TryGetPosition`; else **200** + `PerkStateSnapshotResponse` built from `IPlayerPerkStateStore.GetSnapshot`. + - **`POST /game/players/{id}/perk-state`** — body `PerkBranchSelectRequest` (`schemaVersion` **1**, `skillId`, `tierIndex`, `branchId`). **400** if body null or `schemaVersion != 1`. **404** same player gate. Call **`perkUnlockEngine.TrySelectBranch(trimmedId, skillId, tierIndex, branchId)`**; map outcome to `PerkBranchSelectResponse`. + +2. **Snapshot builder:** `PerkStateApi.BuildSnapshot(playerId, IPlayerPerkStateStore)` maps `PerkStateSnapshot` → JSON DTO: + - **`branchPicks`:** one entry per skill with any picks; each **`picks`** row has `tierIndex` + `branchId`. + - **`unlockedPerkIds`:** sorted ordinal list from set. + - Preserve **`schemaVersion` 1**, **`playerId`**. + +3. **POST success (`selected: true`):** + - **`perkState`:** same shape as GET body (without re-wrapping `playerId` if shared type — use shared `PerkStateSnapshotResponse` or embed identical fields). + - **`unlockedEvents`:** map each `PerkUnlockEvent` → `{ perkId, skillId, tierIndex, branchId, source }` where **`source`** is JSON string **`branch_pick`** | **`level_up`** (snake_case; maps `PerkUnlockSource.BranchPick` / `LevelUp`). Empty array when tier-1 pick unlocks no perks yet. + +4. **POST deny (`selected: false`):** + - **`reasonCode`:** pass through engine `ReasonCode` (`unknown_track`, `unknown_tier`, `unknown_branch`, `level_too_low`, `branch_already_chosen`, `tier_branch_not_selectable`, `player_not_found`). + - **`perkState`:** authoritative snapshot from outcome (unchanged on deny except where engine returns updated snapshot — use `outcome.Snapshot` always). + - **`unlockedEvents`:** empty array on deny. + +5. **Wire-up:** `app.MapPerkStateApi()` in **`Program.cs`** after mastery/perk DI is available (`PerkUnlockEngine` already registered in `AddPerkStateStore`). No new store or engine logic in this story. + +6. **Bruno / README / manual QA:** + - Sequence: optional **`POST …/skill-progression`** grant to reach salvage level 2, then **POST perk-state** tier-1 pick, **GET** verifies pick; deny example: second pick or `level_too_low` without grant. + - Document paths, schema fields, reason codes, and link to this plan + **`docs/manual-qa/NEO-48.md`**. + +7. **Module doc (implementation batch):** Update [E2_M3](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) NEO-47 handoff line to note HTTP landed; refresh [documentation_and_implementation_alignment.md](../decomposition/modules/documentation_and_implementation_alignment.md) E2.M3 row when complete. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs` | `PerkStateSnapshotResponse`, `PerkBranchPickTrackJson`, `PerkBranchPickRowJson`, `PerkBranchSelectRequest`, `PerkBranchSelectResponse`, `PerkUnlockedEventJson`. | +| `server/NeonSprawl.Server/Game/Mastery/PerkStateApi.cs` | `MapPerkStateApi`, `BuildSnapshot`, GET/POST handlers. | +| `server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs` | In-memory host: GET empty defaults; POST tier-1 happy after XP grant; deny `level_too_low` / `branch_already_chosen`; GET reflects POST; **404** / **400** gates. | +| `bruno/neon-sprawl-server/perk-state/folder.bru` | Collection folder metadata. | +| `bruno/neon-sprawl-server/perk-state/Get perk state.bru` | GET smoke + schema assertions. | +| `bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru` | Happy path (docs: grant salvage XP first if needed). | +| `bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru` | Deny without level gate. | +| `docs/manual-qa/NEO-48.md` | Curl/Bruno checklist for GET/POST/deny. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register **`MapPerkStateApi()`**. | +| `server/README.md` | New **Perk state (NEO-48)** section: GET/POST paths, schema v1, `selected`/`reasonCode`/`unlockedEvents`, reason code table (link `PerkUnlockReasonCodes`), Bruno path, manual QA link. | +| `docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md` | Note NEO-48 HTTP landed (replace “HTTP is NEO-48” pending wording in NEO-47 handoff). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E2.M3 row: NEO-48 perk-state HTTP. | +| `docs/decomposition/modules/module_dependency_register.md` | E2.M3 note: NEO-48 planned → landed when shipped. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `PerkStateApiTests.cs` | **AAA** integration via `InMemoryWebApplicationFactory`: **GET** returns v1 empty state for dev player; **POST** after `POST …/skill-progression` grant (100 XP → level 2) selects `scrap_efficiency` tier 1 → `selected: true`, pick in **GET**; **POST** deny `level_too_low` without grant; **POST** deny `branch_already_chosen` on second pick; **404** unknown player; **400** bad `schemaVersion`. Optional: **POST** at level 4+ includes non-empty **`unlockedEvents`** when tier-2 perk unlocks on same request (path-auto). | +| Existing `PerkUnlockEngineTests.cs` | No changes required — engine rules remain covered; API tests are thin HTTP mapping. | + +Bruno + **`docs/manual-qa/NEO-48.md`** supplement automated coverage. Postgres persistence of picks is already covered by **`PerkStatePersistenceIntegrationTests`** (NEO-47); no new Postgres API test unless a regression gap appears during implementation. + +## Open questions / risks + +- **Field name `selected` vs `granted`:** Chosen to distinguish perk branch pick from XP grant; document clearly in README to avoid client confusion. +- **Large `unlockedEvents` batches:** Prototype catalog emits at most a handful per request; no pagination needed. +- **Concurrent POST picks:** Same as NEO-47 store semantics; no new transactional requirement for this story. + +## Decisions + +| Decision | Rationale | +|----------|-----------| +| POST on same path as GET | User choice; matches skill-progression / hotbar. | +| Include `unlockedEvents` on success | User choice; parallels `levelUps`; supports client/QA without parsing perk id diffs. | +| Branch picks as sorted arrays in JSON | Stable serialization; consistent with skill-progression “unordered by contract, sorted for bytes” pattern. | +| Reuse NEO-47 `reasonCode` strings verbatim | README + tests + Bruno share one vocabulary with engine. | From 39c54c38811979f6f099292209d69efaad11e11e Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:10:31 -0400 Subject: [PATCH 02/14] NEO-48: perk-state HTTP, Bruno, tests, and docs GET/POST /game/players/{id}/perk-state with versioned JSON, engine-backed branch selection, PerkStateApiTests, Bruno collection, README, and E2.M3 alignment. --- .../perk-state/Get perk state.bru | 28 +++ .../Post branch select deny level too low.bru | 39 ++++ .../perk-state/Post branch select tier1.bru | 45 ++++ .../neon-sprawl-server/perk-state/folder.bru | 3 + .../modules/E2_M3_MasteryAndPerkUnlocks.md | 6 +- ...umentation_and_implementation_alignment.md | 2 +- .../modules/module_dependency_register.md | 2 +- docs/manual-qa/NEO-48.md | 54 +++++ docs/plans/NEO-48-implementation-plan.md | 10 +- .../Game/Mastery/PerkStateApiTests.cs | 217 ++++++++++++++++++ .../Game/Mastery/PerkStateApi.cs | 124 ++++++++++ .../Game/Mastery/PerkStateDtos.cs | 97 ++++++++ server/NeonSprawl.Server/Program.cs | 1 + server/README.md | 17 ++ 14 files changed, 637 insertions(+), 8 deletions(-) create mode 100644 bruno/neon-sprawl-server/perk-state/Get perk state.bru create mode 100644 bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru create mode 100644 bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru create mode 100644 bruno/neon-sprawl-server/perk-state/folder.bru create mode 100644 docs/manual-qa/NEO-48.md create mode 100644 server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs create mode 100644 server/NeonSprawl.Server/Game/Mastery/PerkStateApi.cs create mode 100644 server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs diff --git a/bruno/neon-sprawl-server/perk-state/Get perk state.bru b/bruno/neon-sprawl-server/perk-state/Get perk state.bru new file mode 100644 index 0000000..8567c13 --- /dev/null +++ b/bruno/neon-sprawl-server/perk-state/Get perk state.bru @@ -0,0 +1,28 @@ +meta { + name: GET perk state + type: http + seq: 1 +} + +docs { + NEO-48: per-player perk state read model (branch picks + unlocked perk ids). + Empty state until branch picks; pair with skill-progression grant + POST branch select. +} + +get { + url: {{baseUrl}}/game/players/dev-local-1/perk-state + body: none + auth: none +} + +tests { + test("returns 200 JSON schema v1", function () { + expect(res.getStatus()).to.equal(200); + expect(res.getHeader("content-type")).to.contain("application/json"); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.playerId).to.equal("dev-local-1"); + expect(body.branchPicks).to.be.an("array"); + expect(body.unlockedPerkIds).to.be.an("array"); + }); +} diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru new file mode 100644 index 0000000..7a80656 --- /dev/null +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru @@ -0,0 +1,39 @@ +meta { + name: POST branch select deny level too low + type: http + seq: 3 +} + +docs { + NEO-48: salvage tier 1 requires skill level 2. On a fresh server (no prior salvage XP), expects selected false + level_too_low. + Restart server or use a fresh dev DB to hit this deny; skip if salvage already leveled from other Bruno runs. +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/perk-state + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "skillId": "salvage", + "tierIndex": 1, + "branchId": "scrap_efficiency" + } +} + +tests { + test("denies with level_too_low when salvage below gate", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + if (body.selected === false && body.reasonCode === "level_too_low") { + expect(body.unlockedEvents).to.be.an("array").that.is.empty; + expect(body.perkState.branchPicks).to.be.an("array"); + return; + } + expect(body.selected).to.equal(true, "salvage already at level 2+ — restart server for deny smoke"); + }); +} diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru b/bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru new file mode 100644 index 0000000..1c7d8c7 --- /dev/null +++ b/bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru @@ -0,0 +1,45 @@ +meta { + name: POST branch select tier1 + type: http + seq: 2 +} + +docs { + NEO-48: tier-1 salvage branch pick. Run skill-progression grant (100+ XP for level 2) first if this denies level_too_low. +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/perk-state + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "skillId": "salvage", + "tierIndex": 1, + "branchId": "scrap_efficiency" + } +} + +tests { + test("selects branch or denies with structured envelope", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.selected).to.be.a("boolean"); + expect(body.perkState).to.be.an("object"); + expect(body.perkState.schemaVersion).to.equal(1); + expect(body.unlockedEvents).to.be.an("array"); + if (body.selected) { + expect(body.reasonCode).to.be.undefined; + const salvage = body.perkState.branchPicks.find((t) => t.skillId === "salvage"); + expect(salvage).to.be.an("object"); + const pick = salvage.picks.find((p) => p.tierIndex === 1); + expect(pick.branchId).to.equal("scrap_efficiency"); + } else { + expect(body.reasonCode).to.be.a("string"); + } + }); +} diff --git a/bruno/neon-sprawl-server/perk-state/folder.bru b/bruno/neon-sprawl-server/perk-state/folder.bru new file mode 100644 index 0000000..62bd4c9 --- /dev/null +++ b/bruno/neon-sprawl-server/perk-state/folder.bru @@ -0,0 +1,3 @@ +meta { + name: perk-state +} diff --git a/docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md b/docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md index ef38de1..4370fbb 100644 --- a/docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md +++ b/docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md @@ -79,7 +79,11 @@ Each catalog file has top-level **`schemaVersion`**, a **`perks`** map (`perkId` ## NEO-47 handoff (perk unlock engine + PerkState persistence) -**Landed ([NEO-47](https://linear.app/neon-sprawl/issue/NEO-47)):** `IPlayerPerkStateStore` (in-memory + Postgres **`V004`**) persists branch picks and unlocked `perkId`s. **`PerkUnlockEngine`** evaluates tier-1 explicit branch picks and **path-auto** tier-2 unlocks from the tier-1 `branchId` when skill level meets gates (no second pick at tier 2 for prototype salvage). Level-up re-evaluation runs inside **`SkillProgressionGrantOperations`** after XP grants (HTTP POST, gather interact, mission/refine helpers). Stable deny **`reasonCode`** values for invalid picks. Internal **`PerkUnlockEvent`** returned from engine methods. **HTTP read/write** for perk state is **NEO-48**. +**Landed ([NEO-47](https://linear.app/neon-sprawl/issue/NEO-47)):** `IPlayerPerkStateStore` (in-memory + Postgres **`V004`**) persists branch picks and unlocked `perkId`s. **`PerkUnlockEngine`** evaluates tier-1 explicit branch picks and **path-auto** tier-2 unlocks from the tier-1 `branchId` when skill level meets gates (no second pick at tier 2 for prototype salvage). Level-up re-evaluation runs inside **`SkillProgressionGrantOperations`** after XP grants (HTTP POST, gather interact, mission/refine helpers). Stable deny **`reasonCode`** values for invalid picks. Internal **`PerkUnlockEvent`** returned from engine methods. + +## NEO-48 handoff (perk state HTTP + Bruno) + +**Landed ([NEO-48](https://linear.app/neon-sprawl/issue/NEO-48)):** Versioned **`GET`/`POST /game/players/{id}/perk-state`** — read model + branch selection via **`PerkUnlockEngine`**; structured **`selected`** / **`reasonCode`** / **`unlockedEvents`** envelopes. Bruno: `bruno/neon-sprawl-server/perk-state/`; [server README — Perk state (NEO-48)](../../../server/README.md#perk-state-neo-48); plan [NEO-48](../../plans/NEO-48-implementation-plan.md); manual QA [`NEO-48.md`](../../manual-qa/NEO-48.md). ## NEO-49 handoff (perk_unlock telemetry hook sites) diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 74acafc..05193da 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.M3 | In Progress | **NEO-45 landed:** prototype **`salvage`** mastery catalog + CI gates (see [NEO-45 plan](../../plans/NEO-45-implementation-plan.md)). **NEO-46 landed:** fail-fast server load under `server/NeonSprawl.Server/Game/Mastery/` — `MasteryCatalogLoader`, `IMasteryCatalogRegistry`, cross-check vs `ISkillDefinitionRegistry`, Slice 4 + **`tierIndex`** 1..N gate; see [NEO-46 plan](../../plans/NEO-46-implementation-plan.md). **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants; see [NEO-47 plan](../../plans/NEO-47-implementation-plan.md). **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook site in [`PerkUnlockEngine.TryUnlockPerks`](../../../server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs) ([NEO-49 plan](../../plans/NEO-49-implementation-plan.md), [`NEO-49` manual QA](../../manual-qa/NEO-49.md)); [server README — Perk unlock telemetry (NEO-49)](../../../server/README.md#perk-unlock-engine-and-telemetry-hooks-neo-47-neo-49). **Still planned:** perk HTTP + Bruno ([NEO-48](https://linear.app/neon-sprawl/issue/NEO-48)). | [NEO-45](../../plans/NEO-45-implementation-plan.md), [NEO-46](../../plans/NEO-46-implementation-plan.md), [NEO-47](../../plans/NEO-47-implementation-plan.md), [NEO-49](../../plans/NEO-49-implementation-plan.md), [E2M3-pre-production-backlog](../../plans/E2M3-pre-production-backlog.md), [E2_M3](E2_M3_MasteryAndPerkUnlocks.md) | +| E2.M3 | In Progress | **NEO-45 landed:** prototype **`salvage`** mastery catalog + CI gates (see [NEO-45 plan](../../plans/NEO-45-implementation-plan.md)). **NEO-46 landed:** fail-fast server load under `server/NeonSprawl.Server/Game/Mastery/` — `MasteryCatalogLoader`, `IMasteryCatalogRegistry`, cross-check vs `ISkillDefinitionRegistry`, Slice 4 + **`tierIndex`** 1..N gate; see [NEO-46 plan](../../plans/NEO-46-implementation-plan.md). **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants; see [NEO-47 plan](../../plans/NEO-47-implementation-plan.md). **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook site in [`PerkUnlockEngine.TryUnlockPerks`](../../../server/NeonSprawl.Server/Game/Mastery/PerkUnlockEngine.cs) ([NEO-49 plan](../../plans/NEO-49-implementation-plan.md), [`NEO-49` manual QA](../../manual-qa/NEO-49.md)); [server README — Perk unlock telemetry (NEO-49)](../../../server/README.md#perk-unlock-engine-and-telemetry-hooks-neo-47-neo-49). **NEO-48 landed:** **`GET`/`POST /game/players/{id}/perk-state`** — `PerkStateApi` + DTOs in `Game/Mastery/` ([NEO-48](../../plans/NEO-48-implementation-plan.md), [`NEO-48` manual QA](../../manual-qa/NEO-48.md)); [server README — Perk state (NEO-48)](../../../server/README.md#perk-state-neo-48); Bruno `bruno/neon-sprawl-server/perk-state/`. | [NEO-45](../../plans/NEO-45-implementation-plan.md), [NEO-46](../../plans/NEO-46-implementation-plan.md), [NEO-47](../../plans/NEO-47-implementation-plan.md), [NEO-48](../../plans/NEO-48-implementation-plan.md), [NEO-49](../../plans/NEO-49-implementation-plan.md), [E2M3-pre-production-backlog](../../plans/E2M3-pre-production-backlog.md), [E2_M3](E2_M3_MasteryAndPerkUnlocks.md) | | 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/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 13d8f9d..6f26ff0 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -36,7 +36,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen **E2.M2 note:** Epic 2 Slice 2 — [NEO-37](https://linear.app/neon-sprawl/issue/NEO-37) (progression snapshot **GET**) and [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (grant **POST**, persistence, level-up payloads); label **`E2.M2`**. Module doc [E2_M2_XpAwardAndLevelEngine.md](E2_M2_XpAwardAndLevelEngine.md). Slice 3 integration: [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41)–[NEO-43](https://linear.app/neon-sprawl/issue/NEO-43) landed; **NEO-44** (gig XP, Epic 5) remains backlog. Keep aligned with [documentation and implementation alignment](documentation_and_implementation_alignment.md). -**E2.M3 note:** Epic 2 Slice 4 — [NEO-45](https://linear.app/neon-sprawl/issue/NEO-45) → [NEO-49](https://linear.app/neon-sprawl/issue/NEO-49); label **`E2.M3`**. **NEO-45 landed:** prototype **`salvage`** mastery catalog + schema + CI. **NEO-46 landed:** fail-fast server mastery catalog load. **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants. **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook in `PerkUnlockEngine.TryUnlockPerks`. **Still planned:** perk HTTP + Bruno ([NEO-48](https://linear.app/neon-sprawl/issue/NEO-48)). Flagship track; see [E2_M3_MasteryAndPerkUnlocks.md](E2_M3_MasteryAndPerkUnlocks.md), [E2M3-pre-production-backlog.md](../../plans/E2M3-pre-production-backlog.md), and [documentation_and_implementation_alignment.md](documentation_and_implementation_alignment.md) E2.M3 row. +**E2.M3 note:** Epic 2 Slice 4 — [NEO-45](https://linear.app/neon-sprawl/issue/NEO-45) → [NEO-49](https://linear.app/neon-sprawl/issue/NEO-49); label **`E2.M3`**. **NEO-45 landed:** prototype **`salvage`** mastery catalog + schema + CI. **NEO-46 landed:** fail-fast server mastery catalog load. **NEO-47 landed:** `PerkUnlockEngine`, `IPlayerPerkStateStore` + **`V004`**, level-up hook in skill XP grants. **NEO-49 landed:** comment-only **`perk_unlock`** telemetry hook in `PerkUnlockEngine.TryUnlockPerks`. **NEO-48 landed:** **`GET`/`POST /game/players/{id}/perk-state`** + Bruno `perk-state/`. Flagship track; see [E2_M3_MasteryAndPerkUnlocks.md](E2_M3_MasteryAndPerkUnlocks.md), [E2M3-pre-production-backlog.md](../../plans/E2M3-pre-production-backlog.md), and [documentation_and_implementation_alignment.md](documentation_and_implementation_alignment.md) E2.M3 row. ### Epic 3 — Crafting Economy diff --git a/docs/manual-qa/NEO-48.md b/docs/manual-qa/NEO-48.md new file mode 100644 index 0000000..21b1c3c --- /dev/null +++ b/docs/manual-qa/NEO-48.md @@ -0,0 +1,54 @@ +# Manual QA — NEO-48 (perk state GET + branch selection POST) + +Reference: [implementation plan](../plans/NEO-48-implementation-plan.md), [NEO-47 plan](../plans/NEO-47-implementation-plan.md) (unlock engine). + +## Preconditions + +- Run `NeonSprawl.Server` (`dotnet run` from `server/NeonSprawl.Server`; default `http://localhost:5253`). +- Dev player `dev-local-1` exists in position state (default in-memory / seeded Postgres). + +## GET empty state + +```bash +curl -sS "http://localhost:5253/game/players/dev-local-1/perk-state" | jq . +``` + +- [ ] HTTP **200**; `schemaVersion` **1**; `branchPicks` **[]**; `unlockedPerkIds` **[]** (fresh server). + +## POST tier-1 pick (happy path) + +1. Grant salvage XP to reach level **2** (prototype curve: **100** total XP): + +```bash +curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":100,"sourceKind":"activity"}' | jq . +``` + +2. Select tier-1 branch: + +```bash +curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","tierIndex":1,"branchId":"scrap_efficiency"}' | jq . +``` + +- [ ] `selected: true`; `perkState.branchPicks` contains salvage tier 1 `scrap_efficiency`; `unlockedEvents` empty at tier 1. +- [ ] Repeat **GET** — same pick visible. + +## POST deny + +- [ ] Second pick same tier different branch → `selected: false`, `reasonCode` **`branch_already_chosen`**. +- [ ] On fresh server without XP grant → `level_too_low`. + +## Bruno + +- [ ] Run folder `bruno/neon-sprawl-server/perk-state/` (GET + POST happy + deny smoke). + +## Regression + +```bash +cd server && dotnet test NeonSprawl.sln +``` + +Expect all tests pass including `PerkStateApiTests`. diff --git a/docs/plans/NEO-48-implementation-plan.md b/docs/plans/NEO-48-implementation-plan.md index 6be9695..49b5749 100644 --- a/docs/plans/NEO-48-implementation-plan.md +++ b/docs/plans/NEO-48-implementation-plan.md @@ -42,11 +42,11 @@ ## Acceptance criteria checklist -- [ ] `GET /game/players/{id}/perk-state` returns versioned JSON v1. -- [ ] `POST` branch selection applies NEO-47 unlock rules; structured denies on invalid tier/branch. -- [ ] Bruno collection `bruno/neon-sprawl-server/perk-state/` (GET + POST + deny). -- [ ] `server/README.md` section for perk-state endpoints. -- [ ] Unit/integration tests (AAA) for API happy/deny paths. +- [x] `GET /game/players/{id}/perk-state` returns versioned JSON v1. +- [x] `POST` branch selection applies NEO-47 unlock rules; structured denies on invalid tier/branch. +- [x] Bruno collection `bruno/neon-sprawl-server/perk-state/` (GET + POST + deny). +- [x] `server/README.md` section for perk-state endpoints. +- [x] Unit/integration tests (AAA) for API happy/deny paths. ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs new file mode 100644 index 0000000..7ee0265 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs @@ -0,0 +1,217 @@ +using System.Linq; +using System.Net; +using System.Net.Http.Json; +using NeonSprawl.Server.Game.Mastery; +using NeonSprawl.Server.Game.Skills; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Mastery; + +public sealed class PerkStateApiTests +{ + private const string DevPlayer = "dev-local-1"; + + private static PerkBranchSelectRequest SelectBranch( + string skillId, + int tierIndex, + string branchId, + int schemaVersion = PerkBranchSelectRequest.CurrentSchemaVersion) => + new PerkBranchSelectRequest + { + SchemaVersion = schemaVersion, + SkillId = skillId, + TierIndex = tierIndex, + BranchId = branchId, + }; + + private static SkillProgressionGrantRequest SalvageGrant(int amount) => + new SkillProgressionGrantRequest + { + SchemaVersion = SkillProgressionGrantRequest.CurrentSchemaVersion, + SkillId = "salvage", + Amount = amount, + SourceKind = "activity", + }; + + [Fact] + public async Task GetPerkState_ShouldReturnNotFound_WhenPlayerUnknown() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.GetAsync("/game/players/missing-player/perk-state"); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task GetPerkState_ShouldReturnSchemaV1_WithEmptyState_ForDevPlayer() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.GetAsync($"/game/players/{DevPlayer}/perk-state"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal(PerkStateSnapshotResponse.CurrentSchemaVersion, body!.SchemaVersion); + Assert.Equal(DevPlayer, body.PlayerId); + Assert.Empty(body.BranchPicks); + Assert.Empty(body.UnlockedPerkIds); + } + + [Fact] + public async Task PostPerkState_ShouldReturnBadRequest_WhenSchemaVersionMismatch() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var request = SelectBranch("salvage", 1, "scrap_efficiency", PerkBranchSelectRequest.CurrentSchemaVersion + 42); + + // Act + var response = await client.PostAsJsonAsync($"/game/players/{DevPlayer}/perk-state", request); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostPerkState_ShouldReturnNotFound_WhenPlayerUnknown() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsJsonAsync( + "/game/players/missing-player/perk-state", + SelectBranch("salvage", 1, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task PostPerkState_ShouldDenyLevelTooLow_WhenSalvageBelowTierGate() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 1, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.LevelTooLow, envelope.ReasonCode); + Assert.Empty(envelope.UnlockedEvents); + Assert.Empty(envelope.PerkState.BranchPicks); + } + + [Fact] + public async Task PostPerkState_ShouldSelectTier1_AndGetReflectsPick_AfterSalvageLevel2() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var grant = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + SalvageGrant(100)); + Assert.Equal(HttpStatusCode.OK, grant.StatusCode); + + // Act + var post = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 1, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, post.StatusCode); + var envelope = await post.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.True(envelope!.Selected); + Assert.Null(envelope.ReasonCode); + Assert.Empty(envelope.UnlockedEvents); + var track = Assert.Single(envelope.PerkState.BranchPicks); + Assert.Equal("salvage", track.SkillId); + var pick = Assert.Single(track.Picks); + Assert.Equal(1, pick.TierIndex); + Assert.Equal("scrap_efficiency", pick.BranchId); + Assert.Empty(envelope.PerkState.UnlockedPerkIds); + + var get = await client.GetFromJsonAsync($"/game/players/{DevPlayer}/perk-state"); + Assert.NotNull(get); + var getTrack = Assert.Single(get!.BranchPicks); + Assert.Equal("scrap_efficiency", Assert.Single(getTrack.Picks).BranchId); + } + + [Fact] + public async Task PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + SalvageGrant(100))).StatusCode); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 1, "scrap_efficiency"))).StatusCode); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 1, "bulk_haul")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.BranchAlreadyChosen, envelope.ReasonCode); + Assert.Equal("scrap_efficiency", Assert.Single(Assert.Single(envelope.PerkState.BranchPicks).Picks).BranchId); + } + + [Fact] + public async Task PostPerkState_ShouldEmitUnlockedEvents_WhenTier2PerkUnlocksAtLevel4() + { + // Arrange — 100 + 400 XP → level 4 before tier-1 pick; path-auto tier 2 on same POST + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + SalvageGrant(100))).StatusCode); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + SalvageGrant(400))).StatusCode); + + // Act + var post = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 1, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, post.StatusCode); + var envelope = await post.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.True(envelope!.Selected); + var unlocked = Assert.Single(envelope.UnlockedEvents); + Assert.Equal("salvage_scrap_efficiency_1", unlocked.PerkId); + Assert.Equal("salvage", unlocked.SkillId); + Assert.Equal(2, unlocked.TierIndex); + Assert.Equal("scrap_efficiency", unlocked.BranchId); + Assert.Equal("level_up", unlocked.Source); + Assert.Contains("salvage_scrap_efficiency_1", envelope.PerkState.UnlockedPerkIds); + } +} diff --git a/server/NeonSprawl.Server/Game/Mastery/PerkStateApi.cs b/server/NeonSprawl.Server/Game/Mastery/PerkStateApi.cs new file mode 100644 index 0000000..0665be3 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Mastery/PerkStateApi.cs @@ -0,0 +1,124 @@ +using NeonSprawl.Server.Game.PositionState; + +namespace NeonSprawl.Server.Game.Mastery; + +/// +/// Maps GET/POST /game/players/{{id}}/perk-state — read snapshot and branch selection (NEO-48). +/// +public static class PerkStateApi +{ + public static WebApplication MapPerkStateApi(this WebApplication app) + { + app.MapGet( + "/game/players/{id}/perk-state", + (string id, IPositionStateStore positions, IPlayerPerkStateStore perkStore) => + { + var trimmedId = id.Trim(); + if (trimmedId.Length == 0 || !positions.TryGetPosition(trimmedId, out _)) + { + return Results.NotFound(); + } + + return Results.Json(BuildSnapshot(trimmedId, perkStore)); + }); + + app.MapPost( + "/game/players/{id}/perk-state", + (string id, PerkBranchSelectRequest? body, IPositionStateStore positions, PerkUnlockEngine perkUnlockEngine) => + { + if (body is null || body.SchemaVersion != PerkBranchSelectRequest.CurrentSchemaVersion) + { + return Results.BadRequest(); + } + + var trimmedId = id.Trim(); + if (trimmedId.Length == 0 || !positions.TryGetPosition(trimmedId, out _)) + { + return Results.NotFound(); + } + + var outcome = perkUnlockEngine.TrySelectBranch( + trimmedId, + body.SkillId, + body.TierIndex, + body.BranchId); + + var perkState = BuildSnapshot(trimmedId, outcome.Snapshot); + if (!outcome.Success) + { + return Results.Json( + new PerkBranchSelectResponse + { + Selected = false, + ReasonCode = outcome.ReasonCode, + PerkState = perkState, + UnlockedEvents = [], + }); + } + + return Results.Json( + new PerkBranchSelectResponse + { + Selected = true, + PerkState = perkState, + UnlockedEvents = MapEvents(outcome.Events), + }); + }); + + return app; + } + + internal static PerkStateSnapshotResponse BuildSnapshot(string playerId, IPlayerPerkStateStore store) => + BuildSnapshot(playerId, store.GetSnapshot(playerId)); + + internal static PerkStateSnapshotResponse BuildSnapshot(string playerId, PerkStateSnapshot snapshot) + { + var branchPicks = new List(snapshot.BranchPicksBySkillId.Count); + foreach (var (skillId, tiers) in snapshot.BranchPicksBySkillId.OrderBy(static kv => kv.Key, StringComparer.Ordinal)) + { + var picks = tiers + .OrderBy(static kv => kv.Key) + .Select(static kv => new PerkBranchPickRowJson { TierIndex = kv.Key, BranchId = kv.Value }) + .ToList(); + branchPicks.Add(new PerkBranchPickTrackJson { SkillId = skillId, Picks = picks }); + } + + var unlocked = snapshot.UnlockedPerkIds.OrderBy(static id => id, StringComparer.Ordinal).ToList(); + + return new PerkStateSnapshotResponse + { + PlayerId = playerId, + BranchPicks = branchPicks, + UnlockedPerkIds = unlocked, + }; + } + + private static IReadOnlyList MapEvents(IReadOnlyList events) + { + if (events.Count == 0) + { + return []; + } + + var mapped = new List(events.Count); + foreach (var e in events) + { + mapped.Add( + new PerkUnlockedEventJson + { + PerkId = e.PerkId, + SkillId = e.SkillId, + TierIndex = e.TierIndex, + BranchId = e.BranchId, + Source = e.Source switch + { + PerkUnlockSource.BranchPick => "branch_pick", + PerkUnlockSource.LevelUp => "level_up", + _ => throw new InvalidOperationException($"Unexpected perk unlock source: {e.Source}"), + }, + }); + } + + return mapped; + } +} diff --git a/server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs b/server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs new file mode 100644 index 0000000..f2f6a78 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs @@ -0,0 +1,97 @@ +using System.Text.Json.Serialization; + +namespace NeonSprawl.Server.Game.Mastery; + +/// JSON body for GET /game/players/{{id}}/perk-state (NEO-48). +public sealed class PerkStateSnapshotResponse +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + [JsonPropertyName("playerId")] + public required string PlayerId { get; init; } + + /// Branch picks per skill. Consumers key by — array order is not part of the contract. + [JsonPropertyName("branchPicks")] + public required IReadOnlyList BranchPicks { get; init; } + + [JsonPropertyName("unlockedPerkIds")] + public required IReadOnlyList UnlockedPerkIds { get; init; } +} + +public sealed class PerkBranchPickTrackJson +{ + [JsonPropertyName("skillId")] + public required string SkillId { get; init; } + + [JsonPropertyName("picks")] + public required IReadOnlyList Picks { get; init; } +} + +public sealed class PerkBranchPickRowJson +{ + [JsonPropertyName("tierIndex")] + public int TierIndex { get; init; } + + [JsonPropertyName("branchId")] + public required string BranchId { get; init; } +} + +/// POST body for branch selection on POST …/perk-state (NEO-48). +public sealed class PerkBranchSelectRequest +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } + + [JsonPropertyName("skillId")] + public required string SkillId { get; init; } + + [JsonPropertyName("tierIndex")] + public int TierIndex { get; init; } + + [JsonPropertyName("branchId")] + public required string BranchId { get; init; } +} + +/// POST responses for branch selection — success () or structured deny (). +public sealed class PerkBranchSelectResponse +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + [JsonPropertyName("selected")] + public bool Selected { get; init; } + + [JsonPropertyName("reasonCode")] + public string? ReasonCode { get; init; } + + [JsonPropertyName("perkState")] + public required PerkStateSnapshotResponse PerkState { get; init; } + + [JsonPropertyName("unlockedEvents")] + public required IReadOnlyList UnlockedEvents { get; init; } +} + +public sealed class PerkUnlockedEventJson +{ + [JsonPropertyName("perkId")] + public required string PerkId { get; init; } + + [JsonPropertyName("skillId")] + public required string SkillId { get; init; } + + [JsonPropertyName("tierIndex")] + public int TierIndex { get; init; } + + [JsonPropertyName("branchId")] + public string? BranchId { get; init; } + + [JsonPropertyName("source")] + public required string Source { get; init; } +} diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index 2ec626d..dae8a54 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -35,6 +35,7 @@ app.MapInteractionApi(); app.MapInteractablesWorldApi(); app.MapSkillDefinitionsWorldApi(); app.MapSkillProgressionSnapshotApi(); +app.MapPerkStateApi(); app.MapTargetingApi(); app.MapHotbarLoadoutApi(); app.MapCooldownSnapshotApi(); diff --git a/server/README.md b/server/README.md index 3f5156f..ab0fba2 100644 --- a/server/README.md +++ b/server/README.md @@ -56,6 +56,23 @@ On success, **Information** logs include the resolved mastery directory, track c **NEO-49:** comment-only **telemetry hook site** for future catalog event **`perk_unlock`** in **`PerkUnlockEngine.TryUnlockPerks`** when new perk ids are persisted (one emit per unlocked perk; **`TODO(E9.M1)`** — no production ingest). Manual QA: [`docs/manual-qa/NEO-49.md`](../../docs/manual-qa/NEO-49.md); plan: [NEO-49 implementation plan](../../docs/plans/NEO-49-implementation-plan.md). +## Perk state (NEO-48) + +**`GET /game/players/{id}/perk-state`** returns versioned JSON (`schemaVersion` **1**, **`playerId`**, **`branchPicks`**, **`unlockedPerkIds`**). **`branchPicks`** is an array of `{ skillId, picks: [{ tierIndex, branchId }] }` — key by **`skillId`**; array order is not part of the contract. **`unlockedPerkIds`** lists perk ids currently unlocked for the player. + +**`POST /game/players/{id}/perk-state`** commits one mastery branch pick (`schemaVersion` **1**, **`skillId`**, **`tierIndex`**, **`branchId`**). Delegates to **`PerkUnlockEngine.TrySelectBranch`** (NEO-47). **404** when the player is unknown to position state; **400** when **`schemaVersion`** is missing or not **1**. Persistence uses **`IPlayerPerkStateStore`** + Flyway **`V004`** when Postgres is configured (same [NEO-29-style](#position-persistence-neo-8) split as skill progression). + +**Structured responses (HTTP 200):** **`selected`** (`true`/`false`), optional **`reasonCode`** on deny, authoritative **`perkState`** (same shape as GET), and **`unlockedEvents`** on success (empty on deny). Stable deny codes (see `PerkUnlockReasonCodes`): **`unknown_track`**, **`unknown_tier`**, **`unknown_branch`**, **`level_too_low`**, **`branch_already_chosen`**, **`tier_branch_not_selectable`**, **`player_not_found`**. On **`selected: true`**, **`unlockedEvents`** lists perks newly unlocked on this request (`perkId`, `skillId`, `tierIndex`, `branchId`, **`source`**: **`branch_pick`** | **`level_up`**). Tier-1 prototype salvage picks may emit zero events; path-auto tier-2 unlocks at level 4+ may emit **`level_up`** events on the same POST when level gates are already met. + +Plan: [NEO-48 implementation plan](../../docs/plans/NEO-48-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-48.md`](../../docs/manual-qa/NEO-48.md); Bruno: `bruno/neon-sprawl-server/perk-state/`. + +```bash +curl -sS -i "http://localhost:5253/game/players/dev-local-1/perk-state" +curl -sS -i -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","tierIndex":1,"branchId":"scrap_efficiency"}' +``` + ## Skill definitions (NEO-36) **`GET /game/world/skill-definitions`** returns a versioned JSON body (`schemaVersion` **1**, **`skills`**) backed by **`ISkillDefinitionRegistry`** — the same prototype rows loaded at startup (no second source of truth). Plan: [NEO-36 implementation plan](../../docs/plans/NEO-36-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-36.md`](../../docs/manual-qa/NEO-36.md); Bruno: `bruno/neon-sprawl-server/skill-definitions/`. From 0838e661c9109ad8f5ce5708b67f7c7fdf7e9a92 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:16:46 -0400 Subject: [PATCH 03/14] NEO-48: add code review for perk-state HTTP --- docs/reviews/2026-05-17-NEO-48.md | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 docs/reviews/2026-05-17-NEO-48.md diff --git a/docs/reviews/2026-05-17-NEO-48.md b/docs/reviews/2026-05-17-NEO-48.md new file mode 100644 index 0000000..f25ae3f --- /dev/null +++ b/docs/reviews/2026-05-17-NEO-48.md @@ -0,0 +1,58 @@ +# Code review — NEO-48 perk state GET + branch selection POST + +**Date:** 2026-05-17 +**Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `5290044`–`39c54c3` vs `main` +**Base:** `main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-48 adds thin HTTP mapping for versioned **`GET`/`POST /game/players/{id}/perk-state`**: read model (`branchPicks`, `unlockedPerkIds`), branch selection via **`PerkUnlockEngine.TrySelectBranch`**, and structured **`selected`** / **`reasonCode`** / **`perkState`** / **`unlockedEvents`** envelopes mirroring NEO-38 grant patterns. Eight integration tests cover 404/400 gates, happy tier-1 pick after XP grant, `level_too_low` / `branch_already_chosen` denies, and path-auto **`unlockedEvents`** at level 4+. Bruno folder, README, manual QA, and E2.M3 decomposition rows align with the implementation plan. **185** server tests pass locally. Low merge risk; no engine or persistence logic duplicated in the API layer. + +## Documentation checked + +| Document | Result | +|----------|--------| +| [`docs/plans/NEO-48-implementation-plan.md`](../plans/NEO-48-implementation-plan.md) | **Matches** — same-path GET/POST, schema v1, deny envelope, `unlockedEvents` on success, position-state 404 gate, Bruno/README/tests/manual QA; acceptance checklist complete. | +| [`docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md`](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) | **Matches** — NEO-48 handoff section; HTTP responsibility listed under module responsibilities. | +| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E2.M3 row cites NEO-48 landed + README/Bruno links. | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E2.M3 note includes NEO-48 perk-state HTTP. | +| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — branch picks and unlock evaluation remain server-authoritative; no client changes (out of scope). | +| [`docs/plans/NEO-47-implementation-plan.md`](../plans/NEO-47-implementation-plan.md) | **Matches** — API delegates to engine/store; no duplicate unlock rules; telemetry stays in `TryUnlockPerks` (NEO-49). | + +Register/tracking: E2.M3 module doc, alignment row, and register note updated in implementation commit. + +## Blocking issues + +None. + +## Suggestions + +1. **Bruno deny for `branch_already_chosen`** — Plan and manual QA describe a second POST to the same tier with a different branch; automated tests cover it (`PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick`), but Bruno only ships `Post branch select deny level too low.bru`. A fourth request (grant + first pick + conflicting second pick) would make the collection self-contained for CI/manual runs without relying on manual QA alone. +2. **Optional thin HTTP deny smoke** — Engine tests lock `unknown_track`, `unknown_tier`, and `unknown_branch`; one API test each (bad `skillId`, bad `tierIndex`, bad `branchId` on dev player after level gate) would guard JSON field mapping if DTOs change. Non-blocking given `PerkUnlockEngineTests` coverage. +3. **POST null body** — Handlers return **400** for `body is null` (same as skill-progression). Neither API suite asserts empty-body **400** today; add a one-liner in `PerkStateApiTests` if you want parity documentation in tests. + +## Nits + +- Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often. +- Nit: Path-auto tier-2 unlocks surfaced in **`unlockedEvents`** use **`source: level_up`** (not `branch_pick`) — consistent with NEO-47/NEO-49 notes; clients/telemetry should treat `source` as unlock channel, not HTTP verb. +- Nit: `PostPerkState_ShouldSelectTier1_AndGetReflectsPick_AfterSalvageLevel2` issues a follow-up **GET** inside **Assert** for end-to-end verification — valid AAA (verification in Assert); grant status checks in **Arrange** match other API tests in the repo. + +## Verification + +```bash +# From repo root +dotnet test NeonSprawl.sln --filter "FullyQualifiedName~PerkStateApi" + +# Full server suite (185 tests at review) +dotnet test NeonSprawl.sln + +# Manual / Bruno +# docs/manual-qa/NEO-48.md +# bruno/neon-sprawl-server/perk-state/ +``` + +Confirm **GET** empty state, **POST** tier-1 happy path after salvage grant (100 XP), **POST** deny codes, and **GET** reflects persisted pick; optional path-auto **`unlockedEvents`** at level 4+ (500 total XP before pick). From 3ba830065206a7e01b5d7c56b952012a962ac2f3 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:21:42 -0400 Subject: [PATCH 04/14] =?UTF-8?q?NEO-48:=20address=20code=20review=20?= =?UTF-8?q?=E2=80=94=20Bruno=20deny,=20API=20deny=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add self-contained branch_already_chosen Bruno request, HTTP deny smoke for unknown track/tier/branch and null-body 400, folder docs for level_too_low. --- ...anch select deny branch already chosen.bru | 72 ++++++++++++++ .../neon-sprawl-server/perk-state/folder.bru | 6 ++ docs/manual-qa/NEO-48.md | 1 + docs/reviews/2026-05-17-NEO-48.md | 11 ++- .../Game/Mastery/PerkStateApiTests.cs | 93 ++++++++++++++++++- 5 files changed, 175 insertions(+), 8 deletions(-) create mode 100644 bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru new file mode 100644 index 0000000..c64f45d --- /dev/null +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru @@ -0,0 +1,72 @@ +meta { + name: POST branch select deny branch already chosen + type: http + seq: 4 +} + +docs { + NEO-48: self-contained deny — pre-request grants salvage to level 2 and picks scrap_efficiency; + main request conflicts with bulk_haul on the same tier. Expects selected false + branch_already_chosen. +} + +script:pre-request { + const baseUrl = bru.getVar("baseUrl"); + const grant = await bru.sendRequest({ + method: "POST", + url: `${baseUrl}/game/players/dev-local-1/skill-progression`, + headers: { "Content-Type": "application/json" }, + data: { + schemaVersion: 1, + skillId: "salvage", + amount: 100, + sourceKind: "activity", + }, + }); + if (grant.status !== 200) { + throw new Error(`salvage grant failed: ${grant.status}`); + } + const firstPick = await bru.sendRequest({ + method: "POST", + url: `${baseUrl}/game/players/dev-local-1/perk-state`, + headers: { "Content-Type": "application/json" }, + data: { + schemaVersion: 1, + skillId: "salvage", + tierIndex: 1, + branchId: "scrap_efficiency", + }, + }); + if (firstPick.status !== 200 || firstPick.data?.selected !== true) { + throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`); + } +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/perk-state + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "skillId": "salvage", + "tierIndex": 1, + "branchId": "bulk_haul" + } +} + +tests { + test("denies second pick with branch_already_chosen", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.selected).to.equal(false); + expect(body.reasonCode).to.equal("branch_already_chosen"); + expect(body.unlockedEvents).to.be.an("array").that.is.empty; + const salvage = body.perkState.branchPicks.find((t) => t.skillId === "salvage"); + expect(salvage).to.be.an("object"); + const pick = salvage.picks.find((p) => p.tierIndex === 1); + expect(pick.branchId).to.equal("scrap_efficiency"); + }); +} diff --git a/bruno/neon-sprawl-server/perk-state/folder.bru b/bruno/neon-sprawl-server/perk-state/folder.bru index 62bd4c9..3a755c2 100644 --- a/bruno/neon-sprawl-server/perk-state/folder.bru +++ b/bruno/neon-sprawl-server/perk-state/folder.bru @@ -1,3 +1,9 @@ meta { name: perk-state } + +docs { + NEO-48 perk-state HTTP smoke. Run folder in seq order for full coverage. + Post branch select deny level too low: passes if salvage is still level 1; if dev-local-1 already has salvage XP from other runs, restart the server (or reset DB in Postgres mode) before expecting level_too_low. + Post branch select deny branch already chosen: self-contained (pre-request grant + first pick); safe to re-run. +} diff --git a/docs/manual-qa/NEO-48.md b/docs/manual-qa/NEO-48.md index 21b1c3c..a6e4506 100644 --- a/docs/manual-qa/NEO-48.md +++ b/docs/manual-qa/NEO-48.md @@ -44,6 +44,7 @@ curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \ ## Bruno - [ ] Run folder `bruno/neon-sprawl-server/perk-state/` (GET + POST happy + deny smoke). +- [ ] Run **`Post branch select deny branch already chosen`** (self-contained; pre-request grant + first pick). ## Regression diff --git a/docs/reviews/2026-05-17-NEO-48.md b/docs/reviews/2026-05-17-NEO-48.md index f25ae3f..b2a376f 100644 --- a/docs/reviews/2026-05-17-NEO-48.md +++ b/docs/reviews/2026-05-17-NEO-48.md @@ -2,7 +2,8 @@ **Date:** 2026-05-17 **Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `5290044`–`39c54c3` vs `main` -**Base:** `main` +**Base:** `main` +**Follow-up:** Code-review suggestions 1–3 and nit 1 addressed in a follow-up commit on the same branch. ## Verdict @@ -31,13 +32,13 @@ None. ## Suggestions -1. **Bruno deny for `branch_already_chosen`** — Plan and manual QA describe a second POST to the same tier with a different branch; automated tests cover it (`PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick`), but Bruno only ships `Post branch select deny level too low.bru`. A fourth request (grant + first pick + conflicting second pick) would make the collection self-contained for CI/manual runs without relying on manual QA alone. -2. **Optional thin HTTP deny smoke** — Engine tests lock `unknown_track`, `unknown_tier`, and `unknown_branch`; one API test each (bad `skillId`, bad `tierIndex`, bad `branchId` on dev player after level gate) would guard JSON field mapping if DTOs change. Non-blocking given `PerkUnlockEngineTests` coverage. -3. **POST null body** — Handlers return **400** for `body is null` (same as skill-progression). Neither API suite asserts empty-body **400** today; add a one-liner in `PerkStateApiTests` if you want parity documentation in tests. +1. ~~**Bruno deny for `branch_already_chosen`** — Plan and manual QA describe a second POST to the same tier with a different branch; automated tests cover it (`PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick`), but Bruno only ships `Post branch select deny level too low.bru`. A fourth request (grant + first pick + conflicting second pick) would make the collection self-contained for CI/manual runs without relying on manual QA alone.~~ **Done.** Added `Post branch select deny branch already chosen.bru` with `script:pre-request` grant + first pick via `bru.sendRequest`. +2. ~~**Optional thin HTTP deny smoke** — Engine tests lock `unknown_track`, `unknown_tier`, and `unknown_branch`; one API test each (bad `skillId`, bad `tierIndex`, bad `branchId` on dev player after level gate) would guard JSON field mapping if DTOs change. Non-blocking given `PerkUnlockEngineTests` coverage.~~ **Done.** `PostPerkState_ShouldDenyUnknownTrack_*`, `UnknownTier_*`, `UnknownBranch_*` in `PerkStateApiTests`. +3. ~~**POST null body** — Handlers return **400** for `body is null` (same as skill-progression). Neither API suite asserts empty-body **400** today; add a one-liner in `PerkStateApiTests` if you want parity documentation in tests.~~ **Done.** `PostPerkState_ShouldReturnBadRequest_WhenBodyNull`. ## Nits -- Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often. +- ~~Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often.~~ **Done.** Folder `docs` block in `perk-state/folder.bru`. - Nit: Path-auto tier-2 unlocks surfaced in **`unlockedEvents`** use **`source: level_up`** (not `branch_pick`) — consistent with NEO-47/NEO-49 notes; clients/telemetry should treat `source` as unlock channel, not HTTP verb. - Nit: `PostPerkState_ShouldSelectTier1_AndGetReflectsPick_AfterSalvageLevel2` issues a follow-up **GET** inside **Assert** for end-to-end verification — valid AAA (verification in Assert); grant status checks in **Arrange** match other API tests in the repo. diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs index 7ee0265..d83c843 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs @@ -1,6 +1,8 @@ using System.Linq; using System.Net; +using System.Net.Http; using System.Net.Http.Json; +using System.Text; using NeonSprawl.Server.Game.Mastery; using NeonSprawl.Server.Game.Skills; using Xunit; @@ -67,6 +69,22 @@ public sealed class PerkStateApiTests Assert.Empty(body.UnlockedPerkIds); } + [Fact] + public async Task PostPerkState_ShouldReturnBadRequest_WhenBodyNull() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsync( + $"/game/players/{DevPlayer}/perk-state", + new StringContent(string.Empty, Encoding.UTF8, "application/json")); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + [Fact] public async Task PostPerkState_ShouldReturnBadRequest_WhenSchemaVersionMismatch() { @@ -156,15 +174,84 @@ public sealed class PerkStateApiTests Assert.Equal("scrap_efficiency", Assert.Single(getTrack.Picks).BranchId); } + private static async Task GrantSalvageLevel2Async(HttpClient client) + { + var grant = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + SalvageGrant(100)); + Assert.Equal(HttpStatusCode.OK, grant.StatusCode); + } + + [Fact] + public async Task PostPerkState_ShouldDenyUnknownTrack_WhenSkillIdInvalid() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await GrantSalvageLevel2Async(client); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("refine", 1, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.UnknownTrack, envelope.ReasonCode); + } + + [Fact] + public async Task PostPerkState_ShouldDenyUnknownTier_WhenTierIndexInvalid() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await GrantSalvageLevel2Async(client); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 99, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.UnknownTier, envelope.ReasonCode); + } + + [Fact] + public async Task PostPerkState_ShouldDenyUnknownBranch_WhenBranchIdInvalid() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await GrantSalvageLevel2Async(client); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 1, "not_a_branch")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.UnknownBranch, envelope.ReasonCode); + } + [Fact] public async Task PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick() { // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); - Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( - $"/game/players/{DevPlayer}/skill-progression", - SalvageGrant(100))).StatusCode); + await GrantSalvageLevel2Async(client); Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( $"/game/players/{DevPlayer}/perk-state", SelectBranch("salvage", 1, "scrap_efficiency"))).StatusCode); From d5b65c94f67df266d4b8de0098a34754309da2a6 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:22:22 -0400 Subject: [PATCH 05/14] NEO-48: fix Bruno pre-request baseUrl for branch_already_chosen Use getEnvVar fallback and axios like other collection scripts; Invalid URL when baseUrl was undefined in pre-request. --- ...anch select deny branch already chosen.bru | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru index c64f45d..0503cb1 100644 --- a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru @@ -10,39 +10,40 @@ docs { } script:pre-request { - const baseUrl = bru.getVar("baseUrl"); - const grant = await bru.sendRequest({ - method: "POST", - url: `${baseUrl}/game/players/dev-local-1/skill-progression`, - headers: { "Content-Type": "application/json" }, - data: { + const axios = require("axios"); + const baseUrl = bru.getEnvVar("baseUrl") || bru.getVar("baseUrl"); + const playerId = bru.getEnvVar("playerId") || bru.getVar("playerId"); + const jsonHeaders = { headers: { "Content-Type": "application/json" } }; + const grant = await axios.post( + `${baseUrl}/game/players/${playerId}/skill-progression`, + { schemaVersion: 1, skillId: "salvage", amount: 100, sourceKind: "activity", }, - }); + jsonHeaders, + ); if (grant.status !== 200) { throw new Error(`salvage grant failed: ${grant.status}`); } - const firstPick = await bru.sendRequest({ - method: "POST", - url: `${baseUrl}/game/players/dev-local-1/perk-state`, - headers: { "Content-Type": "application/json" }, - data: { + const firstPick = await axios.post( + `${baseUrl}/game/players/${playerId}/perk-state`, + { schemaVersion: 1, skillId: "salvage", tierIndex: 1, branchId: "scrap_efficiency", }, - }); + jsonHeaders, + ); if (firstPick.status !== 200 || firstPick.data?.selected !== true) { throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`); } } post { - url: {{baseUrl}}/game/players/dev-local-1/perk-state + url: {{baseUrl}}/game/players/{{playerId}}/perk-state body: json auth: none } From f4c3fe86e429e2076b459b6cee6dd838aafbe283 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:23:42 -0400 Subject: [PATCH 06/14] NEO-48: skip level_too_low Bruno when dev player already progressed Pre-request checks salvage level and tier-1 pick; bru.skipRequest avoids false failure after branch_already_chosen or skill-progression grants. --- .../Post branch select deny level too low.bru | 41 +++++++++++++++---- .../neon-sprawl-server/perk-state/folder.bru | 2 +- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru index 7a80656..1ce851f 100644 --- a/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru @@ -6,11 +6,38 @@ meta { docs { NEO-48: salvage tier 1 requires skill level 2. On a fresh server (no prior salvage XP), expects selected false + level_too_low. - Restart server or use a fresh dev DB to hit this deny; skip if salvage already leveled from other Bruno runs. + Skipped automatically when dev-local-1 already has salvage level 2+ or a tier-1 branch pick (shared state after other Bruno requests). +} + +script:pre-request { + const axios = require("axios"); + const baseUrl = bru.getEnvVar("baseUrl") || bru.getVar("baseUrl"); + const playerId = bru.getEnvVar("playerId") || bru.getVar("playerId"); + let salvageLevel = 1; + let hasTier1Pick = false; + try { + const progression = await axios.get( + `${baseUrl}/game/players/${playerId}/skill-progression`, + ); + const salvage = progression.data?.skills?.find((s) => s.id === "salvage"); + salvageLevel = salvage?.level ?? 1; + const perkState = await axios.get(`${baseUrl}/game/players/${playerId}/perk-state`); + const salvageTrack = perkState.data?.branchPicks?.find((t) => t.skillId === "salvage"); + hasTier1Pick = Boolean( + salvageTrack?.picks?.some((p) => p.tierIndex === 1), + ); + } catch (_) { + /* server down or player missing — let the request surface the error */ + } + if (salvageLevel >= 2 || hasTier1Pick) { + bru.skipRequest( + "dev-local-1 already progressed (salvage level 2+ or tier-1 pick); restart server for strict level_too_low smoke", + ); + } } post { - url: {{baseUrl}}/game/players/dev-local-1/perk-state + url: {{baseUrl}}/game/players/{{playerId}}/perk-state body: json auth: none } @@ -29,11 +56,9 @@ tests { expect(res.getStatus()).to.equal(200); const body = res.getBody(); expect(body.schemaVersion).to.equal(1); - if (body.selected === false && body.reasonCode === "level_too_low") { - expect(body.unlockedEvents).to.be.an("array").that.is.empty; - expect(body.perkState.branchPicks).to.be.an("array"); - return; - } - expect(body.selected).to.equal(true, "salvage already at level 2+ — restart server for deny smoke"); + expect(body.selected).to.equal(false); + expect(body.reasonCode).to.equal("level_too_low"); + expect(body.unlockedEvents).to.be.an("array").that.is.empty; + expect(body.perkState.branchPicks).to.be.an("array"); }); } diff --git a/bruno/neon-sprawl-server/perk-state/folder.bru b/bruno/neon-sprawl-server/perk-state/folder.bru index 3a755c2..9d97eb3 100644 --- a/bruno/neon-sprawl-server/perk-state/folder.bru +++ b/bruno/neon-sprawl-server/perk-state/folder.bru @@ -4,6 +4,6 @@ meta { docs { NEO-48 perk-state HTTP smoke. Run folder in seq order for full coverage. - Post branch select deny level too low: passes if salvage is still level 1; if dev-local-1 already has salvage XP from other runs, restart the server (or reset DB in Postgres mode) before expecting level_too_low. + Post branch select deny level too low: skipped via pre-request when dev-local-1 already has salvage level 2+ or a tier-1 branch pick (normal after other requests in a full collection run). Post branch select deny branch already chosen: self-contained (pre-request grant + first pick); safe to re-run. } From b72201b1f0fabe3e1477106edf3255ad1072cf92 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:26:21 -0400 Subject: [PATCH 07/14] NEO-48: Bruno fixture players for deterministic perk deny tests Seed neo48-perk-fresh and neo48-perk-branch via Game:FixturePlayerIds; level_too_low uses isolated fresh player, branch_already_chosen sets up neo48-perk-branch in pre-request instead of skipping on shared state. --- .../neon-sprawl-server/environments/Local.bru | 2 + ...anch select deny branch already chosen.bru | 63 +++++++++++-------- .../Post branch select deny level too low.bru | 34 +--------- .../neon-sprawl-server/perk-state/folder.bru | 5 +- docs/manual-qa/NEO-48.md | 15 ++++- .../Game/Mastery/PerkStateApiTests.cs | 21 +++++++ .../Mastery/InMemoryPlayerPerkStateStore.cs | 7 ++- .../Game/PositionState/GamePlayerSeed.cs | 30 +++++++++ .../Game/PositionState/GamePositionOptions.cs | 3 + .../InMemoryPositionStateStore.cs | 10 ++- .../PostgresPositionBootstrap.cs | 31 ++++----- .../InMemoryPlayerSkillProgressionStore.cs | 7 ++- server/NeonSprawl.Server/appsettings.json | 4 ++ server/README.md | 2 + 14 files changed, 151 insertions(+), 83 deletions(-) create mode 100644 server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs diff --git a/bruno/neon-sprawl-server/environments/Local.bru b/bruno/neon-sprawl-server/environments/Local.bru index bd622e2..aef1d6b 100644 --- a/bruno/neon-sprawl-server/environments/Local.bru +++ b/bruno/neon-sprawl-server/environments/Local.bru @@ -1,4 +1,6 @@ vars { baseUrl: http://localhost:5253 playerId: dev-local-1 + neo48PerkFreshPlayerId: neo48-perk-fresh + neo48PerkBranchPlayerId: neo48-perk-branch } diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru index 0503cb1..42f4a6a 100644 --- a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru @@ -5,45 +5,58 @@ meta { } docs { - NEO-48: self-contained deny — pre-request grants salvage to level 2 and picks scrap_efficiency; - main request conflicts with bulk_haul on the same tier. Expects selected false + branch_already_chosen. + NEO-48: self-contained deny on fixture player neo48-perk-branch. Pre-request reaches level 2 and picks scrap_efficiency when needed; main request conflicts with bulk_haul. } script:pre-request { const axios = require("axios"); const baseUrl = bru.getEnvVar("baseUrl") || bru.getVar("baseUrl"); - const playerId = bru.getEnvVar("playerId") || bru.getVar("playerId"); + const playerId = bru.getEnvVar("neo48PerkBranchPlayerId") || bru.getVar("neo48PerkBranchPlayerId"); const jsonHeaders = { headers: { "Content-Type": "application/json" } }; - const grant = await axios.post( + const progression = await axios.get( `${baseUrl}/game/players/${playerId}/skill-progression`, - { - schemaVersion: 1, - skillId: "salvage", - amount: 100, - sourceKind: "activity", - }, - jsonHeaders, ); - if (grant.status !== 200) { - throw new Error(`salvage grant failed: ${grant.status}`); + const salvage = progression.data?.skills?.find((s) => s.id === "salvage"); + const salvageLevel = salvage?.level ?? 1; + if (salvageLevel < 2) { + const grant = await axios.post( + `${baseUrl}/game/players/${playerId}/skill-progression`, + { + schemaVersion: 1, + skillId: "salvage", + amount: 100, + sourceKind: "activity", + }, + jsonHeaders, + ); + if (grant.status !== 200) { + throw new Error(`salvage grant failed: ${grant.status}`); + } } - const firstPick = await axios.post( - `${baseUrl}/game/players/${playerId}/perk-state`, - { - schemaVersion: 1, - skillId: "salvage", - tierIndex: 1, - branchId: "scrap_efficiency", - }, - jsonHeaders, + const perkState = await axios.get(`${baseUrl}/game/players/${playerId}/perk-state`); + const salvageTrack = perkState.data?.branchPicks?.find((t) => t.skillId === "salvage"); + const hasTier1Pick = Boolean( + salvageTrack?.picks?.some((p) => p.tierIndex === 1), ); - if (firstPick.status !== 200 || firstPick.data?.selected !== true) { - throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`); + if (!hasTier1Pick) { + const firstPick = await axios.post( + `${baseUrl}/game/players/${playerId}/perk-state`, + { + schemaVersion: 1, + skillId: "salvage", + tierIndex: 1, + branchId: "scrap_efficiency", + }, + jsonHeaders, + ); + if (firstPick.status !== 200 || firstPick.data?.selected !== true) { + throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`); + } } } post { - url: {{baseUrl}}/game/players/{{playerId}}/perk-state + url: {{baseUrl}}/game/players/{{neo48PerkBranchPlayerId}}/perk-state body: json auth: none } diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru index 1ce851f..553fe79 100644 --- a/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru @@ -5,39 +5,11 @@ meta { } docs { - NEO-48: salvage tier 1 requires skill level 2. On a fresh server (no prior salvage XP), expects selected false + level_too_low. - Skipped automatically when dev-local-1 already has salvage level 2+ or a tier-1 branch pick (shared state after other Bruno requests). -} - -script:pre-request { - const axios = require("axios"); - const baseUrl = bru.getEnvVar("baseUrl") || bru.getVar("baseUrl"); - const playerId = bru.getEnvVar("playerId") || bru.getVar("playerId"); - let salvageLevel = 1; - let hasTier1Pick = false; - try { - const progression = await axios.get( - `${baseUrl}/game/players/${playerId}/skill-progression`, - ); - const salvage = progression.data?.skills?.find((s) => s.id === "salvage"); - salvageLevel = salvage?.level ?? 1; - const perkState = await axios.get(`${baseUrl}/game/players/${playerId}/perk-state`); - const salvageTrack = perkState.data?.branchPicks?.find((t) => t.skillId === "salvage"); - hasTier1Pick = Boolean( - salvageTrack?.picks?.some((p) => p.tierIndex === 1), - ); - } catch (_) { - /* server down or player missing — let the request surface the error */ - } - if (salvageLevel >= 2 || hasTier1Pick) { - bru.skipRequest( - "dev-local-1 already progressed (salvage level 2+ or tier-1 pick); restart server for strict level_too_low smoke", - ); - } + NEO-48: salvage tier 1 requires skill level 2. Uses fixture player neo48-perk-fresh (seeded empty; not used by other requests). } post { - url: {{baseUrl}}/game/players/{{playerId}}/perk-state + url: {{baseUrl}}/game/players/{{neo48PerkFreshPlayerId}}/perk-state body: json auth: none } @@ -59,6 +31,6 @@ tests { expect(body.selected).to.equal(false); expect(body.reasonCode).to.equal("level_too_low"); expect(body.unlockedEvents).to.be.an("array").that.is.empty; - expect(body.perkState.branchPicks).to.be.an("array"); + expect(body.perkState.branchPicks).to.be.an("array").that.is.empty; }); } diff --git a/bruno/neon-sprawl-server/perk-state/folder.bru b/bruno/neon-sprawl-server/perk-state/folder.bru index 9d97eb3..a5cd89e 100644 --- a/bruno/neon-sprawl-server/perk-state/folder.bru +++ b/bruno/neon-sprawl-server/perk-state/folder.bru @@ -3,7 +3,6 @@ meta { } docs { - NEO-48 perk-state HTTP smoke. Run folder in seq order for full coverage. - Post branch select deny level too low: skipped via pre-request when dev-local-1 already has salvage level 2+ or a tier-1 branch pick (normal after other requests in a full collection run). - Post branch select deny branch already chosen: self-contained (pre-request grant + first pick); safe to re-run. + NEO-48 perk-state HTTP smoke. Fixture players neo48-perk-fresh and neo48-perk-branch are seeded in server appsettings (Game:FixturePlayerIds) so deny tests do not share dev-local-1 state. + level_too_low uses neo48-perk-fresh (always level 1 at seed). branch_already_chosen uses neo48-perk-branch (pre-request grants/picks when needed). } diff --git a/docs/manual-qa/NEO-48.md b/docs/manual-qa/NEO-48.md index a6e4506..1140680 100644 --- a/docs/manual-qa/NEO-48.md +++ b/docs/manual-qa/NEO-48.md @@ -36,10 +36,19 @@ curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \ - [ ] `selected: true`; `perkState.branchPicks` contains salvage tier 1 `scrap_efficiency`; `unlockedEvents` empty at tier 1. - [ ] Repeat **GET** — same pick visible. -## POST deny +## POST deny (fixture players) -- [ ] Second pick same tier different branch → `selected: false`, `reasonCode` **`branch_already_chosen`**. -- [ ] On fresh server without XP grant → `level_too_low`. +Server seeds **`neo48-perk-fresh`** and **`neo48-perk-branch`** (`Game:FixturePlayerIds` in `appsettings.json`) so Bruno deny tests do not share `dev-local-1` history. + +```bash +# level_too_low (neo48-perk-fresh — no grant needed) +curl -sS -X POST "http://localhost:5253/game/players/neo48-perk-fresh/perk-state" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","tierIndex":1,"branchId":"scrap_efficiency"}' | jq . +``` + +- [ ] `neo48-perk-fresh` → `level_too_low` without prior XP grant. +- [ ] `neo48-perk-branch`: grant 100 salvage XP, pick `scrap_efficiency`, then POST `bulk_haul` → **`branch_already_chosen`**. ## Bruno diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs index d83c843..b35cbbf 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs @@ -116,6 +116,27 @@ public sealed class PerkStateApiTests Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } + [Fact] + public async Task PostPerkState_ShouldDenyLevelTooLow_OnFixturePlayer_WithoutXpGrant() + { + // Arrange — neo48-perk-fresh is seeded with empty progression (appsettings FixturePlayerIds) + const string freshPlayer = "neo48-perk-fresh"; + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{freshPlayer}/perk-state", + SelectBranch("salvage", 1, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.LevelTooLow, envelope.ReasonCode); + } + [Fact] public async Task PostPerkState_ShouldDenyLevelTooLow_WhenSalvageBelowTierGate() { diff --git a/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs b/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs index d47a4de..a4dcb12 100644 --- a/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs +++ b/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs @@ -13,9 +13,12 @@ public sealed class InMemoryPlayerPerkStateStore(IOptions o private static ConcurrentDictionary CreateInitialMap(GamePositionOptions o) { - var id = NormalizePlayerId(o.DevPlayerId); var map = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - map[id] = new PlayerPerkState(); + foreach (var id in GamePlayerSeed.GetSeedPlayerIds(o)) + { + map[NormalizePlayerId(id)] = new PlayerPerkState(); + } + return map; } diff --git a/server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs b/server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs new file mode 100644 index 0000000..43ffbe4 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs @@ -0,0 +1,30 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// Configured player ids seeded at host start (dev player + Bruno/manual QA fixtures). +public static class GamePlayerSeed +{ + /// Distinct normalized player ids from and . + public static IReadOnlyList GetSeedPlayerIds(GamePositionOptions options) + { + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + var list = new List(); + Add(options.DevPlayerId, seen, list); + foreach (var id in options.FixturePlayerIds) + { + Add(id, seen, list); + } + + return list; + } + + private static void Add(string? raw, HashSet seen, List list) + { + var t = raw?.Trim(); + if (string.IsNullOrEmpty(t) || !seen.Add(t)) + { + return; + } + + list.Add(t); + } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs index aacc85b..7124fae 100644 --- a/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs +++ b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs @@ -10,6 +10,9 @@ public sealed class GamePositionOptions /// Logical player id seeded in the in-memory store (URL-safe string). public string DevPlayerId { get; set; } = "dev-local-1"; + /// Additional player ids seeded with default position and empty progression/perk state (Bruno/manual QA). + public List FixturePlayerIds { get; set; } = []; + /// World position for the dev player at process start. public DefaultPositionOptions DefaultPosition { get; set; } = new(); diff --git a/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs index 62edcbf..cfd558e 100644 --- a/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs +++ b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs @@ -10,15 +10,19 @@ public sealed class InMemoryPositionStateStore(IOptions opt private static ConcurrentDictionary CreateInitialMap(GamePositionOptions o) { - var id = o.DevPlayerId.Trim(); - if (id.Length == 0) + var ids = GamePlayerSeed.GetSeedPlayerIds(o); + if (ids.Count == 0) { throw new InvalidOperationException("Game:DevPlayerId must be non-empty."); } var map = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); var p = o.DefaultPosition ?? new DefaultPositionOptions(); - map[id] = new PositionSnapshot(p.X, p.Y, p.Z, 0); + foreach (var id in ids) + { + map[id] = new PositionSnapshot(p.X, p.Y, p.Z, 0); + } + return map; } diff --git a/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs b/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs index 76796b1..91e4b8a 100644 --- a/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs +++ b/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs @@ -45,7 +45,7 @@ public static class PostgresPositionBootstrap } } - /// Idempotent insert for at default position. + /// Idempotent insert for configured seed players at default position. public static void SeedDevPlayer(Npgsql.NpgsqlDataSource dataSource, GamePositionOptions options) { using var conn = dataSource.OpenConnection(); @@ -55,20 +55,23 @@ public static class PostgresPositionBootstrap /// Idempotent insert using an existing open connection (e.g. test harness after TRUNCATE). public static void SeedDevPlayer(Npgsql.NpgsqlConnection connection, GamePositionOptions options) { - var devNorm = NormalizePlayerId(options.DevPlayerId); var p = options.DefaultPosition ?? new DefaultPositionOptions(); - using var cmd = new Npgsql.NpgsqlCommand( - """ - INSERT INTO player_position (player_id, pos_x, pos_y, pos_z, sequence) - VALUES (@pid, @x, @y, @z, 0) - ON CONFLICT (player_id) DO NOTHING; - """, - connection); - cmd.Parameters.AddWithValue("pid", devNorm); - cmd.Parameters.AddWithValue("x", p.X); - cmd.Parameters.AddWithValue("y", p.Y); - cmd.Parameters.AddWithValue("z", p.Z); - cmd.ExecuteNonQuery(); + foreach (var id in GamePlayerSeed.GetSeedPlayerIds(options)) + { + var norm = NormalizePlayerId(id); + using var cmd = new Npgsql.NpgsqlCommand( + """ + INSERT INTO player_position (player_id, pos_x, pos_y, pos_z, sequence) + VALUES (@pid, @x, @y, @z, 0) + ON CONFLICT (player_id) DO NOTHING; + """, + connection); + cmd.Parameters.AddWithValue("pid", norm); + cmd.Parameters.AddWithValue("x", p.X); + cmd.Parameters.AddWithValue("y", p.Y); + cmd.Parameters.AddWithValue("z", p.Z); + cmd.ExecuteNonQuery(); + } } private static string NormalizePlayerId(string? playerId) diff --git a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs index cadd39f..8bdf271 100644 --- a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs @@ -14,9 +14,12 @@ public sealed class InMemoryPlayerSkillProgressionStore(IOptions> CreateInitialMap(GamePositionOptions o) { - var id = NormalizePlayerId(o.DevPlayerId); var map = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); - map[id] = new ConcurrentDictionary(StringComparer.Ordinal); + foreach (var id in GamePlayerSeed.GetSeedPlayerIds(o)) + { + map[NormalizePlayerId(id)] = new ConcurrentDictionary(StringComparer.Ordinal); + } + return map; } diff --git a/server/NeonSprawl.Server/appsettings.json b/server/NeonSprawl.Server/appsettings.json index 394a263..a207a92 100644 --- a/server/NeonSprawl.Server/appsettings.json +++ b/server/NeonSprawl.Server/appsettings.json @@ -12,6 +12,10 @@ }, "Game": { "DevPlayerId": "dev-local-1", + "FixturePlayerIds": [ + "neo48-perk-fresh", + "neo48-perk-branch" + ], "DefaultPosition": { "X": -5, "Y": 0.9, diff --git a/server/README.md b/server/README.md index ab0fba2..886639f 100644 --- a/server/README.md +++ b/server/README.md @@ -64,6 +64,8 @@ On success, **Information** logs include the resolved mastery directory, track c **Structured responses (HTTP 200):** **`selected`** (`true`/`false`), optional **`reasonCode`** on deny, authoritative **`perkState`** (same shape as GET), and **`unlockedEvents`** on success (empty on deny). Stable deny codes (see `PerkUnlockReasonCodes`): **`unknown_track`**, **`unknown_tier`**, **`unknown_branch`**, **`level_too_low`**, **`branch_already_chosen`**, **`tier_branch_not_selectable`**, **`player_not_found`**. On **`selected: true`**, **`unlockedEvents`** lists perks newly unlocked on this request (`perkId`, `skillId`, `tierIndex`, `branchId`, **`source`**: **`branch_pick`** | **`level_up`**). Tier-1 prototype salvage picks may emit zero events; path-auto tier-2 unlocks at level 4+ may emit **`level_up`** events on the same POST when level gates are already met. +**Bruno fixture players (NEO-48):** `Game:FixturePlayerIds` in `appsettings.json` seeds additional player ids (`neo48-perk-fresh`, `neo48-perk-branch`) with default position and empty progression/perk state so deny smokes do not depend on `dev-local-1` history. + Plan: [NEO-48 implementation plan](../../docs/plans/NEO-48-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-48.md`](../../docs/manual-qa/NEO-48.md); Bruno: `bruno/neon-sprawl-server/perk-state/`. ```bash From ffdbed26f27525bbafcbcc84729c0266fb05bc41 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:31:26 -0400 Subject: [PATCH 08/14] NEO-48: dev mastery-fixture API for Bruno state setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace fixture player ids with POST …/__dev/mastery-fixture (resetPerkState, absolute skillXp). Bruno deny tests set state in pre-request on dev-local-1. Gated to Development/Testing or Game:EnableMasteryFixtureApi. --- .../neon-sprawl-server/environments/Local.bru | 2 - ...anch select deny branch already chosen.bru | 63 ++++------- .../Post branch select deny level too low.bru | 23 +++- .../neon-sprawl-server/perk-state/folder.bru | 3 +- docs/manual-qa/NEO-48.md | 17 ++- docs/plans/NEO-48-implementation-plan.md | 1 + .../Game/Mastery/MasteryFixtureApiTests.cs | 103 ++++++++++++++++++ .../Game/Mastery/PerkStateApiTests.cs | 21 ---- .../InMemoryWebApplicationFactory.cs | 1 + .../Game/Mastery/IPlayerPerkStateStore.cs | 3 + .../Mastery/InMemoryPlayerPerkStateStore.cs | 29 ++++- .../Game/Mastery/MasteryFixtureApi.cs | 41 +++++++ .../Game/Mastery/MasteryFixtureDtos.cs | 31 ++++++ .../Game/Mastery/MasteryFixtureOperations.cs | 39 +++++++ .../Mastery/PostgresPlayerPerkStateStore.cs | 40 +++++++ .../Game/PositionState/GamePlayerSeed.cs | 30 ----- .../Game/PositionState/GamePositionOptions.cs | 4 +- .../InMemoryPositionStateStore.cs | 10 +- .../PostgresPositionBootstrap.cs | 29 +++-- .../Skills/IPlayerSkillProgressionStore.cs | 3 + .../InMemoryPlayerSkillProgressionStore.cs | 34 +++++- .../PostgresPlayerSkillProgressionStore.cs | 58 ++++++++++ server/NeonSprawl.Server/Program.cs | 7 ++ .../appsettings.Development.json | 3 + server/NeonSprawl.Server/appsettings.json | 5 +- server/README.md | 2 +- 26 files changed, 460 insertions(+), 142 deletions(-) create mode 100644 server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs create mode 100644 server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs create mode 100644 server/NeonSprawl.Server/Game/Mastery/MasteryFixtureDtos.cs create mode 100644 server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs delete mode 100644 server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs diff --git a/bruno/neon-sprawl-server/environments/Local.bru b/bruno/neon-sprawl-server/environments/Local.bru index aef1d6b..bd622e2 100644 --- a/bruno/neon-sprawl-server/environments/Local.bru +++ b/bruno/neon-sprawl-server/environments/Local.bru @@ -1,6 +1,4 @@ vars { baseUrl: http://localhost:5253 playerId: dev-local-1 - neo48PerkFreshPlayerId: neo48-perk-fresh - neo48PerkBranchPlayerId: neo48-perk-branch } diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru index 42f4a6a..2398c8b 100644 --- a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru @@ -5,58 +5,43 @@ meta { } docs { - NEO-48: self-contained deny on fixture player neo48-perk-branch. Pre-request reaches level 2 and picks scrap_efficiency when needed; main request conflicts with bulk_haul. + NEO-48: pre-request sets salvage level 2 + tier-1 scrap_efficiency pick via mastery-fixture + perk-state; main request conflicts with bulk_haul. } script:pre-request { const axios = require("axios"); const baseUrl = bru.getEnvVar("baseUrl") || bru.getVar("baseUrl"); - const playerId = bru.getEnvVar("neo48PerkBranchPlayerId") || bru.getVar("neo48PerkBranchPlayerId"); + const playerId = bru.getEnvVar("playerId") || bru.getVar("playerId"); const jsonHeaders = { headers: { "Content-Type": "application/json" } }; - const progression = await axios.get( - `${baseUrl}/game/players/${playerId}/skill-progression`, + const reset = await axios.post( + `${baseUrl}/game/players/${playerId}/__dev/mastery-fixture`, + { + schemaVersion: 1, + resetPerkState: true, + skillXp: { salvage: 100 }, + }, + jsonHeaders, ); - const salvage = progression.data?.skills?.find((s) => s.id === "salvage"); - const salvageLevel = salvage?.level ?? 1; - if (salvageLevel < 2) { - const grant = await axios.post( - `${baseUrl}/game/players/${playerId}/skill-progression`, - { - schemaVersion: 1, - skillId: "salvage", - amount: 100, - sourceKind: "activity", - }, - jsonHeaders, - ); - if (grant.status !== 200) { - throw new Error(`salvage grant failed: ${grant.status}`); - } + if (reset.status !== 200 || reset.data?.applied !== true) { + throw new Error(`mastery fixture reset failed: ${reset.status} ${JSON.stringify(reset.data)}`); } - const perkState = await axios.get(`${baseUrl}/game/players/${playerId}/perk-state`); - const salvageTrack = perkState.data?.branchPicks?.find((t) => t.skillId === "salvage"); - const hasTier1Pick = Boolean( - salvageTrack?.picks?.some((p) => p.tierIndex === 1), + const firstPick = await axios.post( + `${baseUrl}/game/players/${playerId}/perk-state`, + { + schemaVersion: 1, + skillId: "salvage", + tierIndex: 1, + branchId: "scrap_efficiency", + }, + jsonHeaders, ); - if (!hasTier1Pick) { - const firstPick = await axios.post( - `${baseUrl}/game/players/${playerId}/perk-state`, - { - schemaVersion: 1, - skillId: "salvage", - tierIndex: 1, - branchId: "scrap_efficiency", - }, - jsonHeaders, - ); - if (firstPick.status !== 200 || firstPick.data?.selected !== true) { - throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`); - } + if (firstPick.status !== 200 || firstPick.data?.selected !== true) { + throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`); } } post { - url: {{baseUrl}}/game/players/{{neo48PerkBranchPlayerId}}/perk-state + url: {{baseUrl}}/game/players/{{playerId}}/perk-state body: json auth: none } diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru index 553fe79..fabb82a 100644 --- a/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny level too low.bru @@ -5,11 +5,30 @@ meta { } docs { - NEO-48: salvage tier 1 requires skill level 2. Uses fixture player neo48-perk-fresh (seeded empty; not used by other requests). + NEO-48: salvage tier 1 requires skill level 2. Pre-request resets dev-local-1 via POST …/__dev/mastery-fixture. +} + +script:pre-request { + const axios = require("axios"); + const baseUrl = bru.getEnvVar("baseUrl") || bru.getVar("baseUrl"); + const playerId = bru.getEnvVar("playerId") || bru.getVar("playerId"); + const jsonHeaders = { headers: { "Content-Type": "application/json" } }; + const reset = await axios.post( + `${baseUrl}/game/players/${playerId}/__dev/mastery-fixture`, + { + schemaVersion: 1, + resetPerkState: true, + skillXp: { salvage: 0 }, + }, + jsonHeaders, + ); + if (reset.status !== 200 || reset.data?.applied !== true) { + throw new Error(`mastery fixture reset failed: ${reset.status} ${JSON.stringify(reset.data)}`); + } } post { - url: {{baseUrl}}/game/players/{{neo48PerkFreshPlayerId}}/perk-state + url: {{baseUrl}}/game/players/{{playerId}}/perk-state body: json auth: none } diff --git a/bruno/neon-sprawl-server/perk-state/folder.bru b/bruno/neon-sprawl-server/perk-state/folder.bru index a5cd89e..d8c5be7 100644 --- a/bruno/neon-sprawl-server/perk-state/folder.bru +++ b/bruno/neon-sprawl-server/perk-state/folder.bru @@ -3,6 +3,5 @@ meta { } docs { - NEO-48 perk-state HTTP smoke. Fixture players neo48-perk-fresh and neo48-perk-branch are seeded in server appsettings (Game:FixturePlayerIds) so deny tests do not share dev-local-1 state. - level_too_low uses neo48-perk-fresh (always level 1 at seed). branch_already_chosen uses neo48-perk-branch (pre-request grants/picks when needed). + NEO-48 perk-state HTTP smoke. Deny requests call POST …/__dev/mastery-fixture in pre-request to set salvage XP and clear perk state (Development / Game:EnableMasteryFixtureApi). } diff --git a/docs/manual-qa/NEO-48.md b/docs/manual-qa/NEO-48.md index 1140680..f5396a4 100644 --- a/docs/manual-qa/NEO-48.md +++ b/docs/manual-qa/NEO-48.md @@ -36,19 +36,24 @@ curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \ - [ ] `selected: true`; `perkState.branchPicks` contains salvage tier 1 `scrap_efficiency`; `unlockedEvents` empty at tier 1. - [ ] Repeat **GET** — same pick visible. -## POST deny (fixture players) +## POST deny (mastery fixture reset) -Server seeds **`neo48-perk-fresh`** and **`neo48-perk-branch`** (`Game:FixturePlayerIds` in `appsettings.json`) so Bruno deny tests do not share `dev-local-1` history. +Requires **`Game:EnableMasteryFixtureApi`** (on in Development) or Development environment. ```bash -# level_too_low (neo48-perk-fresh — no grant needed) -curl -sS -X POST "http://localhost:5253/game/players/neo48-perk-fresh/perk-state" \ +# Reset dev-local-1 to salvage level 1, no perks +curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/__dev/mastery-fixture" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"resetPerkState":true,"skillXp":{"salvage":0}}' | jq . + +curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \ -H "Content-Type: application/json" \ -d '{"schemaVersion":1,"skillId":"salvage","tierIndex":1,"branchId":"scrap_efficiency"}' | jq . +# expect level_too_low ``` -- [ ] `neo48-perk-fresh` → `level_too_low` without prior XP grant. -- [ ] `neo48-perk-branch`: grant 100 salvage XP, pick `scrap_efficiency`, then POST `bulk_haul` → **`branch_already_chosen`**. +- [ ] Fixture reset + branch POST → `level_too_low`. +- [ ] Fixture reset with `skillXp.salvage: 100`, pick `scrap_efficiency`, POST `bulk_haul` → **`branch_already_chosen`**. ## Bruno diff --git a/docs/plans/NEO-48-implementation-plan.md b/docs/plans/NEO-48-implementation-plan.md index 49b5749..2467fde 100644 --- a/docs/plans/NEO-48-implementation-plan.md +++ b/docs/plans/NEO-48-implementation-plan.md @@ -20,6 +20,7 @@ | **GET JSON shape** | Nested maps vs arrays? | **Agent (repo pattern):** serialize branch picks as a **`branchPicks`** array of `{ skillId, picks: [{ tierIndex, branchId }] }` (ordinal sort by `skillId`; picks sorted by `tierIndex`). **`unlockedPerkIds`** as a sorted string array. Internal `PerkStateSnapshot` mapping only — no rename of persisted fields. | | **Deny envelope** | Field names for success/deny? | **Agent (NEO-38 mirror):** HTTP **200** with **`selected`** (`true`/`false`), optional **`reasonCode`** (stable codes from `PerkUnlockReasonCodes`), and authoritative **`perkState`** on both paths. **400** for missing/wrong `schemaVersion`; **404** when player absent from position state (same gate as skill-progression GET). | | **Telemetry at HTTP** | Duplicate NEO-49 hook comments on POST? | **No** — [NEO-49 plan](NEO-49-implementation-plan.md): engine `TryUnlockPerks` is the authoritative hook; HTTP calls `TrySelectBranch` and surfaces returned `Events` only in JSON. | +| **Bruno deny setup** | Separate fixture players vs reset API? | **User (PR follow-up):** `POST …/__dev/mastery-fixture` sets `skillXp` + `resetPerkState`; Bruno pre-request calls it on `dev-local-1` before deny smokes (not separate seeded players). | ## Goal, scope, and out-of-scope diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs new file mode 100644 index 0000000..ecd2a16 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs @@ -0,0 +1,103 @@ +using System.Net; +using System.Net.Http.Json; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using NeonSprawl.Server.Game.Mastery; +using NeonSprawl.Server.Game.Skills; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Mastery; + +public sealed class MasteryFixtureApiTests +{ + private const string DevPlayer = "dev-local-1"; + + [Fact] + public async Task PostMasteryFixture_ShouldReturnNotFound_WhenRouteNotRegistered() + { + // Arrange — Production + fixture flag off => endpoint not mapped + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.WithWebHostBuilder(b => + { + b.UseEnvironment("Production"); + b.UseSetting("Game:EnableMasteryFixtureApi", "false"); + }).CreateClient(); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/__dev/mastery-fixture", + new MasteryFixtureRequest + { + SchemaVersion = MasteryFixtureRequest.CurrentSchemaVersion, + ResetPerkState = true, + SkillXp = new Dictionary { ["salvage"] = 0 }, + }); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task PostMasteryFixture_ShouldResetSalvageXpAndPerkState_ThenLevelTooLowDenies() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + new SkillProgressionGrantRequest + { + SchemaVersion = SkillProgressionGrantRequest.CurrentSchemaVersion, + SkillId = "salvage", + Amount = 100, + SourceKind = "activity", + })).StatusCode); + Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + new PerkBranchSelectRequest + { + SchemaVersion = PerkBranchSelectRequest.CurrentSchemaVersion, + SkillId = "salvage", + TierIndex = 1, + BranchId = "scrap_efficiency", + })).StatusCode); + + // Act + var reset = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/__dev/mastery-fixture", + new MasteryFixtureRequest + { + SchemaVersion = MasteryFixtureRequest.CurrentSchemaVersion, + ResetPerkState = true, + SkillXp = new Dictionary { ["salvage"] = 0 }, + }); + var deny = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + new PerkBranchSelectRequest + { + SchemaVersion = PerkBranchSelectRequest.CurrentSchemaVersion, + SkillId = "salvage", + TierIndex = 1, + BranchId = "scrap_efficiency", + }); + + // Assert + Assert.Equal(HttpStatusCode.OK, reset.StatusCode); + var resetBody = await reset.Content.ReadFromJsonAsync(); + Assert.NotNull(resetBody); + Assert.True(resetBody!.Applied); + var progression = await client.GetFromJsonAsync( + $"/game/players/{DevPlayer}/skill-progression"); + var salvage = Assert.Single(progression!.Skills!, static s => s.Id == "salvage"); + Assert.Equal(0, salvage.Xp); + Assert.Equal(1, salvage.Level); + var perkGet = await client.GetFromJsonAsync( + $"/game/players/{DevPlayer}/perk-state"); + Assert.Empty(perkGet!.BranchPicks); + Assert.Equal(HttpStatusCode.OK, deny.StatusCode); + var envelope = await deny.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.LevelTooLow, envelope.ReasonCode); + } +} diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs index b35cbbf..d83c843 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs @@ -116,27 +116,6 @@ public sealed class PerkStateApiTests Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } - [Fact] - public async Task PostPerkState_ShouldDenyLevelTooLow_OnFixturePlayer_WithoutXpGrant() - { - // Arrange — neo48-perk-fresh is seeded with empty progression (appsettings FixturePlayerIds) - const string freshPlayer = "neo48-perk-fresh"; - await using var factory = new InMemoryWebApplicationFactory(); - var client = factory.CreateClient(); - - // Act - var response = await client.PostAsJsonAsync( - $"/game/players/{freshPlayer}/perk-state", - SelectBranch("salvage", 1, "scrap_efficiency")); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - var envelope = await response.Content.ReadFromJsonAsync(); - Assert.NotNull(envelope); - Assert.False(envelope!.Selected); - Assert.Equal(PerkUnlockReasonCodes.LevelTooLow, envelope.ReasonCode); - } - [Fact] public async Task PostPerkState_ShouldDenyLevelTooLow_WhenSalvageBelowTierGate() { diff --git a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs index a768863..4eb9aa1 100644 --- a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs +++ b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs @@ -32,6 +32,7 @@ public sealed class InMemoryWebApplicationFactory : WebApplicationFactory { diff --git a/server/NeonSprawl.Server/Game/Mastery/IPlayerPerkStateStore.cs b/server/NeonSprawl.Server/Game/Mastery/IPlayerPerkStateStore.cs index 758c273..5ddc70e 100644 --- a/server/NeonSprawl.Server/Game/Mastery/IPlayerPerkStateStore.cs +++ b/server/NeonSprawl.Server/Game/Mastery/IPlayerPerkStateStore.cs @@ -16,4 +16,7 @@ public interface IPlayerPerkStateStore /// Removes a branch pick (engine rollback when perk unlock fails after pick). bool TryRemoveBranchPick(string playerId, string skillId, int tierIndex); + + /// Clears all branch picks and unlocked perks for the player (NEO-48 dev fixture API). + bool TryResetPerkState(string playerId); } diff --git a/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs b/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs index a4dcb12..8e904fa 100644 --- a/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs +++ b/server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs @@ -13,12 +13,9 @@ public sealed class InMemoryPlayerPerkStateStore(IOptions o private static ConcurrentDictionary CreateInitialMap(GamePositionOptions o) { + var id = NormalizePlayerId(o.DevPlayerId); var map = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); - foreach (var id in GamePlayerSeed.GetSeedPlayerIds(o)) - { - map[NormalizePlayerId(id)] = new PlayerPerkState(); - } - + map[id] = new PlayerPerkState(); return map; } @@ -110,6 +107,22 @@ public sealed class InMemoryPlayerPerkStateStore(IOptions o } } + /// + public bool TryResetPerkState(string playerId) + { + var key = NormalizePlayerId(playerId); + if (key.Length == 0 || !byPlayer.TryGetValue(key, out var inner)) + { + return false; + } + + lock (playerLocks.GetOrAdd(key, _ => new object())) + { + inner.Reset(); + return true; + } + } + private static string NormalizePlayerId(string? playerId) { var t = playerId?.Trim(); @@ -146,6 +159,12 @@ public sealed class InMemoryPlayerPerkStateStore(IOptions o public bool RemoveBranchPick(string skillId, int tierIndex) => branchPicks.TryGetValue(skillId, out var tiers) && tiers.Remove(tierIndex); + public void Reset() + { + branchPicks.Clear(); + unlockedPerks.Clear(); + } + public PerkStateSnapshot ToSnapshot() { var bySkill = new Dictionary>(StringComparer.Ordinal); diff --git a/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs new file mode 100644 index 0000000..5a06d1e --- /dev/null +++ b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs @@ -0,0 +1,41 @@ +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Skills; + +namespace NeonSprawl.Server.Game.Mastery; + +/// Dev-only route to reset/set mastery fixture state for Bruno and manual QA (NEO-48). +public static class MasteryFixtureApi +{ + public static WebApplication MapMasteryFixtureApi(this WebApplication app) + { + app.MapPost( + "/game/players/{id}/__dev/mastery-fixture", + (string id, MasteryFixtureRequest? body, IPositionStateStore positions, + IPlayerPerkStateStore perkStore, IPlayerSkillProgressionStore xpStore) => + { + if (body is null || body.SchemaVersion != MasteryFixtureRequest.CurrentSchemaVersion) + { + return Results.BadRequest(); + } + + var trimmedId = id.Trim(); + if (trimmedId.Length == 0 || !positions.TryGetPosition(trimmedId, out _)) + { + return Results.NotFound(); + } + + if (!MasteryFixtureOperations.TryApply(trimmedId, body, perkStore, xpStore)) + { + return Results.NotFound(); + } + + return Results.Json( + new MasteryFixtureResponse + { + Applied = true, + }); + }); + + return app; + } +} diff --git a/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureDtos.cs b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureDtos.cs new file mode 100644 index 0000000..18c9d31 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureDtos.cs @@ -0,0 +1,31 @@ +using System.Text.Json.Serialization; + +namespace NeonSprawl.Server.Game.Mastery; + +/// POST body for POST /game/players/{{id}}/__dev/mastery-fixture (NEO-48 Bruno/manual QA). +public sealed class MasteryFixtureRequest +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } + + /// When true, clears all branch picks and unlocked perks for the player. + [JsonPropertyName("resetPerkState")] + public bool ResetPerkState { get; init; } + + /// Absolute XP totals to set per skillId (omitted skills unchanged). + [JsonPropertyName("skillXp")] + public Dictionary? SkillXp { get; init; } +} + +public sealed class MasteryFixtureResponse +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + [JsonPropertyName("applied")] + public bool Applied { get; init; } +} diff --git a/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs new file mode 100644 index 0000000..3b1c0f5 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs @@ -0,0 +1,39 @@ +using NeonSprawl.Server.Game.Skills; + +namespace NeonSprawl.Server.Game.Mastery; + +/// Applies dev-only mastery fixture resets (NEO-48). +public static class MasteryFixtureOperations +{ + public static bool TryApply( + string playerId, + MasteryFixtureRequest body, + IPlayerPerkStateStore perkStore, + IPlayerSkillProgressionStore xpStore) + { + if (body.ResetPerkState && !perkStore.TryResetPerkState(playerId)) + { + return false; + } + + if (body.SkillXp is null || body.SkillXp.Count == 0) + { + return true; + } + + foreach (var (skillId, xp) in body.SkillXp) + { + if (skillId.Trim().Length == 0 || xp < 0) + { + return false; + } + + if (!xpStore.TrySetSkillXpTotal(playerId, skillId, xp)) + { + return false; + } + } + + return true; + } +} diff --git a/server/NeonSprawl.Server/Game/Mastery/PostgresPlayerPerkStateStore.cs b/server/NeonSprawl.Server/Game/Mastery/PostgresPlayerPerkStateStore.cs index 464eecc..d455cf8 100644 --- a/server/NeonSprawl.Server/Game/Mastery/PostgresPlayerPerkStateStore.cs +++ b/server/NeonSprawl.Server/Game/Mastery/PostgresPlayerPerkStateStore.cs @@ -217,6 +217,46 @@ public sealed class PostgresPlayerPerkStateStore(Npgsql.NpgsqlDataSource dataSou return cmd.ExecuteNonQuery() > 0; } + /// + public bool TryResetPerkState(string playerId) + { + var norm = NormalizePlayerId(playerId); + if (norm.Length == 0) + { + return false; + } + + PostgresPerkStateBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var tx = conn.BeginTransaction(); + if (!PlayerExists(conn, norm, tx)) + { + tx.Rollback(); + return false; + } + + using (var picks = new Npgsql.NpgsqlCommand( + "DELETE FROM player_mastery_branch_pick WHERE player_id = @pid;", + conn, + tx)) + { + picks.Parameters.AddWithValue("pid", norm); + picks.ExecuteNonQuery(); + } + + using (var perks = new Npgsql.NpgsqlCommand( + "DELETE FROM player_unlocked_perk WHERE player_id = @pid;", + conn, + tx)) + { + perks.Parameters.AddWithValue("pid", norm); + perks.ExecuteNonQuery(); + } + + tx.Commit(); + return true; + } + private static bool PlayerExists(Npgsql.NpgsqlConnection conn, string playerIdNormalized, Npgsql.NpgsqlTransaction tx) { using var cmd = new Npgsql.NpgsqlCommand( diff --git a/server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs b/server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs deleted file mode 100644 index 43ffbe4..0000000 --- a/server/NeonSprawl.Server/Game/PositionState/GamePlayerSeed.cs +++ /dev/null @@ -1,30 +0,0 @@ -namespace NeonSprawl.Server.Game.PositionState; - -/// Configured player ids seeded at host start (dev player + Bruno/manual QA fixtures). -public static class GamePlayerSeed -{ - /// Distinct normalized player ids from and . - public static IReadOnlyList GetSeedPlayerIds(GamePositionOptions options) - { - var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - var list = new List(); - Add(options.DevPlayerId, seen, list); - foreach (var id in options.FixturePlayerIds) - { - Add(id, seen, list); - } - - return list; - } - - private static void Add(string? raw, HashSet seen, List list) - { - var t = raw?.Trim(); - if (string.IsNullOrEmpty(t) || !seen.Add(t)) - { - return; - } - - list.Add(t); - } -} diff --git a/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs index 7124fae..334b824 100644 --- a/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs +++ b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs @@ -10,8 +10,8 @@ public sealed class GamePositionOptions /// Logical player id seeded in the in-memory store (URL-safe string). public string DevPlayerId { get; set; } = "dev-local-1"; - /// Additional player ids seeded with default position and empty progression/perk state (Bruno/manual QA). - public List FixturePlayerIds { get; set; } = []; + /// When true, maps POST …/__dev/mastery-fixture for Bruno/manual QA state setup (also enabled in Development). + public bool EnableMasteryFixtureApi { get; set; } /// World position for the dev player at process start. public DefaultPositionOptions DefaultPosition { get; set; } = new(); diff --git a/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs index cfd558e..62edcbf 100644 --- a/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs +++ b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs @@ -10,19 +10,15 @@ public sealed class InMemoryPositionStateStore(IOptions opt private static ConcurrentDictionary CreateInitialMap(GamePositionOptions o) { - var ids = GamePlayerSeed.GetSeedPlayerIds(o); - if (ids.Count == 0) + var id = o.DevPlayerId.Trim(); + if (id.Length == 0) { throw new InvalidOperationException("Game:DevPlayerId must be non-empty."); } var map = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); var p = o.DefaultPosition ?? new DefaultPositionOptions(); - foreach (var id in ids) - { - map[id] = new PositionSnapshot(p.X, p.Y, p.Z, 0); - } - + map[id] = new PositionSnapshot(p.X, p.Y, p.Z, 0); return map; } diff --git a/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs b/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs index 91e4b8a..090c57a 100644 --- a/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs +++ b/server/NeonSprawl.Server/Game/PositionState/PostgresPositionBootstrap.cs @@ -55,23 +55,20 @@ public static class PostgresPositionBootstrap /// Idempotent insert using an existing open connection (e.g. test harness after TRUNCATE). public static void SeedDevPlayer(Npgsql.NpgsqlConnection connection, GamePositionOptions options) { + var devNorm = NormalizePlayerId(options.DevPlayerId); var p = options.DefaultPosition ?? new DefaultPositionOptions(); - foreach (var id in GamePlayerSeed.GetSeedPlayerIds(options)) - { - var norm = NormalizePlayerId(id); - using var cmd = new Npgsql.NpgsqlCommand( - """ - INSERT INTO player_position (player_id, pos_x, pos_y, pos_z, sequence) - VALUES (@pid, @x, @y, @z, 0) - ON CONFLICT (player_id) DO NOTHING; - """, - connection); - cmd.Parameters.AddWithValue("pid", norm); - cmd.Parameters.AddWithValue("x", p.X); - cmd.Parameters.AddWithValue("y", p.Y); - cmd.Parameters.AddWithValue("z", p.Z); - cmd.ExecuteNonQuery(); - } + using var cmd = new Npgsql.NpgsqlCommand( + """ + INSERT INTO player_position (player_id, pos_x, pos_y, pos_z, sequence) + VALUES (@pid, @x, @y, @z, 0) + ON CONFLICT (player_id) DO NOTHING; + """, + connection); + cmd.Parameters.AddWithValue("pid", devNorm); + cmd.Parameters.AddWithValue("x", p.X); + cmd.Parameters.AddWithValue("y", p.Y); + cmd.Parameters.AddWithValue("z", p.Z); + cmd.ExecuteNonQuery(); } private static string NormalizePlayerId(string? playerId) diff --git a/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs index bd183ed..b4ba656 100644 --- a/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs @@ -11,4 +11,7 @@ public interface IPlayerSkillProgressionStore /// Returns false when the player cannot be written (e.g. in-memory store has no bucket; Postgres player missing from player_position). /// bool TryApplyXpDelta(string playerId, string skillId, int amount, out int previousXp, out int newXp); + + /// Sets absolute XP for one skill (NEO-48 dev fixture API). must be >= 0. + bool TrySetSkillXpTotal(string playerId, string skillId, int xp); } diff --git a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs index 8bdf271..61f691d 100644 --- a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs @@ -14,12 +14,9 @@ public sealed class InMemoryPlayerSkillProgressionStore(IOptions> CreateInitialMap(GamePositionOptions o) { + var id = NormalizePlayerId(o.DevPlayerId); var map = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); - foreach (var id in GamePlayerSeed.GetSeedPlayerIds(o)) - { - map[NormalizePlayerId(id)] = new ConcurrentDictionary(StringComparer.Ordinal); - } - + map[id] = new ConcurrentDictionary(StringComparer.Ordinal); return map; } @@ -66,6 +63,33 @@ public sealed class InMemoryPlayerSkillProgressionStore(IOptions + public bool TrySetSkillXpTotal(string playerId, string skillId, int xp) + { + if (xp < 0) + { + return false; + } + + var key = NormalizePlayerId(playerId); + var sid = skillId.Trim(); + if (key.Length == 0 || sid.Length == 0 || !byPlayer.TryGetValue(key, out var inner)) + { + return false; + } + + lock (playerLocks.GetOrAdd(key, _ => new object())) + { + if (xp == 0) + { + return inner.TryRemove(sid, out _); + } + + inner[sid] = xp; + return true; + } + } + private static string NormalizePlayerId(string? playerId) { var t = playerId?.Trim(); diff --git a/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs index 0a0f221..c802601 100644 --- a/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs @@ -98,6 +98,64 @@ public sealed class PostgresPlayerSkillProgressionStore(Npgsql.NpgsqlDataSource return true; } + /// + public bool TrySetSkillXpTotal(string playerId, string skillId, int xp) + { + if (xp < 0) + { + return false; + } + + var norm = NormalizePlayerId(playerId); + var sid = skillId.Trim(); + if (norm.Length == 0 || sid.Length == 0) + { + return false; + } + + PostgresSkillProgressionBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var tx = conn.BeginTransaction(); + if (!PlayerExists(conn, norm, tx)) + { + tx.Rollback(); + return false; + } + + if (xp == 0) + { + using var del = new Npgsql.NpgsqlCommand( + """ + DELETE FROM player_skill_progression + WHERE player_id = @pid AND skill_id = @sid; + """, + conn, + tx); + del.Parameters.AddWithValue("pid", norm); + del.Parameters.AddWithValue("sid", sid); + del.ExecuteNonQuery(); + } + else + { + using var upsert = new Npgsql.NpgsqlCommand( + """ + INSERT INTO player_skill_progression (player_id, skill_id, xp, updated_at) + VALUES (@pid, @sid, @xp, now()) + ON CONFLICT (player_id, skill_id) + DO UPDATE SET xp = EXCLUDED.xp, updated_at = now(); + """, + conn, + tx); + upsert.Parameters.AddWithValue("pid", norm); + upsert.Parameters.AddWithValue("sid", sid); + upsert.Parameters.AddWithValue("xp", xp); + upsert.ExecuteNonQuery(); + } + + tx.Commit(); + return true; + } + private static bool PlayerExists(Npgsql.NpgsqlConnection conn, string playerIdNormalized, Npgsql.NpgsqlTransaction tx) { using var cmd = new Npgsql.NpgsqlCommand( diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index dae8a54..a7aaf11 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -36,6 +36,13 @@ app.MapInteractablesWorldApi(); app.MapSkillDefinitionsWorldApi(); app.MapSkillProgressionSnapshotApi(); app.MapPerkStateApi(); +if (app.Environment.IsDevelopment() || + app.Environment.IsEnvironment("Testing") || + app.Configuration.GetValue("Game:EnableMasteryFixtureApi")) +{ + app.MapMasteryFixtureApi(); +} + app.MapTargetingApi(); app.MapHotbarLoadoutApi(); app.MapCooldownSnapshotApi(); diff --git a/server/NeonSprawl.Server/appsettings.Development.json b/server/NeonSprawl.Server/appsettings.Development.json index 0c208ae..d37883a 100644 --- a/server/NeonSprawl.Server/appsettings.Development.json +++ b/server/NeonSprawl.Server/appsettings.Development.json @@ -4,5 +4,8 @@ "Default": "Information", "Microsoft.AspNetCore": "Warning" } + }, + "Game": { + "EnableMasteryFixtureApi": true } } diff --git a/server/NeonSprawl.Server/appsettings.json b/server/NeonSprawl.Server/appsettings.json index a207a92..be0b44f 100644 --- a/server/NeonSprawl.Server/appsettings.json +++ b/server/NeonSprawl.Server/appsettings.json @@ -12,10 +12,7 @@ }, "Game": { "DevPlayerId": "dev-local-1", - "FixturePlayerIds": [ - "neo48-perk-fresh", - "neo48-perk-branch" - ], + "EnableMasteryFixtureApi": false, "DefaultPosition": { "X": -5, "Y": 0.9, diff --git a/server/README.md b/server/README.md index 886639f..1243b57 100644 --- a/server/README.md +++ b/server/README.md @@ -64,7 +64,7 @@ On success, **Information** logs include the resolved mastery directory, track c **Structured responses (HTTP 200):** **`selected`** (`true`/`false`), optional **`reasonCode`** on deny, authoritative **`perkState`** (same shape as GET), and **`unlockedEvents`** on success (empty on deny). Stable deny codes (see `PerkUnlockReasonCodes`): **`unknown_track`**, **`unknown_tier`**, **`unknown_branch`**, **`level_too_low`**, **`branch_already_chosen`**, **`tier_branch_not_selectable`**, **`player_not_found`**. On **`selected: true`**, **`unlockedEvents`** lists perks newly unlocked on this request (`perkId`, `skillId`, `tierIndex`, `branchId`, **`source`**: **`branch_pick`** | **`level_up`**). Tier-1 prototype salvage picks may emit zero events; path-auto tier-2 unlocks at level 4+ may emit **`level_up`** events on the same POST when level gates are already met. -**Bruno fixture players (NEO-48):** `Game:FixturePlayerIds` in `appsettings.json` seeds additional player ids (`neo48-perk-fresh`, `neo48-perk-branch`) with default position and empty progression/perk state so deny smokes do not depend on `dev-local-1` history. +**Dev mastery fixture (NEO-48, Bruno/manual QA):** When `Game:EnableMasteryFixtureApi` is **true** (default in **Development** via `appsettings.Development.json`) or the host environment is **Development** / **Testing**, **`POST /game/players/{id}/__dev/mastery-fixture`** accepts `schemaVersion` **1**, optional **`resetPerkState`** (clear branch picks + unlocked perks), and optional **`skillXp`** map (absolute XP per `skillId`). **404** when disabled, player unknown, or store write fails. Not for production. Plan: [NEO-48 implementation plan](../../docs/plans/NEO-48-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-48.md`](../../docs/manual-qa/NEO-48.md); Bruno: `bruno/neon-sprawl-server/perk-state/`. From 2bb02d76e92c522ba680f7262a29a393ffce0e26 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:34:36 -0400 Subject: [PATCH 09/14] NEO-48: add re-review for mastery fixture testing refactor --- docs/reviews/2026-05-17-NEO-48-r2.md | 66 ++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 docs/reviews/2026-05-17-NEO-48-r2.md diff --git a/docs/reviews/2026-05-17-NEO-48-r2.md b/docs/reviews/2026-05-17-NEO-48-r2.md new file mode 100644 index 0000000..98d4a0f --- /dev/null +++ b/docs/reviews/2026-05-17-NEO-48-r2.md @@ -0,0 +1,66 @@ +# Code review (re-review) — NEO-48 testing refactor + mastery fixture API + +**Date:** 2026-05-17 +**Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `3ba8300`–`ffdbed2` (delta since [first review](2026-05-17-NEO-48.md)) · full branch vs `main` for merge readiness +**Base:** `main` +**Prior review:** [`2026-05-17-NEO-48.md`](2026-05-17-NEO-48.md) (perk-state HTTP + initial Bruno/tests) + +## Verdict + +**Approve with nits** + +## Summary + +Follow-up work closes the first-review Bruno/test gaps and replaces a short-lived **fixture-player seeding** experiment with a cleaner **dev-only mastery fixture API**. **`POST /game/players/{id}/__dev/mastery-fixture`** (gated to Development, Testing, or `Game:EnableMasteryFixtureApi`) resets perk state and sets absolute skill XP without going through grant/level-up hooks—giving deterministic Bruno deny smokes on shared `dev-local-1`. **`PerkStateApiTests`** adds null-body **400** and HTTP deny smoke for `unknown_track` / `unknown_tier` / `unknown_branch`; **`MasteryFixtureApiTests`** verifies Production+flag-off returns **404** and that reset + XP zero yields **`level_too_low`** on the next branch POST. Bruno deny requests use `script:pre-request` + `axios` against the fixture endpoint; `{{playerId}}` from `Local.bru` is used consistently on deny flows. **191** server tests pass. Merge risk remains low; the new surface is explicitly non-production and tested for disable behavior. + +## Documentation checked + +| Document | Result | +|----------|--------| +| [`docs/plans/NEO-48-implementation-plan.md`](../plans/NEO-48-implementation-plan.md) | **Partially matches** — perk-state HTTP/Bruno acceptance unchanged; kickoff table documents **`__dev/mastery-fixture`** for Bruno deny setup. **Files to add/modify** tables do not list `MasteryFixture*.cs` or store `TryReset*` / `TrySetSkillXpTotal` (implementation detail worth a one-line plan add). | +| [`docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md`](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) | **Matches** — NEO-48 handoff covers HTTP only; dev fixture correctly omitted from module contracts. | +| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — production perk rules still flow through `PerkUnlockEngine`; fixture bypasses grant path by design (dev/QA only). | +| [`docs/manual-qa/NEO-48.md`](../manual-qa/NEO-48.md) | **Matches** — fixture curl examples + Bruno deny checklist updated. | +| [`server/README.md`](../../server/README.md) | **Matches** — Perk state section documents fixture API, gating, and **404** when disabled. | +| Prior [`2026-05-17-NEO-48.md`](2026-05-17-NEO-48.md) | **Superseded for merge** by this re-review for the testing-refactor delta; first-review suggestions 1–3 marked done there remain valid. | + +## Blocking issues + +None. + +## Suggestions + +1. **Atomic fixture apply** — `MasteryFixtureOperations.TryApply` can leave mixed state if `resetPerkState` succeeds and a later `skillXp` entry fails (or an earlier XP write succeeds before a later one fails). For a dev endpoint this is unlikely with Bruno’s fixed payloads, but consider applying XP first then reset, a single transaction on Postgres, or returning **409**/**500** with a note when partial apply is detected. +2. **Fixture API test coverage** — Add small tests for **400** (null body / bad `schemaVersion`) and **404** (unknown player, negative XP in `skillXp`) on `__dev/mastery-fixture` to mirror perk-state gate tests. +3. **Document fixture vs grant semantics** — README could note that **`skillXp` on the fixture sets absolute totals without running `SkillProgressionGrantOperations` / perk level-up reevaluation** (intentional for deny setup; use normal grant API when testing unlock side effects). + +## Nits + +- Nit: `Get perk state.bru` still asserts `playerId === "dev-local-1"` while deny requests use `{{playerId}}` from the environment—harmless if `Local.bru` keeps the default; align assertions for copy-paste environments. +- Nit: `TryResetPerkState` / `TrySetSkillXpTotal` live on production store interfaces—acceptable for prototype; if the surface grows, consider a dev-only `IMasteryFixtureStore` facade later. +- Nit: Bruno deny flows mutate shared `dev-local-1`; folder `docs` explain fixture pre-requests—run deny requests before happy-path smokes or accept order sensitivity in manual runs. +- Nit: `MasteryFixtureApiTests.PostMasteryFixture_ShouldResetSalvageXpAndPerkState_ThenLevelTooLowDenies` performs reset and deny POST both under **Act**; valid for an integration scenario, but heavy **Arrange** (grant + pick) could move to a private helper like `PerkStateApiTests.GrantSalvageLevel2Async`. + +## Delta since first review (testing refactor) + +| Area | Change | +|------|--------| +| **Bruno** | `Post branch select deny branch already chosen.bru`; `level_too_low` uses fixture pre-request (no skip-on-already-leveled); `{{playerId}}` + `baseUrl` resolution fixed. | +| **Server** | `MasteryFixtureApi` / `MasteryFixtureOperations` / DTOs; `Program.cs` conditional map; `EnableMasteryFixtureApi` in appsettings; store `TryResetPerkState` + `TrySetSkillXpTotal` (in-memory + Postgres). | +| **Tests** | `MasteryFixtureApiTests` (2); `PerkStateApiTests` +4 deny/null-body; `InMemoryWebApplicationFactory` enables fixture flag for Testing. | +| **Reverted approach** | `GamePlayerSeed` / extra fixture player ids (commit `b72201b`) removed in favor of fixture API (`ffdbed2`)—simpler ops story. | + +## Verification + +```bash +# From repo root +dotnet test NeonSprawl.sln --filter "FullyQualifiedName~PerkState|MasteryFixture" + +# Full server suite (191 tests at re-review) +dotnet test NeonSprawl.sln + +# Bruno (Development server, EnableMasteryFixtureApi on) +# bruno/neon-sprawl-server/perk-state/ +``` + +Confirm deny Bruno requests pass on a fresh dev server; confirm `POST …/__dev/mastery-fixture` returns **404** when `ASPNETCORE_ENVIRONMENT=Production` and `Game:EnableMasteryFixtureApi=false`. From aba26446d330a1d902eeca5c1ad47d22a06ef6c8 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:37:00 -0400 Subject: [PATCH 10/14] =?UTF-8?q?NEO-48:=20address=20r2=20review=20?= =?UTF-8?q?=E2=80=94=20atomic=20fixture,=20tests,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch skillXp then resetPerkState with XP rollback; 400 for invalid skillXp; MasteryFixtureApi gate tests; README fixture vs grant semantics; Bruno GET playerId. --- .../perk-state/Get perk state.bru | 4 +- .../neon-sprawl-server/perk-state/folder.bru | 1 + docs/plans/NEO-48-implementation-plan.md | 12 +- docs/reviews/2026-05-17-NEO-48-r2.md | 17 ++- .../Game/Mastery/MasteryFixtureApiTests.cs | 143 +++++++++++++----- .../Game/Mastery/MasteryFixtureApi.cs | 5 + .../Game/Mastery/MasteryFixtureOperations.cs | 57 +++++-- .../Skills/IPlayerSkillProgressionStore.cs | 3 + .../InMemoryPlayerSkillProgressionStore.cs | 36 +++++ .../PostgresPlayerSkillProgressionStore.cs | 64 ++++++++ server/README.md | 2 +- 11 files changed, 285 insertions(+), 59 deletions(-) diff --git a/bruno/neon-sprawl-server/perk-state/Get perk state.bru b/bruno/neon-sprawl-server/perk-state/Get perk state.bru index 8567c13..80270ac 100644 --- a/bruno/neon-sprawl-server/perk-state/Get perk state.bru +++ b/bruno/neon-sprawl-server/perk-state/Get perk state.bru @@ -10,7 +10,7 @@ docs { } get { - url: {{baseUrl}}/game/players/dev-local-1/perk-state + url: {{baseUrl}}/game/players/{{playerId}}/perk-state body: none auth: none } @@ -21,7 +21,7 @@ tests { expect(res.getHeader("content-type")).to.contain("application/json"); const body = res.getBody(); expect(body.schemaVersion).to.equal(1); - expect(body.playerId).to.equal("dev-local-1"); + expect(body.playerId).to.equal(bru.getEnvVar("playerId") || bru.getVar("playerId")); expect(body.branchPicks).to.be.an("array"); expect(body.unlockedPerkIds).to.be.an("array"); }); diff --git a/bruno/neon-sprawl-server/perk-state/folder.bru b/bruno/neon-sprawl-server/perk-state/folder.bru index d8c5be7..43b8d00 100644 --- a/bruno/neon-sprawl-server/perk-state/folder.bru +++ b/bruno/neon-sprawl-server/perk-state/folder.bru @@ -4,4 +4,5 @@ meta { docs { NEO-48 perk-state HTTP smoke. Deny requests call POST …/__dev/mastery-fixture in pre-request to set salvage XP and clear perk state (Development / Game:EnableMasteryFixtureApi). + Run deny requests (seq 3–4) before happy-path POST tier1 if you need a clean dev-local-1; deny pre-requests reset shared player state. } diff --git a/docs/plans/NEO-48-implementation-plan.md b/docs/plans/NEO-48-implementation-plan.md index 2467fde..d612f9e 100644 --- a/docs/plans/NEO-48-implementation-plan.md +++ b/docs/plans/NEO-48-implementation-plan.md @@ -83,6 +83,9 @@ |------|---------| | `server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs` | `PerkStateSnapshotResponse`, `PerkBranchPickTrackJson`, `PerkBranchPickRowJson`, `PerkBranchSelectRequest`, `PerkBranchSelectResponse`, `PerkUnlockedEventJson`. | | `server/NeonSprawl.Server/Game/Mastery/PerkStateApi.cs` | `MapPerkStateApi`, `BuildSnapshot`, GET/POST handlers. | +| `server/NeonSprawl.Server/Game/Mastery/MasteryFixtureDtos.cs` | Dev fixture request/response (`resetPerkState`, `skillXp`). | +| `server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs` | `POST …/__dev/mastery-fixture` (gated). | +| `server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs` | Batch XP apply + perk reset with XP rollback on reset failure. | | `server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs` | In-memory host: GET empty defaults; POST tier-1 happy after XP grant; deny `level_too_low` / `branch_already_chosen`; GET reflects POST; **404** / **400** gates. | | `bruno/neon-sprawl-server/perk-state/folder.bru` | Collection folder metadata. | | `bruno/neon-sprawl-server/perk-state/Get perk state.bru` | GET smoke + schema assertions. | @@ -94,7 +97,13 @@ | Path | Rationale | |------|-----------| -| `server/NeonSprawl.Server/Program.cs` | Register **`MapPerkStateApi()`**. | +| `server/NeonSprawl.Server/Program.cs` | Register **`MapPerkStateApi()`** and conditional **`MapMasteryFixtureApi()`**. | +| `server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs` | **`TrySetSkillXpTotal`** / **`TrySetSkillXpTotals`** for dev fixture. | +| `server/NeonSprawl.Server/Game/Mastery/IPlayerPerkStateStore.cs` | **`TryResetPerkState`** for dev fixture. | +| `server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs` | Fixture XP set implementations. | +| `server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs` | Fixture XP set implementations. | +| `server/NeonSprawl.Server/Game/Mastery/InMemoryPlayerPerkStateStore.cs` | **`TryResetPerkState`**. | +| `server/NeonSprawl.Server/Game/Mastery/PostgresPlayerPerkStateStore.cs` | **`TryResetPerkState`**. | | `server/README.md` | New **Perk state (NEO-48)** section: GET/POST paths, schema v1, `selected`/`reasonCode`/`unlockedEvents`, reason code table (link `PerkUnlockReasonCodes`), Bruno path, manual QA link. | | `docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md` | Note NEO-48 HTTP landed (replace “HTTP is NEO-48” pending wording in NEO-47 handoff). | | `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E2.M3 row: NEO-48 perk-state HTTP. | @@ -105,6 +114,7 @@ | Test file | What it covers | |-----------|----------------| | `PerkStateApiTests.cs` | **AAA** integration via `InMemoryWebApplicationFactory`: **GET** returns v1 empty state for dev player; **POST** after `POST …/skill-progression` grant (100 XP → level 2) selects `scrap_efficiency` tier 1 → `selected: true`, pick in **GET**; **POST** deny `level_too_low` without grant; **POST** deny `branch_already_chosen` on second pick; **404** unknown player; **400** bad `schemaVersion`. Optional: **POST** at level 4+ includes non-empty **`unlockedEvents`** when tier-2 perk unlocks on same request (path-auto). | +| `MasteryFixtureApiTests.cs` | Fixture route disabled → **404**; **400** null body / bad schema / negative XP; **404** unknown player; reset after progressed state → **`level_too_low`** on next branch POST. | | Existing `PerkUnlockEngineTests.cs` | No changes required — engine rules remain covered; API tests are thin HTTP mapping. | Bruno + **`docs/manual-qa/NEO-48.md`** supplement automated coverage. Postgres persistence of picks is already covered by **`PerkStatePersistenceIntegrationTests`** (NEO-47); no new Postgres API test unless a regression gap appears during implementation. diff --git a/docs/reviews/2026-05-17-NEO-48-r2.md b/docs/reviews/2026-05-17-NEO-48-r2.md index 98d4a0f..13a4f02 100644 --- a/docs/reviews/2026-05-17-NEO-48-r2.md +++ b/docs/reviews/2026-05-17-NEO-48-r2.md @@ -3,7 +3,8 @@ **Date:** 2026-05-17 **Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `3ba8300`–`ffdbed2` (delta since [first review](2026-05-17-NEO-48.md)) · full branch vs `main` for merge readiness **Base:** `main` -**Prior review:** [`2026-05-17-NEO-48.md`](2026-05-17-NEO-48.md) (perk-state HTTP + initial Bruno/tests) +**Prior review:** [`2026-05-17-NEO-48.md`](2026-05-17-NEO-48.md) (perk-state HTTP + initial Bruno/tests) +**Follow-up:** R2 suggestions 1–3 and nits 1 + 3 addressed in a follow-up commit on the same branch. ## Verdict @@ -17,7 +18,7 @@ Follow-up work closes the first-review Bruno/test gaps and replaces a short-live | Document | Result | |----------|--------| -| [`docs/plans/NEO-48-implementation-plan.md`](../plans/NEO-48-implementation-plan.md) | **Partially matches** — perk-state HTTP/Bruno acceptance unchanged; kickoff table documents **`__dev/mastery-fixture`** for Bruno deny setup. **Files to add/modify** tables do not list `MasteryFixture*.cs` or store `TryReset*` / `TrySetSkillXpTotal` (implementation detail worth a one-line plan add). | +| [`docs/plans/NEO-48-implementation-plan.md`](../plans/NEO-48-implementation-plan.md) | **Matches** — perk-state HTTP/Bruno acceptance unchanged; kickoff documents fixture API; **Files to add/modify** lists `MasteryFixture*` and store fixture methods. | | [`docs/decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md`](../decomposition/modules/E2_M3_MasteryAndPerkUnlocks.md) | **Matches** — NEO-48 handoff covers HTTP only; dev fixture correctly omitted from module contracts. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — production perk rules still flow through `PerkUnlockEngine`; fixture bypasses grant path by design (dev/QA only). | | [`docs/manual-qa/NEO-48.md`](../manual-qa/NEO-48.md) | **Matches** — fixture curl examples + Bruno deny checklist updated. | @@ -30,16 +31,16 @@ None. ## Suggestions -1. **Atomic fixture apply** — `MasteryFixtureOperations.TryApply` can leave mixed state if `resetPerkState` succeeds and a later `skillXp` entry fails (or an earlier XP write succeeds before a later one fails). For a dev endpoint this is unlikely with Bruno’s fixed payloads, but consider applying XP first then reset, a single transaction on Postgres, or returning **409**/**500** with a note when partial apply is detected. -2. **Fixture API test coverage** — Add small tests for **400** (null body / bad `schemaVersion`) and **404** (unknown player, negative XP in `skillXp`) on `__dev/mastery-fixture` to mirror perk-state gate tests. -3. **Document fixture vs grant semantics** — README could note that **`skillXp` on the fixture sets absolute totals without running `SkillProgressionGrantOperations` / perk level-up reevaluation** (intentional for deny setup; use normal grant API when testing unlock side effects). +1. ~~**Atomic fixture apply** — `MasteryFixtureOperations.TryApply` can leave mixed state if `resetPerkState` succeeds and a later `skillXp` entry fails (or an earlier XP write succeeds before a later one fails). For a dev endpoint this is unlikely with Bruno’s fixed payloads, but consider applying XP first then reset, a single transaction on Postgres, or returning **409**/**500** with a note when partial apply is detected.~~ **Done.** Batch **`TrySetSkillXpTotals`** then **`resetPerkState`**; XP rollback via previous snapshot if reset fails; **400** for invalid `skillXp` before writes. +2. ~~**Fixture API test coverage** — Add small tests for **400** (null body / bad `schemaVersion`) and **404** (unknown player, negative XP in `skillXp`) on `__dev/mastery-fixture` to mirror perk-state gate tests.~~ **Done.** Four new tests in `MasteryFixtureApiTests`. +3. ~~**Document fixture vs grant semantics** — README could note that **`skillXp` on the fixture sets absolute totals without running `SkillProgressionGrantOperations` / perk level-up reevaluation** (intentional for deny setup; use normal grant API when testing unlock side effects).~~ **Done.** README Perk state section. ## Nits -- Nit: `Get perk state.bru` still asserts `playerId === "dev-local-1"` while deny requests use `{{playerId}}` from the environment—harmless if `Local.bru` keeps the default; align assertions for copy-paste environments. +- ~~Nit: `Get perk state.bru` still asserts `playerId === "dev-local-1"` while deny requests use `{{playerId}}` from the environment—harmless if `Local.bru` keeps the default; align assertions for copy-paste environments.~~ **Done.** GET uses `{{playerId}}` in URL and assertion. - Nit: `TryResetPerkState` / `TrySetSkillXpTotal` live on production store interfaces—acceptable for prototype; if the surface grows, consider a dev-only `IMasteryFixtureStore` facade later. -- Nit: Bruno deny flows mutate shared `dev-local-1`; folder `docs` explain fixture pre-requests—run deny requests before happy-path smokes or accept order sensitivity in manual runs. -- Nit: `MasteryFixtureApiTests.PostMasteryFixture_ShouldResetSalvageXpAndPerkState_ThenLevelTooLowDenies` performs reset and deny POST both under **Act**; valid for an integration scenario, but heavy **Arrange** (grant + pick) could move to a private helper like `PerkStateApiTests.GrantSalvageLevel2Async`. +- ~~Nit: Bruno deny flows mutate shared `dev-local-1`; folder `docs` explain fixture pre-requests—run deny requests before happy-path smokes or accept order sensitivity in manual runs.~~ **Done.** Folder `docs` note on run order. +- ~~Nit: `MasteryFixtureApiTests.PostMasteryFixture_ShouldResetSalvageXpAndPerkState_ThenLevelTooLowDenies` performs reset and deny POST both under **Act**; valid for an integration scenario, but heavy **Arrange** (grant + pick) could move to a private helper like `PerkStateApiTests.GrantSalvageLevel2Async`.~~ **Done.** `ArrangeSalvageProgressedWithTier1PickAsync`. ## Delta since first review (testing refactor) diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs index ecd2a16..959254a 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/MasteryFixtureApiTests.cs @@ -1,5 +1,7 @@ using System.Net; +using System.Net.Http; using System.Net.Http.Json; +using System.Text; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Testing; using NeonSprawl.Server.Game.Mastery; @@ -11,11 +13,46 @@ namespace NeonSprawl.Server.Tests.Game.Mastery; public sealed class MasteryFixtureApiTests { private const string DevPlayer = "dev-local-1"; + private const string FixturePath = $"/game/players/{DevPlayer}/__dev/mastery-fixture"; + + private static MasteryFixtureRequest ValidFixture( + bool resetPerkState = true, + Dictionary? skillXp = null) => + new MasteryFixtureRequest + { + SchemaVersion = MasteryFixtureRequest.CurrentSchemaVersion, + ResetPerkState = resetPerkState, + SkillXp = skillXp ?? new Dictionary { ["salvage"] = 0 }, + }; + + private static async Task ArrangeSalvageProgressedWithTier1PickAsync(HttpClient client) + { + var grant = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + new SkillProgressionGrantRequest + { + SchemaVersion = SkillProgressionGrantRequest.CurrentSchemaVersion, + SkillId = "salvage", + Amount = 100, + SourceKind = "activity", + }); + Assert.Equal(HttpStatusCode.OK, grant.StatusCode); + var pick = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + new PerkBranchSelectRequest + { + SchemaVersion = PerkBranchSelectRequest.CurrentSchemaVersion, + SkillId = "salvage", + TierIndex = 1, + BranchId = "scrap_efficiency", + }); + Assert.Equal(HttpStatusCode.OK, pick.StatusCode); + } [Fact] public async Task PostMasteryFixture_ShouldReturnNotFound_WhenRouteNotRegistered() { - // Arrange — Production + fixture flag off => endpoint not mapped + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.WithWebHostBuilder(b => { @@ -23,15 +60,77 @@ public sealed class MasteryFixtureApiTests b.UseSetting("Game:EnableMasteryFixtureApi", "false"); }).CreateClient(); + // Act + var response = await client.PostAsJsonAsync(FixturePath, ValidFixture()); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task PostMasteryFixture_ShouldReturnBadRequest_WhenBodyNull() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsync( + FixturePath, + new StringContent(string.Empty, Encoding.UTF8, "application/json")); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostMasteryFixture_ShouldReturnBadRequest_WhenSchemaVersionMismatch() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var request = ValidFixture(); + request = new MasteryFixtureRequest + { + SchemaVersion = MasteryFixtureRequest.CurrentSchemaVersion + 99, + ResetPerkState = request.ResetPerkState, + SkillXp = request.SkillXp, + }; + + // Act + var response = await client.PostAsJsonAsync(FixturePath, request); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostMasteryFixture_ShouldReturnBadRequest_WhenSkillXpNegative() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + // Act var response = await client.PostAsJsonAsync( - $"/game/players/{DevPlayer}/__dev/mastery-fixture", - new MasteryFixtureRequest - { - SchemaVersion = MasteryFixtureRequest.CurrentSchemaVersion, - ResetPerkState = true, - SkillXp = new Dictionary { ["salvage"] = 0 }, - }); + FixturePath, + ValidFixture(skillXp: new Dictionary { ["salvage"] = -1 })); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostMasteryFixture_ShouldReturnNotFound_WhenPlayerUnknown() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsJsonAsync( + "/game/players/missing-player/__dev/mastery-fixture", + ValidFixture()); // Assert Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); @@ -43,34 +142,10 @@ public sealed class MasteryFixtureApiTests // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); - Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( - $"/game/players/{DevPlayer}/skill-progression", - new SkillProgressionGrantRequest - { - SchemaVersion = SkillProgressionGrantRequest.CurrentSchemaVersion, - SkillId = "salvage", - Amount = 100, - SourceKind = "activity", - })).StatusCode); - Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( - $"/game/players/{DevPlayer}/perk-state", - new PerkBranchSelectRequest - { - SchemaVersion = PerkBranchSelectRequest.CurrentSchemaVersion, - SkillId = "salvage", - TierIndex = 1, - BranchId = "scrap_efficiency", - })).StatusCode); + await ArrangeSalvageProgressedWithTier1PickAsync(client); // Act - var reset = await client.PostAsJsonAsync( - $"/game/players/{DevPlayer}/__dev/mastery-fixture", - new MasteryFixtureRequest - { - SchemaVersion = MasteryFixtureRequest.CurrentSchemaVersion, - ResetPerkState = true, - SkillXp = new Dictionary { ["salvage"] = 0 }, - }); + var reset = await client.PostAsJsonAsync(FixturePath, ValidFixture()); var deny = await client.PostAsJsonAsync( $"/game/players/{DevPlayer}/perk-state", new PerkBranchSelectRequest diff --git a/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs index 5a06d1e..835b87c 100644 --- a/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs +++ b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureApi.cs @@ -18,6 +18,11 @@ public static class MasteryFixtureApi return Results.BadRequest(); } + if (!MasteryFixtureOperations.TryNormalizeSkillXp(body.SkillXp, out _)) + { + return Results.BadRequest(); + } + var trimmedId = id.Trim(); if (trimmedId.Length == 0 || !positions.TryGetPosition(trimmedId, out _)) { diff --git a/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs index 3b1c0f5..e4612f6 100644 --- a/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs +++ b/server/NeonSprawl.Server/Game/Mastery/MasteryFixtureOperations.cs @@ -5,35 +5,66 @@ namespace NeonSprawl.Server.Game.Mastery; /// Applies dev-only mastery fixture resets (NEO-48). public static class MasteryFixtureOperations { + /// Validates entries; returns false when any key is blank or XP is negative. + public static bool TryNormalizeSkillXp( + IReadOnlyDictionary? skillXp, + out Dictionary normalized) + { + normalized = new Dictionary(StringComparer.Ordinal); + if (skillXp is null || skillXp.Count == 0) + { + return true; + } + + foreach (var (skillId, xp) in skillXp) + { + var sid = skillId.Trim(); + if (sid.Length == 0 || xp < 0) + { + normalized.Clear(); + return false; + } + + normalized[sid] = xp; + } + + return true; + } + + /// Applies skill XP (batch, then perk reset). Rolls back XP when perk reset fails after XP writes. public static bool TryApply( string playerId, MasteryFixtureRequest body, IPlayerPerkStateStore perkStore, IPlayerSkillProgressionStore xpStore) { - if (body.ResetPerkState && !perkStore.TryResetPerkState(playerId)) + if (!TryNormalizeSkillXp(body.SkillXp, out var skillXp)) { return false; } - if (body.SkillXp is null || body.SkillXp.Count == 0) + var previousXp = new Dictionary(xpStore.GetXpTotals(playerId), StringComparer.Ordinal); + + if (skillXp.Count > 0 && !xpStore.TrySetSkillXpTotals(playerId, skillXp)) + { + return false; + } + + if (!body.ResetPerkState) { return true; } - foreach (var (skillId, xp) in body.SkillXp) + if (perkStore.TryResetPerkState(playerId)) { - if (skillId.Trim().Length == 0 || xp < 0) - { - return false; - } - - if (!xpStore.TrySetSkillXpTotal(playerId, skillId, xp)) - { - return false; - } + return true; } - return true; + if (skillXp.Count > 0) + { + _ = xpStore.TrySetSkillXpTotals(playerId, previousXp); + } + + return false; } } diff --git a/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs index b4ba656..c743f01 100644 --- a/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs @@ -14,4 +14,7 @@ public interface IPlayerSkillProgressionStore /// Sets absolute XP for one skill (NEO-48 dev fixture API). must be >= 0. bool TrySetSkillXpTotal(string playerId, string skillId, int xp); + + /// Sets absolute XP for multiple skills atomically (NEO-48 dev fixture API). + bool TrySetSkillXpTotals(string playerId, IReadOnlyDictionary skillXpTotals); } diff --git a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs index 61f691d..fa1a1bd 100644 --- a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs @@ -90,6 +90,42 @@ public sealed class InMemoryPlayerSkillProgressionStore(IOptions + public bool TrySetSkillXpTotals(string playerId, IReadOnlyDictionary skillXpTotals) + { + var key = NormalizePlayerId(playerId); + if (key.Length == 0 || !byPlayer.TryGetValue(key, out var inner)) + { + return false; + } + + foreach (var (skillId, xp) in skillXpTotals) + { + if (skillId.Trim().Length == 0 || xp < 0) + { + return false; + } + } + + lock (playerLocks.GetOrAdd(key, _ => new object())) + { + foreach (var (skillId, xp) in skillXpTotals) + { + var sid = skillId.Trim(); + if (xp == 0) + { + inner.TryRemove(sid, out _); + } + else + { + inner[sid] = xp; + } + } + + return true; + } + } + private static string NormalizePlayerId(string? playerId) { var t = playerId?.Trim(); diff --git a/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs index c802601..2f10add 100644 --- a/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs @@ -156,6 +156,70 @@ public sealed class PostgresPlayerSkillProgressionStore(Npgsql.NpgsqlDataSource return true; } + /// + public bool TrySetSkillXpTotals(string playerId, IReadOnlyDictionary skillXpTotals) + { + var norm = NormalizePlayerId(playerId); + if (norm.Length == 0) + { + return false; + } + + foreach (var (skillId, xp) in skillXpTotals) + { + if (skillId.Trim().Length == 0 || xp < 0) + { + return false; + } + } + + PostgresSkillProgressionBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var tx = conn.BeginTransaction(); + if (!PlayerExists(conn, norm, tx)) + { + tx.Rollback(); + return false; + } + + foreach (var (skillId, xp) in skillXpTotals) + { + var sid = skillId.Trim(); + if (xp == 0) + { + using var del = new Npgsql.NpgsqlCommand( + """ + DELETE FROM player_skill_progression + WHERE player_id = @pid AND skill_id = @sid; + """, + conn, + tx); + del.Parameters.AddWithValue("pid", norm); + del.Parameters.AddWithValue("sid", sid); + del.ExecuteNonQuery(); + } + else + { + using var upsert = new Npgsql.NpgsqlCommand( + """ + INSERT INTO player_skill_progression (player_id, skill_id, xp, updated_at) + VALUES (@pid, @sid, @xp, now()) + ON CONFLICT (player_id, skill_id) + DO UPDATE SET xp = EXCLUDED.xp, updated_at = now(); + """, + conn, + tx); + upsert.Parameters.AddWithValue("pid", norm); + upsert.Parameters.AddWithValue("sid", sid); + upsert.Parameters.AddWithValue("xp", xp); + upsert.ExecuteNonQuery(); + } + } + + tx.Commit(); + return true; + } + private static bool PlayerExists(Npgsql.NpgsqlConnection conn, string playerIdNormalized, Npgsql.NpgsqlTransaction tx) { using var cmd = new Npgsql.NpgsqlCommand( diff --git a/server/README.md b/server/README.md index 1243b57..8eef9d9 100644 --- a/server/README.md +++ b/server/README.md @@ -64,7 +64,7 @@ On success, **Information** logs include the resolved mastery directory, track c **Structured responses (HTTP 200):** **`selected`** (`true`/`false`), optional **`reasonCode`** on deny, authoritative **`perkState`** (same shape as GET), and **`unlockedEvents`** on success (empty on deny). Stable deny codes (see `PerkUnlockReasonCodes`): **`unknown_track`**, **`unknown_tier`**, **`unknown_branch`**, **`level_too_low`**, **`branch_already_chosen`**, **`tier_branch_not_selectable`**, **`player_not_found`**. On **`selected: true`**, **`unlockedEvents`** lists perks newly unlocked on this request (`perkId`, `skillId`, `tierIndex`, `branchId`, **`source`**: **`branch_pick`** | **`level_up`**). Tier-1 prototype salvage picks may emit zero events; path-auto tier-2 unlocks at level 4+ may emit **`level_up`** events on the same POST when level gates are already met. -**Dev mastery fixture (NEO-48, Bruno/manual QA):** When `Game:EnableMasteryFixtureApi` is **true** (default in **Development** via `appsettings.Development.json`) or the host environment is **Development** / **Testing**, **`POST /game/players/{id}/__dev/mastery-fixture`** accepts `schemaVersion` **1**, optional **`resetPerkState`** (clear branch picks + unlocked perks), and optional **`skillXp`** map (absolute XP per `skillId`). **404** when disabled, player unknown, or store write fails. Not for production. +**Dev mastery fixture (NEO-48, Bruno/manual QA):** When `Game:EnableMasteryFixtureApi` is **true** (default in **Development** via `appsettings.Development.json`) or the host environment is **Development** / **Testing**, **`POST /game/players/{id}/__dev/mastery-fixture`** accepts `schemaVersion` **1**, optional **`resetPerkState`** (clear branch picks + unlocked perks), and optional **`skillXp`** map (**absolute** XP per `skillId`; omitted skills unchanged). Fixture writes set totals **directly** — they do **not** run **`SkillProgressionGrantOperations`** or perk **level-up reevaluation**; use normal **`POST …/skill-progression`** when testing unlock side effects. Apply order: batch **`skillXp`** first, then **`resetPerkState`** (XP rolls back if reset fails). **400** for bad schema or invalid **`skillXp`**; **404** when disabled, player unknown, or store write fails. Not for production. Plan: [NEO-48 implementation plan](../../docs/plans/NEO-48-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-48.md`](../../docs/manual-qa/NEO-48.md); Bruno: `bruno/neon-sprawl-server/perk-state/`. From e9c02eded794865341c3b2baf204e22b42c40137 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:38:02 -0400 Subject: [PATCH 11/14] =?UTF-8?q?NEO-48:=20mark=20review=20items=20resolve?= =?UTF-8?q?d=20=E2=80=94=20Approve=20verdict?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/reviews/2026-05-17-NEO-48-r2.md | 8 ++++---- docs/reviews/2026-05-17-NEO-48.md | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/reviews/2026-05-17-NEO-48-r2.md b/docs/reviews/2026-05-17-NEO-48-r2.md index 13a4f02..8256452 100644 --- a/docs/reviews/2026-05-17-NEO-48-r2.md +++ b/docs/reviews/2026-05-17-NEO-48-r2.md @@ -4,11 +4,11 @@ **Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `3ba8300`–`ffdbed2` (delta since [first review](2026-05-17-NEO-48.md)) · full branch vs `main` for merge readiness **Base:** `main` **Prior review:** [`2026-05-17-NEO-48.md`](2026-05-17-NEO-48.md) (perk-state HTTP + initial Bruno/tests) -**Follow-up:** R2 suggestions 1–3 and nits 1 + 3 addressed in a follow-up commit on the same branch. +**Follow-up:** R2 suggestions 1–3 and nits 1 + 3 + 4 addressed in `aba2644`; re-verified at **195** passing server tests. ## Verdict -**Approve with nits** +**Approve** ## Summary @@ -38,7 +38,7 @@ None. ## Nits - ~~Nit: `Get perk state.bru` still asserts `playerId === "dev-local-1"` while deny requests use `{{playerId}}` from the environment—harmless if `Local.bru` keeps the default; align assertions for copy-paste environments.~~ **Done.** GET uses `{{playerId}}` in URL and assertion. -- Nit: `TryResetPerkState` / `TrySetSkillXpTotal` live on production store interfaces—acceptable for prototype; if the surface grows, consider a dev-only `IMasteryFixtureStore` facade later. +- ~~Nit: `TryResetPerkState` / `TrySetSkillXpTotal` live on production store interfaces—acceptable for prototype; if the surface grows, consider a dev-only `IMasteryFixtureStore` facade later.~~ **Deferred.** Acceptable for E2.M3 prototype; revisit if more dev-only store methods accumulate. - ~~Nit: Bruno deny flows mutate shared `dev-local-1`; folder `docs` explain fixture pre-requests—run deny requests before happy-path smokes or accept order sensitivity in manual runs.~~ **Done.** Folder `docs` note on run order. - ~~Nit: `MasteryFixtureApiTests.PostMasteryFixture_ShouldResetSalvageXpAndPerkState_ThenLevelTooLowDenies` performs reset and deny POST both under **Act**; valid for an integration scenario, but heavy **Arrange** (grant + pick) could move to a private helper like `PerkStateApiTests.GrantSalvageLevel2Async`.~~ **Done.** `ArrangeSalvageProgressedWithTier1PickAsync`. @@ -57,7 +57,7 @@ None. # From repo root dotnet test NeonSprawl.sln --filter "FullyQualifiedName~PerkState|MasteryFixture" -# Full server suite (191 tests at re-review) +# Full server suite (195 tests after aba2644) dotnet test NeonSprawl.sln # Bruno (Development server, EnableMasteryFixtureApi on) diff --git a/docs/reviews/2026-05-17-NEO-48.md b/docs/reviews/2026-05-17-NEO-48.md index b2a376f..f29e369 100644 --- a/docs/reviews/2026-05-17-NEO-48.md +++ b/docs/reviews/2026-05-17-NEO-48.md @@ -3,11 +3,11 @@ **Date:** 2026-05-17 **Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `5290044`–`39c54c3` vs `main` **Base:** `main` -**Follow-up:** Code-review suggestions 1–3 and nit 1 addressed in a follow-up commit on the same branch. +**Follow-up:** Suggestions 1–3 and nit 1 addressed on branch; r2 follow-up (`aba2644`) closed fixture/testing items — see [`2026-05-17-NEO-48-r2.md`](2026-05-17-NEO-48-r2.md). ## Verdict -**Approve with nits** +**Approve** ## Summary @@ -39,8 +39,8 @@ None. ## Nits - ~~Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often.~~ **Done.** Folder `docs` block in `perk-state/folder.bru`. -- Nit: Path-auto tier-2 unlocks surfaced in **`unlockedEvents`** use **`source: level_up`** (not `branch_pick`) — consistent with NEO-47/NEO-49 notes; clients/telemetry should treat `source` as unlock channel, not HTTP verb. -- Nit: `PostPerkState_ShouldSelectTier1_AndGetReflectsPick_AfterSalvageLevel2` issues a follow-up **GET** inside **Assert** for end-to-end verification — valid AAA (verification in Assert); grant status checks in **Arrange** match other API tests in the repo. +- ~~Nit: Path-auto tier-2 unlocks surfaced in **`unlockedEvents`** use **`source: level_up`** (not `branch_pick`) — consistent with NEO-47/NEO-49 notes; clients/telemetry should treat `source` as unlock channel, not HTTP verb.~~ **Deferred.** Documented in NEO-47/NEO-49; no API change required for NEO-48. +- ~~Nit: `PostPerkState_ShouldSelectTier1_AndGetReflectsPick_AfterSalvageLevel2` issues a follow-up **GET** inside **Assert** for end-to-end verification — valid AAA (verification in Assert); grant status checks in **Arrange** match other API tests in the repo.~~ **Deferred.** Accepted integration-test pattern; no change. ## Verification @@ -48,7 +48,7 @@ None. # From repo root dotnet test NeonSprawl.sln --filter "FullyQualifiedName~PerkStateApi" -# Full server suite (185 tests at review) +# Full server suite (195 tests at final verification) dotnet test NeonSprawl.sln # Manual / Bruno From aa837907036504d6edbc47304ff9bf43610d7ae0 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:46:28 -0400 Subject: [PATCH 12/14] NEO-48: omit null reasonCode from success JSON responses WhenWritingNull on PerkBranchSelectResponse and SkillProgressionGrantResponse matches hotbar/cast/interaction; fixes Bruno tier1 undefined check. --- .../perk-state/Post branch select tier1.bru | 1 + .../Game/Mastery/PerkStateApiTests.cs | 9 ++++++++- server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs | 1 + .../Game/Skills/SkillProgressionGrantDtos.cs | 1 + 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru b/bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru index 1c7d8c7..4c8ffc2 100644 --- a/bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru +++ b/bruno/neon-sprawl-server/perk-state/Post branch select tier1.bru @@ -6,6 +6,7 @@ meta { docs { NEO-48: tier-1 salvage branch pick. Run skill-progression grant (100+ XP for level 2) first if this denies level_too_low. + On success, reasonCode is omitted from JSON (not null); test expects undefined. } post { diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs index d83c843..d6372a4 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs @@ -2,6 +2,7 @@ using System.Linq; using System.Net; using System.Net.Http; using System.Net.Http.Json; +using System.Text.Json; using System.Text; using NeonSprawl.Server.Game.Mastery; using NeonSprawl.Server.Game.Skills; @@ -156,7 +157,13 @@ public sealed class PerkStateApiTests // Assert Assert.Equal(HttpStatusCode.OK, post.StatusCode); - var envelope = await post.Content.ReadFromJsonAsync(); + var json = await post.Content.ReadAsStringAsync(); + using (var doc = JsonDocument.Parse(json)) + { + Assert.False(doc.RootElement.TryGetProperty("reasonCode", out _)); + } + + var envelope = JsonSerializer.Deserialize(json); Assert.NotNull(envelope); Assert.True(envelope!.Selected); Assert.Null(envelope.ReasonCode); diff --git a/server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs b/server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs index f2f6a78..e45a1a5 100644 --- a/server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs +++ b/server/NeonSprawl.Server/Game/Mastery/PerkStateDtos.cs @@ -69,6 +69,7 @@ public sealed class PerkBranchSelectResponse public bool Selected { get; init; } [JsonPropertyName("reasonCode")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? ReasonCode { get; init; } [JsonPropertyName("perkState")] diff --git a/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs b/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs index b853314..d3ba3e9 100644 --- a/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs +++ b/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs @@ -34,6 +34,7 @@ public sealed class SkillProgressionGrantResponse /// Set when is false; stable snake_case values (see README). [JsonPropertyName("reasonCode")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? ReasonCode { get; init; } [JsonPropertyName("progression")] From 12e52bccc90a3c0e89c8def66f9348531132581c Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:46:55 -0400 Subject: [PATCH 13/14] NEO-48: fix in-memory TrySetSkillXpTotal zero on absent skill Setting XP to 0 is a no-op success when no row exists, matching Postgres DELETE semantics and TrySetSkillXpTotals behavior. --- ...nMemoryPlayerSkillProgressionStoreTests.cs | 23 +++++++++++++++++++ .../InMemoryPlayerSkillProgressionStore.cs | 3 ++- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 server/NeonSprawl.Server.Tests/Game/Skills/InMemoryPlayerSkillProgressionStoreTests.cs diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/InMemoryPlayerSkillProgressionStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/InMemoryPlayerSkillProgressionStoreTests.cs new file mode 100644 index 0000000..8b4fe69 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/InMemoryPlayerSkillProgressionStoreTests.cs @@ -0,0 +1,23 @@ +using Microsoft.Extensions.DependencyInjection; +using NeonSprawl.Server.Game.Skills; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +public sealed class InMemoryPlayerSkillProgressionStoreTests +{ + [Fact] + public void TrySetSkillXpTotal_WhenXpZeroAndSkillAbsent_ShouldReturnTrue() + { + // Arrange + using var factory = new InMemoryWebApplicationFactory(); + var store = factory.Services.GetRequiredService(); + + // Act + var applied = store.TrySetSkillXpTotal("dev-local-1", "intrusion", xp: 0); + + // Assert + Assert.True(applied); + Assert.False(store.GetXpTotals("dev-local-1").ContainsKey("intrusion")); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs index fa1a1bd..acab145 100644 --- a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs +++ b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs @@ -82,7 +82,8 @@ public sealed class InMemoryPlayerSkillProgressionStore(IOptions Date: Sun, 17 May 2026 20:59:37 -0400 Subject: [PATCH 14/14] NEO-48: revert WhenWritingNull on skill-progression grant API Keep omit-null reasonCode only on new perk-state responses; restoring the existing grant endpoint wire format (reasonCode: null on success). --- .../Post skill progression grant.bru | 2 ++ .../Skills/SkillProgressionGrantApiTests.cs | 21 +++++++++++++++++++ .../Game/Skills/SkillProgressionGrantDtos.cs | 1 - 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru b/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru index 4ccbead..9595d4a 100644 --- a/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru +++ b/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru @@ -8,6 +8,7 @@ docs { NEO-38: single skill XP grant + allowlist + level-up metadata; see server README. NEO-40: same contract; server adds comment-only telemetry hook markers (no request/response change). NEO-47: on level-up, server re-evaluates mastery perk unlocks internally (no JSON change until NEO-48 perk-state HTTP). + Success responses include reasonCode: null (stable since NEO-38); perk-state POST omits reasonCode on success instead. E2.M2 tracking row in docs/decomposition/modules/documentation_and_implementation_alignment.md lists NEO-40 landed with plan + manual QA links. } @@ -32,6 +33,7 @@ tests { const body = res.getBody(); expect(body.schemaVersion).to.equal(1); expect(body.granted).to.equal(true); + expect(body.reasonCode).to.equal(null); expect(body.progression).to.be.an("object"); expect(body.progression.schemaVersion).to.equal(1); expect(body.progression.playerId).to.equal("dev-local-1"); diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs index 57657a9..3d142c9 100644 --- a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs @@ -1,6 +1,7 @@ using System.Linq; using System.Net; using System.Net.Http.Json; +using System.Text.Json; using NeonSprawl.Server.Game.Skills; using Xunit; @@ -120,6 +121,26 @@ public sealed class SkillProgressionGrantApiTests Assert.Equal(SkillProgressionSnapshotApi.ReasonInvalidAmount, envelope.ReasonCode); } + [Fact] + public async Task PostSkillProgression_ShouldSerializeReasonCodeNull_OnSuccess() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/skill-progression", + ValidGrant("salvage", amount: 10)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var json = await response.Content.ReadAsStringAsync(); + using var doc = JsonDocument.Parse(json); + Assert.True(doc.RootElement.TryGetProperty("reasonCode", out var reasonCode)); + Assert.Equal(JsonValueKind.Null, reasonCode.ValueKind); + } + [Fact] public async Task PostSkillProgression_ShouldApplyGrant_AndGetMatches_WhenTrainerAllowedForRefine() { diff --git a/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs b/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs index d3ba3e9..b853314 100644 --- a/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs +++ b/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs @@ -34,7 +34,6 @@ public sealed class SkillProgressionGrantResponse /// Set when is false; stable snake_case values (see README). [JsonPropertyName("reasonCode")] - [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? ReasonCode { get; init; } [JsonPropertyName("progression")]