From 044b4f823c8a2d0a9b21370b72a24bdbcfb6f465 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 4 May 2026 20:33:53 -0400 Subject: [PATCH 1/4] NEO-36: add implementation plan for skill catalog HTTP API --- docs/plans/NEO-36-implementation-plan.md | 81 ++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/plans/NEO-36-implementation-plan.md diff --git a/docs/plans/NEO-36-implementation-plan.md b/docs/plans/NEO-36-implementation-plan.md new file mode 100644 index 0000000..804c95b --- /dev/null +++ b/docs/plans/NEO-36-implementation-plan.md @@ -0,0 +1,81 @@ +# NEO-36 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-36 | +| **Title** | E2.M1: Read-only skill catalog HTTP projection + Bruno | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-36/e2m1-read-only-skill-catalog-http-projection-bruno | +| **Module** | [E2.M1 — SkillDefinitionRegistry](../decomposition/modules/E2_M1_SkillDefinitionRegistry.md) | + +## Kickoff clarifications + +No questions were put to the user during kickoff. + +**Reason:** The issue specifies the response fields (`id`, `displayName`, `category`, `allowedXpSourceKinds`), excludes mutations and client polish, and points at “match server routing style.” The repo already exposes a read-only world projection at `GET /game/world/interactables` (`InteractablesWorldApi`) with a versioned JSON envelope—this story can mirror that pattern. **`ISkillDefinitionRegistry`** and the frozen prototype trio (`salvage`, `refine`, `intrusion`) are present on `main` from **NEO-33–NEO-35** (`prototype_skills.json`, `SkillDefinitionRegistryTests`). Linear may still list **blockedBy NEO-35**; the implementation branch is based on **`main`** where that work is already merged. + +**If you want a different public path** than the default in Technical approach (below), say so before implementation. + +## Goal, scope, and out-of-scope + +**Goal:** Expose a stable, read-only JSON endpoint that returns all loaded prototype skill definitions for dev tooling, Bruno, and manual QA, backed by **`ISkillDefinitionRegistry`** without duplicating catalog truth outside the server. + +**In scope (from Linear):** + +- One GET route consistent with existing APIs (under `/game/`, same style as other read-only projections). +- JSON rows including **`id`**, **`displayName`**, **`category`**, **`allowedXpSourceKinds`** per skill. +- Bruno request(s) under `bruno/neon-sprawl-server/`, mirroring existing collections (e.g. `interaction/Get interactables list.bru`). +- `docs/manual-qa/NEO-36.md` checklist (endpoint is user-visible for QA per repo rules). + +**Out of scope (from Linear):** + +- Mutations or authoring APIs. +- Client UX / debug panel (optional and not required). + +## Acceptance criteria checklist + +- [ ] Bruno (and/or automated test) can call the endpoint against a running dev server and observe the **three frozen ids** (`salvage`, `refine`, `intrusion`). +- [ ] `docs/manual-qa/NEO-36.md` exists with a short checklist for hitting the endpoint. + +## Technical approach + +1. **Route:** Add **`GET /game/world/skill-definitions`** (parallel to **`/game/world/interactables`**) as a parameterless read. If “world” reads odd for catalog-only data, acceptable alternative is **`GET /game/content/skill-definitions`**—pick one in implementation and keep Bruno/tests aligned; default **`/game/world/skill-definitions`** for consistency with the other static registry projection. + +2. **Response shape:** Follow the interactables pattern: top-level **`schemaVersion`** (const `1` for first ship of this contract) plus an array (e.g. **`skills`**) of objects with **`id`**, **`displayName`**, **`category`**, **`allowedXpSourceKinds`**. Order skills by **`id`** ordinal to match **`ISkillDefinitionRegistry.GetDefinitionsInIdOrder()`**. + +3. **Implementation:** New static **`SkillDefinitionsWorldApi`** (or **`SkillCatalogApi`**) in `Game/Skills/` that injects **`ISkillDefinitionRegistry`**, maps rows to DTOs (either anonymous projection types or small sealed DTO types in a `*Dtos.cs` file next to the API), returns **`Results.Json`**. Wire **`app.MapSkillDefinitionsWorldApi()`** (name TBD) from **`Program.cs`** next to other `Map*Api` calls. + +4. **Bruno:** New folder `bruno/neon-sprawl-server/skill-definitions/` with **`Get skill definitions.bru`** (and optional **`folder.bru`** if the collection uses per-folder metadata like `ability-cast`). Reuse **`{{baseUrl}}`** from environments; tests assert 200, JSON, schema version, array length ≥ 3, and presence of the three frozen ids (mirror style of **`Get interactables list.bru`**). + +5. **Automated tests:** Add **`SkillDefinitionsWorldApiTests`** (or equivalent) using **`InMemoryWebApplicationFactory`**, **`HttpClient.GetAsync`**, **`ReadFromJsonAsync`** on the response DTO—same structure as **`InteractablesWorldApiTests`**, asserting the trio and ordering. + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionsWorldApi.cs` (or chosen name) | `Map*` extension registering GET route; maps registry → JSON. | +| `server/NeonSprawl.Server/Game/Skills/SkillDefinitionsListDtos.cs` (or inline records in API file if kept tiny) | Versioned response + row DTOs for JSON serialization. | +| `server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionsWorldApiTests.cs` | HTTP integration: 200, schema, three ids, id order. | +| `bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru` | Manual / Bruno verification against dev server. | +| `bruno/neon-sprawl-server/skill-definitions/folder.bru` | Optional; match Bruno folder conventions used elsewhere. | +| `docs/manual-qa/NEO-36.md` | Checklist: start server, call GET, confirm trio (and note Bruno path). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register the new `Map*Api()` alongside existing game APIs. | + +## Tests + +| Test file | What it covers | +|-----------|------------------| +| `server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionsWorldApiTests.cs` | **Integration:** `GET` new route via **`InMemoryWebApplicationFactory`**; **`ReadFromJsonAsync`**; assert **`schemaVersion`**, ordered **`id`** list equals **`intrusion`**, **`refine`**, **`salvage`** (ordinal sort); spot-check one row’s **`allowedXpSourceKinds`**. **AAA** comments per [csharp-style](../../.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert). | + +Bruno scripts are manual verification; they complement but do not replace the automated test above. + +## Open questions / risks + +- **Linear relation:** Issue may still show **blockedBy NEO-35**; code on **`main`** already includes the registry. If **`main`** lags a fork, rebase onto **`main`** after NEO-35 merge. +- **Path naming:** If product prefers **`/content/`** over **`/world/`**, swap in one place (route + Bruno + tests) before shipping. From ab597ff350caf8f8dd812c32ea19d3aac4ee014c Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 4 May 2026 20:36:06 -0400 Subject: [PATCH 2/4] NEO-36: expose GET /game/world/skill-definitions with tests, Bruno, QA doc --- .../Get skill definitions.bru | 46 +++++++++++++++++++ .../skill-definitions/folder.bru | 3 ++ docs/manual-qa/NEO-36.md | 14 ++++++ docs/plans/NEO-36-implementation-plan.md | 4 +- .../Skills/SkillDefinitionsWorldApiTests.cs | 39 ++++++++++++++++ .../Game/Skills/SkillDefinitionsListDtos.cs | 32 +++++++++++++ .../Game/Skills/SkillDefinitionsWorldApi.cs | 36 +++++++++++++++ server/NeonSprawl.Server/Program.cs | 1 + 8 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru create mode 100644 bruno/neon-sprawl-server/skill-definitions/folder.bru create mode 100644 docs/manual-qa/NEO-36.md create mode 100644 server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionsWorldApiTests.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillDefinitionsListDtos.cs create mode 100644 server/NeonSprawl.Server/Game/Skills/SkillDefinitionsWorldApi.cs diff --git a/bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru b/bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru new file mode 100644 index 0000000..d4cf25e --- /dev/null +++ b/bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru @@ -0,0 +1,46 @@ +meta { + name: GET skill definitions + type: http + seq: 1 +} + +get { + url: {{baseUrl}}/game/world/skill-definitions + body: none + auth: none +} + +tests { + test("returns 200 JSON with schema v1 and skills array", function () { + expect(res.getStatus()).to.equal(200); + expect(res.getHeader("content-type")).to.contain("application/json"); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.skills).to.be.an("array"); + expect(body.skills.length).to.equal(3); + }); + + test("skills are ascending by id (ordinal)", function () { + const body = res.getBody(); + const ids = body.skills.map((x) => x.id); + const sorted = [...ids].sort((a, b) => a.localeCompare(b)); + expect(ids).to.eql(sorted); + }); + + test("frozen prototype trio is present", function () { + const body = res.getBody(); + 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); + }); + + test("salvage row matches catalog", function () { + const body = res.getBody(); + const row = body.skills.find((x) => x.id === "salvage"); + expect(row).to.be.an("object"); + expect(row.displayName).to.equal("Salvage"); + expect(row.category).to.equal("gather"); + expect(row.allowedXpSourceKinds).to.include.members(["activity", "mission_reward"]); + }); +} diff --git a/bruno/neon-sprawl-server/skill-definitions/folder.bru b/bruno/neon-sprawl-server/skill-definitions/folder.bru new file mode 100644 index 0000000..6813433 --- /dev/null +++ b/bruno/neon-sprawl-server/skill-definitions/folder.bru @@ -0,0 +1,3 @@ +meta { + name: skill-definitions +} diff --git a/docs/manual-qa/NEO-36.md b/docs/manual-qa/NEO-36.md new file mode 100644 index 0000000..0b8f8db --- /dev/null +++ b/docs/manual-qa/NEO-36.md @@ -0,0 +1,14 @@ +# Manual QA — NEO-36 (read-only skill catalog HTTP) + +## Preconditions + +- Server built and configured with default `Content:SkillsDirectory` pointing at repo `content/skills` (local dev / `InMemoryWebApplicationFactory` tests use the same layout). + +## Checklist + +1. Start **`NeonSprawl.Server`** (e.g. `dotnet run` from `server/NeonSprawl.Server`). +2. **`GET /game/world/skill-definitions`** — expect **200** and **`Content-Type`** containing **`application/json`**. +3. Parse JSON — expect **`schemaVersion`** === **1**, **`skills`** array length **3**. +4. Confirm **`id`** values include **`intrusion`**, **`refine`**, **`salvage`** (frozen prototype trio). +5. Spot-check **`salvage`**: **`displayName`** “Salvage”, **`category`** **`gather`**, **`allowedXpSourceKinds`** includes **`activity`** and **`mission_reward`**. +6. Optional: run **`bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru`** against the same **`baseUrl`** (see `environments/Local.bru`). diff --git a/docs/plans/NEO-36-implementation-plan.md b/docs/plans/NEO-36-implementation-plan.md index 804c95b..df07127 100644 --- a/docs/plans/NEO-36-implementation-plan.md +++ b/docs/plans/NEO-36-implementation-plan.md @@ -35,8 +35,8 @@ No questions were put to the user during kickoff. ## Acceptance criteria checklist -- [ ] Bruno (and/or automated test) can call the endpoint against a running dev server and observe the **three frozen ids** (`salvage`, `refine`, `intrusion`). -- [ ] `docs/manual-qa/NEO-36.md` exists with a short checklist for hitting the endpoint. +- [x] Bruno (and/or automated test) can call the endpoint against a running dev server and observe the **three frozen ids** (`salvage`, `refine`, `intrusion`). +- [x] `docs/manual-qa/NEO-36.md` exists with a short checklist for hitting the endpoint. ## Technical approach diff --git a/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionsWorldApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionsWorldApiTests.cs new file mode 100644 index 0000000..7a2b25f --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Skills/SkillDefinitionsWorldApiTests.cs @@ -0,0 +1,39 @@ +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 class SkillDefinitionsWorldApiTests +{ + [Fact] + public async Task GetSkillDefinitions_ShouldReturnSchemaV1_WithFrozenTrioInIdOrder() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + // Act + var response = await client.GetAsync("/game/world/skill-definitions"); + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal(SkillDefinitionsListResponse.CurrentSchemaVersion, body!.SchemaVersion); + Assert.NotNull(body.Skills); + var ids = body.Skills.Select(static s => s.Id).ToList(); + Assert.Equal(new[] { "intrusion", "refine", "salvage" }, ids); + + var salvage = body.Skills.Single(s => s.Id == "salvage"); + Assert.Equal("Salvage", salvage.DisplayName); + Assert.Equal("gather", salvage.Category); + Assert.Equal(new[] { "activity", "mission_reward" }, salvage.AllowedXpSourceKinds); + + var refine = body.Skills.Single(s => s.Id == "refine"); + Assert.Contains("trainer", refine.AllowedXpSourceKinds); + + var intrusion = body.Skills.Single(s => s.Id == "intrusion"); + Assert.Contains("book_or_item", intrusion.AllowedXpSourceKinds); + } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionsListDtos.cs b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionsListDtos.cs new file mode 100644 index 0000000..7531c6a --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionsListDtos.cs @@ -0,0 +1,32 @@ +using System.Text.Json.Serialization; + +namespace NeonSprawl.Server.Game.Skills; + +/// JSON body for GET /game/world/skill-definitions (NEO-36). +public sealed class SkillDefinitionsListResponse +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + /// Loaded skills ordered by stable id (ordinal), matching . + [JsonPropertyName("skills")] + public required IReadOnlyList Skills { get; init; } +} + +/// One row in the read-only skill definition projection. +public sealed class SkillDefinitionJson +{ + [JsonPropertyName("id")] + public required string Id { get; init; } + + [JsonPropertyName("displayName")] + public required string DisplayName { get; init; } + + [JsonPropertyName("category")] + public required string Category { get; init; } + + [JsonPropertyName("allowedXpSourceKinds")] + public required IReadOnlyList AllowedXpSourceKinds { get; init; } +} diff --git a/server/NeonSprawl.Server/Game/Skills/SkillDefinitionsWorldApi.cs b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionsWorldApi.cs new file mode 100644 index 0000000..1658417 --- /dev/null +++ b/server/NeonSprawl.Server/Game/Skills/SkillDefinitionsWorldApi.cs @@ -0,0 +1,36 @@ +namespace NeonSprawl.Server.Game.Skills; + +/// Maps GET /game/world/skill-definitions (NEO-36). +public static class SkillDefinitionsWorldApi +{ + public static WebApplication MapSkillDefinitionsWorldApi(this WebApplication app) + { + app.MapGet( + "/game/world/skill-definitions", + (ISkillDefinitionRegistry registry) => + { + var defs = registry.GetDefinitionsInIdOrder(); + var skills = new List(defs.Count); + foreach (var d in defs) + { + skills.Add( + new SkillDefinitionJson + { + Id = d.Id, + DisplayName = d.DisplayName, + Category = d.Category, + AllowedXpSourceKinds = d.AllowedXpSourceKinds, + }); + } + + return Results.Json( + new SkillDefinitionsListResponse + { + SchemaVersion = SkillDefinitionsListResponse.CurrentSchemaVersion, + Skills = skills, + }); + }); + + return app; + } +} diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index 3a645f9..94ed641 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -27,6 +27,7 @@ app.MapGet("/health", () => Results.Json(new app.MapPositionStateApi(); app.MapInteractionApi(); app.MapInteractablesWorldApi(); +app.MapSkillDefinitionsWorldApi(); app.MapTargetingApi(); app.MapHotbarLoadoutApi(); app.MapCooldownSnapshotApi(); From 28fcd52c8c4da492a48703eaafae48876581660d Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 4 May 2026 20:37:20 -0400 Subject: [PATCH 3/4] NEO-36: add curl example to manual QA checklist --- docs/manual-qa/NEO-36.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/manual-qa/NEO-36.md b/docs/manual-qa/NEO-36.md index 0b8f8db..deb33bd 100644 --- a/docs/manual-qa/NEO-36.md +++ b/docs/manual-qa/NEO-36.md @@ -7,7 +7,11 @@ ## Checklist 1. Start **`NeonSprawl.Server`** (e.g. `dotnet run` from `server/NeonSprawl.Server`). -2. **`GET /game/world/skill-definitions`** — expect **200** and **`Content-Type`** containing **`application/json`**. +2. **`GET /game/world/skill-definitions`** — expect **200** and **`Content-Type`** containing **`application/json`**. Example (default dev URL from `Properties/launchSettings.json` and Bruno `environments/Local.bru`; change the host/port if yours differs): + + ```bash + curl -sS -i "http://localhost:5253/game/world/skill-definitions" + ``` 3. Parse JSON — expect **`schemaVersion`** === **1**, **`skills`** array length **3**. 4. Confirm **`id`** values include **`intrusion`**, **`refine`**, **`salvage`** (frozen prototype trio). 5. Spot-check **`salvage`**: **`displayName`** “Salvage”, **`category`** **`gather`**, **`allowedXpSourceKinds`** includes **`activity`** and **`mission_reward`**. From a0c35995256e2621aa5e17770adcf958db95b70f Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 4 May 2026 20:44:52 -0400 Subject: [PATCH 4/4] NEO-36: address code review (manual QA header, alignment, README, Bruno) --- .../Get skill definitions.bru | 3 +- ...umentation_and_implementation_alignment.md | 2 +- docs/manual-qa/NEO-36.md | 11 ++++- docs/reviews/2026-05-04-NEO-36.md | 47 +++++++++++++++++++ server/README.md | 8 ++++ 5 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 docs/reviews/2026-05-04-NEO-36.md diff --git a/bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru b/bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru index d4cf25e..04cb342 100644 --- a/bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru +++ b/bruno/neon-sprawl-server/skill-definitions/Get skill definitions.bru @@ -23,7 +23,8 @@ tests { test("skills are ascending by id (ordinal)", function () { const body = res.getBody(); const ids = body.skills.map((x) => x.id); - const sorted = [...ids].sort((a, b) => a.localeCompare(b)); + // Match server StringComparer.Ordinal (prototype ids are ASCII snake_case). + const sorted = [...ids].sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); expect(ids).to.eql(sorted); }); diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 06c9950..f1f4d2b 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -50,7 +50,7 @@ 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)). **Follow-on:** read HTTP + Bruno ([NEO-36](https://linear.app/neon-sprawl/issue/NEO-36)). | [NEO-33](../../plans/NEO-33-implementation-plan.md), [NEO-34](../../plans/NEO-34-implementation-plan.md), [NEO-35](../../plans/NEO-35-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/` | +| 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) | --- diff --git a/docs/manual-qa/NEO-36.md b/docs/manual-qa/NEO-36.md index deb33bd..c6b2531 100644 --- a/docs/manual-qa/NEO-36.md +++ b/docs/manual-qa/NEO-36.md @@ -1,4 +1,12 @@ -# Manual QA — NEO-36 (read-only skill catalog HTTP) +# NEO-36 — Manual QA checklist + +| Field | Value | +|-------|-------| +| Key | NEO-36 | +| Title | E2.M1: Read-only skill catalog HTTP projection + Bruno | +| Linear | https://linear.app/neon-sprawl/issue/NEO-36/e2m1-read-only-skill-catalog-http-projection-bruno | +| Plan | `docs/plans/NEO-36-implementation-plan.md` | +| Branch | `NEO-36-e2m1-read-only-skill-catalog-http-projection` | ## Preconditions @@ -12,6 +20,7 @@ ```bash curl -sS -i "http://localhost:5253/game/world/skill-definitions" ``` + 3. Parse JSON — expect **`schemaVersion`** === **1**, **`skills`** array length **3**. 4. Confirm **`id`** values include **`intrusion`**, **`refine`**, **`salvage`** (frozen prototype trio). 5. Spot-check **`salvage`**: **`displayName`** “Salvage”, **`category`** **`gather`**, **`allowedXpSourceKinds`** includes **`activity`** and **`mission_reward`**. diff --git a/docs/reviews/2026-05-04-NEO-36.md b/docs/reviews/2026-05-04-NEO-36.md new file mode 100644 index 0000000..aec843b --- /dev/null +++ b/docs/reviews/2026-05-04-NEO-36.md @@ -0,0 +1,47 @@ +# Code review: NEO-36 (read-only skill catalog HTTP) + +**Date:** 2026-05-04 + +**Scope:** Branch `NEO-36-e2m1-read-only-skill-catalog-http-projection` vs merge-base `a5f7f1b4aedfb7928440ac2e1fef7035895544a6` (`origin/main` at review time): `GET /game/world/skill-definitions`, `SkillDefinitionsWorldApi`, `SkillDefinitionsListDtos`, `SkillDefinitionsWorldApiTests`, Bruno `skill-definitions/`, `docs/plans/NEO-36-implementation-plan.md`, `docs/manual-qa/NEO-36.md`. + +**Base:** `origin/main` (merge-base above) + +## Verdict + +**Approve with nits** + +## Summary + +The branch adds a versioned read-only JSON projection backed by `ISkillDefinitionRegistry.GetDefinitionsInIdOrder()`, wired next to existing world projections in `Program.cs`, with integration tests mirroring `InteractablesWorldApiTests` (HTTP 200, `ReadFromJsonAsync`, schema version, frozen trio order, and spot-checks on `allowedXpSourceKinds`). Bruno requests assert the same contract. Behavior matches the NEO-36 plan and E2.M1 catalog intent: no mutations, no duplicate source of truth beyond the registry. + +## Documentation checked + +| Document | Result | +|----------|--------| +| `docs/plans/NEO-36-implementation-plan.md` | **Matches** — route `/game/world/skill-definitions`, `schemaVersion` + `skills` rows with the four fields, registry-backed mapping, tests + Bruno + manual QA as listed. | +| `docs/decomposition/modules/E2_M1_SkillDefinitionRegistry.md` | **Matches** — read-only exposure of stable ids, display names, categories, and `allowedXpSourceKinds`; aligns with “dev tooling / QA” style use; no XP grant enforcement here (E2.M2). | +| `docs/decomposition/modules/module_dependency_register.md` | **Matches** — E2.M1 in progress; HTTP projection is consistent with module scope. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E2.M1 row updated for NEO-36 landed (route, paths, README + manual QA pointers). | +| `docs/decomposition/modules/contracts.md` | **N/A** (not opened end-to-end) — response uses explicit `JsonPropertyName` and a `schemaVersion` const; same pattern as interactables list. | + +## Blocking issues + +(none) + +## Suggestions + +1. ~~**`docs/manual-qa/NEO-36.md` header** — [planning-implementation-docs](../../.cursor/rules/planning-implementation-docs.md) asks for a small table with **Key**, **Title**, **Linear** link, **Plan** link, and **Branch** on ticketed manual QA files. Add that table so the checklist matches repo convention (same pattern as other `docs/manual-qa/NEO-*.md` files that follow the rule).~~ **Done.** + +2. ~~**`documentation_and_implementation_alignment.md`** — Extend the E2.M1 row to note NEO-36: `GET /game/world/skill-definitions`, `Game/Skills/SkillDefinitionsWorldApi.cs` / DTOs, and pointer to `docs/plans/NEO-36-implementation-plan.md` (same PR or immediate follow-up per [documentation_and_implementation_alignment.md](../decomposition/modules/documentation_and_implementation_alignment.md)).~~ **Done.** + +3. ~~**`server/README.md` (optional)** — Skill catalog section documents load policy (NEO-34) but not the read projection. A one-line anchor to the GET route would match how NEO-25 interactables are surfaced for operators; only if you want parity with other HTTP surfaces.~~ **Done.** + +## Nits + +1. ~~**Nit:** Bruno test sorts ids with `localeCompare`; server order is ordinal (`GetDefinitionsInIdOrder`). For current ASCII `snake_case` ids they agree; if ids ever used non-ASCII, consider documenting or aligning sort semantics in Bruno.~~ **Done.** (ordinal sort + comment in Bruno.) + +## Verification + +- `dotnet test NeonSprawl.sln --filter "FullyQualifiedName~SkillDefinitionsWorldApiTests"` — passed at review time. +- Optional: full `dotnet test NeonSprawl.sln` before merge. +- Manual: follow `docs/manual-qa/NEO-36.md` (after adding the header table if you take suggestion 1) and/or run the Bruno request against `Local` environment. diff --git a/server/README.md b/server/README.md index 787decb..91d27a0 100644 --- a/server/README.md +++ b/server/README.md @@ -37,6 +37,14 @@ On startup the host loads every **`*.json`** under the skills directory, validat On success, **Information** logs include the resolved skills directory path and distinct skill count. +## Skill definitions (NEO-36) + +**`GET /game/world/skill-definitions`** returns a versioned JSON body (`schemaVersion` **1**, **`skills`**) backed by **`ISkillDefinitionRegistry`** — the same prototype rows loaded at startup (no second source of truth). Plan: [NEO-36 implementation plan](../../docs/plans/NEO-36-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-36.md`](../../docs/manual-qa/NEO-36.md); Bruno: `bruno/neon-sprawl-server/skill-definitions/`. + +```bash +curl -sS -i "http://localhost:5253/game/world/skill-definitions" +``` + ## 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).