From 374b4b5a537a82d4aedc31194d6b718220f3f4e2 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 22:20:39 -0400 Subject: [PATCH 1/4] NEO-78: Add kickoff implementation plan for ability-definitions GET. --- docs/plans/NEO-78-implementation-plan.md | 119 +++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 docs/plans/NEO-78-implementation-plan.md diff --git a/docs/plans/NEO-78-implementation-plan.md b/docs/plans/NEO-78-implementation-plan.md new file mode 100644 index 0000000..1d5fd85 --- /dev/null +++ b/docs/plans/NEO-78-implementation-plan.md @@ -0,0 +1,119 @@ +# NEO-78 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-78 | +| **Title** | E5M1-04: GET /game/world/ability-definitions | +| **Linear** | https://linear.app/neon-sprawl/issue/NEO-78/e5m1-04-get-gameworldability-definitions | +| **Module** | [E5.M1 — CombatRulesEngine](../decomposition/modules/E5_M1_CombatRulesEngine.md) · Epic 5 Slice 1 · backlog **E5M1-04** | +| **Branch** | `NEO-78-get-world-ability-definitions` | +| **Precursor** | [NEO-79](https://linear.app/neon-sprawl/issue/NEO-79) — `IAbilityDefinitionRegistry` + DI (**Done** on `main`) | +| **Pattern** | [NEO-68](https://linear.app/neon-sprawl/issue/NEO-68) — `GET /game/world/recipe-definitions`; [NEO-53](https://linear.app/neon-sprawl/issue/NEO-53) — item definitions world GET | +| **Client counterpart** | None (server-only); optional client enrichment in [NEO-85](https://linear.app/neon-sprawl/issue/NEO-85) via this HTTP route | + +## Kickoff clarifications + +| Topic | Question | Agent recommendation | Answer | +|--------|----------|----------------------|--------| +| **Manual QA doc** | Add `docs/manual-qa/NEO-78.md`? | **Yes** — NEO-68 pattern for world GET HTTP. | **User:** skip — manual QA only when there are client-facing changes; this story is server-only (Bruno + automated tests suffice). | +| **Optional `abilityKind`** | Emit when row has no kind? | **Omit property when null** — NEO-68 “loaded fields only”; all four prototype rows have a kind today. | **User:** omit when null. | + +No other blocking decisions — route path, envelope (`schemaVersion` **1**, **`abilities`** array), row fields, registry injection, and frozen four-id order are specified in [E5M1-04](E5M1-prototype-backlog.md#e5m1-04--get-gameworldability-definitions) and [E5.M1 freeze box](../decomposition/modules/E5_M1_CombatRulesEngine.md#prototype-slice-1-freeze-e5m1-01). + +## Goal, scope, and out-of-scope + +**Goal:** Expose a stable, read-only JSON endpoint that returns all four loaded prototype ability definitions for Bruno, tooling, and optional client preview ([NEO-85](https://linear.app/neon-sprawl/issue/NEO-85)) — backed by **`IAbilityDefinitionRegistry`** without duplicating catalog truth. + +**In scope (from Linear + [E5M1-04](E5M1-prototype-backlog.md#e5m1-04--get-gameworldability-definitions)):** + +- **`GET /game/world/ability-definitions`** with versioned envelope (`schemaVersion` **1**, **`abilities`** array). +- Row fields: **`id`**, **`displayName`**, **`baseDamage`**, **`cooldownSeconds`**, **`abilityKind`** (omit when null). +- Abilities ordered by **`id`** ordinal, matching **`GetDefinitionsInIdOrder()`**. +- Bruno folder `bruno/neon-sprawl-server/ability-definitions/`. +- `server/README.md` section. +- API integration tests (AAA). + +**Out of scope (from Linear + backlog):** + +- Godot HUD / client wiring ([NEO-85](https://linear.app/neon-sprawl/issue/NEO-85)). +- Combat resolution, damage application, cooldown commit ([NEO-80+](E5M1-prototype-backlog.md)). +- Per-player ability state or loadout mutation. +- `docs/manual-qa/NEO-78.md` (server-only; no client-facing changes per kickoff). + +## Acceptance criteria checklist + +- [ ] GET returns all **four** prototype abilities with **`schemaVersion`** **1** and stable field names (`id`, `displayName`, `baseDamage`, `cooldownSeconds`, `abilityKind` when present). +- [ ] Bruno happy path documents response shape and runs against dev server. +- [ ] Automated API tests (AAA) assert four ids in registry order and spot-check metadata. + +## Technical approach + +1. **Route:** **`GET /game/world/ability-definitions`** — parameterless read; inject **`IAbilityDefinitionRegistry`** only (not **`AbilityDefinitionCatalog`**). + +2. **Response shape:** Top-level **`schemaVersion`** (`AbilityDefinitionsListResponse.CurrentSchemaVersion` = **1**) plus **`abilities`** array. Each row maps from **`AbilityDefRow`**: **`id`**, **`displayName`**, **`baseDamage`**, **`cooldownSeconds`**, **`abilityKind`**. Use **`JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)`** on **`abilityKind`** so absent kinds are omitted from JSON. + +3. **Ordering:** Build list from **`registry.GetDefinitionsInIdOrder()`** (already ordinal by id). Tests assert exact id sequence (frozen four from [E5.M1 freeze box](../decomposition/modules/E5_M1_CombatRulesEngine.md#prototype-slice-1-freeze-e5m1-01)): + + `prototype_burst`, `prototype_dash`, `prototype_guard`, `prototype_pulse` + + (ordinal sort of the four ids — same order as **`AbilityDefinitionRegistryTests`** / catalog keys). + +4. **Implementation:** New **`AbilityDefinitionsWorldApi`** + **`AbilityDefinitionsListDtos.cs`** in `Game/Combat/` (mirror [`RecipeDefinitionsWorldApi`](../../server/NeonSprawl.Server/Game/Crafting/RecipeDefinitionsWorldApi.cs)). Wire **`app.MapAbilityDefinitionsWorldApi()`** from **`Program.cs`** next to other world definition APIs (after **`MapRecipeDefinitionsWorldApi()`**). + +5. **Bruno:** `bruno/neon-sprawl-server/ability-definitions/` with **`Get ability definitions.bru`** + **`folder.bru`**. Tests: 200, JSON, **`schemaVersion` 1**, **`abilities.length === 4`**, ascending id order, **exact frozen-four registry sequence**, spot-check **`prototype_pulse`** (`baseDamage` 25, `cooldownSeconds` 3.0, `abilityKind` `attack`) and **`prototype_burst`** (`baseDamage` 40, `cooldownSeconds` 5.0, `abilityKind` `attack`). + +6. **Docs:** Update **`server/README.md`** ability-catalog section with GET + curl example. Update [E5_M1](E5_M1_CombatRulesEngine.md) **Related implementation slices** and [documentation_and_implementation_alignment.md](documentation_and_implementation_alignment.md) E5.M1 row when landed. + +### Expected prototype row values (from content) + +| `id` | `displayName` | `baseDamage` | `cooldownSeconds` | `abilityKind` | +|------|---------------|--------------|-------------------|---------------| +| `prototype_pulse` | Prototype Pulse | 25 | 3.0 | `attack` | +| `prototype_guard` | Prototype Guard | 0 | 6.0 | `utility` | +| `prototype_dash` | Prototype Dash | 0 | 4.0 | `movement` | +| `prototype_burst` | Prototype Burst | 40 | 5.0 | `attack` | + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsWorldApi.cs` | `Map*` extension registering GET route; maps registry → JSON. | +| `server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsListDtos.cs` | Versioned response + row DTOs for JSON serialization. | +| `server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs` | HTTP integration: 200, schema v1, four ids in order, spot-check rows. | +| `bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru` | Manual / Bruno verification against dev server. | +| `bruno/neon-sprawl-server/ability-definitions/folder.bru` | Bruno folder metadata (match `recipe-definitions`). | +| `docs/plans/NEO-78-implementation-plan.md` | This plan. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register `MapAbilityDefinitionsWorldApi()` alongside other game world definition APIs. | +| `server/README.md` | Document `GET /game/world/ability-definitions`, curl example, links to plan/Bruno. | +| `docs/decomposition/modules/E5_M1_CombatRulesEngine.md` | **Related implementation slices** — HTTP read model bullet (NEO-78). | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E5.M1 row — note NEO-78 HTTP projection when landed. | + +## Tests + +| File | Coverage | +|------|----------| +| `server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs` | **Integration:** `GET /game/world/ability-definitions` via **`InMemoryWebApplicationFactory`**; **`ReadFromJsonAsync`**; assert **`schemaVersion` 1**; **`abilities`** count **4**; ordered **`id`** list equals frozen four in ordinal id order; spot-check **`prototype_pulse`** and **`prototype_burst`** damage/cooldown/kind. **AAA** per [csharp-style](../../.cursor/rules/csharp-style.md). | + +Bruno scripts complement automated tests for manual dev-server verification. No `docs/manual-qa/NEO-78.md` (server-only per kickoff). + +## Open questions / risks + +| Question / risk | Agent recommendation | Status | +|-----------------|---------------------|--------| +| **Linear blockedBy NEO-79** | Branch from **`main`** where NEO-79 is merged (confirmed at kickoff; branch fast-forwarded). | **adopted** | +| **Optional fields later** | When content adds new optional ability-def fields, bump **`schemaVersion`** or extend v1 in a follow-up issue. | **deferred** | +| **Client not in this story** | Cross-link **NEO-85** in plan; Bruno is not prototype-complete per [full-stack epic decomposition](../../.cursor/rules/full-stack-epic-decomposition.md). | **adopted** | +| **`cooldownSeconds` JSON type** | Emit as JSON **number** (double) matching content schema; tests assert numeric equality. | **adopted** | + +## Decisions (kickoff) + +- **No manual QA doc** — server-only HTTP; Bruno + automated tests (user confirmed). +- **`abilityKind` omitted when null** — v1 contract emits loaded fields only (user confirmed). +- **Registry-only injection** — HTTP layer depends on **`IAbilityDefinitionRegistry`**, not **`AbilityDefinitionCatalog`**. From 44e0056b37f122d04a369364ea5d55fada3e5ff7 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 22:22:48 -0400 Subject: [PATCH 2/4] NEO-78: Add GET /game/world/ability-definitions HTTP projection. Expose versioned read-only ability catalog via IAbilityDefinitionRegistry, with integration tests, Bruno folder, and docs alignment. --- .../Get ability definitions.bru | 69 +++++++++++++++++++ .../ability-definitions/folder.bru | 3 + .../modules/E5_M1_CombatRulesEngine.md | 4 +- ...umentation_and_implementation_alignment.md | 2 +- docs/plans/E5M1-prototype-backlog.md | 6 +- docs/plans/NEO-78-implementation-plan.md | 13 +++- .../Combat/AbilityDefinitionsWorldApiTests.cs | 49 +++++++++++++ .../Game/Combat/AbilityDefinitionsListDtos.cs | 36 ++++++++++ .../Game/Combat/AbilityDefinitionsWorldApi.cs | 37 ++++++++++ server/NeonSprawl.Server/Program.cs | 1 + server/README.md | 8 +++ 11 files changed, 221 insertions(+), 7 deletions(-) create mode 100644 bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru create mode 100644 bruno/neon-sprawl-server/ability-definitions/folder.bru create mode 100644 server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs create mode 100644 server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsListDtos.cs create mode 100644 server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsWorldApi.cs diff --git a/bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru b/bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru new file mode 100644 index 0000000..b1cc67a --- /dev/null +++ b/bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru @@ -0,0 +1,69 @@ +meta { + name: GET ability definitions + type: http + seq: 1 +} + +get { + url: {{baseUrl}}/game/world/ability-definitions + body: none + auth: none +} + +tests { + test("returns 200 JSON with schema v1 and abilities 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.abilities).to.be.an("array"); + expect(body.abilities.length).to.equal(4); + }); + + test("abilities are ascending by id (ordinal)", function () { + const body = res.getBody(); + const ids = body.abilities.map((x) => x.id); + const sorted = [...ids].sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); + expect(ids).to.eql(sorted); + }); + + test("frozen prototype four matches registry id order", function () { + const body = res.getBody(); + const ids = body.abilities.map((x) => x.id); + expect(ids).to.eql([ + "prototype_burst", + "prototype_dash", + "prototype_guard", + "prototype_pulse", + ]); + }); + + test("frozen prototype four is present", function () { + const body = res.getBody(); + const ids = new Set(body.abilities.map((x) => x.id)); + expect(ids.has("prototype_burst")).to.equal(true); + expect(ids.has("prototype_dash")).to.equal(true); + expect(ids.has("prototype_guard")).to.equal(true); + expect(ids.has("prototype_pulse")).to.equal(true); + }); + + test("prototype_pulse row matches catalog", function () { + const body = res.getBody(); + const row = body.abilities.find((x) => x.id === "prototype_pulse"); + expect(row).to.be.an("object"); + expect(row.displayName).to.equal("Prototype Pulse"); + expect(row.baseDamage).to.equal(25); + expect(row.cooldownSeconds).to.equal(3); + expect(row.abilityKind).to.equal("attack"); + }); + + test("prototype_burst row matches catalog", function () { + const body = res.getBody(); + const row = body.abilities.find((x) => x.id === "prototype_burst"); + expect(row).to.be.an("object"); + expect(row.displayName).to.equal("Prototype Burst"); + expect(row.baseDamage).to.equal(40); + expect(row.cooldownSeconds).to.equal(5); + expect(row.abilityKind).to.equal("attack"); + }); +} diff --git a/bruno/neon-sprawl-server/ability-definitions/folder.bru b/bruno/neon-sprawl-server/ability-definitions/folder.bru new file mode 100644 index 0000000..575839c --- /dev/null +++ b/bruno/neon-sprawl-server/ability-definitions/folder.bru @@ -0,0 +1,3 @@ +meta { + name: ability-definitions +} diff --git a/docs/decomposition/modules/E5_M1_CombatRulesEngine.md b/docs/decomposition/modules/E5_M1_CombatRulesEngine.md index 4b588a7..7e72b5e 100644 --- a/docs/decomposition/modules/E5_M1_CombatRulesEngine.md +++ b/docs/decomposition/modules/E5_M1_CombatRulesEngine.md @@ -7,7 +7,7 @@ | **Module ID** | E5.M1 | | **Epic** | [Epic 5 — PvE Combat](../epics/epic_05_pve_combat.md) | | **Stage target** | Prototype | -| **Status** | In Progress — Slice 1 backlog [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md): E5M1-01 [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) ability catalog + CI landed; E5M1-02 [NEO-77](https://linear.app/neon-sprawl/issue/NEO-77) server load landed; E5M1-03 [NEO-79](https://linear.app/neon-sprawl/issue/NEO-79) registry + hotbar/cast migration landed; E5M1-04+ pending (see [dependency register](module_dependency_register.md)) | +| **Status** | In Progress — Slice 1 backlog [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md): E5M1-01 [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) ability catalog + CI landed; E5M1-02 [NEO-77](https://linear.app/neon-sprawl/issue/NEO-77) server load landed; E5M1-03 [NEO-79](https://linear.app/neon-sprawl/issue/NEO-79) registry + hotbar/cast migration landed; E5M1-04 [NEO-78](https://linear.app/neon-sprawl/issue/NEO-78) ability-definitions GET landed; E5M1-05+ pending (see [dependency register](module_dependency_register.md)) | | **Linear** | Label **`E5.M1`** · [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md) **E5M1-01** [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) → **E5M1-12** [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); gig XP **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44) | ## Purpose @@ -50,6 +50,8 @@ Core tab-target combat resolution: valid actions against a locked target, hit re See Epic 5 **Slice 1 — Combat rules MVP**: target lock, basic attacks, 4–6 abilities, cooldowns/resources; telemetry `encounter_start`, `ability_used`, `player_death`, `enemy_defeat`. +**E5M1-04 (NEO-78):** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78 plan](../../plans/NEO-78-implementation-plan.md)); [server README — Ability definitions (NEO-78)](../../../server/README.md#ability-definitions-neo-78); Bruno `bruno/neon-sprawl-server/ability-definitions/`. + ## Linear backlog (decomposed) Full story tables, kickoff defaults, and **`blockedBy` graph:** [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md). diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 1d59e0d..c24cbfe 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -56,7 +56,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E3.M1 | Ready | **NEO-41 landed (prototype, superseded on interact by NEO-63):** inline XP on **`resource_node`** interact. **NEO-57–NEO-61** — content, registry, HTTP defs, depletion store (see prior alignment). **NEO-62 landed:** **`GatherOperations`** + **`GatherResult`**. **NEO-63 landed:** **`POST …/interact`** **`resource_node`** → gather engine; **four** **`PrototypeInteractableRegistry`** anchors; Bruno gather → inventory + depletion deny ([NEO-63](../../plans/NEO-63-implementation-plan.md), [`NEO-63` manual QA](../../manual-qa/NEO-63.md)); [server README — Gather via interact (NEO-63)](../../../server/README.md#gather-via-interact-neo-63). **NEO-64 landed:** comment-only **`resource_gathered`** / **`gather_node_depleted`** hook sites in **`GatherOperations.TryGather`** ([NEO-64](../../plans/NEO-64-implementation-plan.md), [`NEO-64` manual QA](../../manual-qa/NEO-64.md)); Epic 3 Slice 2 backlog **E3M1-01**–**E3M1-08** complete. **NEO-73 landed:** client gather feedback — **`skill_progression_client.gd`**, **`prototype_interactable_picker.gd`** (nearest in-range **R**), **`GatherFeedbackLabel`** + **`SkillProgressionLabel`**, auto inventory/skill refresh after successful gather ([NEO-73](../../plans/NEO-73-implementation-plan.md), [`NEO-73` manual QA](../../manual-qa/NEO-73.md)); `client/README.md` gather section. **NEO-75 landed:** capstone gather→refine→make loop in Godot — [`NEO-75` manual QA](../../manual-qa/NEO-75.md), `client/README.md` end-to-end section; Epic 3 Slice 5 client complete. | [NEO-41](../../plans/NEO-41-implementation-plan.md) … [NEO-64](../../plans/NEO-64-implementation-plan.md), [NEO-73](../../plans/NEO-73-implementation-plan.md), [NEO-75](../../plans/NEO-75-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M1](E3_M1_ResourceNodeAndGatherLoop.md); `server/NeonSprawl.Server/Game/Gathering/`, `Game/Interaction/`, `client/scripts/skill_progression_client.gd`, `client/scripts/prototype_interactable_picker.gd` | | E3.M3 | In Progress | **NEO-50 landed:** frozen prototype six-item catalog in [`content/items/prototype_items.json`](../../../content/items/prototype_items.json); [`item-def.schema.json`](../../../content/schemas/item-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py). **NEO-51 landed:** fail-fast server load of `content/items/*_items.json` at startup — `server/NeonSprawl.Server/Game/Items/` ([NEO-51](../../plans/NEO-51-implementation-plan.md)); [server README — Item catalog](../../../server/README.md#item-catalog-contentitems-neo-51). **NEO-52 landed:** injectable **`IItemDefinitionRegistry`** + lookup tests ([NEO-52](../../plans/NEO-52-implementation-plan.md)). **NEO-53 landed:** **`GET /game/world/item-definitions`** — `ItemDefinitionsWorldApi` + DTOs in `Game/Items/` ([NEO-53](../../plans/NEO-53-implementation-plan.md), [`NEO-53` manual QA](../../manual-qa/NEO-53.md)); [server README — Item definitions (NEO-53)](../../../server/README.md#item-definitions-neo-53); Bruno `bruno/neon-sprawl-server/item-definitions/`. **NEO-54 landed:** **`IPlayerInventoryStore`** + **`PlayerInventoryOperations`** — 24 bag + 1 equipment slots, stack rules, `V005` migration ([NEO-54](../../plans/NEO-54-implementation-plan.md)). **NEO-55 landed:** **`GET`/`POST /game/players/{id}/inventory`** — `PlayerInventoryApi` + DTOs in `Game/Items/` ([NEO-55](../../plans/NEO-55-implementation-plan.md)); [server README — Player inventory (NEO-54 store, NEO-55 HTTP)](../../../server/README.md#player-inventory-neo-54-store-neo-55-http); Bruno `bruno/neon-sprawl-server/inventory/`. **NEO-56 landed:** comment-only **`item_created`** / **`inventory_transfer_denied`** telemetry hook sites in [`PlayerInventoryOperations`](../../../server/NeonSprawl.Server/Game/Items/PlayerInventoryOperations.cs) ([NEO-56](../../plans/NEO-56-implementation-plan.md), [`NEO-56` manual QA](../../manual-qa/NEO-56.md)); no E9.M1 ingest. **NEO-72 landed:** client inventory HUD — **`inventory_client.gd`**, **`item_definitions_client.gd`**, **`InventoryLabel`**, boot hydrate + **`I`** refresh ([NEO-72](../../plans/NEO-72-implementation-plan.md), [`NEO-72` manual QA](../../manual-qa/NEO-72.md)); `client/README.md` inventory HUD section. **NEO-75 landed:** **`prototype_economy_hud_section.gd`** collapse toggle + capstone manual QA ([NEO-75](../../plans/NEO-75-implementation-plan.md), [`NEO-75` manual QA](../../manual-qa/NEO-75.md)); Epic 3 Slice 5 client complete. | [NEO-50](../../plans/NEO-50-implementation-plan.md), [NEO-51](../../plans/NEO-51-implementation-plan.md), [NEO-52](../../plans/NEO-52-implementation-plan.md), [NEO-53](../../plans/NEO-53-implementation-plan.md), [NEO-54](../../plans/NEO-54-implementation-plan.md), [NEO-55](../../plans/NEO-55-implementation-plan.md), [NEO-56](../../plans/NEO-56-implementation-plan.md), [NEO-72](../../plans/NEO-72-implementation-plan.md), [NEO-75](../../plans/NEO-75-implementation-plan.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3M3-prototype-backlog](../../plans/E3M3-prototype-backlog.md), [E3_M3](E3_M3_ItemizationAndInventorySchema.md) | | E3.M2 | Ready | **NEO-65 landed:** frozen prototype eight-recipe catalog in [`content/recipes/prototype_recipes.json`](../../../content/recipes/prototype_recipes.json); [`recipe-def.schema.json`](../../../content/schemas/recipe-def.schema.json) + [`recipe-io-row.schema.json`](../../../content/schemas/recipe-io-row.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) Slice 3 gates ([NEO-65](../../plans/NEO-65-implementation-plan.md)). **NEO-66 landed:** fail-fast server load of `content/recipes/*_recipes.json` at startup — `server/NeonSprawl.Server/Game/Crafting/` ([NEO-66](../../plans/NEO-66-implementation-plan.md)); [server README — Recipe catalog](../../../server/README.md#recipe-catalog-contentrecipes-neo-66). **NEO-67 landed:** injectable **`IRecipeDefinitionRegistry`** + lookup tests ([NEO-67](../../plans/NEO-67-implementation-plan.md)). **NEO-68 landed:** **`GET /game/world/recipe-definitions`** — `RecipeDefinitionsWorldApi` + DTOs in `Game/Crafting/` ([NEO-68](../../plans/NEO-68-implementation-plan.md), [`NEO-68` manual QA](../../manual-qa/NEO-68.md)); [server README — Recipe definitions (NEO-68)](../../../server/README.md#recipe-definitions-neo-68); Bruno `bruno/neon-sprawl-server/recipe-definitions/`. **NEO-69 landed:** **`CraftOperations.TryCraft`** + **`CraftResult`** — pre-flight inputs/outputs, inventory mutation, refine XP ([NEO-69](../../plans/NEO-69-implementation-plan.md)); [server README — Craft engine (NEO-69)](../../../server/README.md#craft-engine-neo-69-and-craft-http-neo-70). **NEO-70 landed:** **`POST /game/players/{id}/craft`** — `PlayerCraftApi` + wire **`CraftResponse`**; Bruno gather→refine→make spine ([NEO-70](../../plans/NEO-70-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/craft/`. **NEO-71 landed:** comment-only **`item_crafted`** / **`craft_failed`** telemetry hook sites in **`CraftOperations.TryCraft`** ([NEO-71](../../plans/NEO-71-implementation-plan.md)); [server README — Craft telemetry hooks (NEO-71)](../../../server/README.md#craft-telemetry-hooks-neo-71). **NEO-42 landed (prep):** **`RefineActivitySkillXpGrant`** / **`RefineSkillXpConstants`** — wired from craft engine success path. Epic 3 Slice 3 server backlog **E3M2-01**–**E3M2-07** complete. Upstream: E3.M1 **Ready**, E3.M3 inventory landed. **NEO-74 landed:** client craft UI — **`recipe_definitions_client.gd`**, **`craft_client.gd`**, **`craft_recipe_panel.gd`**, **`CraftFeedbackLabel`**, **refine** skill row ([NEO-74](../../plans/NEO-74-implementation-plan.md), [`NEO-74` manual QA](../../manual-qa/NEO-74.md)); `client/README.md` craft section. **NEO-75 landed:** capstone playable gather→refine→make loop — [`NEO-75` manual QA](../../manual-qa/NEO-75.md), **`prototype_economy_hud_section.gd`**; Epic 3 Slice 5 client complete. | [NEO-42](../../plans/NEO-42-implementation-plan.md), [NEO-65](../../plans/NEO-65-implementation-plan.md), [NEO-66](../../plans/NEO-66-implementation-plan.md), [NEO-67](../../plans/NEO-67-implementation-plan.md), [NEO-68](../../plans/NEO-68-implementation-plan.md), [NEO-69](../../plans/NEO-69-implementation-plan.md), [NEO-70](../../plans/NEO-70-implementation-plan.md), [NEO-71](../../plans/NEO-71-implementation-plan.md), [NEO-74](../../plans/NEO-74-implementation-plan.md), [NEO-75](../../plans/NEO-75-implementation-plan.md), [E3M2-prototype-backlog](../../plans/E3M2-prototype-backlog.md), [E3S5-client-prototype-backlog](../../plans/E3S5-client-prototype-backlog.md), [E3_M2](E3_M2_RefinementAndRecipeExecution.md) | -| E5.M1 | In Progress | **NEO-76 landed:** frozen prototype four-ability catalog in [`content/abilities/prototype_abilities.json`](../../../content/abilities/prototype_abilities.json); [`ability-def.schema.json`](../../../content/schemas/ability-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M1 four-id gate ([NEO-76](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` at startup — `server/NeonSprawl.Server/Game/Combat/` ([NEO-77](../../plans/NEO-77-implementation-plan.md)); [server README — Ability catalog](../../../server/README.md#ability-catalog-contentabilities-neo-77). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79](../../plans/NEO-79-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/hotbar-loadout/` + `ability-cast/` unknown-ability deny smokes. **Backlog decomposed:** Epic 5 Slice 1 — [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md) **E5M1-04** [NEO-78](https://linear.app/neon-sprawl/issue/NEO-78) through **E5M1-12** [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); gig XP **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). **Precursor landed:** E1.M4 cast funnel (NEO-28/31/32) — **`POST …/ability-cast`** accepts without **`CombatResolution`** yet. | [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1](E5_M1_CombatRulesEngine.md), [NEO-77](../../plans/NEO-77-implementation-plan.md), [NEO-79](../../plans/NEO-79-implementation-plan.md), [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) | +| E5.M1 | In Progress | **NEO-76 landed:** frozen prototype four-ability catalog in [`content/abilities/prototype_abilities.json`](../../../content/abilities/prototype_abilities.json); [`ability-def.schema.json`](../../../content/schemas/ability-def.schema.json); PR gate + [`validate_content.py`](../../../scripts/validate_content.py) E5M1 four-id gate ([NEO-76](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` at startup — `server/NeonSprawl.Server/Game/Combat/` ([NEO-77](../../plans/NEO-77-implementation-plan.md)); [server README — Ability catalog](../../../server/README.md#ability-catalog-contentabilities-neo-77). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79](../../plans/NEO-79-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/hotbar-loadout/` + `ability-cast/` unknown-ability deny smokes. **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78](../../plans/NEO-78-implementation-plan.md)); [server README — Ability definitions (NEO-78)](../../../server/README.md#ability-definitions-neo-78); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **Backlog decomposed:** Epic 5 Slice 1 — [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md) **E5M1-05** [NEO-80](https://linear.app/neon-sprawl/issue/NEO-80) through **E5M1-12** [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); gig XP **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). **Precursor landed:** E1.M4 cast funnel (NEO-28/31/32) — **`POST …/ability-cast`** accepts without **`CombatResolution`** yet. | [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1](E5_M1_CombatRulesEngine.md), [NEO-77](../../plans/NEO-77-implementation-plan.md), [NEO-79](../../plans/NEO-79-implementation-plan.md), [NEO-78](../../plans/NEO-78-implementation-plan.md), [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) | --- diff --git a/docs/plans/E5M1-prototype-backlog.md b/docs/plans/E5M1-prototype-backlog.md index 47fd303..ed4c589 100644 --- a/docs/plans/E5M1-prototype-backlog.md +++ b/docs/plans/E5M1-prototype-backlog.md @@ -162,8 +162,10 @@ Working backlog for **Epic 5 — Slice 1** ([combat rules MVP](../decomposition/ **Acceptance criteria** -- [ ] GET returns all four prototype abilities with stable field names. -- [ ] Bruno happy path documents response shape. +- [x] GET returns all four prototype abilities with stable field names. +- [x] Bruno happy path documents response shape. + +**Landed ([NEO-78](https://linear.app/neon-sprawl/issue/NEO-78)):** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78-implementation-plan.md](NEO-78-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/ability-definitions/`. **Client counterpart:** optional enrichment in [NEO-85](#e5m1-11--client-combat-feedback--target-hp-hud) — not a separate issue. diff --git a/docs/plans/NEO-78-implementation-plan.md b/docs/plans/NEO-78-implementation-plan.md index 1d5fd85..0fc6031 100644 --- a/docs/plans/NEO-78-implementation-plan.md +++ b/docs/plans/NEO-78-implementation-plan.md @@ -44,9 +44,9 @@ No other blocking decisions — route path, envelope (`schemaVersion` **1**, **` ## Acceptance criteria checklist -- [ ] GET returns all **four** prototype abilities with **`schemaVersion`** **1** and stable field names (`id`, `displayName`, `baseDamage`, `cooldownSeconds`, `abilityKind` when present). -- [ ] Bruno happy path documents response shape and runs against dev server. -- [ ] Automated API tests (AAA) assert four ids in registry order and spot-check metadata. +- [x] GET returns all **four** prototype abilities with **`schemaVersion`** **1** and stable field names (`id`, `displayName`, `baseDamage`, `cooldownSeconds`, `abilityKind` when present). +- [x] Bruno happy path documents response shape and runs against dev server. +- [x] Automated API tests (AAA) assert four ids in registry order and spot-check metadata. ## Technical approach @@ -117,3 +117,10 @@ Bruno scripts complement automated tests for manual dev-server verification. No - **No manual QA doc** — server-only HTTP; Bruno + automated tests (user confirmed). - **`abilityKind` omitted when null** — v1 contract emits loaded fields only (user confirmed). - **Registry-only injection** — HTTP layer depends on **`IAbilityDefinitionRegistry`**, not **`AbilityDefinitionCatalog`**. + +## Reconciliation (implementation) + +- **`AbilityDefinitionsWorldApi`** + **`AbilityDefinitionsListDtos`** registered via **`MapAbilityDefinitionsWorldApi()`** in **`Program.cs`**; injects **`IAbilityDefinitionRegistry`** only. +- **`AbilityDefinitionsWorldApiTests`** — integration test for schema v1, frozen-four id order, **`prototype_pulse`** / **`prototype_burst`** spot-checks. +- **Bruno:** `bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru` with response-shape tests. +- **`server/README.md`**, **E5.M1** module doc, **alignment register**, and **E5M1 backlog** updated. diff --git a/server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs new file mode 100644 index 0000000..d56f097 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs @@ -0,0 +1,49 @@ +using System.Linq; +using System.Net; +using System.Net.Http.Json; +using NeonSprawl.Server.Game.Combat; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.Combat; + +public class AbilityDefinitionsWorldApiTests +{ + /// Frozen prototype four in registry id order (ordinal). Keep in sync with Bruno. + public static readonly string[] FrozenFourInIdOrder = + [ + "prototype_burst", + "prototype_dash", + "prototype_guard", + "prototype_pulse", + ]; + + [Fact] + public async Task GetAbilityDefinitions_ShouldReturnSchemaV1_WithFrozenFourInIdOrder() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + // Act + var response = await client.GetAsync("/game/world/ability-definitions"); + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal(AbilityDefinitionsListResponse.CurrentSchemaVersion, body!.SchemaVersion); + Assert.NotNull(body.Abilities); + var ids = body.Abilities.Select(static a => a.Id).ToList(); + Assert.Equal(FrozenFourInIdOrder, ids); + + var pulse = body.Abilities.Single(a => a.Id == "prototype_pulse"); + Assert.Equal("Prototype Pulse", pulse.DisplayName); + Assert.Equal(25, pulse.BaseDamage); + Assert.Equal(3.0, pulse.CooldownSeconds); + Assert.Equal("attack", pulse.AbilityKind); + + var burst = body.Abilities.Single(a => a.Id == "prototype_burst"); + Assert.Equal("Prototype Burst", burst.DisplayName); + Assert.Equal(40, burst.BaseDamage); + Assert.Equal(5.0, burst.CooldownSeconds); + Assert.Equal("attack", burst.AbilityKind); + } +} diff --git a/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsListDtos.cs b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsListDtos.cs new file mode 100644 index 0000000..036949c --- /dev/null +++ b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsListDtos.cs @@ -0,0 +1,36 @@ +using System.Text.Json.Serialization; + +namespace NeonSprawl.Server.Game.Combat; + +/// JSON body for GET /game/world/ability-definitions (NEO-78). +public sealed class AbilityDefinitionsListResponse +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + /// Loaded abilities ordered by stable id (ordinal), matching . + [JsonPropertyName("abilities")] + public required IReadOnlyList Abilities { get; init; } +} + +/// One row in the read-only ability definition projection. +public sealed class AbilityDefinitionJson +{ + [JsonPropertyName("id")] + public required string Id { get; init; } + + [JsonPropertyName("displayName")] + public required string DisplayName { get; init; } + + [JsonPropertyName("baseDamage")] + public required int BaseDamage { get; init; } + + [JsonPropertyName("cooldownSeconds")] + public required double CooldownSeconds { get; init; } + + [JsonPropertyName("abilityKind")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? AbilityKind { get; init; } +} diff --git a/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsWorldApi.cs b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsWorldApi.cs new file mode 100644 index 0000000..7d4d28e --- /dev/null +++ b/server/NeonSprawl.Server/Game/Combat/AbilityDefinitionsWorldApi.cs @@ -0,0 +1,37 @@ +namespace NeonSprawl.Server.Game.Combat; + +/// Maps GET /game/world/ability-definitions (NEO-78). +public static class AbilityDefinitionsWorldApi +{ + public static WebApplication MapAbilityDefinitionsWorldApi(this WebApplication app) + { + app.MapGet( + "/game/world/ability-definitions", + (IAbilityDefinitionRegistry registry) => + { + var defs = registry.GetDefinitionsInIdOrder(); + var abilities = new List(defs.Count); + foreach (var d in defs) + { + abilities.Add( + new AbilityDefinitionJson + { + Id = d.Id, + DisplayName = d.DisplayName, + BaseDamage = d.BaseDamage, + CooldownSeconds = d.CooldownSeconds, + AbilityKind = d.AbilityKind, + }); + } + + return Results.Json( + new AbilityDefinitionsListResponse + { + SchemaVersion = AbilityDefinitionsListResponse.CurrentSchemaVersion, + Abilities = abilities, + }); + }); + + return app; + } +} diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index 87ae584..7e52634 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -48,6 +48,7 @@ app.MapInteractablesWorldApi(); app.MapSkillDefinitionsWorldApi(); app.MapItemDefinitionsWorldApi(); app.MapRecipeDefinitionsWorldApi(); +app.MapAbilityDefinitionsWorldApi(); app.MapResourceNodeDefinitionsWorldApi(); app.MapPlayerInventoryApi(); app.MapPlayerCraftApi(); diff --git a/server/README.md b/server/README.md index 45021c9..eb26759 100644 --- a/server/README.md +++ b/server/README.md @@ -93,6 +93,14 @@ On startup the host loads every **`*_abilities.json`** under the abilities direc On success, **Information** logs include the resolved abilities directory path, distinct ability count, and catalog file count. Game code should use **`IAbilityDefinitionRegistry`** for lookups (NEO-79). The catalog singleton remains for fail-fast startup only; do not inject **`AbilityDefinitionCatalog`** in new game code. Hotbar loadout and ability cast routes validate ability ids via **`IAbilityDefinitionRegistry.TryNormalizeKnown`** (backed by the loaded catalog, not a separate static allowlist). +## Ability definitions (NEO-78) + +**`GET /game/world/ability-definitions`** returns a versioned JSON body (`schemaVersion` **1**, **`abilities`**) backed by **`IAbilityDefinitionRegistry`** — the same prototype rows loaded at startup (no second source of truth). Each row includes **`id`**, **`displayName`**, **`baseDamage`**, **`cooldownSeconds`**, and **`abilityKind`** when present in content. Plan: [NEO-78 implementation plan](../../docs/plans/NEO-78-implementation-plan.md); Bruno: `bruno/neon-sprawl-server/ability-definitions/`. + +```bash +curl -sS -i "http://localhost:5253/game/world/ability-definitions" +``` + ## Recipe definitions (NEO-68) **`GET /game/world/recipe-definitions`** returns a versioned JSON body (`schemaVersion` **1**, **`recipes`**) backed by **`IRecipeDefinitionRegistry`** — the same prototype rows loaded at startup (no second source of truth). Each row includes **`id`**, **`displayName`**, **`recipeKind`**, **`requiredSkillId`**, and nested **`inputs`** / **`outputs`** (`itemId`, `quantity`). Plan: [NEO-68 implementation plan](../../docs/plans/NEO-68-implementation-plan.md); manual QA: [`docs/manual-qa/NEO-68.md`](../../docs/manual-qa/NEO-68.md); Bruno: `bruno/neon-sprawl-server/recipe-definitions/`. From 8397e74ec147dae1a8b6d930294bcf5ad884c83f Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 22:24:33 -0400 Subject: [PATCH 3/4] NEO-78: Add code review for ability-definitions GET story. --- docs/reviews/2026-05-24-NEO-78.md | 61 +++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 docs/reviews/2026-05-24-NEO-78.md diff --git a/docs/reviews/2026-05-24-NEO-78.md b/docs/reviews/2026-05-24-NEO-78.md new file mode 100644 index 0000000..aa18f6b --- /dev/null +++ b/docs/reviews/2026-05-24-NEO-78.md @@ -0,0 +1,61 @@ +# Code review — NEO-78 GET world ability-definitions + +**Date:** 2026-05-24 +**Scope:** Branch `NEO-78-get-world-ability-definitions` · commits `374b4b5`–`44e0056` vs `origin/main` +**Base:** `origin/main` + +## Verdict + +**Approve with nits** + +## Summary + +NEO-78 delivers **E5M1-04**: a read-only **`GET /game/world/ability-definitions`** endpoint backed by **`IAbilityDefinitionRegistry.GetDefinitionsInIdOrder()`**, with versioned DTOs (`schemaVersion` **1**, **`abilities`** array), row projection (`id`, `displayName`, `baseDamage`, `cooldownSeconds`, optional **`abilityKind`** omitted when null), Bruno coverage, and an AAA integration test asserting the frozen-four id order plus **`prototype_pulse`** / **`prototype_burst`** spot-checks. Implementation closely mirrors the landed NEO-68 recipe-definitions and NEO-53 item-definitions patterns; no auth, no second catalog source, no client wiring (deferred to NEO-85). Residual risk is low — additive HTTP read model on existing registry truth. + +## Documentation checked + +| Document | Result | +|----------|--------| +| [`docs/plans/NEO-78-implementation-plan.md`](../plans/NEO-78-implementation-plan.md) | **Matches** — kickoff decisions (no manual QA, omit null `abilityKind`, registry-only injection); acceptance checklist complete; reconciliation section reflects landed files. | +| [`docs/plans/E5M1-prototype-backlog.md`](../plans/E5M1-prototype-backlog.md) · **E5M1-04** | **Matches** — AC checkboxes checked; landed note with plan + Bruno links. In-scope line still mentions “manual QA stub if needed” (kickoff override documented in plan only). | +| [`docs/decomposition/modules/E5_M1_CombatRulesEngine.md`](../decomposition/modules/E5_M1_CombatRulesEngine.md) | **Matches** — Status cites NEO-78 landed; **Related implementation slices** HTTP bullet added; freeze table ids align with test assertions. | +| [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E5.M1 row includes NEO-78 HTTP projection + Bruno folder. | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E5.M1 note still ends at **NEO-79 landed**; does not yet cite NEO-78 GET (see suggestion). | +| [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — read-only world catalog; server source of truth; no client mutation in this story. | +| NEO-68 / NEO-53 reference implementations | **Matches** — `AbilityDefinitionsWorldApi` + separate DTO file mirror `RecipeDefinitionsWorldApi` / `ItemDefinitionsWorldApi`; integration test shape matches `RecipeDefinitionsWorldApiTests`. | +| Full-stack epic decomposition | **Matches** — plan documents no client counterpart; NEO-85 optional enrichment; does not claim prototype slice complete without Godot. | +| [`server/README.md`](../../server/README.md) | **Matches** — new **Ability definitions (NEO-78)** section with curl example and plan/Bruno links. | + +## Blocking issues + +None. + +## Suggestions + +1. **Dependency register E5.M1 note** — Extend the **E5.M1 note** in `docs/decomposition/modules/module_dependency_register.md` with **NEO-78 landed:** `GET /game/world/ability-definitions` — `AbilityDefinitionsWorldApi` + DTOs; Bruno `ability-definitions/` (same pattern as NEO-79 review follow-up for registry DI). + +2. **E5M1 backlog in-scope line** — In **E5M1-04** “In scope”, consider replacing “manual QA stub if needed” with “no manual QA (server-only per NEO-78 kickoff)” so the backlog does not contradict the plan decision. + +## Nits + +- Nit: Bruno test **“frozen prototype four is present”** is redundant given **“frozen prototype four matches registry id order”** already asserts the exact four ids. + +- Nit: C# integration test infers count **4** via `FrozenFourInIdOrder` length only; an explicit `Assert.Equal(4, body.Abilities.Count)` would match Bruno’s explicit length check (optional — recipe test uses the same frozen-array pattern). + +- Nit: Spot-checks cover **`prototype_pulse`** and **`prototype_burst`** only; **`prototype_guard`** (`utility`) and **`prototype_dash`** (`movement`) kinds are untested in automated/Bruno paths — acceptable per plan scope but easy to add later if desired. + +## Verification + +```bash +cd /home/don/neon-sprawl/server +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \ + --filter "FullyQualifiedName~AbilityDefinitionsWorldApiTests|FullyQualifiedName~AbilityDefinitionRegistryTests" +dotnet test NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj \ + --filter "FullyQualifiedName~Combat" +``` + +Manual (optional): + +- Bruno: `bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru` against a running dev server (`http://localhost:5253`). + +**Reviewer note:** Full `dotnet test` reported **32** failures in `GatherOperationsTests` (likely environment/DB-dependent, unrelated to this diff). All **27** Combat-filtered tests and the new ability-definitions integration test passed. From 94b6c67b07d997a795909d2707c5bc4eb55e072c Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 24 May 2026 22:26:54 -0400 Subject: [PATCH 4/4] NEO-78: Address code review follow-ups for ability-definitions GET. Update dependency register and E5M1 backlog, tighten tests and Bruno spot-checks, strike through resolved review items. --- .../Get ability definitions.bru | 27 ++++++++++++------- .../modules/module_dependency_register.md | 2 +- docs/plans/E5M1-prototype-backlog.md | 2 +- docs/reviews/2026-05-24-NEO-78.md | 16 ++++++----- .../Combat/AbilityDefinitionsWorldApiTests.cs | 11 ++++++++ 5 files changed, 40 insertions(+), 18 deletions(-) diff --git a/bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru b/bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru index b1cc67a..f851f56 100644 --- a/bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru +++ b/bruno/neon-sprawl-server/ability-definitions/Get ability definitions.bru @@ -38,15 +38,6 @@ tests { ]); }); - test("frozen prototype four is present", function () { - const body = res.getBody(); - const ids = new Set(body.abilities.map((x) => x.id)); - expect(ids.has("prototype_burst")).to.equal(true); - expect(ids.has("prototype_dash")).to.equal(true); - expect(ids.has("prototype_guard")).to.equal(true); - expect(ids.has("prototype_pulse")).to.equal(true); - }); - test("prototype_pulse row matches catalog", function () { const body = res.getBody(); const row = body.abilities.find((x) => x.id === "prototype_pulse"); @@ -66,4 +57,22 @@ tests { expect(row.cooldownSeconds).to.equal(5); expect(row.abilityKind).to.equal("attack"); }); + + test("prototype_guard row matches catalog", function () { + const body = res.getBody(); + const row = body.abilities.find((x) => x.id === "prototype_guard"); + expect(row).to.be.an("object"); + expect(row.abilityKind).to.equal("utility"); + expect(row.baseDamage).to.equal(0); + expect(row.cooldownSeconds).to.equal(6); + }); + + test("prototype_dash row matches catalog", function () { + const body = res.getBody(); + const row = body.abilities.find((x) => x.id === "prototype_dash"); + expect(row).to.be.an("object"); + expect(row.abilityKind).to.equal("movement"); + expect(row.baseDamage).to.equal(0); + expect(row.cooldownSeconds).to.equal(4); + }); } diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index d76623c..b7ae1e8 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -76,7 +76,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen | E5.M3 | EncounterAndRewardTables | E5.M1, E3.M3, E7.M2 | EncounterDef, RewardTable, EncounterCompleteEvent | Prototype | Planned | | E5.M4 | GroupCombatScaling | E5.M3, E8.M1 | ScalingProfile, PartySizeModifier, CombatDifficultyBand | Pre-production | Planned | -**E5.M1 note:** Epic 5 **Slice 1** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) → [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); label **`E5.M1`**. Gig XP on defeat: **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1_CombatRulesEngine.md](E5_M1_CombatRulesEngine.md). Builds on E1.M4 cast funnel (NEO-28/31/32); extends **`POST …/ability-cast`** with **`CombatResolution`**. **NEO-76 landed:** frozen four-ability catalog + CI ([NEO-76 plan](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` ([NEO-77 plan](../../plans/NEO-77-implementation-plan.md)). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79 plan](../../plans/NEO-79-implementation-plan.md)). +**E5.M1 note:** Epic 5 **Slice 1** backlog in Linear ([Epic 5 — PvE Combat and Encounter Content](https://linear.app/neon-sprawl/project/epic-5-pve-combat-and-encounter-content-cf3c94c3-5346-40e0-8aaf-281e846f2846)): [NEO-76](https://linear.app/neon-sprawl/issue/NEO-76) → [NEO-86](https://linear.app/neon-sprawl/issue/NEO-86); label **`E5.M1`**. Gig XP on defeat: **E5M1-10** [NEO-44](https://linear.app/neon-sprawl/issue/NEO-44). See [E5M1-prototype-backlog.md](../../plans/E5M1-prototype-backlog.md), [E5_M1_CombatRulesEngine.md](E5_M1_CombatRulesEngine.md). Builds on E1.M4 cast funnel (NEO-28/31/32); extends **`POST …/ability-cast`** with **`CombatResolution`**. **NEO-76 landed:** frozen four-ability catalog + CI ([NEO-76 plan](../../plans/NEO-76-implementation-plan.md)). **NEO-77 landed:** fail-fast server load of `content/abilities/*_abilities.json` ([NEO-77 plan](../../plans/NEO-77-implementation-plan.md)). **NEO-79 landed:** injectable **`IAbilityDefinitionRegistry`** + DI; hotbar/cast use **`TryNormalizeKnown`** ([NEO-79 plan](../../plans/NEO-79-implementation-plan.md)). **NEO-78 landed:** **`GET /game/world/ability-definitions`** — `AbilityDefinitionsWorldApi` + DTOs in `Game/Combat/` ([NEO-78 plan](../../plans/NEO-78-implementation-plan.md)); Bruno `bruno/neon-sprawl-server/ability-definitions/`. ### Epic 6 — PvP Security diff --git a/docs/plans/E5M1-prototype-backlog.md b/docs/plans/E5M1-prototype-backlog.md index ed4c589..45dc327 100644 --- a/docs/plans/E5M1-prototype-backlog.md +++ b/docs/plans/E5M1-prototype-backlog.md @@ -154,7 +154,7 @@ Working backlog for **Epic 5 — Slice 1** ([combat rules MVP](../decomposition/ - `GET /game/world/ability-definitions` → list DTO (`schemaVersion`, `abilities[]` with id, displayName, baseDamage, cooldownSeconds, abilityKind). - API + integration tests (AAA); Bruno folder `bruno/neon-sprawl-server/ability-definitions/`. -- `server/README.md`; manual QA stub if needed. +- `server/README.md`; no manual QA (server-only per NEO-78 kickoff). **Out of scope** diff --git a/docs/reviews/2026-05-24-NEO-78.md b/docs/reviews/2026-05-24-NEO-78.md index aa18f6b..dc03be1 100644 --- a/docs/reviews/2026-05-24-NEO-78.md +++ b/docs/reviews/2026-05-24-NEO-78.md @@ -4,6 +4,8 @@ **Scope:** Branch `NEO-78-get-world-ability-definitions` · commits `374b4b5`–`44e0056` vs `origin/main` **Base:** `origin/main` +**Follow-up:** Code review suggestions addressed (dependency register, E5M1 backlog in-scope line, test count assert, Bruno cleanup + guard/dash spot-checks). + ## Verdict **Approve with nits** @@ -17,10 +19,10 @@ NEO-78 delivers **E5M1-04**: a read-only **`GET /game/world/ability-definitions` | Document | Result | |----------|--------| | [`docs/plans/NEO-78-implementation-plan.md`](../plans/NEO-78-implementation-plan.md) | **Matches** — kickoff decisions (no manual QA, omit null `abilityKind`, registry-only injection); acceptance checklist complete; reconciliation section reflects landed files. | -| [`docs/plans/E5M1-prototype-backlog.md`](../plans/E5M1-prototype-backlog.md) · **E5M1-04** | **Matches** — AC checkboxes checked; landed note with plan + Bruno links. In-scope line still mentions “manual QA stub if needed” (kickoff override documented in plan only). | +| [`docs/plans/E5M1-prototype-backlog.md`](../plans/E5M1-prototype-backlog.md) · **E5M1-04** | **Matches** — AC checkboxes checked; landed note with plan + Bruno links; in-scope line reflects no manual QA (server-only kickoff). | | [`docs/decomposition/modules/E5_M1_CombatRulesEngine.md`](../decomposition/modules/E5_M1_CombatRulesEngine.md) | **Matches** — Status cites NEO-78 landed; **Related implementation slices** HTTP bullet added; freeze table ids align with test assertions. | | [`docs/decomposition/modules/documentation_and_implementation_alignment.md`](../decomposition/modules/documentation_and_implementation_alignment.md) | **Matches** — E5.M1 row includes NEO-78 HTTP projection + Bruno folder. | -| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Partially matches** — E5.M1 note still ends at **NEO-79 landed**; does not yet cite NEO-78 GET (see suggestion). | +| [`docs/decomposition/modules/module_dependency_register.md`](../decomposition/modules/module_dependency_register.md) | **Matches** — E5.M1 note includes NEO-78 GET + Bruno folder. | | [`docs/decomposition/modules/client_server_authority.md`](../decomposition/modules/client_server_authority.md) | **Matches** — read-only world catalog; server source of truth; no client mutation in this story. | | NEO-68 / NEO-53 reference implementations | **Matches** — `AbilityDefinitionsWorldApi` + separate DTO file mirror `RecipeDefinitionsWorldApi` / `ItemDefinitionsWorldApi`; integration test shape matches `RecipeDefinitionsWorldApiTests`. | | Full-stack epic decomposition | **Matches** — plan documents no client counterpart; NEO-85 optional enrichment; does not claim prototype slice complete without Godot. | @@ -32,17 +34,17 @@ None. ## Suggestions -1. **Dependency register E5.M1 note** — Extend the **E5.M1 note** in `docs/decomposition/modules/module_dependency_register.md` with **NEO-78 landed:** `GET /game/world/ability-definitions` — `AbilityDefinitionsWorldApi` + DTOs; Bruno `ability-definitions/` (same pattern as NEO-79 review follow-up for registry DI). +1. ~~**Dependency register E5.M1 note** — Extend the **E5.M1 note** in `docs/decomposition/modules/module_dependency_register.md` with **NEO-78 landed:** `GET /game/world/ability-definitions` — `AbilityDefinitionsWorldApi` + DTOs; Bruno `ability-definitions/` (same pattern as NEO-79 review follow-up for registry DI).~~ **Done.** -2. **E5M1 backlog in-scope line** — In **E5M1-04** “In scope”, consider replacing “manual QA stub if needed” with “no manual QA (server-only per NEO-78 kickoff)” so the backlog does not contradict the plan decision. +2. ~~**E5M1 backlog in-scope line** — In **E5M1-04** “In scope”, consider replacing “manual QA stub if needed” with “no manual QA (server-only per NEO-78 kickoff)” so the backlog does not contradict the plan decision.~~ **Done.** ## Nits -- Nit: Bruno test **“frozen prototype four is present”** is redundant given **“frozen prototype four matches registry id order”** already asserts the exact four ids. +- ~~Nit: Bruno test **“frozen prototype four is present”** is redundant given **“frozen prototype four matches registry id order”** already asserts the exact four ids.~~ **Done.** Removed redundant test. -- Nit: C# integration test infers count **4** via `FrozenFourInIdOrder` length only; an explicit `Assert.Equal(4, body.Abilities.Count)` would match Bruno’s explicit length check (optional — recipe test uses the same frozen-array pattern). +- ~~Nit: C# integration test infers count **4** via `FrozenFourInIdOrder` length only; an explicit `Assert.Equal(4, body.Abilities.Count)` would match Bruno’s explicit length check (optional — recipe test uses the same frozen-array pattern).~~ **Done.** -- Nit: Spot-checks cover **`prototype_pulse`** and **`prototype_burst`** only; **`prototype_guard`** (`utility`) and **`prototype_dash`** (`movement`) kinds are untested in automated/Bruno paths — acceptable per plan scope but easy to add later if desired. +- ~~Nit: Spot-checks cover **`prototype_pulse`** and **`prototype_burst`** only; **`prototype_guard`** (`utility`) and **`prototype_dash`** (`movement`) kinds are untested in automated/Bruno paths — acceptable per plan scope but easy to add later if desired.~~ **Done.** Guard/dash spot-checks added to C# test and Bruno. ## Verification diff --git a/server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs index d56f097..6c27309 100644 --- a/server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Combat/AbilityDefinitionsWorldApiTests.cs @@ -31,6 +31,7 @@ public class AbilityDefinitionsWorldApiTests Assert.NotNull(body); Assert.Equal(AbilityDefinitionsListResponse.CurrentSchemaVersion, body!.SchemaVersion); Assert.NotNull(body.Abilities); + Assert.Equal(4, body.Abilities.Count); var ids = body.Abilities.Select(static a => a.Id).ToList(); Assert.Equal(FrozenFourInIdOrder, ids); @@ -45,5 +46,15 @@ public class AbilityDefinitionsWorldApiTests Assert.Equal(40, burst.BaseDamage); Assert.Equal(5.0, burst.CooldownSeconds); Assert.Equal("attack", burst.AbilityKind); + + var guard = body.Abilities.Single(a => a.Id == "prototype_guard"); + Assert.Equal("utility", guard.AbilityKind); + Assert.Equal(0, guard.BaseDamage); + Assert.Equal(6.0, guard.CooldownSeconds); + + var dash = body.Abilities.Single(a => a.Id == "prototype_dash"); + Assert.Equal("movement", dash.AbilityKind); + Assert.Equal(0, dash.BaseDamage); + Assert.Equal(4.0, dash.CooldownSeconds); } }