From 58b9016d40a0153b38d46adcfe4131aee6656dba Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 6 May 2026 22:11:16 -0400 Subject: [PATCH 1/8] NEO-38: add implementation plan for skill XP grant and level-up --- docs/plans/NEO-38-implementation-plan.md | 107 +++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 docs/plans/NEO-38-implementation-plan.md diff --git a/docs/plans/NEO-38-implementation-plan.md b/docs/plans/NEO-38-implementation-plan.md new file mode 100644 index 0000000..e8d5c7f --- /dev/null +++ b/docs/plans/NEO-38-implementation-plan.md @@ -0,0 +1,107 @@ +# NEO-38 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-38 | +| **Title** | Apply skill XP grant + allowlist + level-up | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-38/apply-skill-xp-grant-allowlist-level-up | +| **Module** | [E2.M2 — XpAwardAndLevelEngine](../decomposition/modules/E2_M2_XpAwardAndLevelEngine.md) | +| **Blocked by** | [NEO-37](https://linear.app/neon-sprawl/issue/NEO-37) (read snapshot) — implemented on `main`; this branch builds on that contract. | + +## Kickoff clarifications + +| Topic | Question / note | Resolution | +|--------|-----------------|------------| +| **Persistence** | NEO-38 AC: follow NEO-29 pattern vs in-memory-only + follow-up | **User:** mirror **Hotbar / NEO-29** — abstract store, **`InMemory*`** fallback, **`Postgres*`** when `ConnectionStrings__NeonSprawl` is set, with schema bootstrap analogous to **`PostgresHotbarLoadoutBootstrap`**. | +| **Successful POST body** | Surface level-up in response vs minimal + GET only | User asked for recommendation → **Agent:** **HTTP 200** with **versioned JSON** that includes **full progression snapshot** (aligned with **`GET …/skill-progression`**) plus an explicit **`levelUps`** (or equivalent) array for thresholds crossed on this grant. **User:** confirmed snapshot + level-ups. | + +## Goal, scope, and out-of-scope + +**Goal:** Server-authoritative **apply XP grant** path: validate **`skillId`** (`ISkillDefinitionRegistry.TryGetDefinition`) and **`sourceKind`** against **`SkillDefRow.AllowedXpSourceKinds`**; persist per-player XP; recompute level from an **inline placeholder curve** (acceptable until [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39)); return **structured denies** (stable reason codes, hotbar-style **200 + `granted: false`** where that pattern already fits the repo) and **level-up** data on success. + +**In scope (from Linear):** + +- Reject unknown skill id and disallowed `sourceKind` with **stable reason codes**. +- Valid grant updates persistence; **level** increases when thresholds crossed (**`LevelUpEvent`** semantics surfaced in JSON for this story). +- Versioned **POST** + automated tests; **Bruno** requests; **`server/README.md`**. +- Persistence: **in-memory + Postgres when configured**, per kickoff decision. + +**Out of scope (from Linear):** + +- Data-driven level curve file + CI (**NEO-39**). +- Telemetry catalog (**NEO-40**). +- Slice 3 activity wiring (**NEO-41**–**43**). + +## Acceptance criteria checklist + +- [ ] Unknown `skillId` rejected; `sourceKind` not in allowlist rejected (**stable reason codes**). +- [ ] Valid grant updates stored progression; level increases when thresholds crossed (**level-up** payload on success). +- [ ] Versioned **POST** + tests; Bruno; README. +- [ ] Persistence: **NEO-29-style** dual backend (in-memory + Postgres when configured). + +## Technical approach + +1. **Route:** **`POST /game/players/{id}/skill-progression`** (same resource as NEO-37 **GET**, mirroring **`GET` + `POST` …/hotbar-loadout**). **Reject** missing/invalid **`schemaVersion`** with **400** (same idea as hotbar update). + +2. **Player gate:** **`404`** when **`!IPositionStateStore.TryGetPosition`**, unchanged from NEO-37. + +3. **Request body (versioned):** e.g. **`schemaVersion`**, **`skillId`**, **`amount`** (positive int), **`sourceKind`** (string matching catalog values: `activity`, `mission_reward`, `trainer`, `book_or_item`). Single grant per request for a small, testable surface; batch grants can be a later issue if needed. + +4. **Validation:** Use **`ISkillDefinitionRegistry`**. If unknown skill → deny with **`unknown_skill`** (exact string TBD but stable and documented). If known skill but **`sourceKind`** not in **`AllowedXpSourceKinds`** (case/whitespace normalization consistent with catalog) → **`source_kind_not_allowed`**. Non-positive **`amount`** → **`invalid_amount`** (or reuse a generic **`bad_grant`** only if we want fewer codes — prefer specific codes per AC). + +5. **Persistence:** Introduce **`IPlayerSkillProgressionStore`** (name may match existing naming conventions) with operations sufficient to **read XP per skill** and **apply a delta** atomically per request, returning new XP and whether level changed. **In-memory** implementation keyed by normalized player id; **Postgres** implementation with **`EnsureSchema`**, **`player_id` + `skill_id`** uniqueness, XP column (level **derived** from XP + curve helper to avoid drift). **No row** for a skill ⇒ treat as **XP 0** / **level 1** at read time (GET and POST deny paths that return current snapshot). + +6. **GET snapshot (NEO-37 evolution):** Replace the hard-coded **0 / 1** builder in **`SkillProgressionSnapshotApi.BuildSnapshot`** with **registry + store merge** so persisted XP appears on **`GET /game/players/{id}/skill-progression`**. Preserve **`schemaVersion` 1** if the JSON shape is unchanged (only values differ). Keep **ordinal sort** for stable serialization; **unordered-by-contract** rule unchanged. + +7. **Level curve (placeholder):** Inline C# thresholds or formula (documented in code comments, **subsumed by NEO-39**). Example pattern: simple per-level XP thresholds up to a small cap so tests can cross **at least one** level-up with realistic amounts. + +8. **Success response:** **HTTP 200**, versioned envelope: e.g. **`granted: true`**, **`progression`** (same shape as **`SkillProgressionSnapshotResponse`** or a shared type), **`levelUps`**: array of **`{ skillId, previousLevel, newLevel }`** (field names camelCase JSON). Empty **`levelUps`** when no threshold crossed. + +9. **Deny response:** **HTTP 200** with **`granted: false`**, **`reasonCode`**, and current **`progression`** snapshot (hotbar **`Updated: false`** pattern) so clients always receive authoritative state for UI sync — unless a case clearly fits **400** (malformed body / wrong schema version). + +10. **Wire-up:** **`AddSkillProgressionStore`** (or similar) in **`Program.cs`** next to **`AddHotbarLoadoutStore`**. **`InMemoryWebApplicationFactory`** forces the in-memory implementation for isolated tests (same replacement pattern as hotbar/position). + +11. **Bruno / docs:** Add **`Post … skill progression grant.bru`** (exact name follows collection style) under **`bruno/neon-sprawl-server/skill-progression/`**; extend **`server/README.md`**. Add **`docs/manual-qa/NEO-38.md`** during implementation per project convention. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs` | Abstraction: read XP map / apply grant for one player+skill. | +| `server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs` | In-memory persistence; normalized player keys. | +| `server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs` | Postgres implementation when connection string present. | +| `server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs` | Schema ensure (mirror hotbar bootstrap style). | +| `server/NeonSprawl.Server/Game/Skills/SkillProgressionServiceCollectionExtensions.cs` | Chooses Postgres vs in-memory from configuration. | +| `server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs` | Versioned POST request + success/deny response DTOs (`levelUps`, `reasonCode`, etc.). | +| `server/NeonSprawl.Server/Game/Skills/SkillLevelCurvePlaceholder.cs` | Inline threshold/floor helpers until NEO-39 (pure functions, unit-testable). | +| `server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs` | In-memory host: unknown skill, bad source kind, success, level-up, GET reflects POST, **404** player gate, bad schema → **400**. | +| `server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs` | Postgres harness: grant survives new host/process (mirror **`HotbarLoadoutPersistenceIntegrationTests`**). | +| `bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru` | Manual smoke for happy + one deny path. | +| `docs/manual-qa/NEO-38.md` | Curl/Bruno checklist for grant + GET verification. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs` | Register **POST**; inject store; change **GET** builder to merge registry + stored XP via placeholder level curve. | +| `server/NeonSprawl.Server/Program.cs` | Register **`AddSkillProgressionStore`** and ensure catalog/store order remains valid. | +| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Swap/replace **`IPlayerSkillProgressionStore`** with in-memory implementation for tests. | +| `server/README.md` | Document **POST** grant, reason codes, persistence note, Bruno path, links to plan + manual QA. | + +## Tests + +| Test file | What it covers | +|-----------|----------------| +| `SkillProgressionGrantApiTests.cs` | **AAA** integration tests: denies (**unknown_skill**, **source_kind_not_allowed**); success increases XP; **levelUps** when threshold crossed; **GET** matches after grant; **404** without position; malformed / wrong **`schemaVersion`** → **400**. | +| `SkillProgressionGrantPersistenceIntegrationTests.cs` | Postgres: write grant, new factory/client, **GET** (or POST read-back) shows persisted XP. | +| `SkillProgressionSnapshotApiTests.cs` (existing) | Update expectations if **GET** implementation changes from hard-coded zeros to store-backed merge (same defaults when store empty). | + +Bruno + **`docs/manual-qa/NEO-38.md`** supplement automated coverage. + +## Open questions / risks + +- **Reason code naming:** Final strings should be listed once in README and reused in tests/Bruno to avoid drift (prefer **`snake_case`** consistent with **`HotbarLoadoutApi`** constants). +- **Placeholder curve vs. content:** Keep thresholds boring and documented; **NEO-39** replaces the helper without changing persistence schema if XP remains the stored value. +- **Concurrency:** Postgres store should use a transaction per grant; in-memory uses concurrent structures or per-player locks if needed for tests under parallel load (prototype: acceptable to document single-threaded assumption if minimal). From 33b2e917e46537cf4083b390be0d4562cd3e8bf3 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 6 May 2026 22:17:02 -0400 Subject: [PATCH 2/8] NEO-38: skill XP POST grant, persistence, level-up response --- .../Post skill progression grant.bru | 40 ++++ docs/manual-qa/NEO-38.md | 29 +++ docs/plans/NEO-38-implementation-plan.md | 8 +- .../Skills/SkillProgressionGrantApiTests.cs | 190 ++++++++++++++++++ ...ressionGrantPersistenceIntegrationTests.cs | 77 +++++++ .../InMemoryWebApplicationFactory.cs | 2 + .../NeonSprawl.Server.Tests.csproj | 2 +- .../Skills/IPlayerSkillProgressionStore.cs | 14 ++ .../InMemoryPlayerSkillProgressionStore.cs | 81 ++++++++ .../PostgresPlayerSkillProgressionStore.cs | 121 +++++++++++ .../PostgresSkillProgressionBootstrap.cs | 39 ++++ .../Game/Skills/SkillLevelCurvePlaceholder.cs | 16 ++ .../Game/Skills/SkillProgressionGrantDtos.cs | 58 ++++++ ...lProgressionServiceCollectionExtensions.cs | 22 ++ .../Skills/SkillProgressionSnapshotApi.cs | 103 +++++++++- server/NeonSprawl.Server/Program.cs | 1 + server/README.md | 18 +- .../V003__player_skill_progression.sql | 10 + 18 files changed, 817 insertions(+), 14 deletions(-) create mode 100644 bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru create mode 100644 docs/manual-qa/NEO-38.md create mode 100644 server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs create mode 100644 server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillLevelCurvePlaceholder.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillProgressionServiceCollectionExtensions.cs create mode 100644 server/db/migrations/V003__player_skill_progression.sql 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 new file mode 100644 index 0000000..8bfa23d --- /dev/null +++ b/bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru @@ -0,0 +1,40 @@ +meta { + name: POST skill progression grant + type: http + seq: 2 +} + +docs { + NEO-38: single skill XP grant + allowlist + level-up metadata; see server README. +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/skill-progression + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "skillId": "salvage", + "amount": 10, + "sourceKind": "activity" + } +} + +tests { + test("grant returns 200 with granted true and progression", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.granted).to.equal(true); + expect(body.progression).to.be.an("object"); + expect(body.progression.schemaVersion).to.equal(1); + expect(body.progression.playerId).to.equal("dev-local-1"); + expect(body.levelUps).to.be.an("array"); + const salvage = body.progression.skills.find((x) => x.id === "salvage"); + expect(salvage).to.be.an("object"); + expect(salvage.xp).to.be.a("number"); + }); +} diff --git a/docs/manual-qa/NEO-38.md b/docs/manual-qa/NEO-38.md new file mode 100644 index 0000000..72e4a80 --- /dev/null +++ b/docs/manual-qa/NEO-38.md @@ -0,0 +1,29 @@ +# Manual QA — NEO-38 (skill XP grant) + +Reference: [implementation plan](../plans/NEO-38-implementation-plan.md), [server README](../../server/README.md#skill-progression-grant-neo-38). + +## Preconditions + +- Run `NeonSprawl.Server` from `server/NeonSprawl.Server` (`dotnet run`; default `http://localhost:5253`). +- Use a **known** player id (e.g. `dev-local-1`, present in position state). + +## Happy path — grant + read back + +1. `POST /game/players/dev-local-1/skill-progression` with JSON: + - `schemaVersion` **1** + - `skillId` **`salvage`** + - `amount` **10** + - `sourceKind` **`activity`** +2. Expect **HTTP 200**, body `granted` **true**, `progression.skills` contains `salvage` with increased `xp` vs a prior `GET`. +3. `GET /game/players/dev-local-1/skill-progression` — **`salvage.xp`** matches the POST `progression`. + +## Deny — source not allowed + +1. `POST` same path with `skillId` **`salvage`**, `sourceKind` **`trainer`** (not in prototype catalog for salvage). +2. Expect **HTTP 200**, `granted` **false**, **`reasonCode`** **`source_kind_not_allowed`**, and `progression` still present. + +## Bruno + +- Folder: `bruno/neon-sprawl-server/skill-progression/` +- `Post skill progression grant.bru` — happy grant. +- `Get skill progression.bru` — confirms read model after grants. diff --git a/docs/plans/NEO-38-implementation-plan.md b/docs/plans/NEO-38-implementation-plan.md index e8d5c7f..54fc607 100644 --- a/docs/plans/NEO-38-implementation-plan.md +++ b/docs/plans/NEO-38-implementation-plan.md @@ -36,10 +36,10 @@ ## Acceptance criteria checklist -- [ ] Unknown `skillId` rejected; `sourceKind` not in allowlist rejected (**stable reason codes**). -- [ ] Valid grant updates stored progression; level increases when thresholds crossed (**level-up** payload on success). -- [ ] Versioned **POST** + tests; Bruno; README. -- [ ] Persistence: **NEO-29-style** dual backend (in-memory + Postgres when configured). +- [x] Unknown `skillId` rejected; `sourceKind` not in allowlist rejected (**stable reason codes**). +- [x] Valid grant updates stored progression; level increases when thresholds crossed (**level-up** payload on success). +- [x] Versioned **POST** + tests; Bruno; README. +- [x] Persistence: **NEO-29-style** dual backend (in-memory + Postgres when configured). ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs new file mode 100644 index 0000000..8688b41 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantApiTests.cs @@ -0,0 +1,190 @@ +using System.Linq; +using System.Net; +using System.Net.Http.Json; +using NeonSprawl.Server.Game.Skills; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +public sealed class SkillProgressionGrantApiTests +{ + private static SkillProgressionGrantRequest Grant( + string skillId, + int schemaVersion, + int amount = 50, + string sourceKind = "activity") => + new SkillProgressionGrantRequest + { + SchemaVersion = schemaVersion, + SkillId = skillId, + Amount = amount, + SourceKind = sourceKind, + }; + + private static SkillProgressionGrantRequest ValidGrant(string skillId, int amount = 50, string sourceKind = "activity") => + Grant(skillId, SkillProgressionGrantRequest.CurrentSchemaVersion, amount, sourceKind); + + [Fact] + public async Task PostSkillProgression_ShouldReturnBadRequest_WhenSchemaVersionMismatch() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var request = Grant("salvage", SkillProgressionGrantRequest.CurrentSchemaVersion + 42, amount: 1); + + // Act + var response = await client.PostAsJsonAsync("/game/players/dev-local-1/skill-progression", request); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostSkillProgression_ShouldReturnNotFound_WhenPlayerUnknown() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsJsonAsync( + "/game/players/missing-player/skill-progression", + ValidGrant("salvage")); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task PostSkillProgression_ShouldDenyUnknownSkill_WithReasonUnknownSkill_AndEchoProgression() + { + // 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("not-a-prototype-skill")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Granted); + Assert.Equal(SkillProgressionSnapshotApi.ReasonUnknownSkill, envelope.ReasonCode); + Assert.NotNull(envelope.Progression); + var salvage = envelope.Progression!.Skills!.Single(static s => s.Id == "salvage"); + Assert.Equal(0, salvage.Xp); + Assert.Equal(1, salvage.Level); + } + + [Fact] + public async Task PostSkillProgression_ShouldDenyTrainerOnSalvage_WithReasonSourceKindNotAllowed() + { + // Arrange — prototype_skills: salvage allowedXpSourceKinds = activity, mission_reward only + 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, sourceKind: "trainer")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Granted); + Assert.Equal(SkillProgressionSnapshotApi.ReasonSourceKindNotAllowed, envelope.ReasonCode); + } + + [Fact] + public async Task PostSkillProgression_ShouldDenyNonPositiveAmount_WithReasonInvalidAmount() + { + // 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: 0)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Granted); + Assert.Equal(SkillProgressionSnapshotApi.ReasonInvalidAmount, envelope.ReasonCode); + } + + [Fact] + public async Task PostSkillProgression_ShouldApplyGrant_AndGetMatches_WhenTrainerAllowedForRefine() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var post = await client.PostAsJsonAsync( + "/game/players/dev-local-1/skill-progression", + ValidGrant("refine", amount: 25, sourceKind: "trainer")); + + // Assert + Assert.Equal(HttpStatusCode.OK, post.StatusCode); + var envelope = await post.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.True(envelope!.Granted); + Assert.Empty(envelope.LevelUps); + Assert.Single(envelope.Progression!.Skills!, static s => s.Id == "refine" && s.Xp == 25 && s.Level == 1); + + var get = await client.GetFromJsonAsync( + "/game/players/dev-local-1/skill-progression"); + Assert.NotNull(get); + var refine = Assert.Single(get!.Skills!, static s => s.Id == "refine"); + Assert.Equal(25, refine.Xp); + Assert.Equal(1, refine.Level); + } + + [Fact] + public async Task PostSkillProgression_ShouldReportLevelUps_WhenThresholdCrossed() + { + // Arrange — placeholder curve: +100 xp from 0 → level 2 + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var post = await client.PostAsJsonAsync( + "/game/players/dev-local-1/skill-progression", + ValidGrant("intrusion", amount: 100, sourceKind: "activity")); + + // Assert + var envelope = await post.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.True(envelope!.Granted); + var up = Assert.Single(envelope.LevelUps!); + Assert.Equal("intrusion", up.SkillId); + Assert.Equal(1, up.PreviousLevel); + Assert.Equal(2, up.NewLevel); + Assert.Single(envelope.Progression!.Skills!, static s => s.Id == "intrusion" && s.Xp == 100 && s.Level == 2); + } + + [Fact] + public async Task PostSkillProgression_ShouldAcceptSourceKind_WithIgnoreCase_OnAllowedList() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var post = await client.PostAsJsonAsync( + "/game/players/dev-local-1/skill-progression", + ValidGrant("salvage", amount: 5, sourceKind: "ACTIVITY")); + + // Assert + var envelope = await post.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.True(envelope!.Granted); + } +} diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs new file mode 100644 index 0000000..43293dc --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs @@ -0,0 +1,77 @@ +using System.Net.Http.Json; +using NeonSprawl.Server.Game.Skills; +using NeonSprawl.Server.Tests.Game.PositionState; +using Npgsql; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Skills; + +[Collection("Postgres integration")] +public sealed class SkillProgressionGrantPersistenceIntegrationTests(PostgresIntegrationHarness harness) +{ + private PostgresWebApplicationFactory Factory => harness.Factory; + + [RequirePostgresFact] + public async Task PostGrantThenGetAcrossNewFactory_ShouldPersistSkillXp() + { + // Arrange + await ResetProgressionTableAsync(); + var grant = new SkillProgressionGrantRequest + { + SchemaVersion = SkillProgressionGrantRequest.CurrentSchemaVersion, + SkillId = "salvage", + Amount = 50, + SourceKind = "activity", + }; + HttpResponseMessage postResponse; + + // Act + using (var firstClient = Factory.CreateClient()) + { + postResponse = await firstClient.PostAsJsonAsync( + "/game/players/dev-local-1/skill-progression", + grant); + } + + await using var secondFactory = new PostgresWebApplicationFactory(); + using var secondClient = secondFactory.CreateClient(); + var snapshot = + await secondClient.GetFromJsonAsync( + "/game/players/dev-local-1/skill-progression"); + + // Assert + Assert.True(postResponse.IsSuccessStatusCode); + Assert.NotNull(snapshot); + var salvage = Assert.Single(snapshot!.Skills!, static s => s.Id == "salvage"); + Assert.Equal(50, salvage.Xp); + Assert.Equal(1, salvage.Level); + } + + private static async Task ResetProgressionTableAsync() + { + var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); + if (string.IsNullOrWhiteSpace(cs)) + { + throw new InvalidOperationException("ConnectionStrings__NeonSprawl is not set."); + } + + var ddlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V003__player_skill_progression.sql"); + if (!File.Exists(ddlPath)) + { + throw new FileNotFoundException($"Test DDL not found at '{ddlPath}'.", ddlPath); + } + + var ddl = await File.ReadAllTextAsync(ddlPath); + await using var conn = new NpgsqlConnection(cs); + await conn.OpenAsync(); + await using (var apply = new NpgsqlCommand(ddl, conn)) + { + await apply.ExecuteNonQueryAsync(); + } + + await using (var truncate = new NpgsqlCommand("TRUNCATE player_skill_progression;", conn)) + { + await truncate.ExecuteNonQueryAsync(); + } + } +} diff --git a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs index 7395778..69e4047 100644 --- a/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs +++ b/server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs @@ -31,6 +31,7 @@ public sealed class InMemoryWebApplicationFactory : WebApplicationFactory(fakeTime); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); }); } diff --git a/server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj b/server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj index 22d6cbb..cd49d16 100644 --- a/server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj +++ b/server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj @@ -27,7 +27,7 @@ - + diff --git a/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs new file mode 100644 index 0000000..bd183ed --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/IPlayerSkillProgressionStore.cs @@ -0,0 +1,14 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// Persisted totals of skill XP per player (NEO-38); level is derived, not stored. +public interface IPlayerSkillProgressionStore +{ + /// Keyed by catalog SkillDef.id; omitted skills imply XP 0. + IReadOnlyDictionary GetXpTotals(string playerId); + + /// + /// Adds to for . + /// 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); +} diff --git a/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs new file mode 100644 index 0000000..cadd39f --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/InMemoryPlayerSkillProgressionStore.cs @@ -0,0 +1,81 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.Options; +using NeonSprawl.Server.Game.PositionState; + +namespace NeonSprawl.Server.Game.Skills; + +/// Thread-safe in-memory progression; seeds the configured dev player with an empty XP map (NEO-38). +public sealed class InMemoryPlayerSkillProgressionStore(IOptions options) : IPlayerSkillProgressionStore +{ + private readonly ConcurrentDictionary> byPlayer = CreateInitialMap(options.Value); + + /// Locks per normalized player id for coherent read/compare/write on inner dictionaries. + private readonly ConcurrentDictionary playerLocks = new(StringComparer.OrdinalIgnoreCase); + + private static ConcurrentDictionary> CreateInitialMap(GamePositionOptions o) + { + var id = NormalizePlayerId(o.DevPlayerId); + var map = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); + map[id] = new ConcurrentDictionary(StringComparer.Ordinal); + return map; + } + + /// + public IReadOnlyDictionary GetXpTotals(string playerId) + { + var key = NormalizePlayerId(playerId); + if (key.Length == 0 || !byPlayer.TryGetValue(key, out var inner)) + { + return ReadOnlyDic.Empty; + } + + lock (playerLocks.GetOrAdd(key, _ => new object())) + { + return new Dictionary(inner, StringComparer.Ordinal); + } + } + + /// + public bool TryApplyXpDelta(string playerId, string skillId, int amount, out int previousXp, out int newXp) + { + previousXp = 0; + newXp = 0; + var key = NormalizePlayerId(playerId); + if (key.Length == 0 || skillId.Trim().Length == 0 || !byPlayer.TryGetValue(key, out var inner)) + { + return false; + } + + var sid = skillId.Trim(); + + lock (playerLocks.GetOrAdd(key, _ => new object())) + { + inner.TryGetValue(sid, out var prev); + previousXp = prev; + newXp = prev + amount; + if (newXp < 0) + { + return false; + } + + inner[sid] = newXp; + return true; + } + } + + private static string NormalizePlayerId(string? playerId) + { + var t = playerId?.Trim(); + if (string.IsNullOrEmpty(t)) + { + return string.Empty; + } + + return t.ToLowerInvariant(); + } + + private static class ReadOnlyDic + { + internal static readonly IReadOnlyDictionary Empty = new Dictionary(StringComparer.Ordinal); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs b/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs new file mode 100644 index 0000000..0a0f221 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/PostgresPlayerSkillProgressionStore.cs @@ -0,0 +1,121 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// PostgreSQL-backed skill XP totals keyed by normalized player id (NEO-38). +public sealed class PostgresPlayerSkillProgressionStore(Npgsql.NpgsqlDataSource dataSource) : IPlayerSkillProgressionStore +{ + /// + public IReadOnlyDictionary GetXpTotals(string playerId) + { + var norm = NormalizePlayerId(playerId); + if (norm.Length == 0) + { + return new Dictionary(StringComparer.Ordinal); + } + + PostgresSkillProgressionBootstrap.EnsureSchema(dataSource); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand( + """ + SELECT skill_id, xp + FROM player_skill_progression + WHERE player_id = @pid; + """, + conn); + cmd.Parameters.AddWithValue("pid", norm); + var map = new Dictionary(StringComparer.Ordinal); + using var reader = cmd.ExecuteReader(); + while (reader.Read()) + { + map[reader.GetString(0)] = reader.GetInt32(1); + } + + return map; + } + + /// + public bool TryApplyXpDelta(string playerId, string skillId, int amount, out int previousXp, out int newXp) + { + previousXp = 0; + newXp = 0; + 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; + } + + using (var sel = new Npgsql.NpgsqlCommand( + """ + SELECT xp + FROM player_skill_progression + WHERE player_id = @pid AND skill_id = @sid + LIMIT 1 + FOR UPDATE; + """, + conn, + tx)) + { + sel.Parameters.AddWithValue("pid", norm); + sel.Parameters.AddWithValue("sid", sid); + var scalar = sel.ExecuteScalar(); + previousXp = scalar is null || Equals(scalar, DBNull.Value) + ? 0 + : scalar is int xi + ? xi + : Convert.ToInt32(scalar, System.Globalization.CultureInfo.InvariantCulture); + } + + newXp = previousXp + amount; + if (newXp < 0) + { + tx.Rollback(); + return false; + } + + 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", newXp); + upsert.ExecuteNonQuery(); + tx.Commit(); + return true; + } + + private static bool PlayerExists(Npgsql.NpgsqlConnection conn, string playerIdNormalized, Npgsql.NpgsqlTransaction tx) + { + using var cmd = new Npgsql.NpgsqlCommand( + "SELECT 1 FROM player_position WHERE player_id = @pid LIMIT 1;", + conn, + tx); + cmd.Parameters.AddWithValue("pid", playerIdNormalized); + return cmd.ExecuteScalar() is not null; + } + + private static string NormalizePlayerId(string? playerId) + { + var t = playerId?.Trim(); + if (string.IsNullOrEmpty(t)) + { + return string.Empty; + } + + return t.ToLowerInvariant(); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs b/server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs new file mode 100644 index 0000000..2eb3844 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/PostgresSkillProgressionBootstrap.cs @@ -0,0 +1,39 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// Applies NEO-38 skill progression table DDL once per process. +public static class PostgresSkillProgressionBootstrap +{ + private static readonly string DdlRelativePath = Path.Combine("db", "migrations", "V003__player_skill_progression.sql"); + + private static readonly object SchemaGate = new(); + + private static int _schemaReady; + + public static void EnsureSchema(Npgsql.NpgsqlDataSource dataSource) + { + if (System.Threading.Volatile.Read(ref _schemaReady) != 0) + { + return; + } + + lock (SchemaGate) + { + if (System.Threading.Volatile.Read(ref _schemaReady) != 0) + { + return; + } + + var ddlPath = Path.Combine(AppContext.BaseDirectory, DdlRelativePath); + if (!File.Exists(ddlPath)) + { + throw new FileNotFoundException($"NEO-38 DDL not found at '{ddlPath}'.", ddlPath); + } + + var ddl = File.ReadAllText(ddlPath); + using var conn = dataSource.OpenConnection(); + using var cmd = new Npgsql.NpgsqlCommand(ddl, conn); + cmd.ExecuteNonQuery(); + System.Threading.Volatile.Write(ref _schemaReady, 1); + } + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillLevelCurvePlaceholder.cs b/server/NeonSprawl.Server/Game/Skills/SkillLevelCurvePlaceholder.cs new file mode 100644 index 0000000..055ebca --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillLevelCurvePlaceholder.cs @@ -0,0 +1,16 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// +/// Inline level curve replaced by content-driven thresholds in NEO-39. Rule (v1): level = 1 + floor(xp / 100) for non-negative XP (uncapped prototype). +/// +public static class SkillLevelCurvePlaceholder +{ + internal const int XpPerLevelStep = 100; + + /// Returns skill level (>= 1) for the given total accumulated XP. + public static int LevelFromTotalXp(int totalXp) + { + var xp = Math.Max(0, totalXp); + return 1 + (xp / XpPerLevelStep); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs b/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs new file mode 100644 index 0000000..b853314 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillProgressionGrantDtos.cs @@ -0,0 +1,58 @@ +using System.Text.Json.Serialization; + +namespace NeonSprawl.Server.Game.Skills; + +/// POST body for applying one skill XP grant (NEO-38). +public sealed class SkillProgressionGrantRequest +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } + + [JsonPropertyName("skillId")] + public required string SkillId { get; init; } + + [JsonPropertyName("amount")] + public int Amount { get; init; } + + /// Validated against target skill's allowedXpSourceKinds (ordinal ignore-case). + [JsonPropertyName("sourceKind")] + public required string SourceKind { get; init; } +} + +/// POST responses for progression grants — success () or structured deny (). +public sealed class SkillProgressionGrantResponse +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + [JsonPropertyName("granted")] + public bool Granted { get; init; } + + /// Set when is false; stable snake_case values (see README). + [JsonPropertyName("reasonCode")] + public string? ReasonCode { get; init; } + + [JsonPropertyName("progression")] + public required SkillProgressionSnapshotResponse Progression { get; init; } + + /// Present on success only; empty array when XP changed but level did not. + [JsonPropertyName("levelUps")] + public required IReadOnlyList LevelUps { get; init; } +} + +/// One skill crossed a level boundary on this grant (previousLevel < newLevel). +public sealed class SkillLevelUpJson +{ + [JsonPropertyName("skillId")] + public required string SkillId { get; init; } + + [JsonPropertyName("previousLevel")] + public int PreviousLevel { get; init; } + + [JsonPropertyName("newLevel")] + public int NewLevel { get; init; } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillProgressionServiceCollectionExtensions.cs b/server/NeonSprawl.Server/Game/Skills/SkillProgressionServiceCollectionExtensions.cs new file mode 100644 index 0000000..883061c --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillProgressionServiceCollectionExtensions.cs @@ -0,0 +1,22 @@ +using NeonSprawl.Server.Game.PositionState; + +namespace NeonSprawl.Server.Game.Skills; + +/// Registers skill XP persistence: PostgreSQL when configured, otherwise in-memory fallback (NEO-38). +public static class SkillProgressionServiceCollectionExtensions +{ + public static IServiceCollection AddSkillProgressionStore(this IServiceCollection services, IConfiguration configuration) + { + var cs = configuration.GetConnectionString(PositionStateServiceCollectionExtensions.NeonSprawlConnectionStringName); + if (!string.IsNullOrWhiteSpace(cs)) + { + services.AddSingleton(); + } + else + { + services.AddSingleton(); + } + + return services; + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs b/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs index 7631190..390eb55 100644 --- a/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs +++ b/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs @@ -2,14 +2,22 @@ using NeonSprawl.Server.Game.PositionState; namespace NeonSprawl.Server.Game.Skills; -/// Maps GET /game/players/{{id}}/skill-progression (NEO-37). +/// +/// Maps GET/POST /game/players/{{id}}/skill-progression — read snapshot (NEO-37) and XP grant apply (NEO-38). +/// public static class SkillProgressionSnapshotApi { + /// + public const string ReasonUnknownSkill = "unknown_skill"; + + public const string ReasonSourceKindNotAllowed = "source_kind_not_allowed"; + public const string ReasonInvalidAmount = "invalid_amount"; + public static WebApplication MapSkillProgressionSnapshotApi(this WebApplication app) { app.MapGet( "/game/players/{id}/skill-progression", - (string id, IPositionStateStore positions, ISkillDefinitionRegistry registry) => + (string id, IPositionStateStore positions, ISkillDefinitionRegistry registry, IPlayerSkillProgressionStore xpStore) => { if (!positions.TryGetPosition(id, out _)) { @@ -17,7 +25,85 @@ public static class SkillProgressionSnapshotApi } var trimmedId = id.Trim(); - return Results.Json(BuildSnapshot(trimmedId, registry)); + return Results.Json(BuildSnapshot(trimmedId, registry, xpStore)); + }); + + app.MapPost( + "/game/players/{id}/skill-progression", + (string id, SkillProgressionGrantRequest? body, IPositionStateStore positions, + ISkillDefinitionRegistry registry, IPlayerSkillProgressionStore xpStore) => + { + var trimmedId = id.Trim(); + + static SkillProgressionGrantResponse Deny( + SkillProgressionSnapshotResponse snapshot, + string reason) => + new SkillProgressionGrantResponse + { + Granted = false, + ReasonCode = reason, + Progression = snapshot, + LevelUps = [], + }; + + if (body is null || body.SchemaVersion != SkillProgressionGrantRequest.CurrentSchemaVersion) + { + return Results.BadRequest(); + } + + if (!positions.TryGetPosition(id, out _)) + { + return Results.NotFound(); + } + + var beforeSnapshot = BuildSnapshot(trimmedId, registry, xpStore); + + if (body.Amount <= 0) + { + return Results.Json(Deny(beforeSnapshot, ReasonInvalidAmount)); + } + + var skillLookup = body.SkillId.Trim(); + if (skillLookup.Length == 0 || !registry.TryGetDefinition(skillLookup, out var def)) + { + return Results.Json(Deny(beforeSnapshot, ReasonUnknownSkill)); + } + + var source = body.SourceKind.Trim(); + var allowed = def.AllowedXpSourceKinds.Any(k => + string.Equals(k, source, StringComparison.OrdinalIgnoreCase)); + if (!allowed) + { + return Results.Json(Deny(beforeSnapshot, ReasonSourceKindNotAllowed)); + } + + if (!xpStore.TryApplyXpDelta(trimmedId, def.Id, body.Amount, out var prevXp, out var newXp)) + { + return Results.NotFound(); + } + + var prevLevel = SkillLevelCurvePlaceholder.LevelFromTotalXp(prevXp); + var nextLevel = SkillLevelCurvePlaceholder.LevelFromTotalXp(newXp); + var levelUps = new List(); + if (nextLevel > prevLevel) + { + levelUps.Add( + new SkillLevelUpJson + { + SkillId = def.Id, + PreviousLevel = prevLevel, + NewLevel = nextLevel, + }); + } + + var afterSnapshot = BuildSnapshot(trimmedId, registry, xpStore); + return Results.Json( + new SkillProgressionGrantResponse + { + Granted = true, + Progression = afterSnapshot, + LevelUps = levelUps, + }); }); return app; @@ -25,22 +111,25 @@ public static class SkillProgressionSnapshotApi internal static SkillProgressionSnapshotResponse BuildSnapshot( string playerId, - ISkillDefinitionRegistry registry) + ISkillDefinitionRegistry registry, + IPlayerSkillProgressionStore store) { + var xpBySkill = store.GetXpTotals(playerId); var defs = registry.GetDefinitionsInIdOrder(); var skills = new List(defs.Count); foreach (var d in defs) { + xpBySkill.TryGetValue(d.Id, out var xp); + var level = SkillLevelCurvePlaceholder.LevelFromTotalXp(xp); skills.Add( new SkillProgressionRowJson { Id = d.Id, - Xp = 0, - Level = 1, + Xp = xp, + Level = level, }); } - // Ordinal sort for stable serialization; consumers must index rows by skill id (NEO-37). skills.Sort(static (a, b) => string.CompareOrdinal(a.Id, b.Id)); return new SkillProgressionSnapshotResponse diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index b9241ec..5a5c4a9 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -7,6 +7,7 @@ using NeonSprawl.Server.Game.Targeting; var builder = WebApplication.CreateBuilder(args); builder.Services.AddPositionStateStore(builder.Configuration); builder.Services.AddHotbarLoadoutStore(builder.Configuration); +builder.Services.AddSkillProgressionStore(builder.Configuration); builder.Services.AddAbilityCooldownStore(); builder.Services.AddSingleton(); builder.Services.AddSkillDefinitionCatalog(builder.Configuration); diff --git a/server/README.md b/server/README.md index 2f19752..4931cce 100644 --- a/server/README.md +++ b/server/README.md @@ -47,12 +47,26 @@ curl -sS -i "http://localhost:5253/game/world/skill-definitions" ## Skill progression snapshot (NEO-37) -**`GET /game/players/{id}/skill-progression`** returns a versioned JSON body (`schemaVersion` **1**, **`playerId`**, **`skills`**) with one row per registered skill (**`id`**, **`xp`**, **`level`**). Treat **`skills`** as **unordered** and index rows by **`id`** — array order is not part of the contract (the server may sort for stable serialization). Before XP grants (**NEO-38**) each row boots with **`xp`** **0** and **`level`** **1**. Unknown players (**no row in position state**) receive **404**, same gate as **`GET …/cooldown-snapshot`**. Plan: [NEO-37 implementation plan](../../docs/plans/NEO-37-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-37.md`](../../docs/manual-qa/NEO-37.md); Bruno: `bruno/neon-sprawl-server/skill-progression/`. +**`GET /game/players/{id}/skill-progression`** returns a versioned JSON body (`schemaVersion` **1**, **`playerId`**, **`skills`**) with one row per registered skill (**`id`**, **`xp`**, **`level`**). Treat **`skills`** as **unordered** and index rows by **`id`** — array order is not part of the contract (the server may sort for stable serialization). Before any stored XP, each row reflects **`xp`** **0** and **`level`** **1** (levels are derived from total XP via an inline placeholder curve until [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39): **`level = 1 + floor(xp / 100)`** for non-negative XP). Unknown players (**no row in position state**) receive **404**, same gate as **`GET …/cooldown-snapshot`**. Plan: [NEO-37 implementation plan](../../docs/plans/NEO-37-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-37.md`](../../docs/manual-qa/NEO-37.md); Bruno: `bruno/neon-sprawl-server/skill-progression/Get skill progression.bru`. ```bash curl -sS -i "http://localhost:5253/game/players/dev-local-1/skill-progression" ``` +## Skill progression grant (NEO-38) + +**`POST /game/players/{id}/skill-progression`** accepts a versioned body (`schemaVersion` **1**, **`skillId`**, **`amount`** positive int, **`sourceKind`**) and validates the skill against **`ISkillDefinitionRegistry`**, then validates **`sourceKind`** against that skill’s **`allowedXpSourceKinds`** (case-insensitive). **404** when the player is unknown to position state; **400** when **`schemaVersion`** is missing or not **1**. Persisted XP totals use the same **[NEO-29-style](#position-persistence-neo-8) split** as hotbar: **`player_skill_progression`** in PostgreSQL when **`ConnectionStrings:NeonSprawl`** is set (DDL [`V003__player_skill_progression.sql`](../db/migrations/V003__player_skill_progression.sql)), otherwise the in-memory fallback (dev bucket only, matching hotbar semantics). + +**Structured responses (HTTP 200):** **`granted`** (`true`/`false`), optional **`reasonCode`** on deny, plus **`progression`** mirroring the GET snapshot. Stable deny codes: **`unknown_skill`**, **`source_kind_not_allowed`**, **`invalid_amount`**. On **`granted: true`**, **`levelUps`** lists skills whose level increased on this request (`skillId`, `previousLevel`, `newLevel`). + +Plan: [NEO-38 implementation plan](../../docs/plans/NEO-38-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-38.md`](../../docs/manual-qa/NEO-38.md); Bruno: `bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru`. + +```bash +curl -sS -i -X POST "http://localhost:5253/game/players/dev-local-1/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":10,"sourceKind":"activity"}' +``` + ## Position persistence (NEO-8) When **`ConnectionStrings:NeonSprawl`** is set to a valid PostgreSQL connection string, the server uses the **`player_position`** table (relational columns `pos_x` … `sequence`, not JSONB). DDL lives in [`server/db/migrations/V001__player_position.sql`](../db/migrations/V001__player_position.sql); the app applies that script on startup and on first store use (idempotent `CREATE TABLE IF NOT EXISTS`). The configured dev player (`Game:DevPlayerId` / `Game:DefaultPosition`) is seeded once at host startup, matching the in-memory store’s constructor seed (not on every HTTP request). @@ -72,7 +86,7 @@ Match credentials with [root `docker-compose.yml`](../../docker-compose.yml) for **Postgres integration tests** require `ConnectionStrings__NeonSprawl` (set automatically in [`.github/workflows/dotnet.yml`](../../.github/workflows/dotnet.yml)). The test project sets this via **`NeonSprawl.Server.Tests/postgres.runsettings`** (`RunSettingsFilePath` in the `.csproj`) so **`dotnet test`** and **Rider** pick it up without shell `export`. **`launchSettings.json`** in the same project is for IDE launch profiles and is **not** the xUnit “config file” in Rider settings. -**Contributors / PRs:** With **`postgres.runsettings`** checked in, **`dotnet test`** always injects that connection string, so the **three Postgres integration tests run** (they are **not** skipped). If nothing is listening on Postgres and **Docker** cannot start Compose (or you have no DB), those tests **fail**—that is expected, not a random flake. **Mitigations:** `docker compose up -d` from the repo root (tests may auto-start Compose locally when not in CI), or temporarily remove **`RunSettingsFilePath`** from `NeonSprawl.Server.Tests.csproj` while hacking. **GitHub Actions** provides Postgres in [`.github/workflows/dotnet.yml`](../../.github/workflows/dotnet.yml), so merges stay green when the workflow runs. +**Contributors / PRs:** With **`postgres.runsettings`** checked in, **`dotnet test`** always injects that connection string, so the **four Postgres integration tests run** (they are **not** skipped). If nothing is listening on Postgres and **Docker** cannot start Compose (or you have no DB), those tests **fail**—that is expected, not a random flake. **Mitigations:** `docker compose up -d` from the repo root (tests may auto-start Compose locally when not in CI), or temporarily remove **`RunSettingsFilePath`** from `NeonSprawl.Server.Tests.csproj` while hacking. **GitHub Actions** provides Postgres in [`.github/workflows/dotnet.yml`](../../.github/workflows/dotnet.yml), so merges stay green when the workflow runs. **Rider:** Under **Settings → … → Unit Testing → xUnit.net**, leave **“Use specific configuration file”** off unless you add a real **`xunit.runner.json`**. Do **not** point that field at `launchSettings.json`. diff --git a/server/db/migrations/V003__player_skill_progression.sql b/server/db/migrations/V003__player_skill_progression.sql new file mode 100644 index 0000000..4f04997 --- /dev/null +++ b/server/db/migrations/V003__player_skill_progression.sql @@ -0,0 +1,10 @@ +-- NEO-38: per-player non-combat skill XP totals (level derived server-side until data-driven curves). +CREATE TABLE IF NOT EXISTS player_skill_progression ( + player_id TEXT NOT NULL REFERENCES player_position(player_id) ON DELETE CASCADE, + skill_id TEXT NOT NULL, + xp INTEGER NOT NULL CHECK (xp >= 0), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (player_id, skill_id) +); + +COMMENT ON TABLE player_skill_progression IS 'Persisted skill XP per player (NEO-38); level from SkillLevelCurvePlaceholder until NEO-39.'; From 2bcde45891161052c6bb3f752145ec6ead518e8e Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 6 May 2026 22:20:14 -0400 Subject: [PATCH 3/8] NEO-38: add curl examples to manual QA --- docs/manual-qa/NEO-38.md | 56 +++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/docs/manual-qa/NEO-38.md b/docs/manual-qa/NEO-38.md index 72e4a80..e8576d8 100644 --- a/docs/manual-qa/NEO-38.md +++ b/docs/manual-qa/NEO-38.md @@ -7,20 +7,58 @@ Reference: [implementation plan](../plans/NEO-38-implementation-plan.md), [serve - Run `NeonSprawl.Server` from `server/NeonSprawl.Server` (`dotnet run`; default `http://localhost:5253`). - Use a **known** player id (e.g. `dev-local-1`, present in position state). +Use a shell variable so you only change the base URL once: + +```bash +BASE=http://localhost:5253 +ID=dev-local-1 +``` + ## Happy path — grant + read back -1. `POST /game/players/dev-local-1/skill-progression` with JSON: - - `schemaVersion` **1** - - `skillId` **`salvage`** - - `amount` **10** - - `sourceKind` **`activity`** -2. Expect **HTTP 200**, body `granted` **true**, `progression.skills` contains `salvage` with increased `xp` vs a prior `GET`. -3. `GET /game/players/dev-local-1/skill-progression` — **`salvage.xp`** matches the POST `progression`. +1. **Baseline read** (optional): note `salvage.xp` before the grant (usually **0** on a fresh store). + +```bash +curl -sS "${BASE}/game/players/${ID}/skill-progression" +``` + +2. **Grant** — `schemaVersion` **1**, `skillId` **`salvage`**, `amount` **10**, `sourceKind` **`activity`**. + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":10,"sourceKind":"activity"}' +``` + +Expect **HTTP 200**, JSON `granted` **true**, `levelUps` may be empty if no level boundary crossed (10 XP stays at level 1 with the prototype curve). + +3. **Read back**: `salvage.xp` in the snapshot should reflect the persisted total (baseline + **10**, or cumulative if you repeated the POST). + +```bash +curl -sS "${BASE}/game/players/${ID}/skill-progression" +``` ## Deny — source not allowed -1. `POST` same path with `skillId` **`salvage`**, `sourceKind` **`trainer`** (not in prototype catalog for salvage). -2. Expect **HTTP 200**, `granted` **false**, **`reasonCode`** **`source_kind_not_allowed`**, and `progression` still present. +`skillId` **`salvage`** does **not** allow **`trainer`** in the prototype catalog; the server still returns **200** with structured deny. + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"salvage","amount":10,"sourceKind":"trainer"}' +``` + +Expect **HTTP 200**, `granted` **false**, **`reasonCode`** **`source_kind_not_allowed`**, and `progression` present (snapshot unchanged except what other requests already applied). + +## Bonus — observe a level-up in one response + +With the prototype curve `level = 1 + floor(xp / 100)`, a grant of **100** XP from **0** on `intrusion` should set `granted` **true** and include **`levelUps`** (`previousLevel` **1**, `newLevel` **2**). + +```bash +curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"skillId":"intrusion","amount":100,"sourceKind":"activity"}' +``` ## Bruno From bab7336e5b52d52520470cef4f18f30518acf591 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 6 May 2026 22:23:11 -0400 Subject: [PATCH 4/8] NEO-38: Bruno GET skill progression tolerant of persisted XP --- .../Get skill progression.bru | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/bruno/neon-sprawl-server/skill-progression/Get skill progression.bru b/bruno/neon-sprawl-server/skill-progression/Get skill progression.bru index 86fc043..8a09bf6 100644 --- a/bruno/neon-sprawl-server/skill-progression/Get skill progression.bru +++ b/bruno/neon-sprawl-server/skill-progression/Get skill progression.bru @@ -6,6 +6,7 @@ meta { docs { NEO-37: per-player read model; join with GET /game/world/skill-definitions for display names. + NEO-38: XP persists (in-memory/Postgres). These checks do not assume a fresh server—level must match placeholder curve level = 1 + floor(max(0,xp)/100). Restart server (or truncate DB in Postgres mode) to see all-zero defaults again. } get { @@ -25,15 +26,24 @@ tests { expect(body.skills.length).to.equal(3); }); - test("frozen prototype trio with default xp and level", function () { + test("frozen prototype trio with levels consistent with server placeholder curve", function () { const body = res.getBody(); - const intrusion = body.skills.find((x) => x.id === "intrusion"); - expect(intrusion).to.be.an("object"); - expect(intrusion.xp).to.equal(0); - expect(intrusion.level).to.equal(1); const ids = new Set(body.skills.map((x) => x.id)); expect(ids.has("salvage")).to.equal(true); expect(ids.has("refine")).to.equal(true); expect(ids.has("intrusion")).to.equal(true); + + /** Matches SkillLevelCurvePlaceholder (NEO-38): level = 1 + floor(max(0,xp)/100). */ + const expectedLevel = (xp) => 1 + Math.floor(Math.max(0, xp) / 100); + + for (const id of ["salvage", "refine", "intrusion"]) { + const row = body.skills.find((x) => x.id === id); + expect(row).to.be.an("object"); + expect(row.xp).to.be.a("number"); + expect(row.level).to.be.a("number"); + expect(row.xp).to.be.at.least(0); + expect(row.level).to.be.at.least(1); + expect(row.level).to.equal(expectedLevel(row.xp)); + } }); } From 9ed4034140963ec4991cfa5ba16fb566b4ccf297 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 6 May 2026 22:24:17 -0400 Subject: [PATCH 5/8] NEO-38: add code review (2026-05-06) --- docs/reviews/2026-05-06-NEO-38.md | 49 +++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 docs/reviews/2026-05-06-NEO-38.md diff --git a/docs/reviews/2026-05-06-NEO-38.md b/docs/reviews/2026-05-06-NEO-38.md new file mode 100644 index 0000000..74a00c3 --- /dev/null +++ b/docs/reviews/2026-05-06-NEO-38.md @@ -0,0 +1,49 @@ +# Code review — NEO-38 (skill XP grant) + +**Date:** 2026-05-06 + +**Scope:** Branch `NEO-38-apply-skill-xp-grant-allowlist-level-up` vs `origin/main` (four commits ending `bab7336`; working tree clean at review time). + +**Base:** `origin/main` + +--- + +## Verdict + +**Approve** + +## Summary + +The branch implements `POST /game/players/{id}/skill-progression` with versioned bodies, stable deny reason codes (`unknown_skill`, `source_kind_not_allowed`, `invalid_amount`), hotbar-style **200 + `granted: false`** denies with echoed **`progression`**, dual persistence (**`InMemoryPlayerSkillProgressionStore`** / **`PostgresPlayerSkillProgressionStore`** + **`V003__player_skill_progression.sql`**), and GET snapshot merge of registry + stored XP via **`SkillLevelCurvePlaceholder`**. Integration tests (**`SkillProgressionGrantApiTests`**), Postgres persistence (**`SkillProgressionGrantPersistenceIntegrationTests`**), Bruno, **`server/README.md`**, **`docs/manual-qa/NEO-38.md`**, and **`docs/plans/NEO-38-implementation-plan.md`** are in place. **`dotnet test`** for `NeonSprawl.Server.Tests` passed (123 tests). Overall risk is **low** with normal prototype caveats (placeholder curve, synchronous ADO.NET in stores). + +## Documentation checked + +| Document | Assessment | +|---------|-------------| +| `docs/plans/NEO-38-implementation-plan.md` | **Matches** — routing, gates, persistence shape, denial pattern, Bruno/README/manual QA covered by implementation. Minor filename drift (`SkillProgressionServiceCollectionExtensions.cs` shipped vs plan’s wording; benign). | +| `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` | **Partially matches** — responsibilities (validate skill + `sourceKind`, derive level, surface level-ups) align. The page’s **Implementation snapshot** paragraph still reads as if **NEO-38** is backlog (“Still backlog … applying XP grants”). **Should update after merge** (or in same PR) so the slice-2 narrative matches landed code — see Suggestions. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E2.M2 tracking row still lists **NEO-38** under “Follow-on” only. Merge should refresh that row (**NEO-38** landed paths, README anchors, Bruno) per this doc’s own rule (**§ Status column / tracking table**). E2.M1 row’s **Follow-on (E2.M2): XP grant validation** line is stale once NEO-38 merges (**should fix** in same sweep). | +| `docs/decomposition/modules/module_dependency_register.md` | Referenced implicitly via alignment doc; register row likely needs the same E2.M2 status/snapshot tweak when NEO-38 merges — treat as **post-merge bookkeeping** alongside the alignment table (not reviewed line-by-line in this pass). | + +**Register / tracking table:** yes — **`documentation_and_implementation_alignment.md`** (and **`E2_M2_…` Implementation snapshot**) should be updated so Status / snapshot bullets do not disagree with merged code. + +## Blocking issues + +_None._ + +## Suggestions + +1. **Decomposition freshness (same PR or immediate follow-up):** Update **`E2_M2_XpAwardAndLevelEngine.md`** implementation snapshot plus **`documentation_and_implementation_alignment.md`** E2.M2 (and tighten E2.M1 “follow-on”) so they describe **NEO-38** as landed with links to **`NEO-38-implementation-plan.md`**, **`docs/manual-qa/NEO-38.md`**, and **`server/README.md`** grant section — matching the checklist in **`planning-implementation-docs`**. +2. **`levelUps` semantics for multi-step jumps:** A single grant that crosses more than one level boundary (e.g. +250 XP under `level = 1 + floor(xp/100)`) emits **one** `levelUps` row `{ previousLevel: 1, newLevel: 3 }`. If callers need one entry per crossed level later, note that explicitly in README or backlog; otherwise a one-line clarification that “span reflects final levels after grant” avoids ambiguity. +3. **Bruno coverage:** **`Post skill progression grant.bru`** exercises the happy path only; **`docs/manual-qa/NEO-38.md`** covers deny via curl. Optional: add a second Bruno request for **`source_kind_not_allowed`** to mirror hotbar-style smoke patterns. + +## Nits + +- **Nit:** `MapPost` uses `positions.TryGetPosition(id, …)` with the raw route parameter while snapshots use `trimmedId`; `MapGet` does the same. Harmless if ids are never padded with whitespace; using the same trimmed key everywhere would be slightly tidier. +- **Nit:** **`SkillProgressionGrantPersistenceIntegrationTests`** uses a broad `// Act` block that includes opening a second factory and GET — acceptable for an integration test, but strict AAA purists might split “write” vs “read-back” with clearer phase comments. + +## Verification + +- `cd server && dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj` — **passed** (123 tests) at review time. +- Optional: run **`docs/manual-qa/NEO-38.md`** curl steps against a local host; run Bruno folder `bruno/neon-sprawl-server/skill-progression/` for smoke. +- With Postgres: confirm **`V003__player_skill_progression.sql`** applied in real migrations flow if something other than bootstrap applies DDL in your environment (app uses bootstrap + test harness re-applies file — OK for tests). From b2999f3ed9b3f8d5b68c1cbd95c3aad153f59aee Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 6 May 2026 22:26:16 -0400 Subject: [PATCH 6/8] =?UTF-8?q?NEO-38:=20address=20review=20=E2=80=94=20do?= =?UTF-8?q?cs=20alignment,=20Bruno=20deny,=20README=20levelUps=20trim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...ill progression grant deny source kind.bru | 36 +++++++++++++++++++ .../modules/E2_M1_SkillDefinitionRegistry.md | 2 +- .../modules/E2_M2_XpAwardAndLevelEngine.md | 6 +++- ...umentation_and_implementation_alignment.md | 4 +-- .../modules/module_dependency_register.md | 2 ++ docs/manual-qa/NEO-38.md | 1 + docs/reviews/2026-05-06-NEO-38.md | 21 ++++++----- ...ressionGrantPersistenceIntegrationTests.cs | 3 +- .../Skills/SkillProgressionSnapshotApi.cs | 9 +++-- server/README.md | 6 ++-- 10 files changed, 68 insertions(+), 22 deletions(-) create mode 100644 bruno/neon-sprawl-server/skill-progression/Post skill progression grant deny source kind.bru diff --git a/bruno/neon-sprawl-server/skill-progression/Post skill progression grant deny source kind.bru b/bruno/neon-sprawl-server/skill-progression/Post skill progression grant deny source kind.bru new file mode 100644 index 0000000..bc111c6 --- /dev/null +++ b/bruno/neon-sprawl-server/skill-progression/Post skill progression grant deny source kind.bru @@ -0,0 +1,36 @@ +meta { + name: POST skill progression grant deny source kind + type: http + seq: 3 +} + +docs { + NEO-38: salvage does not allow trainer in prototype_skills.json; expects granted false + source_kind_not_allowed. +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/skill-progression + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "skillId": "salvage", + "amount": 10, + "sourceKind": "trainer" + } +} + +tests { + test("denies with source_kind_not_allowed and echoes progression", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.granted).to.equal(false); + expect(body.reasonCode).to.equal("source_kind_not_allowed"); + expect(body.progression).to.be.an("object"); + expect(body.progression.skills.length).to.equal(3); + }); +} diff --git a/docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md b/docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md index e74c5ba..ffd5157 100644 --- a/docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md +++ b/docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md @@ -62,7 +62,7 @@ Every `SkillDef` row **must** include a **non-empty** `allowedXpSourceKinds` arr - **`mission_reward`** — wire **explicit** mission/contract hand-ins or scripted bonuses only; not a stand-in for combat encounter XP ([`docs/game-design/progression.md`](../../game-design/progression.md) — mission vs combat). - **`trainer`** / **`book_or_item`** — wire NPC teaching beats and consumable/one-shot grants respectively; do not use for combat kills. -**Combat encounters award gig XP, not skill XP.** If a grant channel is missing from a skill’s list, [E2.M2](E2_M2_XpAwardAndLevelEngine.md) must reject the grant once implemented—keep catalogs honest so call sites do not drift. +**Combat encounters award gig XP, not skill XP.** If a grant channel is missing from a skill’s list, [E2.M2](E2_M2_XpAwardAndLevelEngine.md) rejects the grant at apply time (**NEO-38** HTTP grant path)—keep catalogs honest so call sites do not drift. ## Prototype Slice 1 freeze ([NEO-33](https://linear.app/neon-sprawl/issue/NEO-33)) diff --git a/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md b/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md index de9c254..a68493a 100644 --- a/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md +++ b/docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md @@ -13,7 +13,11 @@ **Linear (label `E2.M2`):** [Epic 2 project](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6) — **Slice 2:** [NEO-37](https://linear.app/neon-sprawl/issue/NEO-37) (progression read snapshot) → [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (apply grant + allowlist + level-up) → [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) (data-driven `LevelCurve` + CI); [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites) after NEO-38, parallel with NEO-39. **Slice 3 integration** (multi-label where noted): [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41), [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42), [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43), [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). -**NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([implementation plan](../../plans/NEO-37-implementation-plan.md)) — `SkillProgressionSnapshotApi` / `SkillProgressionSnapshotDtos` under `server/NeonSprawl.Server/Game/Skills/`; one row per `ISkillDefinitionRegistry` skill (**xp** 0, **level** 1 until grants persist); **`404`** when the player has no position (same gate as **`GET …/cooldown-snapshot`**). Bruno **`bruno/neon-sprawl-server/skill-progression/`**; manual QA **[`NEO-37.md`](../../manual-qa/NEO-37.md)**; **[server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37)**. **Still backlog within this module:** applying XP grants, `LevelCurve`, `LevelUpEvent`, telemetry hook sites (**NEO-38** onward). Keep this paragraph and [documentation tracking](documentation_and_implementation_alignment.md) in sync as Slice 2 stories merge. +**NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([implementation plan](../../plans/NEO-37-implementation-plan.md)) — `SkillProgressionSnapshotApi` / `SkillProgressionSnapshotDtos` under `server/NeonSprawl.Server/Game/Skills/`; one row per `ISkillDefinitionRegistry` skill; **`404`** when the player has no position (same gate as **`GET …/cooldown-snapshot`**). Manual QA **[`NEO-37.md`](../../manual-qa/NEO-37.md)**; **[server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37)**. + +**NEO-38 landed:** versioned **`POST /game/players/{id}/skill-progression`** ([implementation plan](../../plans/NEO-38-implementation-plan.md)) — applies one grant per request with **`ISkillDefinitionRegistry`** + **`allowedXpSourceKinds`** validation, stable deny **`reasonCode`** values, dual persistence (**`IPlayerSkillProgressionStore`**: in-memory / Postgres + [`V003__player_skill_progression.sql`](../../../server/db/migrations/V003__player_skill_progression.sql)), GET merges stored XP with the registry; inline **`SkillLevelCurvePlaceholder`** until [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39); success payload includes **`levelUps`** (see README for multi-step semantics). Bruno **`bruno/neon-sprawl-server/skill-progression/`** (GET + POST + deny); manual QA **[`NEO-38.md`](../../manual-qa/NEO-38.md)**; **[server README — Skill progression grant (NEO-38)](../../../server/README.md#skill-progression-grant-neo-38)**. + +**Still backlog within this module:** data-driven **`LevelCurve`** + CI (**NEO-39**); telemetry hook sites (**NEO-40**); callers in Slice 3 (**NEO-41**–**NEO-43**). Keep this paragraph and [documentation tracking](documentation_and_implementation_alignment.md) in sync as Slice 2 stories merge. ## Purpose diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 63b3c75..bd65b52 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -50,8 +50,8 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E1.M2 | Ready | **Slice 2 prototype slice closed:** follow + `CameraState` ([NEO-15](https://linear.app/neon-sprawl/issue/NEO-15)); zoom bands ([NEO-16](https://linear.app/neon-sprawl/issue/NEO-16)); occlusion ([NEO-17](https://linear.app/neon-sprawl/issue/NEO-17)); occluder pick-through ([NEO-20](https://linear.app/neon-sprawl/issue/NEO-20)); contracts + hardening ([NEO-18](https://linear.app/neon-sprawl/issue/NEO-18)). Client-local; no server use of camera pose. | [NEO-15](../../plans/NEO-15-implementation-plan.md), [NEO-16](../../plans/NEO-16-implementation-plan.md), [NEO-17](../../plans/NEO-17-implementation-plan.md), [NEO-18](../../plans/NEO-18-implementation-plan.md), [NEO-20](../../plans/NEO-20-implementation-plan.md); `client/scripts/isometric_follow_camera.gd`, `camera_state.gd`, `zoom_band_config.gd`, `occlusion_policy.gd`, `ground_pick.gd`; [E1_M2_IsometricCameraController.md](E1_M2_IsometricCameraController.md) | | 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). **Follow-on (E2.M2):** XP grant validation vs `allowedXpSourceKinds` per [E2_M1](E2_M1_SkillDefinitionRegistry.md). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-implementation-plan.md), [NEO-36](../../plans/NEO-36-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/`; [`docs/manual-qa/NEO-36.md`](../../manual-qa/NEO-36.md); [server README — Skill definitions (NEO-36)](../../../server/README.md#skill-definitions-neo-36) | -| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — `SkillProgressionSnapshotApi` / `SkillProgressionSnapshotDtos` in `server/NeonSprawl.Server/Game/Skills/`; bootstrap read model (**xp** 0, **level** 1) for every registered skill; known-player gate via `IPositionStateStore` (same pattern as cooldown snapshot); Bruno `bruno/neon-sprawl-server/skill-progression/`; manual QA [`NEO-37`](../../manual-qa/NEO-37.md); [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37). **Follow-on:** [NEO-38](https://linear.app/neon-sprawl/issue/NEO-38) (apply XP grant + `allowedXpSourceKinds` validation + level-up) → [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) (data-driven `LevelCurve` + CI); [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites). Slice 3 integration — [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41) / [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42) / [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43); [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-40, NEO-41–NEO-43 | +| E2.M1 | In Progress | **NEO-33 landed:** frozen prototype trio **`salvage`** / **`refine`** / **`intrusion`** in [`content/skills/prototype_skills.json`](../../../content/skills/prototype_skills.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) enforce schema, duplicate `id`, exact trio ids, and category coverage; designer note on **`allowedXpSourceKinds`** in [E2_M1](E2_M1_SkillDefinitionRegistry.md) + [`content/README.md`](../../../content/README.md). **NEO-34 landed:** fail-fast server load of `content/skills/*.json` at startup — `server/NeonSprawl.Server/Game/Skills/` ([NEO-34](../../plans/NEO-34-implementation-plan.md)); config + discovery in [server README — Skill catalog](../../../server/README.md#skill-catalog-contentskills-neo-34). **NEO-35 landed:** `ISkillDefinitionRegistry` / `SkillDefinitionRegistry` + DI ([NEO-35](../../plans/NEO-35-implementation-plan.md)). **NEO-36 landed:** versioned **`GET /game/world/skill-definitions`** ([NEO-36](../../plans/NEO-36-implementation-plan.md)) — `SkillDefinitionsWorldApi` + `SkillDefinitionsListDtos` in `server/NeonSprawl.Server/Game/Skills/`; Bruno `bruno/neon-sprawl-server/skill-definitions/`; manual QA [`NEO-36`](../../manual-qa/NEO-36.md). **E2.M2 consumer (NEO-38 landed):** grant path validates `skillId` + `sourceKind` vs catalog **`allowedXpSourceKinds`** on **`POST …/skill-progression`** ([NEO-38](../../plans/NEO-38-implementation-plan.md)); see [server README — Skill progression grant](../../../server/README.md#skill-progression-grant-neo-38) and [E2_M2](E2_M2_XpAwardAndLevelEngine.md). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-implementation-plan.md), [NEO-36](../../plans/NEO-36-implementation-plan.md); [E2_M1](E2_M1_SkillDefinitionRegistry.md); [module_dependency_register.md](module_dependency_register.md) **E2.M1 note**; `server/NeonSprawl.Server/Game/Skills/`; [`docs/manual-qa/NEO-36.md`](../../manual-qa/NEO-36.md); [server README — Skill definitions (NEO-36)](../../../server/README.md#skill-definitions-neo-36) | +| E2.M2 | In Progress | **NEO-37 landed:** versioned **`GET /game/players/{id}/skill-progression`** ([NEO-37](../../plans/NEO-37-implementation-plan.md)) — read model for every registered skill; known-player gate via `IPositionStateStore`; [server README — Skill progression snapshot (NEO-37)](../../../server/README.md#skill-progression-snapshot-neo-37); manual QA [`NEO-37`](../../manual-qa/NEO-37.md). **NEO-38 landed:** **`POST`** same path — grant apply, persistence (`IPlayerSkillProgressionStore`, `V003` migration), **`SkillLevelCurvePlaceholder`**, 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). **Follow-on:** [NEO-39](https://linear.app/neon-sprawl/issue/NEO-39) (data-driven `LevelCurve` + CI); [NEO-40](https://linear.app/neon-sprawl/issue/NEO-40) (telemetry hook sites). Slice 3 integration — [NEO-41](https://linear.app/neon-sprawl/issue/NEO-41) / [NEO-42](https://linear.app/neon-sprawl/issue/NEO-42) / [NEO-43](https://linear.app/neon-sprawl/issue/NEO-43); [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [epic_02 — E2.M2 + Slice 3](../epics/epic_02_skills_and_progression.md). | [NEO-37](../../plans/NEO-37-implementation-plan.md), [NEO-38](../../plans/NEO-38-implementation-plan.md), [E2_M2](E2_M2_XpAwardAndLevelEngine.md); label **`E2.M2`** on NEO-37–NEO-40, NEO-41–NEO-43 | --- diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 34d539c..05071cb 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -34,6 +34,8 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen **E2.M1 note:** Epic 2 Slice 1 backlog in Linear ([Epic 2 — Classless Skill and Progression Framework](https://linear.app/neon-sprawl/project/epic-2-classless-skill-and-progression-framework-5200f84bf8b6)): [NEO-33](https://linear.app/neon-sprawl/issue/NEO-33) → [NEO-34](https://linear.app/neon-sprawl/issue/NEO-34) → [NEO-35](https://linear.app/neon-sprawl/issue/NEO-35) → [NEO-36](https://linear.app/neon-sprawl/issue/NEO-36); label **`E2.M1`**. See [E2_M1_SkillDefinitionRegistry.md](E2_M1_SkillDefinitionRegistry.md). **NEO-33** (content + CI + docs) moves the register row to **In Progress**; **NEO-34+** cover server registry and HTTP. Update [documentation and implementation alignment](documentation_and_implementation_alignment.md) when slices land. +**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). Keep aligned with [documentation and implementation alignment](documentation_and_implementation_alignment.md). + ### Epic 3 — Crafting Economy | Module ID | Module Name | Depends On | Contract Needed | Phase Required | Status | diff --git a/docs/manual-qa/NEO-38.md b/docs/manual-qa/NEO-38.md index e8576d8..2876b28 100644 --- a/docs/manual-qa/NEO-38.md +++ b/docs/manual-qa/NEO-38.md @@ -64,4 +64,5 @@ curl -sS -i -X POST "${BASE}/game/players/${ID}/skill-progression" \ - Folder: `bruno/neon-sprawl-server/skill-progression/` - `Post skill progression grant.bru` — happy grant. +- `Post skill progression grant deny source kind.bru` — `source_kind_not_allowed` on salvage + `trainer`. - `Get skill progression.bru` — confirms read model after grants. diff --git a/docs/reviews/2026-05-06-NEO-38.md b/docs/reviews/2026-05-06-NEO-38.md index 74a00c3..f274127 100644 --- a/docs/reviews/2026-05-06-NEO-38.md +++ b/docs/reviews/2026-05-06-NEO-38.md @@ -21,11 +21,11 @@ The branch implements `POST /game/players/{id}/skill-progression` with versioned | Document | Assessment | |---------|-------------| | `docs/plans/NEO-38-implementation-plan.md` | **Matches** — routing, gates, persistence shape, denial pattern, Bruno/README/manual QA covered by implementation. Minor filename drift (`SkillProgressionServiceCollectionExtensions.cs` shipped vs plan’s wording; benign). | -| `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` | **Partially matches** — responsibilities (validate skill + `sourceKind`, derive level, surface level-ups) align. The page’s **Implementation snapshot** paragraph still reads as if **NEO-38** is backlog (“Still backlog … applying XP grants”). **Should update after merge** (or in same PR) so the slice-2 narrative matches landed code — see Suggestions. | -| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matches** — E2.M2 tracking row still lists **NEO-38** under “Follow-on” only. Merge should refresh that row (**NEO-38** landed paths, README anchors, Bruno) per this doc’s own rule (**§ Status column / tracking table**). E2.M1 row’s **Follow-on (E2.M2): XP grant validation** line is stale once NEO-38 merges (**should fix** in same sweep). | -| `docs/decomposition/modules/module_dependency_register.md` | Referenced implicitly via alignment doc; register row likely needs the same E2.M2 status/snapshot tweak when NEO-38 merges — treat as **post-merge bookkeeping** alongside the alignment table (not reviewed line-by-line in this pass). | +| `docs/decomposition/modules/E2_M2_XpAwardAndLevelEngine.md` | ~~**Partially matches** — responsibilities (validate skill + `sourceKind`, derive level, surface level-ups) align. The page’s **Implementation snapshot** paragraph still reads as if **NEO-38** is backlog (“Still backlog … applying XP grants”). **Should update after merge** (or in same PR) so the slice-2 narrative matches landed code — see Suggestions.~~ **Done.** (2026-05-07) — Implementation snapshot refreshed for **NEO-38** landed + remaining backlog (**NEO-39** onward). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | ~~**Partially matches** — E2.M2 tracking row still lists **NEO-38** under “Follow-on” only. Merge should refresh that row (**NEO-38** landed paths, README anchors, Bruno) per this doc’s own rule (**§ Status column / tracking table**). E2.M1 row’s **Follow-on (E2.M2): XP grant validation** line is stale once NEO-38 merges (**should fix** in same sweep).~~ **Done.** (2026-05-07) — E2.M2 row includes **NEO-38**; E2.M1 row references consumer grant validation. | +| `docs/decomposition/modules/module_dependency_register.md` | ~~Referenced implicitly via alignment doc; register row likely needs the same E2.M2 status/snapshot tweak when NEO-38 merges — treat as **post-merge bookkeeping** alongside the alignment table (not reviewed line-by-line in this pass).~~ **Done.** (2026-05-07) — **E2.M2 note** added (NEO-37/NEO-38 pointers). | -**Register / tracking table:** yes — **`documentation_and_implementation_alignment.md`** (and **`E2_M2_…` Implementation snapshot**) should be updated so Status / snapshot bullets do not disagree with merged code. +**Register / tracking table:** yes — ~~**`documentation_and_implementation_alignment.md`** (and **`E2_M2_…` Implementation snapshot**) should be updated so Status / snapshot bullets do not disagree with merged code.~~ **Done.** ## Blocking issues @@ -33,14 +33,17 @@ _None._ ## Suggestions -1. **Decomposition freshness (same PR or immediate follow-up):** Update **`E2_M2_XpAwardAndLevelEngine.md`** implementation snapshot plus **`documentation_and_implementation_alignment.md`** E2.M2 (and tighten E2.M1 “follow-on”) so they describe **NEO-38** as landed with links to **`NEO-38-implementation-plan.md`**, **`docs/manual-qa/NEO-38.md`**, and **`server/README.md`** grant section — matching the checklist in **`planning-implementation-docs`**. -2. **`levelUps` semantics for multi-step jumps:** A single grant that crosses more than one level boundary (e.g. +250 XP under `level = 1 + floor(xp/100)`) emits **one** `levelUps` row `{ previousLevel: 1, newLevel: 3 }`. If callers need one entry per crossed level later, note that explicitly in README or backlog; otherwise a one-line clarification that “span reflects final levels after grant” avoids ambiguity. -3. **Bruno coverage:** **`Post skill progression grant.bru`** exercises the happy path only; **`docs/manual-qa/NEO-38.md`** covers deny via curl. Optional: add a second Bruno request for **`source_kind_not_allowed`** to mirror hotbar-style smoke patterns. +1. ~~**Decomposition freshness (same PR or immediate follow-up):** Update **`E2_M2_XpAwardAndLevelEngine.md`** implementation snapshot plus **`documentation_and_implementation_alignment.md`** E2.M2 (and tighten E2.M1 “follow-on”) so they describe **NEO-38** as landed with links to **`NEO-38-implementation-plan.md`**, **`docs/manual-qa/NEO-38.md`**, and **`server/README.md`** grant section — matching the checklist in **`planning-implementation-docs`**.~~ **Done.** + +2. ~~**`levelUps` semantics for multi-step jumps:** A single grant that crosses more than one level boundary (e.g. +250 XP under `level = 1 + floor(xp/100)`) emits **one** `levelUps` row `{ previousLevel: 1, newLevel: 3 }`. If callers need one entry per crossed level later, note that explicitly in README or backlog; otherwise a one-line clarification that “span reflects final levels after grant” avoids ambiguity.~~ **Done.** (clarified in **`server/README.md`** grant section; optional future backlog called out.) + +3. ~~**Bruno coverage:** **`Post skill progression grant.bru`** exercises the happy path only; **`docs/manual-qa/NEO-38.md`** covers deny via curl. Optional: add a second Bruno request for **`source_kind_not_allowed`** to mirror hotbar-style smoke patterns.~~ **Done.** — `Post skill progression grant deny source kind.bru`; manual QA updated. ## Nits -- **Nit:** `MapPost` uses `positions.TryGetPosition(id, …)` with the raw route parameter while snapshots use `trimmedId`; `MapGet` does the same. Harmless if ids are never padded with whitespace; using the same trimmed key everywhere would be slightly tidier. -- **Nit:** **`SkillProgressionGrantPersistenceIntegrationTests`** uses a broad `// Act` block that includes opening a second factory and GET — acceptable for an integration test, but strict AAA purists might split “write” vs “read-back” with clearer phase comments. +- ~~**Nit:** `MapPost` uses `positions.TryGetPosition(id, …)` with the raw route parameter while snapshots use `trimmedId`; `MapGet` does the same. Harmless if ids are never padded with whitespace; using the same trimmed key everywhere would be slightly tidier.~~ **Done.** — GET/POST use **`trimmedId`** (+ empty ⇒ **404**) for **`TryGetPosition`**. + +- ~~**Nit:** **`SkillProgressionGrantPersistenceIntegrationTests`** uses a broad `// Act` block that includes opening a second factory and GET — acceptable for an integration test, but strict AAA purists might split “write” vs “read-back” with clearer phase comments.~~ **Done.** — two **`// Act`** comment blocks (**write** / **read-back**). ## Verification diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs index 43293dc..5cf5eee 100644 --- a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs @@ -25,7 +25,7 @@ public sealed class SkillProgressionGrantPersistenceIntegrationTests(PostgresInt }; HttpResponseMessage postResponse; - // Act + // Act — write grant through first host using (var firstClient = Factory.CreateClient()) { postResponse = await firstClient.PostAsJsonAsync( @@ -33,6 +33,7 @@ public sealed class SkillProgressionGrantPersistenceIntegrationTests(PostgresInt grant); } + // Act — read back through a fresh host (forces DB round-trip / new DI scope) await using var secondFactory = new PostgresWebApplicationFactory(); using var secondClient = secondFactory.CreateClient(); var snapshot = diff --git a/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs b/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs index 390eb55..9f22600 100644 --- a/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs +++ b/server/NeonSprawl.Server/Game/Skills/SkillProgressionSnapshotApi.cs @@ -19,12 +19,12 @@ public static class SkillProgressionSnapshotApi "/game/players/{id}/skill-progression", (string id, IPositionStateStore positions, ISkillDefinitionRegistry registry, IPlayerSkillProgressionStore xpStore) => { - if (!positions.TryGetPosition(id, out _)) + var trimmedId = id.Trim(); + if (trimmedId.Length == 0 || !positions.TryGetPosition(trimmedId, out _)) { return Results.NotFound(); } - var trimmedId = id.Trim(); return Results.Json(BuildSnapshot(trimmedId, registry, xpStore)); }); @@ -33,8 +33,6 @@ public static class SkillProgressionSnapshotApi (string id, SkillProgressionGrantRequest? body, IPositionStateStore positions, ISkillDefinitionRegistry registry, IPlayerSkillProgressionStore xpStore) => { - var trimmedId = id.Trim(); - static SkillProgressionGrantResponse Deny( SkillProgressionSnapshotResponse snapshot, string reason) => @@ -51,7 +49,8 @@ public static class SkillProgressionSnapshotApi return Results.BadRequest(); } - if (!positions.TryGetPosition(id, out _)) + var trimmedId = id.Trim(); + if (trimmedId.Length == 0 || !positions.TryGetPosition(trimmedId, out _)) { return Results.NotFound(); } diff --git a/server/README.md b/server/README.md index 4931cce..2f74b27 100644 --- a/server/README.md +++ b/server/README.md @@ -57,9 +57,9 @@ curl -sS -i "http://localhost:5253/game/players/dev-local-1/skill-progression" **`POST /game/players/{id}/skill-progression`** accepts a versioned body (`schemaVersion` **1**, **`skillId`**, **`amount`** positive int, **`sourceKind`**) and validates the skill against **`ISkillDefinitionRegistry`**, then validates **`sourceKind`** against that skill’s **`allowedXpSourceKinds`** (case-insensitive). **404** when the player is unknown to position state; **400** when **`schemaVersion`** is missing or not **1**. Persisted XP totals use the same **[NEO-29-style](#position-persistence-neo-8) split** as hotbar: **`player_skill_progression`** in PostgreSQL when **`ConnectionStrings:NeonSprawl`** is set (DDL [`V003__player_skill_progression.sql`](../db/migrations/V003__player_skill_progression.sql)), otherwise the in-memory fallback (dev bucket only, matching hotbar semantics). -**Structured responses (HTTP 200):** **`granted`** (`true`/`false`), optional **`reasonCode`** on deny, plus **`progression`** mirroring the GET snapshot. Stable deny codes: **`unknown_skill`**, **`source_kind_not_allowed`**, **`invalid_amount`**. On **`granted: true`**, **`levelUps`** lists skills whose level increased on this request (`skillId`, `previousLevel`, `newLevel`). +**Structured responses (HTTP 200):** **`granted`** (`true`/`false`), optional **`reasonCode`** on deny, plus **`progression`** mirroring the GET snapshot. Stable deny codes: **`unknown_skill`**, **`source_kind_not_allowed`**, **`invalid_amount`**. On **`granted: true`**, **`levelUps`** lists skills whose level increased on this request (`skillId`, `previousLevel`, `newLevel`). If a single grant crosses **multiple** level boundaries (e.g. a large XP jump under the placeholder curve), the server emits **one** object per skill with **`previousLevel`** and **`newLevel`** set to the **start and end** levels after the grant — not one array entry per boundary crossed. A future issue can add per-step events if product needs them. -Plan: [NEO-38 implementation plan](../../docs/plans/NEO-38-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-38.md`](../../docs/manual-qa/NEO-38.md); Bruno: `bruno/neon-sprawl-server/skill-progression/Post skill progression grant.bru`. +Plan: [NEO-38 implementation plan](../../docs/plans/NEO-38-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-38.md`](../../docs/manual-qa/NEO-38.md); Bruno: `bruno/neon-sprawl-server/skill-progression/` (happy **POST** + deny smoke). ```bash curl -sS -i -X POST "http://localhost:5253/game/players/dev-local-1/skill-progression" \ @@ -71,7 +71,7 @@ curl -sS -i -X POST "http://localhost:5253/game/players/dev-local-1/skill-progre When **`ConnectionStrings:NeonSprawl`** is set to a valid PostgreSQL connection string, the server uses the **`player_position`** table (relational columns `pos_x` … `sequence`, not JSONB). DDL lives in [`server/db/migrations/V001__player_position.sql`](../db/migrations/V001__player_position.sql); the app applies that script on startup and on first store use (idempotent `CREATE TABLE IF NOT EXISTS`). The configured dev player (`Game:DevPlayerId` / `Game:DefaultPosition`) is seeded once at host startup, matching the in-memory store’s constructor seed (not on every HTTP request). -If **`ConnectionStrings:NeonSprawl`** is missing or blank, behavior matches NEO-6/NEO-7 (**in-memory** only). The **test** project’s `postgres.runsettings` sets `ConnectionStrings__NeonSprawl` for the test host so Postgres integration tests run in CI/Rider/`dotnet test`; non-Postgres tests use `InMemoryWebApplicationFactory` and do not require a live DB. Remove or rename `postgres.runsettings` (and the `RunSettingsFilePath` property) if you want those three tests to **skip** again when the variable is unset. +If **`ConnectionStrings:NeonSprawl`** is missing or blank, behavior matches NEO-6/NEO-7 (**in-memory** only). The **test** project’s `postgres.runsettings` sets `ConnectionStrings__NeonSprawl` for the test host so Postgres integration tests run in CI/Rider/`dotnet test`; non-Postgres tests use `InMemoryWebApplicationFactory` and do not require a live DB. Remove or rename `postgres.runsettings` (and the `RunSettingsFilePath` property) if you want those Postgres integration tests to **skip** again when the variable is unset. **Environment variables** (typical — same mapping as other ASP.NET Core config): From ad9259c308771abc2a13f1a12056c2eff27d7b40 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Wed, 6 May 2026 22:34:26 -0400 Subject: [PATCH 7/8] NEO-38: set integration test host to Testing for deterministic JSON errors --- .../Game/PositionState/PostgresWebApplicationFactory.cs | 2 ++ .../NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs index 43f8466..9993dab 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresWebApplicationFactory.cs @@ -18,6 +18,8 @@ public sealed class PostgresWebApplicationFactory : WebApplicationFactory Date: Wed, 6 May 2026 22:39:18 -0400 Subject: [PATCH 8/8] NEO-38: reset player_position before progression persistence test --- ...ressionGrantPersistenceIntegrationTests.cs | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs index 5cf5eee..adf0d4a 100644 --- a/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillProgressionGrantPersistenceIntegrationTests.cs @@ -1,4 +1,7 @@ using System.Net.Http.Json; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using NeonSprawl.Server.Game.PositionState; using NeonSprawl.Server.Game.Skills; using NeonSprawl.Server.Tests.Game.PositionState; using Npgsql; @@ -48,7 +51,7 @@ public sealed class SkillProgressionGrantPersistenceIntegrationTests(PostgresInt Assert.Equal(1, salvage.Level); } - private static async Task ResetProgressionTableAsync() + private async Task ResetProgressionTableAsync() { var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); if (string.IsNullOrWhiteSpace(cs)) @@ -56,23 +59,45 @@ public sealed class SkillProgressionGrantPersistenceIntegrationTests(PostgresInt throw new InvalidOperationException("ConnectionStrings__NeonSprawl is not set."); } - var ddlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V003__player_skill_progression.sql"); - if (!File.Exists(ddlPath)) + _ = Factory.Services; + var options = Factory.Services.GetRequiredService>().Value; + + var positionDdlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V001__player_position.sql"); + var progressionDdlPath = Path.Combine(AppContext.BaseDirectory, "db", "migrations", "V003__player_skill_progression.sql"); + if (!File.Exists(positionDdlPath)) { - throw new FileNotFoundException($"Test DDL not found at '{ddlPath}'.", ddlPath); + throw new FileNotFoundException($"Test DDL not found at '{positionDdlPath}'.", positionDdlPath); } - var ddl = await File.ReadAllTextAsync(ddlPath); + if (!File.Exists(progressionDdlPath)) + { + throw new FileNotFoundException($"Test DDL not found at '{progressionDdlPath}'.", progressionDdlPath); + } + + var positionDdl = await File.ReadAllTextAsync(positionDdlPath); + var progressionDdl = await File.ReadAllTextAsync(progressionDdlPath); await using var conn = new NpgsqlConnection(cs); await conn.OpenAsync(); - await using (var apply = new NpgsqlCommand(ddl, conn)) + await using (var applyPosition = new NpgsqlCommand(positionDdl, conn)) { - await apply.ExecuteNonQueryAsync(); + await applyPosition.ExecuteNonQueryAsync(); } - await using (var truncate = new NpgsqlCommand("TRUNCATE player_skill_progression;", conn)) + await using (var truncate = new NpgsqlCommand("TRUNCATE player_position CASCADE;", conn)) { await truncate.ExecuteNonQueryAsync(); } + + PostgresPositionBootstrap.SeedDevPlayer(conn, options); + + await using (var applyProgression = new NpgsqlCommand(progressionDdl, conn)) + { + await applyProgression.ExecuteNonQueryAsync(); + } + + await using (var truncateProgression = new NpgsqlCommand("TRUNCATE player_skill_progression;", conn)) + { + await truncateProgression.ExecuteNonQueryAsync(); + } } }