From e9fcf46b0248645f7ac579116f7f87b4d99623fd Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:13:10 -0400 Subject: [PATCH 01/12] NEO-31: add E1M4-02 implementation plan (cast request path kickoff) --- docs/plans/NEO-31-implementation-plan.md | 83 ++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 docs/plans/NEO-31-implementation-plan.md diff --git a/docs/plans/NEO-31-implementation-plan.md b/docs/plans/NEO-31-implementation-plan.md new file mode 100644 index 0000000..19ddd5f --- /dev/null +++ b/docs/plans/NEO-31-implementation-plan.md @@ -0,0 +1,83 @@ +# NEO-31 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-31 | +| **Title** | E1M4-02: Input to AbilityCastRequest path | +| **Linear** | [NEO-31](https://linear.app/neon-sprawl/issue/NEO-31/e1m4-02-input-to-abilitycastrequest-path) | +| **Slug** | E1M4-02 | +| **Git branch** | `NEO-31-e1m4-02-input-to-abilitycastrequest-path` | +| **Parent context** | [E1.M4 prototype backlog](E1M4-prototype-backlog.md) · [E1.M4 — AbilityInputScaffold](../decomposition/modules/E1_M4_AbilityInputScaffold.md) | +| **Decomposition** | [E1.M3 — InteractionAndTargetingLayer](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md) (target lock), [E5.M1 — CombatRulesEngine](../decomposition/modules/E5_M1_CombatRulesEngine.md) (future resolution) | + +## Kickoff clarifications + +- **Linear / process:** NEO-29 remained **In Test** on the board while its implementation was already on `main`. The user directed that **NEO-31 must not be treated as blocked** on that basis. Kickoff used Linear MCP to set **In Progress** and **`removeBlockedBy: ["NEO-29"]`** so the board matches reality. +- **Product / technical:** No additional questions were required. **Goal, in/out scope, and acceptance criteria** are explicit in Linear; **target context** is defined as server-acknowledged lock state, which already exists as `TargetSelectionClient._state` / `cached_state()` (see NEO-26 class docs). **Request transport** follows the same minimal JSON + `HTTPRequest` pattern as NEO-29 hotbar loadout. **Combat resolution** stays out of scope per Linear (NEO-28 / E5.M1). + +## Goal, scope, and out-of-scope + +**Goal:** Wire hotbar activation input to a real **`AbilityCastRequest`** payload (slot, ability id, optional target id from E1.M3) and emit it through a defined client→server path suitable for later combat integration. + +**In scope** + +- Client input binding (keyboard slots per prototype; optional click can be deferred if key path satisfies AC first). +- Request payload includes **slot index**, **ability id**, and **target id** when present (from last server-acknowledged target state, not camera heuristics). +- **Integration tests** (server: HTTP POST contract) **and** client harness tests (mock transport) for formation, send triggers, and unbound-slot behavior. + +**Out of scope** + +- Final combat resolution, cooldown UI, and full E5.M1 **`CombatAction`** / **`CombatResolution`** semantics (NEO-28 and follow-on stories). + +## Acceptance criteria checklist + +- [ ] Pressing a bound slot emits **one** cast request with expected payload fields (`schemaVersion`, `slotIndex`, `abilityId`, `targetId` or explicit null/absent rule). +- [ ] Request **target** field is populated from **`TargetSelectionClient`** last acknowledged state (`cached_state()` / `lockedTargetId`), not from camera-only picking. +- [ ] **Unbound** slots: no HTTP cast POST; clear **`push_warning`** or dev log line naming slot + reason (e.g. `empty_slot`) so QA is unambiguous. + +## Technical approach + +1. **Contract (v1 JSON)** — Add a small versioned request DTO aligned with hotbar/target naming: `schemaVersion`, `slotIndex` (0–7), `abilityId` (non-empty string), `targetId` (string or null — mirror `PlayerTargetStateResponse`’s `lockedTargetId` semantics). Response for this slice can be minimal: `accepted` bool + optional `reasonCode` for prototype denies (e.g. unknown player, slot/ability mismatch vs loadout), enough for tests and NEO-28 to extend. + +2. **Server** — New `POST /game/players/{id}/ability-cast` (name fixed in implementation; document in plan **Decisions** if adjusted) mapped in `Program.cs`, implemented beside existing `Game/AbilityInput` types. Validate: player exists (same **player id** gate as hotbar APIs via `IPositionStateStore` or equivalent), slot in range, **non-empty ability** on that slot from **`IPlayerHotbarLoadoutStore`** matches request `abilityId`. Do **not** implement full combat engine; optionally shallow-check `targetId` against current lock store if low-cost, otherwise accept client-provided target for v1 and let NEO-28 tighten. + +3. **Client** — New `AbilityCastClient` (or equivalent) mirroring `HotbarLoadoutClient`: `base_url`, `dev_player_id`, injectable HTTP, `request_cast(slot_index, ability_id, target_id_variant)` that POSTs JSON once. **`main.gd`** (or dedicated small coordinator): on new InputMap actions **`hotbar_slot_1` … `hotbar_slot_8`** (digits 1–8), resolve slot index, read **`HotbarState.slots_snapshot()`**; if empty, warn and return; else read **`TargetSelectionClient.cached_state()`** for `lockedTargetId`, build payload, call cast client. Replace NEO-27 **reserve-only** comment block with a real **`ability_cast_requested`** hook site (still `TODO(E9.M1)` for ingest) immediately **before** successful POST start, per `main.gd` notes. + +4. **Tests** — **Server:** xUnit suite posting valid/invalid bodies (unknown player, empty slot, ability mismatch). **Client:** GdUnit suite with mock HTTP verifying URL/method/body for bound vs unbound paths and correct target id propagation from a stub target state provider. + +5. **Docs / backlog hygiene (light)** — After behavior exists, optionally refresh [E1_M4_AbilityInputScaffold.md](../decomposition/modules/E1_M4_AbilityInputScaffold.md) implementation snapshot + Linear table row for E1M4-02 (still optional if timeboxed). + +## Files to add + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs` | Versioned request/response JSON records for cast POST. | +| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Route handler: validation, loadout cross-check, prototype response. | +| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | HTTP integration tests for accept/deny paths and payload shape. | +| `client/scripts/ability_cast_client.gd` | Thin HTTP client for cast POST (injection pattern matches hotbar client). | +| `client/test/ability_cast_client_test.gd` | Mock-transport tests for payload formation and unbound-slot no-op. | +| `docs/manual-qa/NEO-31.md` | Manual QA checklist (slot keys, bound vs unbound, target id from HUD lock). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Program.cs` | Register `MapAbilityCastApi()` (or named extension) alongside existing maps. | +| `client/project.godot` | Add `hotbar_slot_1` … `hotbar_slot_8` (or compact equivalent) bound to digit keys for prototype. | +| `client/scripts/main.gd` | Wire input → hotbar snapshot + `TargetSelectionClient` + cast client; real `ability_cast_requested` hook site before submit. | +| `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | Optional: snapshot + Linear link for E1M4-02 after implementation lands. | + +## Tests + +| Suite | What it covers | +|--------|----------------| +| `AbilityCastApiTests.cs` | POST success when loadout matches; denies with stable `reasonCode` for bad player, wrong slot/ability, empty binding. | +| `ability_cast_client_test.gd` | Client builds JSON with `targetId` from injected target state; unbound slot does not call `request`; bound slot emits exactly one POST body. | +| Manual `docs/manual-qa/NEO-31.md` | Human verification of keys, logs, and HUD target id alignment. | + +## Open questions / risks + +- **Server target validation:** If we only trust client `targetId` in v1, NEO-28 must reconcile with `IPlayerTargetLockStore`. Call out in NEO-28 plan if we skip server-side lock match in NEO-31. +- **E1M4 backlog doc** still describes a **Linear dependency graph** where E1M4-02 follows E1M4-01; ordering is conceptual — ensure team agrees process-wise when **In Test** lags merge (handled this time via explicit unblock). From e0e9f90329626529f6eefa02a622caca11dfa2b3 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:14:29 -0400 Subject: [PATCH 02/12] NEO-31: plan Bruno requests for ability-cast API parity --- docs/plans/NEO-31-implementation-plan.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/plans/NEO-31-implementation-plan.md b/docs/plans/NEO-31-implementation-plan.md index 19ddd5f..c29bdda 100644 --- a/docs/plans/NEO-31-implementation-plan.md +++ b/docs/plans/NEO-31-implementation-plan.md @@ -47,7 +47,9 @@ 4. **Tests** — **Server:** xUnit suite posting valid/invalid bodies (unknown player, empty slot, ability mismatch). **Client:** GdUnit suite with mock HTTP verifying URL/method/body for bound vs unbound paths and correct target id propagation from a stub target state provider. -5. **Docs / backlog hygiene (light)** — After behavior exists, optionally refresh [E1_M4_AbilityInputScaffold.md](../decomposition/modules/E1_M4_AbilityInputScaffold.md) implementation snapshot + Linear table row for E1M4-02 (still optional if timeboxed). +5. **Bruno (`bruno/neon-sprawl-server/`)** — Add `.bru` requests mirroring the existing **hotbar-loadout** pattern (`{{baseUrl}}`, `dev-local-1`): at minimum **happy-path cast POST**, **bad schema / missing player**, and **loadout mismatch or empty slot** so manual API checks and collection runs stay aligned with server xUnit coverage. + +6. **Docs / backlog hygiene (light)** — After behavior exists, optionally refresh [E1_M4_AbilityInputScaffold.md](../decomposition/modules/E1_M4_AbilityInputScaffold.md) implementation snapshot + Linear table row for E1M4-02 (still optional if timeboxed). ## Files to add @@ -59,6 +61,10 @@ | `client/scripts/ability_cast_client.gd` | Thin HTTP client for cast POST (injection pattern matches hotbar client). | | `client/test/ability_cast_client_test.gd` | Mock-transport tests for payload formation and unbound-slot no-op. | | `docs/manual-qa/NEO-31.md` | Manual QA checklist (slot keys, bound vs unbound, target id from HUD lock). | +| `bruno/neon-sprawl-server/ability-cast-post.bru` | Happy-path `POST …/ability-cast` after loadout binds slot 0 (same workflow as `hotbar-loadout-post.bru`). | +| `bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru` | Wrong/missing `schemaVersion` or malformed body; expect deny + stable shape. | +| `bruno/neon-sprawl-server/ability-cast-post-missing-player.bru` | Unknown `playerId` path segment; parity with `hotbar-loadout-get-missing-player.bru`. | +| `bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru` | Ability/slot does not match persisted hotbar (or unbound slot); documents deny `reasonCode`. | ## Files to modify @@ -76,6 +82,7 @@ | `AbilityCastApiTests.cs` | POST success when loadout matches; denies with stable `reasonCode` for bad player, wrong slot/ability, empty binding. | | `ability_cast_client_test.gd` | Client builds JSON with `targetId` from injected target state; unbound slot does not call `request`; bound slot emits exactly one POST body. | | Manual `docs/manual-qa/NEO-31.md` | Human verification of keys, logs, and HUD target id alignment. | +| Bruno `ability-cast-*.bru` | Embedded `tests { … }` blocks assert status + JSON fields for quick regression (same style as `hotbar-loadout-post.bru`). | ## Open questions / risks From 7e97eb4c1d485fb6e1d64cd9618955480b42272a Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:17:57 -0400 Subject: [PATCH 03/12] NEO-31: ability cast POST, client hotbar keys, tests, Bruno --- .../ability-cast-post-bad-schema.bru | 26 +++ .../ability-cast-post-loadout-mismatch.bru | 29 +++ .../ability-cast-post-missing-player.bru | 26 +++ .../neon-sprawl-server/ability-cast-post.bru | 32 +++ .../ability-cast-preset-slot0.bru | 31 +++ client/README.md | 9 +- client/project.godot | 40 ++++ client/scripts/ability_cast_client.gd | 77 +++++++ client/scripts/main.gd | 51 ++++- client/test/ability_cast_client_test.gd | 112 +++++++++ .../modules/E1_M4_AbilityInputScaffold.md | 4 +- docs/manual-qa/NEO-31.md | 34 +++ docs/plans/NEO-31-implementation-plan.md | 18 +- .../Game/AbilityInput/AbilityCastApiTests.cs | 213 ++++++++++++++++++ .../Game/AbilityInput/AbilityCastApi.cs | 82 +++++++ .../Game/AbilityInput/AbilityCastDtos.cs | 38 ++++ server/NeonSprawl.Server/Program.cs | 1 + 17 files changed, 808 insertions(+), 15 deletions(-) create mode 100644 bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru create mode 100644 bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru create mode 100644 bruno/neon-sprawl-server/ability-cast-post-missing-player.bru create mode 100644 bruno/neon-sprawl-server/ability-cast-post.bru create mode 100644 bruno/neon-sprawl-server/ability-cast-preset-slot0.bru create mode 100644 client/scripts/ability_cast_client.gd create mode 100644 client/test/ability_cast_client_test.gd create mode 100644 docs/manual-qa/NEO-31.md create mode 100644 server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs create mode 100644 server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs create mode 100644 server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs diff --git a/bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru b/bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru new file mode 100644 index 0000000..f32a9db --- /dev/null +++ b/bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru @@ -0,0 +1,26 @@ +meta { + name: ability-cast post bad schema + type: http + seq: 22 +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/ability-cast + body: json + auth: none +} + +body:json { + { + "schemaVersion": 999, + "slotIndex": 0, + "abilityId": "prototype_pulse", + "targetId": null + } +} + +tests { + test("status 400", function () { + expect(res.getStatus()).to.equal(400); + }); +} diff --git a/bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru b/bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru new file mode 100644 index 0000000..87e889d --- /dev/null +++ b/bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru @@ -0,0 +1,29 @@ +meta { + name: ability-cast post loadout mismatch + type: http + seq: 24 +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/ability-cast + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "slotIndex": 0, + "abilityId": "prototype_burst", + "targetId": null + } +} + +tests { + test("status 200 denied", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.accepted).to.equal(false); + expect(body.reasonCode).to.equal("loadout_mismatch"); + }); +} diff --git a/bruno/neon-sprawl-server/ability-cast-post-missing-player.bru b/bruno/neon-sprawl-server/ability-cast-post-missing-player.bru new file mode 100644 index 0000000..88235ce --- /dev/null +++ b/bruno/neon-sprawl-server/ability-cast-post-missing-player.bru @@ -0,0 +1,26 @@ +meta { + name: ability-cast post missing player + type: http + seq: 23 +} + +post { + url: {{baseUrl}}/game/players/missing-player/ability-cast + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "slotIndex": 0, + "abilityId": "prototype_pulse", + "targetId": null + } +} + +tests { + test("status 404", function () { + expect(res.getStatus()).to.equal(404); + }); +} diff --git a/bruno/neon-sprawl-server/ability-cast-post.bru b/bruno/neon-sprawl-server/ability-cast-post.bru new file mode 100644 index 0000000..e30940d --- /dev/null +++ b/bruno/neon-sprawl-server/ability-cast-post.bru @@ -0,0 +1,32 @@ +meta { + name: ability-cast post happy + type: http + seq: 21 +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/ability-cast + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "slotIndex": 0, + "abilityId": "prototype_pulse", + "targetId": null + } +} + +tests { + test("status 200", function () { + expect(res.getStatus()).to.equal(200); + }); + + test("accepted true", function () { + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.accepted).to.equal(true); + }); +} diff --git a/bruno/neon-sprawl-server/ability-cast-preset-slot0.bru b/bruno/neon-sprawl-server/ability-cast-preset-slot0.bru new file mode 100644 index 0000000..0095c85 --- /dev/null +++ b/bruno/neon-sprawl-server/ability-cast-preset-slot0.bru @@ -0,0 +1,31 @@ +meta { + name: ability-cast preset slot0 pulse + type: http + seq: 20 +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/hotbar-loadout + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "slots": [ + { + "slotIndex": 0, + "abilityId": "prototype_pulse" + } + ] + } +} + +tests { + test("status 200 and updated", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.updated).to.equal(true); + }); +} diff --git a/client/README.md b/client/README.md index a67be42..c09a16e 100644 --- a/client/README.md +++ b/client/README.md @@ -112,7 +112,7 @@ Full checklist: [`docs/manual-qa/NEO-25.md`](../manual-qa/NEO-25.md). - **`TargetSelectionClient`** emits **`selection_event(event: Dictionary)`** only when the server-acknowledged **`lockedTargetId`** changes (not on **`validity`-only** updates; those stay on **`target_state_changed`**). - Payload keys: **`previous`** ([String] or `null`), **`next`** (same), **`cause`** (`tab` \| `clear` \| `server_correction`), **`sequence`** (int from the new state). **`POST …/target/select`** from **`request_select_target_id`** (including the path used by Tab) uses cause **`tab`**; **`request_clear_target`** uses **`clear`**; every **`GET …/target`** completion uses **`server_correction`** (including boot sync). - **Slice 3 telemetry hook (NEO-27):** this selection transition maps to product event name **`target_changed`**; with **`log_selection_events`** enabled, Godot Output prints the `target_changed` token for manual verification. **TODO(E9.M1):** replace dev log with cataloged telemetry emit. -- **Ability telemetry reserves (NEO-27):** `scripts/main.gd` documents future hook sites for **`ability_cast_requested`** / **`ability_cast_denied`** (implemented in E1.M4 + E5.M1, not in this story). +- **Ability cast hooks (NEO-27 + NEO-31):** digit keys **`hotbar_slot_1` … `hotbar_slot_8`** (defaults **1**–**8**) issue a prototype cast via `scripts/ability_cast_client.gd`. **`ability_cast_requested`** is printed from `main.gd` immediately before the POST; **`ability_cast_denied`** is logged as `push_warning` from the client on deny responses (TODO(E9.M1): telemetry). - **E1.M4+:** connect to **`$TargetSelectionClient.selection_event`** (or your scene’s path to that node) without reshaping the payload. Full checklist: [`docs/manual-qa/NEO-26.md`](../docs/manual-qa/NEO-26.md). @@ -127,6 +127,13 @@ Full checklist: [`docs/manual-qa/NEO-26.md`](../docs/manual-qa/NEO-26.md). This story hydrates bindings only; cast execution and cooldown behavior stay in later E1.M4 / E5.M1 stories. +## Ability cast request (NEO-31) + +- **`POST /game/players/{id}/ability-cast`** — JSON body `schemaVersion`, `slotIndex` (0–7), `abilityId`, optional `targetId` (mirrors server target lock id). Server accepts when the slot matches persisted hotbar bindings; see `server/…/AbilityCastApi.cs` for prototype `reasonCode` values. +- **Client:** `main.gd` reads `HotbarState.slots_snapshot()` and `TargetSelectionClient.cached_state()` (`lockedTargetId`) so cast payloads use **server-acknowledged** target context, not camera guessing. + +Full checklist: [`docs/manual-qa/NEO-31.md`](../docs/manual-qa/NEO-31.md). + ### Manual check (NEO-24) 1. Run the server and client as in NEO-7 / NEO-9; confirm spawn snap at `(-5, 0.9, -5)`. diff --git a/client/project.godot b/client/project.godot index 207e57a..ce46c18 100644 --- a/client/project.godot +++ b/client/project.godot @@ -98,6 +98,46 @@ move_back={ "events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":83,"physical_keycode":83,"key_label":0,"unicode":115,"location":0,"echo":false,"script":null) ] } +hotbar_slot_1={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":49,"physical_keycode":49,"key_label":0,"unicode":49,"location":0,"echo":false,"script":null) +] +} +hotbar_slot_2={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":50,"physical_keycode":50,"key_label":0,"unicode":50,"location":0,"echo":false,"script":null) +] +} +hotbar_slot_3={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":51,"physical_keycode":51,"key_label":0,"unicode":51,"location":0,"echo":false,"script":null) +] +} +hotbar_slot_4={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":52,"physical_keycode":52,"key_label":0,"unicode":52,"location":0,"echo":false,"script":null) +] +} +hotbar_slot_5={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":53,"physical_keycode":53,"key_label":0,"unicode":53,"location":0,"echo":false,"script":null) +] +} +hotbar_slot_6={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":54,"physical_keycode":54,"key_label":0,"unicode":54,"location":0,"echo":false,"script":null) +] +} +hotbar_slot_7={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":55,"physical_keycode":55,"key_label":0,"unicode":55,"location":0,"echo":false,"script":null) +] +} +hotbar_slot_8={ +"deadzone": 0.5, +"events": [Object(InputEventKey,"resource_local_to_scene":false,"resource_name":"","device":-1,"window_id":0,"alt_pressed":false,"shift_pressed":false,"ctrl_pressed":false,"meta_pressed":false,"pressed":false,"keycode":56,"physical_keycode":56,"key_label":0,"unicode":56,"location":0,"echo":false,"script":null) +] +} [physics] diff --git a/client/scripts/ability_cast_client.gd b/client/scripts/ability_cast_client.gd new file mode 100644 index 0000000..471a7e0 --- /dev/null +++ b/client/scripts/ability_cast_client.gd @@ -0,0 +1,77 @@ +extends Node + +## NEO-31: prototype HTTP client for `POST /game/players/{id}/ability-cast`. +@export var base_url: String = "http://127.0.0.1:5253" +@export var dev_player_id: String = "dev-local-1" +@export var injected_http: Node = null + +var _http: Node +var _busy: bool = false + + +func _ready() -> void: + if injected_http != null: + _http = injected_http + else: + _http = HTTPRequest.new() + add_child(_http) + if _http is HTTPRequest: + (_http as HTTPRequest).timeout = 30.0 + @warning_ignore("unsafe_method_access") + _http.request_completed.connect(_on_request_completed) + + +## [param target_id] server [code]lockedTargetId[/code] string, or [code]null[/code] if none. +func request_cast(slot_index: int, ability_id: String, target_id: Variant) -> void: + if _busy: + return + var tid: Variant = null + if target_id is String: + var s: String = target_id as String + if not s.is_empty(): + tid = s + var payload: Dictionary = { + "schemaVersion": 1, + "slotIndex": slot_index, + "abilityId": ability_id, + "targetId": tid, + } + _busy = true + var url := "%s/game/players/%s/ability-cast" % [_base_root(), _player_path_segment()] + var headers := PackedStringArray(["Content-Type: application/json"]) + var err: Error = _http.request(url, headers, HTTPClient.METHOD_POST, JSON.stringify(payload)) + if err != OK: + push_warning("AbilityCastClient: POST failed to start (%s)" % err) + _busy = false + + +func _base_root() -> String: + return base_url.strip_edges().rstrip("/") + + +func _player_path_segment() -> String: + return dev_player_id.strip_edges() + + +func _on_request_completed( + result: int, response_code: int, _headers: PackedStringArray, body: PackedByteArray +) -> void: + _busy = false + if result != HTTPRequest.RESULT_SUCCESS: + push_warning("AbilityCastClient: HTTP failed (result=%s)" % result) + return + if response_code < 200 or response_code >= 300: + push_warning("AbilityCastClient: HTTP %s" % response_code) + return + var text := body.get_string_from_utf8() + var parsed: Variant = JSON.parse_string(text) + if not parsed is Dictionary: + push_warning("AbilityCastClient: non-JSON body") + return + var data: Dictionary = parsed + var accepted: bool = bool(data.get("accepted", false)) + if not accepted: + var reason_variant: Variant = data.get("reasonCode", "") + var reason: String = reason_variant as String if reason_variant is String else "" + # NEO-27 / NEO-31: product hook name for future telemetry (TODO(E9.M1)). + push_warning("ability_cast_denied reasonCode=%s" % reason) diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 2a0f465..d2ccf67 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -66,6 +66,7 @@ var _dev_saved_obstacle_process_mode: Node.ProcessMode = Node.PROCESS_MODE_INHER var _dev_obstacle_smoke: Node3D var _hotbar_state: Node = null var _hotbar_client: Node = null +var _ability_cast_client: Node = null @onready var _camera: Camera3D = $World/IsometricFollowCamera/Camera3D @onready var _world: Node3D = $World @@ -204,10 +205,10 @@ func _on_authoritative_position(world: Vector3, apply_as_snap: bool) -> void: ## hooks for movement-driven lock refresh — we do not snap the capsule to this value ## (that is `authoritative_position_received`'s job on boot + move rejection). ## -## NEO-27 telemetry reserve (input/cast path not implemented in E1.M3): -## - `ability_cast_requested` hook will attach in E1.M4 input wiring before cast submit. -## - `ability_cast_denied` hook will attach on cast-response denial (`reasonCode`) in E1.M4+E5.M1. -## TODO(E9.M1): map both to the telemetry schema/catalog once available. +## NEO-27 / NEO-31: `ability_cast_requested` is emitted as a dev [code]print[/code] from +## [method _request_hotbar_cast_slot] immediately before the cast HTTP POST starts. +## [code]ability_cast_denied[/code] is surfaced via [code]push_warning[/code] from +## `ability_cast_client.gd` on deny responses (TODO(E9.M1): telemetry schema + ingest). func _on_authoritative_ack_for_hud(world: Vector3) -> void: _last_ack_world = world _have_last_ack = true @@ -243,6 +244,16 @@ func _setup_hotbar_loadout_sync() -> void: _hotbar_client.call("set_hotbar_state", _hotbar_state) if _hotbar_client.has_method("request_sync_from_server"): _hotbar_client.call("request_sync_from_server") + _ability_cast_client = load("res://scripts/ability_cast_client.gd").new() + _ability_cast_client.name = "AbilityCastClient" + add_child(_ability_cast_client) + if is_instance_valid(_authority): + var authority_base_url2: Variant = _authority.get("base_url") + var authority_player_id2: Variant = _authority.get("dev_player_id") + if authority_base_url2 is String and not (authority_base_url2 as String).is_empty(): + _ability_cast_client.set("base_url", authority_base_url2) + if authority_player_id2 is String and not (authority_player_id2 as String).is_empty(): + _ability_cast_client.set("dev_player_id", authority_player_id2) ## Combines cached server state (`_last_target_state`) with per-anchor horizontal @@ -309,6 +320,11 @@ func _on_move_rejected(reason_code: String) -> void: ## **F9** / **F10** are debugger shortcuts in the embedded game; use the Input Map action. ## For a true **freed-node** occluder test, reload the scene or remove the body in the editor. func _unhandled_key_input(event: InputEvent) -> void: + for slot_digit in range(1, 9): + var action_name := "hotbar_slot_%d" % slot_digit + if event.is_action_pressed(action_name): + _request_hotbar_cast_slot(slot_digit - 1) + return # NEO-25: route interact keys here — `InteractionRequestClient._input` is unreliable in the # embedded Game dock / focus order; `_unhandled_key_input` on the scene root still fires. if event.is_action_pressed("interact"): @@ -330,6 +346,33 @@ func _unhandled_key_input(event: InputEvent) -> void: call_deferred("_dev_toggle_obstacle_smoke_deferred") +func _request_hotbar_cast_slot(slot_index: int) -> void: + if not is_instance_valid(_hotbar_state) or not _hotbar_state.has_method("slots_snapshot"): + return + var slots: Array = _hotbar_state.call("slots_snapshot") as Array + if slot_index < 0 or slot_index >= slots.size(): + push_warning("Hotbar cast ignored: slot %d invalid_slot_index" % slot_index) + return + var ability_variant: Variant = slots[slot_index] + if ability_variant == null or not (ability_variant is String) or (ability_variant as String).strip_edges().is_empty(): + push_warning("Hotbar cast ignored: slot %d empty_slot" % slot_index) + return + var ability_id: String = ability_variant as String + var target_id: Variant = null + if is_instance_valid(_target_client) and _target_client.has_method("cached_state"): + var st: Variant = _target_client.call("cached_state") + if st is Dictionary: + var locked: Variant = (st as Dictionary).get("lockedTargetId", null) + if locked is String and not (locked as String).is_empty(): + target_id = locked + var target_label: String = "null" + if target_id != null: + target_label = str(target_id) + print("ability_cast_requested slot=%d ability_id=%s targetId=%s" % [slot_index, ability_id, target_label]) + if is_instance_valid(_ability_cast_client) and _ability_cast_client.has_method("request_cast"): + _ability_cast_client.call("request_cast", slot_index, ability_id, target_id) + + func _forward_interact_post(method: String) -> void: if not is_instance_valid(_interaction_client): return diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd new file mode 100644 index 0000000..5f23329 --- /dev/null +++ b/client/test/ability_cast_client_test.gd @@ -0,0 +1,112 @@ +extends GdUnitTestSuite + +const CastClient := preload("res://scripts/ability_cast_client.gd") + + +class MockHttpTransport: + extends Node + signal request_completed( + result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray + ) + var last_url: String = "" + var last_body: String = "" + var last_method: HTTPClient.Method = HTTPClient.METHOD_GET + var _queue: Array[Dictionary] = [] + + func enqueue(result: int, code: int, body: String) -> void: + _queue.append({"result": result, "code": code, "body": body}) + + func request( + url: String, + _custom_headers: PackedStringArray = PackedStringArray(), + method: HTTPClient.Method = HTTPClient.METHOD_GET, + request_data: String = "" + ) -> Error: + last_url = url + last_method = method + last_body = request_data + if _queue.is_empty(): + return ERR_UNAVAILABLE + var r: Dictionary = _queue.pop_front() + request_completed.emit( + r["result"], r["code"], PackedStringArray(), str(r["body"]).to_utf8_buffer() + ) + return OK + + +class HoldFirstThenAutoTransport: + extends Node + signal request_completed( + result: int, response_code: int, headers: PackedStringArray, body: PackedByteArray + ) + var request_count: int = 0 + var last_body: String = "" + var _first: bool = true + + func request( + _url: String, + _custom_headers: PackedStringArray = PackedStringArray(), + _method: HTTPClient.Method = HTTPClient.METHOD_GET, + request_data: String = "" + ) -> Error: + request_count += 1 + last_body = request_data + var body_json := '{"schemaVersion":1,"accepted":true}' + var body_bytes: PackedByteArray = body_json.to_utf8_buffer() + if _first: + _first = false + call_deferred("_emit", body_bytes) + else: + request_completed.emit(HTTPRequest.RESULT_SUCCESS, 200, PackedStringArray(), body_bytes) + return OK + + func _emit(body_bytes: PackedByteArray) -> void: + request_completed.emit(HTTPRequest.RESULT_SUCCESS, 200, PackedStringArray(), body_bytes) + + +func _make_client(transport: Node) -> Node: + var c: Node = CastClient.new() + c.set("injected_http", transport) + auto_free(transport) + auto_free(c) + add_child(c) + return c + + +func test_request_cast_posts_expected_payload_with_null_target() -> void: + var transport := MockHttpTransport.new() + transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') + var c := _make_client(transport) + c.call("request_cast", 2, "prototype_guard", null) + assert_that(transport.last_url).contains("/ability-cast") + assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST) + var parsed: Variant = JSON.parse_string(transport.last_body) + assert_that(parsed is Dictionary).is_true() + var body: Dictionary = parsed + assert_that(int(body.get("schemaVersion", 0))).is_equal(1) + assert_that(int(body.get("slotIndex", -1))).is_equal(2) + assert_that(body.get("abilityId")).is_equal("prototype_guard") + assert_that(body.get("targetId", "missing")).is_equal(null) + + +func test_request_cast_posts_target_id_string_when_set() -> void: + var transport := MockHttpTransport.new() + transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') + var c := _make_client(transport) + c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha") + var parsed: Variant = JSON.parse_string(transport.last_body) + var body: Dictionary = parsed + assert_that(body.get("targetId")).is_equal("prototype_target_alpha") + + +func test_request_cast_while_busy_is_ignored() -> void: + var transport := HoldFirstThenAutoTransport.new() + var c := _make_client(transport) + c.call("request_cast", 0, "prototype_pulse", null) + c.call("request_cast", 1, "prototype_guard", null) + await get_tree().process_frame + await get_tree().process_frame + assert_that(transport.request_count).is_equal(1) + var parsed: Variant = JSON.parse_string(transport.last_body) + var body: Dictionary = parsed + assert_that(int(body.get("slotIndex", -1))).is_equal(0) diff --git a/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md b/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md index 1205ac5..aa9ba8a 100644 --- a/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md +++ b/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md @@ -42,7 +42,7 @@ Epic 1 **Slice 3** — `ability_cast_requested`, `ability_cast_denied` with reas ## Implementation snapshot -- **Current state:** NEO-29 landed prototype `HotbarLoadout` v1 contract and hydration path (`GET`/`POST /game/players/{id}/hotbar-loadout`, fixed 8 slots, deny reason codes). `AbilityCastRequest` and `CooldownSnapshot` wiring are still pending. +- **Current state:** NEO-29 landed prototype `HotbarLoadout` v1 contract and hydration path (`GET`/`POST /game/players/{id}/hotbar-loadout`, fixed 8 slots, deny reason codes). NEO-31 landed prototype **`POST /game/players/{id}/ability-cast`** plus client digit hotbar → cast payload wiring (target id from server-ack target state). `CooldownSnapshot` wiring is still pending. - **Prototype persistence policy (NEO-29):** per-player loadout key; Postgres when `ConnectionStrings:NeonSprawl` exists, in-memory fallback otherwise. - **Prototype backlog:** [E1M4 prototype backlog](../../plans/E1M4-prototype-backlog.md) defines five vertical slices from hotbar loadout contract to cast telemetry hooks. - **Dependency expectation:** first implementation stories assume E1.M3 target selection flow exists and E5.M1 provides minimum cast accept/deny contract. @@ -56,7 +56,7 @@ Parent epic: [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/pro | Slug | Linear | |------|--------| | **E1M4-01** | [NEO-29](https://linear.app/neon-sprawl/issue/NEO-29/e1m4-01-hotbarloadout-v1-contract-baseline-persistence-path) — `HotbarLoadout` v1 contract + baseline persistence path | -| **E1M4-02** | TBD — Input to `AbilityCastRequest` path | +| **E1M4-02** | [NEO-31](https://linear.app/neon-sprawl/issue/NEO-31/e1m4-02-input-to-abilitycastrequest-path) — Input to `AbilityCastRequest` path | | **E1M4-03** | TBD — Combat accept/deny integration + reason-code UX | | **E1M4-04** | TBD — `CooldownSnapshot` sync + slot presentation | | **E1M4-05** | TBD — Slice 3 telemetry hooks for cast funnel | diff --git a/docs/manual-qa/NEO-31.md b/docs/manual-qa/NEO-31.md new file mode 100644 index 0000000..1a44ba1 --- /dev/null +++ b/docs/manual-qa/NEO-31.md @@ -0,0 +1,34 @@ +# NEO-31 — Manual QA checklist + +| Field | Value | +|-------|-------| +| Key | NEO-31 | +| Title | E1M4-02: Input to AbilityCastRequest path | +| Linear | https://linear.app/neon-sprawl/issue/NEO-31/e1m4-02-input-to-abilitycastrequest-path | +| Plan | `docs/plans/NEO-31-implementation-plan.md` | +| Branch | `NEO-31-e1m4-02-input-to-abilitycastrequest-path` | + +## 1) Server + Bruno + +- [ ] Start server: `cd server/NeonSprawl.Server && dotnet run`. +- [ ] `curl -s http://localhost:5253/health` returns JSON with `status: ok`. +- [ ] Run Bruno collection (or at least): `ability-cast-preset-slot0` (seq **20**) then `ability-cast-post` (seq **21**) — expect cast `accepted: true`. +- [ ] `ability-cast-post-bad-schema` → HTTP **400**. +- [ ] `ability-cast-post-missing-player` → HTTP **404**. +- [ ] **`ability-cast-post-loadout-mismatch`** (seq **24**): run **`ability-cast-preset-slot0`** in the same session first so slot 0 is `prototype_pulse`; then expect `accepted: false` and `reasonCode == "loadout_mismatch"`. + +## 2) Client — bound slot + +- [ ] With server running, open main scene; confirm hotbar hydrated (NEO-29). +- [ ] Bind slot **0** to `prototype_pulse` (Bruno `hotbar-loadout-post` or equivalent `curl`) if not already. +- [ ] Press **1** (InputMap `hotbar_slot_1`): Godot Output shows a line starting with **`ability_cast_requested`** including `slot=0`, `ability_id=prototype_pulse`, and `targetId=` matching current HUD lock (`null` / `—` when cleared, or the locked id after Tab). +- [ ] Server log or network trace shows **one** `POST …/ability-cast` per key press (no spam while key held; Godot `is_action_pressed` fires once per press). + +## 3) Client — unbound slot + +- [ ] Ensure slot **2** has no ability (default empty). +- [ ] Press **3** (`hotbar_slot_3`): **no** cast POST; Output shows **`Hotbar cast ignored: slot 2 empty_slot`** (or equivalent `push_warning` text). + +## 4) Deny UX + +- [ ] Force a server deny (e.g. cast wrong ability for bound slot via `curl`): response `accepted: false`; if exercised through the client, Output includes **`ability_cast_denied`** in a warning. diff --git a/docs/plans/NEO-31-implementation-plan.md b/docs/plans/NEO-31-implementation-plan.md index c29bdda..db1bbc8 100644 --- a/docs/plans/NEO-31-implementation-plan.md +++ b/docs/plans/NEO-31-implementation-plan.md @@ -33,9 +33,9 @@ ## Acceptance criteria checklist -- [ ] Pressing a bound slot emits **one** cast request with expected payload fields (`schemaVersion`, `slotIndex`, `abilityId`, `targetId` or explicit null/absent rule). -- [ ] Request **target** field is populated from **`TargetSelectionClient`** last acknowledged state (`cached_state()` / `lockedTargetId`), not from camera-only picking. -- [ ] **Unbound** slots: no HTTP cast POST; clear **`push_warning`** or dev log line naming slot + reason (e.g. `empty_slot`) so QA is unambiguous. +- [x] Pressing a bound slot emits **one** cast request with expected payload fields (`schemaVersion`, `slotIndex`, `abilityId`, `targetId` or explicit null/absent rule). +- [x] Request **target** field is populated from **`TargetSelectionClient`** last acknowledged state (`cached_state()` / `lockedTargetId`), not from camera-only picking. +- [x] **Unbound** slots: no HTTP cast POST; clear **`push_warning`** or dev log line naming slot + reason (e.g. `empty_slot`) so QA is unambiguous. ## Technical approach @@ -61,10 +61,11 @@ | `client/scripts/ability_cast_client.gd` | Thin HTTP client for cast POST (injection pattern matches hotbar client). | | `client/test/ability_cast_client_test.gd` | Mock-transport tests for payload formation and unbound-slot no-op. | | `docs/manual-qa/NEO-31.md` | Manual QA checklist (slot keys, bound vs unbound, target id from HUD lock). | -| `bruno/neon-sprawl-server/ability-cast-post.bru` | Happy-path `POST …/ability-cast` after loadout binds slot 0 (same workflow as `hotbar-loadout-post.bru`). | -| `bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru` | Wrong/missing `schemaVersion` or malformed body; expect deny + stable shape. | -| `bruno/neon-sprawl-server/ability-cast-post-missing-player.bru` | Unknown `playerId` path segment; parity with `hotbar-loadout-get-missing-player.bru`. | -| `bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru` | Ability/slot does not match persisted hotbar (or unbound slot); documents deny `reasonCode`. | +| `bruno/neon-sprawl-server/ability-cast-preset-slot0.bru` | POST hotbar bind slot 0 → `prototype_pulse` so cast happy/mismatch requests have deterministic loadout state. | +| `bruno/neon-sprawl-server/ability-cast-post.bru` | Happy-path `POST …/ability-cast` (run after preset when exercising alone). | +| `bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru` | Wrong `schemaVersion` → HTTP 400. | +| `bruno/neon-sprawl-server/ability-cast-post-missing-player.bru` | Unknown player path → HTTP 404. | +| `bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru` | Wrong ability for slot 0 → `loadout_mismatch` (requires preset in same run). | ## Files to modify @@ -73,7 +74,8 @@ | `server/NeonSprawl.Server/Program.cs` | Register `MapAbilityCastApi()` (or named extension) alongside existing maps. | | `client/project.godot` | Add `hotbar_slot_1` … `hotbar_slot_8` (or compact equivalent) bound to digit keys for prototype. | | `client/scripts/main.gd` | Wire input → hotbar snapshot + `TargetSelectionClient` + cast client; real `ability_cast_requested` hook site before submit. | -| `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | Optional: snapshot + Linear link for E1M4-02 after implementation lands. | +| `client/README.md` | Document digit hotbar keys, cast endpoint, and manual QA link. | +| `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | Snapshot + Linear link for E1M4-02. | ## Tests diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs new file mode 100644 index 0000000..2962d91 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs @@ -0,0 +1,213 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text; +using NeonSprawl.Server.Game.AbilityInput; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.AbilityInput; + +public sealed class AbilityCastApiTests +{ + private static async Task BindSlot0PulseAsync(HttpClient client) + { + var post = await client.PostAsJsonAsync( + "/game/players/dev-local-1/hotbar-loadout", + new HotbarLoadoutUpdateRequest + { + SchemaVersion = HotbarLoadoutUpdateRequest.CurrentSchemaVersion, + Slots = [new HotbarSlotBindingJson { SlotIndex = 0, AbilityId = PrototypeAbilityRegistry.PrototypePulse }], + }); + post.EnsureSuccessStatusCode(); + } + + [Fact] + public async Task PostAbilityCast_ShouldAccept_WhenLoadoutMatches() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + TargetId = null, + }); + var body = await response.Content.ReadFromJsonAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.True(body!.Accepted); + Assert.Null(body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldAccept_WithTargetId_WhenLoadoutMatches() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + TargetId = "prototype_target_alpha", + }); + var body = await response.Content.ReadFromJsonAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.True(body!.Accepted); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenySlotUnbound_WhenSlotEmpty() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 2, + AbilityId = PrototypeAbilityRegistry.PrototypeGuard, + TargetId = null, + }); + var body = await response.Content.ReadFromJsonAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonSlotUnbound, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenyLoadoutMismatch_WhenAbilityDoesNotMatchSlot() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypeBurst, + TargetId = null, + }); + var body = await response.Content.ReadFromJsonAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonLoadoutMismatch, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenySlotOutOfBounds() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 8, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + TargetId = null, + }); + var body = await response.Content.ReadFromJsonAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonSlotOutOfBounds, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenyUnknownAbility_WhenAbilityIdNotInRegistry() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = "not_a_real_ability", + TargetId = null, + }); + var body = await response.Content.ReadFromJsonAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonUnknownAbility, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldReturnBadRequest_WhenBodyMalformed() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + var response = await client.PostAsync( + "/game/players/dev-local-1/ability-cast", + new StringContent("{\"schemaVersion\":1,\"slotIndex\":", Encoding.UTF8, "application/json")); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldReturnBadRequest_WhenSchemaVersionWrong() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = 999, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + }); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldReturnNotFound_WhenPlayerUnknown() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + var response = await client.PostAsJsonAsync( + "/game/players/missing-player/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + }); + + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } +} diff --git a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs new file mode 100644 index 0000000..ba3cf6c --- /dev/null +++ b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs @@ -0,0 +1,82 @@ +using NeonSprawl.Server.Game.PositionState; + +namespace NeonSprawl.Server.Game.AbilityInput; + +/// Maps prototype ability cast POST (NEO-31). +public static class AbilityCastApi +{ + public const string ReasonSlotOutOfBounds = "slot_out_of_bounds"; + public const string ReasonSlotUnbound = "slot_unbound"; + public const string ReasonLoadoutMismatch = "loadout_mismatch"; + public const string ReasonUnknownAbility = "unknown_ability"; + + public static WebApplication MapAbilityCastApi(this WebApplication app) + { + app.MapPost( + "/game/players/{id}/ability-cast", + (string id, AbilityCastRequest? body, IPositionStateStore positions, IPlayerHotbarLoadoutStore store) => + { + if (body is null || body.SchemaVersion != AbilityCastRequest.CurrentSchemaVersion) + { + return Results.BadRequest(); + } + + if (!positions.TryGetPosition(id, out _)) + { + return Results.NotFound(); + } + + if (!store.TryGetBindings(id, out var bindings)) + { + return Results.NotFound(); + } + + if (body.SlotIndex < 0 || body.SlotIndex >= HotbarLoadoutResponse.SlotCountV1) + { + return Results.Json( + new AbilityCastResponse + { + Accepted = false, + ReasonCode = ReasonSlotOutOfBounds, + }); + } + + if (!bindings.TryGetValue(body.SlotIndex, out var boundAbility) || + string.IsNullOrWhiteSpace(boundAbility)) + { + return Results.Json( + new AbilityCastResponse + { + Accepted = false, + ReasonCode = ReasonSlotUnbound, + }); + } + + if (string.IsNullOrWhiteSpace(body.AbilityId) || + !PrototypeAbilityRegistry.TryNormalizeKnown(body.AbilityId, out var normalizedRequest)) + { + return Results.Json( + new AbilityCastResponse + { + Accepted = false, + ReasonCode = ReasonUnknownAbility, + }); + } + + if (!string.Equals(normalizedRequest, boundAbility, StringComparison.Ordinal)) + { + return Results.Json( + new AbilityCastResponse + { + Accepted = false, + ReasonCode = ReasonLoadoutMismatch, + }); + } + + // TargetId echoed for NEO-28 / E5.M1; server does not validate against lock store in NEO-31. + return Results.Json(new AbilityCastResponse { Accepted = true }); + }); + + return app; + } +} diff --git a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs new file mode 100644 index 0000000..ff08086 --- /dev/null +++ b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs @@ -0,0 +1,38 @@ +using System.Text.Json.Serialization; + +namespace NeonSprawl.Server.Game.AbilityInput; + +/// POST body for ability cast intent (NEO-31). +public sealed class AbilityCastRequest +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } + + [JsonPropertyName("slotIndex")] + public int SlotIndex { get; init; } + + [JsonPropertyName("abilityId")] + public string? AbilityId { get; init; } + + /// Optional; mirrors lockedTargetId from target state v1. + [JsonPropertyName("targetId")] + public string? TargetId { get; init; } +} + +/// POST response for cast submit (prototype accept/deny only; NEO-31). +public sealed class AbilityCastResponse +{ + public const int CurrentSchemaVersion = 1; + + [JsonPropertyName("schemaVersion")] + public int SchemaVersion { get; init; } = CurrentSchemaVersion; + + [JsonPropertyName("accepted")] + public bool Accepted { get; init; } + + [JsonPropertyName("reasonCode")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? ReasonCode { get; init; } +} diff --git a/server/NeonSprawl.Server/Program.cs b/server/NeonSprawl.Server/Program.cs index 0664a57..e4b1fe9 100644 --- a/server/NeonSprawl.Server/Program.cs +++ b/server/NeonSprawl.Server/Program.cs @@ -25,5 +25,6 @@ app.MapInteractionApi(); app.MapInteractablesWorldApi(); app.MapTargetingApi(); app.MapHotbarLoadoutApi(); +app.MapAbilityCastApi(); app.Run(); From b5b8c10d3673c924bc33f718bc306f155ac0a423 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:22:23 -0400 Subject: [PATCH 04/12] chore: add Godot UIDs for hotbar_loadout_client and hotbar_state --- client/scripts/hotbar_loadout_client.gd.uid | 1 + client/scripts/hotbar_state.gd.uid | 1 + 2 files changed, 2 insertions(+) create mode 100644 client/scripts/hotbar_loadout_client.gd.uid create mode 100644 client/scripts/hotbar_state.gd.uid diff --git a/client/scripts/hotbar_loadout_client.gd.uid b/client/scripts/hotbar_loadout_client.gd.uid new file mode 100644 index 0000000..7603f43 --- /dev/null +++ b/client/scripts/hotbar_loadout_client.gd.uid @@ -0,0 +1 @@ +uid://bvy6s17r4m600 diff --git a/client/scripts/hotbar_state.gd.uid b/client/scripts/hotbar_state.gd.uid new file mode 100644 index 0000000..7158391 --- /dev/null +++ b/client/scripts/hotbar_state.gd.uid @@ -0,0 +1 @@ +uid://bt6bdy8qhaj0e From 7c327994eb9c4fdf44dd0b5daaf7316e845906b5 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:23:02 -0400 Subject: [PATCH 05/12] NEO-31: structure AbilityCastApiTests in explicit AAA sections --- .../Game/AbilityInput/AbilityCastApiTests.cs | 65 +++++++++++++------ 1 file changed, 46 insertions(+), 19 deletions(-) diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs index 2962d91..812d40b 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs @@ -23,10 +23,12 @@ public sealed class AbilityCastApiTests [Fact] public async Task PostAbilityCast_ShouldAccept_WhenLoadoutMatches() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); await BindSlot0PulseAsync(client); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/ability-cast", new AbilityCastRequest @@ -38,6 +40,7 @@ public sealed class AbilityCastApiTests }); var body = await response.Content.ReadFromJsonAsync(); + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.True(body!.Accepted); @@ -47,10 +50,12 @@ public sealed class AbilityCastApiTests [Fact] public async Task PostAbilityCast_ShouldAccept_WithTargetId_WhenLoadoutMatches() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); await BindSlot0PulseAsync(client); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/ability-cast", new AbilityCastRequest @@ -62,6 +67,7 @@ public sealed class AbilityCastApiTests }); var body = await response.Content.ReadFromJsonAsync(); + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.True(body!.Accepted); @@ -70,9 +76,11 @@ public sealed class AbilityCastApiTests [Fact] public async Task PostAbilityCast_ShouldDenySlotUnbound_WhenSlotEmpty() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/ability-cast", new AbilityCastRequest @@ -84,6 +92,7 @@ public sealed class AbilityCastApiTests }); var body = await response.Content.ReadFromJsonAsync(); + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -93,10 +102,12 @@ public sealed class AbilityCastApiTests [Fact] public async Task PostAbilityCast_ShouldDenyLoadoutMismatch_WhenAbilityDoesNotMatchSlot() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); await BindSlot0PulseAsync(client); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/ability-cast", new AbilityCastRequest @@ -108,6 +119,7 @@ public sealed class AbilityCastApiTests }); var body = await response.Content.ReadFromJsonAsync(); + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -117,10 +129,12 @@ public sealed class AbilityCastApiTests [Fact] public async Task PostAbilityCast_ShouldDenySlotOutOfBounds() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); await BindSlot0PulseAsync(client); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/ability-cast", new AbilityCastRequest @@ -132,6 +146,7 @@ public sealed class AbilityCastApiTests }); var body = await response.Content.ReadFromJsonAsync(); + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -141,10 +156,12 @@ public sealed class AbilityCastApiTests [Fact] public async Task PostAbilityCast_ShouldDenyUnknownAbility_WhenAbilityIdNotInRegistry() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); await BindSlot0PulseAsync(client); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/ability-cast", new AbilityCastRequest @@ -156,6 +173,7 @@ public sealed class AbilityCastApiTests }); var body = await response.Content.ReadFromJsonAsync(); + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -165,49 +183,58 @@ public sealed class AbilityCastApiTests [Fact] public async Task PostAbilityCast_ShouldReturnBadRequest_WhenBodyMalformed() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); + var malformed = new StringContent( + "{\"schemaVersion\":1,\"slotIndex\":", + Encoding.UTF8, + "application/json"); - var response = await client.PostAsync( - "/game/players/dev-local-1/ability-cast", - new StringContent("{\"schemaVersion\":1,\"slotIndex\":", Encoding.UTF8, "application/json")); + // Act + var response = await client.PostAsync("/game/players/dev-local-1/ability-cast", malformed); + // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); } [Fact] public async Task PostAbilityCast_ShouldReturnBadRequest_WhenSchemaVersionWrong() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); + var request = new AbilityCastRequest + { + SchemaVersion = 999, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + }; - var response = await client.PostAsJsonAsync( - "/game/players/dev-local-1/ability-cast", - new AbilityCastRequest - { - SchemaVersion = 999, - SlotIndex = 0, - AbilityId = PrototypeAbilityRegistry.PrototypePulse, - }); + // Act + var response = await client.PostAsJsonAsync("/game/players/dev-local-1/ability-cast", request); + // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); } [Fact] public async Task PostAbilityCast_ShouldReturnNotFound_WhenPlayerUnknown() { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); + var request = new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + }; - var response = await client.PostAsJsonAsync( - "/game/players/missing-player/ability-cast", - new AbilityCastRequest - { - SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, - SlotIndex = 0, - AbilityId = PrototypeAbilityRegistry.PrototypePulse, - }); + // Act + var response = await client.PostAsJsonAsync("/game/players/missing-player/ability-cast", request); + // Assert Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } } From b34f7365b833edc2323b0b81781d3fc4d57a6582 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:24:20 -0400 Subject: [PATCH 06/12] chore: make C# AAA test layout visible in AGENTS and review checklist --- .cursor/rules/code-review-agent.md | 4 ++-- .cursor/rules/testing-expectations.md | 2 +- AGENTS.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md index a747b74..cb2910d 100644 --- a/.cursor/rules/code-review-agent.md +++ b/.cursor/rules/code-review-agent.md @@ -21,7 +21,7 @@ Align recommendations with repo rules and docs, including: - [architecture-authority](architecture-authority.md) — server authority, client vs spike boundaries. - [testing-expectations](testing-expectations.md) — when automated tests are required vs manual. -- [csharp-style](csharp-style.md) / [gdscript-style](gdscript-style.md) — naming, layout, primary constructors for C#. +- [csharp-style](csharp-style.md) / [gdscript-style](gdscript-style.md) — naming, layout, primary constructors for C#; for C# tests, [AAA sections](csharp-style.md#unit-and-integration-tests-arrange-act-assert) on every **new or changed** `[Fact]` / `[Theory]` method. - [linear-git-naming](linear-git-naming.md) — branch/commit prefixes when the work is ticketed. - [git-workflow](git-workflow.md) — branch vs `main`, story-scoped plans. @@ -46,7 +46,7 @@ Work through what applies to the diff (skip irrelevant sections briefly). 2. **APIs & contracts** — Breaking changes, versioning, serialization shapes, documented public surface. 3. **Security** — Injection, secrets in repo, authz/authn assumptions, unsafe defaults. 4. **Performance** — Obvious hot-path allocations or N+1 patterns in new code only; avoid speculative micro-optimization. -5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. +5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. **C#:** any **new or changed** test method in `*Tests.cs` must include **`// Arrange`**, **`// Act`**, **`// Assert`** (blank lines between phases encouraged). Treat missing AAA on touched tests as **should fix**; only call it a **nit** when the diff is a tiny edit inside an already non-AAA legacy method you did not own end-to-end. 6. **Observability** — Logging/metrics where failures would be hard to diagnose (server/runtime code). 7. **Docs** — README or plan updates when behavior or run instructions change. 8. **Plan & decomposition alignment** — Per **Plan and decomposition documentation** above: relevant `docs/plans/` and module/policy docs cited; implementation checked against them; **Documentation checked** section in the saved review file. diff --git a/.cursor/rules/testing-expectations.md b/.cursor/rules/testing-expectations.md index 9a0bf01..57c208b 100644 --- a/.cursor/rules/testing-expectations.md +++ b/.cursor/rules/testing-expectations.md @@ -9,7 +9,7 @@ alwaysApply: true - **Non-trivial logic** (validation, simulation rules, serialization boundaries, idempotency, security-sensitive paths) should have **automated tests** once a test project exists in the repo. - **Default framework:** **xUnit** for new test projects unless the repo already standardizes on something else—stay consistent with existing `*.Tests` projects. -- **Layout:** use **Arrange / Act / Assert** in every test method; see [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). +- **Layout (mandatory for new/changed tests):** use **Arrange / Act / Assert** with **`// Arrange`**, **`// Act`**, **`// Assert`** comments in every test method you add or edit; see [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). PRs that touch `*Tests.cs` without AAA on changed methods should be caught in review. - **At-write-time default:** start new tests with snippet **`xut`** / **`xutc`** from `.vscode/csharp.code-snippets` (AAA sections pre-inserted) or copy `server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`. - **Names:** **`MethodName_ShouldExpectedOutcome_WhenScenario`**; see [C# style — Test method naming convention](csharp-style.md#test-method-naming-convention). - **No test project yet:** It is acceptable to ship a small change without a suite only while the server is mostly scaffolding. When you add the **first** testable module, **add a `NeonSprawl.Server.Tests` (or equivalent) project** and start covering that area; do not leave new core logic permanently untested. diff --git a/AGENTS.md b/AGENTS.md index dc188eb..5ae8dc5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,6 +13,6 @@ Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or | **Code review** | [`.cursor/rules/code-review-agent.md`](.cursor/rules/code-review-agent.md) | PR / diff / pre-merge review; **always writes** `docs/reviews/YYYY-MM-DD-{slug}.md` with **Documentation checked** vs `docs/plans/` and `docs/decomposition/modules/`; **when suggestions are fixed, strike through those bullets + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); short chat pointer | | **Docs review** | [`.cursor/rules/docs-review-agent.md`](.cursor/rules/docs-review-agent.md) | Coherence / links / dev-guide fitness for `docs/` (especially `docs/game-design/` + decomposition); **writes** `docs/reviews/YYYY-MM-DD-{slug}.md`; **when suggestions are fixed, strike through + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); use when working in **documents** or @ mention | -Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use the **GitHub MCP** **`create_pull_request`** tool (`user-github`) with a title starting **`NEO-123:`** ([commit-and-review](.cursor/rules/commit-and-review.md)); **do not use `gh` CLI for GitHub operations in this repo**. **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file). +Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **C# xUnit tests:** every new or changed test method must use **Arrange → Act → Assert** with explicit **`// Arrange`**, **`// Act`**, **`// Assert`** comments — [csharp-style — Unit tests](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert) and [testing-expectations](.cursor/rules/testing-expectations.md); prefer VS Code snippets **`xut`** / **`xutc`**. **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use the **GitHub MCP** **`create_pull_request`** tool (`user-github`) with a title starting **`NEO-123:`** ([commit-and-review](.cursor/rules/commit-and-review.md)); **do not use `gh` CLI for GitHub operations in this repo**. **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file). **Commits tied to a Linear issue** must start the subject with the **issue id** and a colon (e.g. `NEO-8: …`). Branch naming and exceptions (`chore:` when there is no ticket) — [`.cursor/rules/linear-git-naming.md`](.cursor/rules/linear-git-naming.md). From eac95f0c2772a871bb11320c383d9c9e8980ca45 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:27:53 -0400 Subject: [PATCH 07/12] chore: group Bruno hotbar-loadout and ability-cast under subfolders --- .cursor/rules/testing-expectations.md | 2 +- .../Post cast bad schema.bru} | 2 +- .../Post cast happy.bru} | 2 +- .../Post cast loadout mismatch.bru} | 2 +- .../Post cast missing player.bru} | 2 +- .../Post hotbar bind slot0 pulse.bru} | 2 +- .../Get loadout missing player.bru} | 2 +- .../Get loadout.bru} | 2 +- .../Post loadout bad schema.bru} | 2 +- .../Post loadout bind.bru} | 2 +- client/README.md | 2 +- docs/manual-qa/NEO-31.md | 10 +++++----- docs/plans/NEO-31-implementation-plan.md | 14 +++++++------- 13 files changed, 23 insertions(+), 23 deletions(-) rename bruno/neon-sprawl-server/{ability-cast-post-bad-schema.bru => ability-cast/Post cast bad schema.bru} (90%) rename bruno/neon-sprawl-server/{ability-cast-post.bru => ability-cast/Post cast happy.bru} (94%) rename bruno/neon-sprawl-server/{ability-cast-post-loadout-mismatch.bru => ability-cast/Post cast loadout mismatch.bru} (91%) rename bruno/neon-sprawl-server/{ability-cast-post-missing-player.bru => ability-cast/Post cast missing player.bru} (89%) rename bruno/neon-sprawl-server/{ability-cast-preset-slot0.bru => ability-cast/Post hotbar bind slot0 pulse.bru} (89%) rename bruno/neon-sprawl-server/{hotbar-loadout-get-missing-player.bru => hotbar-loadout/Get loadout missing player.bru} (84%) rename bruno/neon-sprawl-server/{hotbar-loadout-get.bru => hotbar-loadout/Get loadout.bru} (94%) rename bruno/neon-sprawl-server/{hotbar-loadout-post-bad-schema.bru => hotbar-loadout/Post loadout bad schema.bru} (90%) rename bruno/neon-sprawl-server/{hotbar-loadout-post.bru => hotbar-loadout/Post loadout bind.bru} (95%) diff --git a/.cursor/rules/testing-expectations.md b/.cursor/rules/testing-expectations.md index 57c208b..c2d69e5 100644 --- a/.cursor/rules/testing-expectations.md +++ b/.cursor/rules/testing-expectations.md @@ -14,7 +14,7 @@ alwaysApply: true - **Names:** **`MethodName_ShouldExpectedOutcome_WhenScenario`**; see [C# style — Test method naming convention](csharp-style.md#test-method-naming-convention). - **No test project yet:** It is acceptable to ship a small change without a suite only while the server is mostly scaffolding. When you add the **first** testable module, **add a `NeonSprawl.Server.Tests` (or equivalent) project** and start covering that area; do not leave new core logic permanently untested. - **Bug fixes:** Prefer a **regression test** when the fix is non-obvious or has broken before. -- **HTTP endpoints:** Any change set that **adds or changes** a route or wire contract on **`NeonSprawl.Server`** (new `MapGet` / `MapPost`, path change, request/response JSON shape, status codes) must **add or update** the **Bruno** collection under **`bruno/neon-sprawl-server/`** so the API stays manually exercisable from the repo: at least one **.bru** request per new/changed route (happy path plus a representative **4xx/404** case when the server defines one). Prefer **Bruno `tests` blocks** on those requests for simple smoke checks (status + key JSON fields) when the behavior is stable enough not to churn every edit. If Bruno is unsuitable for a given endpoint (e.g. streaming-only), say so briefly in the PR or plan instead of silently omitting coverage. +- **HTTP endpoints:** Any change set that **adds or changes** a route or wire contract on **`NeonSprawl.Server`** (new `MapGet` / `MapPost`, path change, request/response JSON shape, status codes) must **add or update** the **Bruno** collection under **`bruno/neon-sprawl-server/`** so the API stays manually exercisable from the repo: at least one **.bru** request per new/changed route (happy path plus a representative **4xx/404** case when the server defines one). **Group** requests in **subfolders** by area (same pattern as `targeting/`, `position/`, `interaction/`, `hotbar-loadout/`, `ability-cast/`). Prefer **Bruno `tests` blocks** on those requests for simple smoke checks (status + key JSON fields) when the behavior is stable enough not to churn every edit. If Bruno is unsuitable for a given endpoint (e.g. streaming-only), say so briefly in the PR or plan instead of silently omitting coverage. ## Integration testing (data manipulation) diff --git a/bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru b/bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru similarity index 90% rename from bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru rename to bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru index f32a9db..cc55b1b 100644 --- a/bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru @@ -1,5 +1,5 @@ meta { - name: ability-cast post bad schema + name: POST ability cast bad schema type: http seq: 22 } diff --git a/bruno/neon-sprawl-server/ability-cast-post.bru b/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru similarity index 94% rename from bruno/neon-sprawl-server/ability-cast-post.bru rename to bruno/neon-sprawl-server/ability-cast/Post cast happy.bru index e30940d..39ad219 100644 --- a/bruno/neon-sprawl-server/ability-cast-post.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru @@ -1,5 +1,5 @@ meta { - name: ability-cast post happy + name: POST ability cast happy type: http seq: 21 } diff --git a/bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru b/bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru similarity index 91% rename from bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru rename to bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru index 87e889d..81e0341 100644 --- a/bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru @@ -1,5 +1,5 @@ meta { - name: ability-cast post loadout mismatch + name: POST ability cast loadout mismatch type: http seq: 24 } diff --git a/bruno/neon-sprawl-server/ability-cast-post-missing-player.bru b/bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru similarity index 89% rename from bruno/neon-sprawl-server/ability-cast-post-missing-player.bru rename to bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru index 88235ce..a765d2a 100644 --- a/bruno/neon-sprawl-server/ability-cast-post-missing-player.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru @@ -1,5 +1,5 @@ meta { - name: ability-cast post missing player + name: POST ability cast missing player type: http seq: 23 } diff --git a/bruno/neon-sprawl-server/ability-cast-preset-slot0.bru b/bruno/neon-sprawl-server/ability-cast/Post hotbar bind slot0 pulse.bru similarity index 89% rename from bruno/neon-sprawl-server/ability-cast-preset-slot0.bru rename to bruno/neon-sprawl-server/ability-cast/Post hotbar bind slot0 pulse.bru index 0095c85..4e6322d 100644 --- a/bruno/neon-sprawl-server/ability-cast-preset-slot0.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post hotbar bind slot0 pulse.bru @@ -1,5 +1,5 @@ meta { - name: ability-cast preset slot0 pulse + name: POST hotbar bind slot0 pulse (cast preset) type: http seq: 20 } diff --git a/bruno/neon-sprawl-server/hotbar-loadout-get-missing-player.bru b/bruno/neon-sprawl-server/hotbar-loadout/Get loadout missing player.bru similarity index 84% rename from bruno/neon-sprawl-server/hotbar-loadout-get-missing-player.bru rename to bruno/neon-sprawl-server/hotbar-loadout/Get loadout missing player.bru index 9b97978..c94ed16 100644 --- a/bruno/neon-sprawl-server/hotbar-loadout-get-missing-player.bru +++ b/bruno/neon-sprawl-server/hotbar-loadout/Get loadout missing player.bru @@ -1,5 +1,5 @@ meta { - name: hotbar-loadout get missing player + name: GET hotbar loadout missing player type: http seq: 2 } diff --git a/bruno/neon-sprawl-server/hotbar-loadout-get.bru b/bruno/neon-sprawl-server/hotbar-loadout/Get loadout.bru similarity index 94% rename from bruno/neon-sprawl-server/hotbar-loadout-get.bru rename to bruno/neon-sprawl-server/hotbar-loadout/Get loadout.bru index 52e7617..9836605 100644 --- a/bruno/neon-sprawl-server/hotbar-loadout-get.bru +++ b/bruno/neon-sprawl-server/hotbar-loadout/Get loadout.bru @@ -1,5 +1,5 @@ meta { - name: hotbar-loadout get + name: GET hotbar loadout type: http seq: 1 } diff --git a/bruno/neon-sprawl-server/hotbar-loadout-post-bad-schema.bru b/bruno/neon-sprawl-server/hotbar-loadout/Post loadout bad schema.bru similarity index 90% rename from bruno/neon-sprawl-server/hotbar-loadout-post-bad-schema.bru rename to bruno/neon-sprawl-server/hotbar-loadout/Post loadout bad schema.bru index 5421180..bc7ca46 100644 --- a/bruno/neon-sprawl-server/hotbar-loadout-post-bad-schema.bru +++ b/bruno/neon-sprawl-server/hotbar-loadout/Post loadout bad schema.bru @@ -1,5 +1,5 @@ meta { - name: hotbar-loadout post bad schema + name: POST hotbar loadout bad schema type: http seq: 4 } diff --git a/bruno/neon-sprawl-server/hotbar-loadout-post.bru b/bruno/neon-sprawl-server/hotbar-loadout/Post loadout bind.bru similarity index 95% rename from bruno/neon-sprawl-server/hotbar-loadout-post.bru rename to bruno/neon-sprawl-server/hotbar-loadout/Post loadout bind.bru index b7bc2ac..4443859 100644 --- a/bruno/neon-sprawl-server/hotbar-loadout-post.bru +++ b/bruno/neon-sprawl-server/hotbar-loadout/Post loadout bind.bru @@ -1,5 +1,5 @@ meta { - name: hotbar-loadout post bind + name: POST hotbar loadout bind type: http seq: 3 } diff --git a/client/README.md b/client/README.md index c09a16e..b47d147 100644 --- a/client/README.md +++ b/client/README.md @@ -213,7 +213,7 @@ pwsh -File scripts/install-git-hooks.ps1 This installs: -- **pre-commit**: blocks commits when `client/scripts/*.gd` changes lack matching `client/test/*_test.gd` updates, or when server route/contract files change without corresponding `server/NeonSprawl.Server.Tests/` and `bruno/neon-sprawl-server/*.bru` updates. +- **pre-commit**: blocks commits when `client/scripts/*.gd` changes lack matching `client/test/*_test.gd` updates, or when server route/contract files change without corresponding `server/NeonSprawl.Server.Tests/` and `bruno/neon-sprawl-server/**/*.bru` updates. - **pre-push**: runs **`gdlint client/scripts client/test`** and **`gdformat --check client/scripts client/test`**. The pre-push hook prefers **`.venv-gd/bin/`** (Unix) or **`.venv-gd/Scripts/`** (Windows venv) when present; without that venv or a global install, the hook **fails** and blocks the push — same as CI. diff --git a/docs/manual-qa/NEO-31.md b/docs/manual-qa/NEO-31.md index 1a44ba1..245a5b0 100644 --- a/docs/manual-qa/NEO-31.md +++ b/docs/manual-qa/NEO-31.md @@ -12,15 +12,15 @@ - [ ] Start server: `cd server/NeonSprawl.Server && dotnet run`. - [ ] `curl -s http://localhost:5253/health` returns JSON with `status: ok`. -- [ ] Run Bruno collection (or at least): `ability-cast-preset-slot0` (seq **20**) then `ability-cast-post` (seq **21**) — expect cast `accepted: true`. -- [ ] `ability-cast-post-bad-schema` → HTTP **400**. -- [ ] `ability-cast-post-missing-player` → HTTP **404**. -- [ ] **`ability-cast-post-loadout-mismatch`** (seq **24**): run **`ability-cast-preset-slot0`** in the same session first so slot 0 is `prototype_pulse`; then expect `accepted: false` and `reasonCode == "loadout_mismatch"`. +- [ ] Run Bruno collection (or at least): `ability-cast/Post hotbar bind slot0 pulse` (seq **20**) then `ability-cast/Post cast happy` (seq **21**) — expect cast `accepted: true`. +- [ ] `ability-cast/Post cast bad schema` → HTTP **400**. +- [ ] `ability-cast/Post cast missing player` → HTTP **404**. +- [ ] **`ability-cast/Post cast loadout mismatch`** (seq **24**): run **`ability-cast/Post hotbar bind slot0 pulse`** in the same session first so slot 0 is `prototype_pulse`; then expect `accepted: false` and `reasonCode == "loadout_mismatch"`. ## 2) Client — bound slot - [ ] With server running, open main scene; confirm hotbar hydrated (NEO-29). -- [ ] Bind slot **0** to `prototype_pulse` (Bruno `hotbar-loadout-post` or equivalent `curl`) if not already. +- [ ] Bind slot **0** to `prototype_pulse` (Bruno `hotbar-loadout/Post loadout bind` or equivalent `curl`) if not already. - [ ] Press **1** (InputMap `hotbar_slot_1`): Godot Output shows a line starting with **`ability_cast_requested`** including `slot=0`, `ability_id=prototype_pulse`, and `targetId=` matching current HUD lock (`null` / `—` when cleared, or the locked id after Tab). - [ ] Server log or network trace shows **one** `POST …/ability-cast` per key press (no spam while key held; Godot `is_action_pressed` fires once per press). diff --git a/docs/plans/NEO-31-implementation-plan.md b/docs/plans/NEO-31-implementation-plan.md index db1bbc8..8b760e9 100644 --- a/docs/plans/NEO-31-implementation-plan.md +++ b/docs/plans/NEO-31-implementation-plan.md @@ -47,7 +47,7 @@ 4. **Tests** — **Server:** xUnit suite posting valid/invalid bodies (unknown player, empty slot, ability mismatch). **Client:** GdUnit suite with mock HTTP verifying URL/method/body for bound vs unbound paths and correct target id propagation from a stub target state provider. -5. **Bruno (`bruno/neon-sprawl-server/`)** — Add `.bru` requests mirroring the existing **hotbar-loadout** pattern (`{{baseUrl}}`, `dev-local-1`): at minimum **happy-path cast POST**, **bad schema / missing player**, and **loadout mismatch or empty slot** so manual API checks and collection runs stay aligned with server xUnit coverage. +5. **Bruno (`bruno/neon-sprawl-server/ability-cast/`)** — Add `.bru` requests mirroring the **hotbar-loadout** folder pattern (`{{baseUrl}}`, `dev-local-1`): at minimum **happy-path cast POST**, **bad schema / missing player**, and **loadout mismatch** so manual API checks and collection runs stay aligned with server xUnit coverage. Hotbar API requests live under **`bruno/neon-sprawl-server/hotbar-loadout/`**. 6. **Docs / backlog hygiene (light)** — After behavior exists, optionally refresh [E1_M4_AbilityInputScaffold.md](../decomposition/modules/E1_M4_AbilityInputScaffold.md) implementation snapshot + Linear table row for E1M4-02 (still optional if timeboxed). @@ -61,11 +61,11 @@ | `client/scripts/ability_cast_client.gd` | Thin HTTP client for cast POST (injection pattern matches hotbar client). | | `client/test/ability_cast_client_test.gd` | Mock-transport tests for payload formation and unbound-slot no-op. | | `docs/manual-qa/NEO-31.md` | Manual QA checklist (slot keys, bound vs unbound, target id from HUD lock). | -| `bruno/neon-sprawl-server/ability-cast-preset-slot0.bru` | POST hotbar bind slot 0 → `prototype_pulse` so cast happy/mismatch requests have deterministic loadout state. | -| `bruno/neon-sprawl-server/ability-cast-post.bru` | Happy-path `POST …/ability-cast` (run after preset when exercising alone). | -| `bruno/neon-sprawl-server/ability-cast-post-bad-schema.bru` | Wrong `schemaVersion` → HTTP 400. | -| `bruno/neon-sprawl-server/ability-cast-post-missing-player.bru` | Unknown player path → HTTP 404. | -| `bruno/neon-sprawl-server/ability-cast-post-loadout-mismatch.bru` | Wrong ability for slot 0 → `loadout_mismatch` (requires preset in same run). | +| `bruno/neon-sprawl-server/ability-cast/Post hotbar bind slot0 pulse.bru` | POST hotbar bind slot 0 → `prototype_pulse` so cast happy/mismatch requests have deterministic loadout state. | +| `bruno/neon-sprawl-server/ability-cast/Post cast happy.bru` | Happy-path `POST …/ability-cast` (run after preset when exercising alone). | +| `bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru` | Wrong `schemaVersion` → HTTP 400. | +| `bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru` | Unknown player path → HTTP 404. | +| `bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru` | Wrong ability for slot 0 → `loadout_mismatch` (requires preset in same run). | ## Files to modify @@ -84,7 +84,7 @@ | `AbilityCastApiTests.cs` | POST success when loadout matches; denies with stable `reasonCode` for bad player, wrong slot/ability, empty binding. | | `ability_cast_client_test.gd` | Client builds JSON with `targetId` from injected target state; unbound slot does not call `request`; bound slot emits exactly one POST body. | | Manual `docs/manual-qa/NEO-31.md` | Human verification of keys, logs, and HUD target id alignment. | -| Bruno `ability-cast-*.bru` | Embedded `tests { … }` blocks assert status + JSON fields for quick regression (same style as `hotbar-loadout-post.bru`). | +| Bruno `ability-cast/*.bru` | Embedded `tests { … }` blocks assert status + JSON fields for quick regression (same style as `hotbar-loadout/Post loadout bind.bru`). | ## Open questions / risks From e88e59f3a6e1937a6d0952138b7425cfe49fbbf3 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:42:36 -0400 Subject: [PATCH 08/12] NEO-31: hotbar cast resolver tests, doc alignment, review follow-up --- client/scripts/hotbar_cast_slot_resolver.gd | 38 +++++++ client/scripts/main.gd | 33 +++--- client/test/hotbar_cast_slot_resolver_test.gd | 100 ++++++++++++++++++ ...umentation_and_implementation_alignment.md | 2 +- .../modules/module_dependency_register.md | 2 +- docs/plans/NEO-31-implementation-plan.md | 9 +- docs/reviews/2026-04-27-NEO-31.md | 45 ++++++++ 7 files changed, 210 insertions(+), 19 deletions(-) create mode 100644 client/scripts/hotbar_cast_slot_resolver.gd create mode 100644 client/test/hotbar_cast_slot_resolver_test.gd create mode 100644 docs/reviews/2026-04-27-NEO-31.md diff --git a/client/scripts/hotbar_cast_slot_resolver.gd b/client/scripts/hotbar_cast_slot_resolver.gd new file mode 100644 index 0000000..76ea168 --- /dev/null +++ b/client/scripts/hotbar_cast_slot_resolver.gd @@ -0,0 +1,38 @@ +extends Object + +## Pure resolver for digit hotbar → cast intent (NEO-31). [code]main.gd[/code] delegates here so +## GdUnit can cover unbound slots and [code]lockedTargetId[/code] sourcing without booting the scene. +## No [code]class_name[/code] — match [code]prototype_target_constants.gd[/code] preload pattern. + +const KEY_OK := "ok" +const KEY_REASON := "reason" +const KEY_SLOT_INDEX := "slotIndex" +const KEY_ABILITY_ID := "abilityId" +const KEY_TARGET_ID := "targetId" + +const REASON_INVALID_SLOT := "invalid_slot_index" +const REASON_EMPTY_SLOT := "empty_slot" + + +## [param slots] snapshot from [code]HotbarState.slots_snapshot()[/code] (fixed length, entries +## [code]null[/code] or [String] ability ids). +## [param target_cached_state] from [code]TargetSelectionClient.cached_state()[/code] (may be empty). +## Returns [code]{ "ok": true, "slotIndex", "abilityId", "targetId" }[/code] or +## [code]{ "ok": false, "reason" }[/code]. +static func resolve(slot_index: int, slots: Array, target_cached_state: Dictionary) -> Dictionary: + if slot_index < 0 or slot_index >= slots.size(): + return {KEY_OK: false, KEY_REASON: REASON_INVALID_SLOT} + var ability_variant: Variant = slots[slot_index] + if ability_variant == null or not (ability_variant is String) or (ability_variant as String).strip_edges().is_empty(): + return {KEY_OK: false, KEY_REASON: REASON_EMPTY_SLOT} + var ability_id: String = (ability_variant as String).strip_edges() + var target_id: Variant = null + var locked: Variant = target_cached_state.get("lockedTargetId", null) + if locked is String and not (locked as String).is_empty(): + target_id = locked as String + return { + KEY_OK: true, + KEY_SLOT_INDEX: slot_index, + KEY_ABILITY_ID: ability_id, + KEY_TARGET_ID: target_id, + } diff --git a/client/scripts/main.gd b/client/scripts/main.gd index d2ccf67..b35bd0a 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -38,6 +38,7 @@ const AUTH_SNAP_LOCOMOTION_SKIP_MAX_DH: float = 0.55 const AUTH_SNAP_LOCOMOTION_VEL_HZ_SQ: float = 0.12 const PrototypeTargetConstants := preload("res://scripts/prototype_target_constants.gd") +const HotbarCastSlotResolver := preload("res://scripts/hotbar_cast_slot_resolver.gd") ## Bump on each rejection so older one-shot timers do not clear a newer message. var _move_reject_msg_token: int = 0 @@ -350,27 +351,31 @@ func _request_hotbar_cast_slot(slot_index: int) -> void: if not is_instance_valid(_hotbar_state) or not _hotbar_state.has_method("slots_snapshot"): return var slots: Array = _hotbar_state.call("slots_snapshot") as Array - if slot_index < 0 or slot_index >= slots.size(): - push_warning("Hotbar cast ignored: slot %d invalid_slot_index" % slot_index) - return - var ability_variant: Variant = slots[slot_index] - if ability_variant == null or not (ability_variant is String) or (ability_variant as String).strip_edges().is_empty(): - push_warning("Hotbar cast ignored: slot %d empty_slot" % slot_index) - return - var ability_id: String = ability_variant as String - var target_id: Variant = null + var target_state: Dictionary = {} if is_instance_valid(_target_client) and _target_client.has_method("cached_state"): var st: Variant = _target_client.call("cached_state") if st is Dictionary: - var locked: Variant = (st as Dictionary).get("lockedTargetId", null) - if locked is String and not (locked as String).is_empty(): - target_id = locked + target_state = st as Dictionary + var outcome: Dictionary = HotbarCastSlotResolver.resolve(slot_index, slots, target_state) + if not bool(outcome.get(HotbarCastSlotResolver.KEY_OK, false)): + var reason: String = str(outcome.get(HotbarCastSlotResolver.KEY_REASON, "")) + if reason == HotbarCastSlotResolver.REASON_INVALID_SLOT: + push_warning("Hotbar cast ignored: slot %d invalid_slot_index" % slot_index) + elif reason == HotbarCastSlotResolver.REASON_EMPTY_SLOT: + push_warning("Hotbar cast ignored: slot %d empty_slot" % slot_index) + return + var ability_id: String = outcome[HotbarCastSlotResolver.KEY_ABILITY_ID] as String + var resolved_slot: int = int(outcome.get(HotbarCastSlotResolver.KEY_SLOT_INDEX, slot_index)) + var target_id: Variant = outcome.get(HotbarCastSlotResolver.KEY_TARGET_ID, null) var target_label: String = "null" if target_id != null: target_label = str(target_id) - print("ability_cast_requested slot=%d ability_id=%s targetId=%s" % [slot_index, ability_id, target_label]) + print( + "ability_cast_requested slot=%d ability_id=%s targetId=%s" + % [resolved_slot, ability_id, target_label] + ) if is_instance_valid(_ability_cast_client) and _ability_cast_client.has_method("request_cast"): - _ability_cast_client.call("request_cast", slot_index, ability_id, target_id) + _ability_cast_client.call("request_cast", resolved_slot, ability_id, target_id) func _forward_interact_post(method: String) -> void: diff --git a/client/test/hotbar_cast_slot_resolver_test.gd b/client/test/hotbar_cast_slot_resolver_test.gd new file mode 100644 index 0000000..5174618 --- /dev/null +++ b/client/test/hotbar_cast_slot_resolver_test.gd @@ -0,0 +1,100 @@ +extends GdUnitTestSuite + +const Resolver := preload("res://scripts/hotbar_cast_slot_resolver.gd") + + +func _eight_empty_slots() -> Array: + var slots: Array = [] + slots.resize(8) + for i in range(8): + slots[i] = null + return slots + + +func test_resolve_returns_ok_with_target_from_locked_target_id() -> void: + # Arrange + var slots := _eight_empty_slots() + slots[0] = "prototype_pulse" + var target_state := {"lockedTargetId": "prototype_target_alpha", "sequence": 3} + + # Act + var outcome: Dictionary = Resolver.resolve(0, slots, target_state) + + # Assert + assert_that(bool(outcome.get(Resolver.KEY_OK, false))).is_true() + assert_that(int(outcome.get(Resolver.KEY_SLOT_INDEX, -1))).is_equal(0) + assert_that(outcome.get(Resolver.KEY_ABILITY_ID)).is_equal("prototype_pulse") + assert_that(outcome.get(Resolver.KEY_TARGET_ID)).is_equal("prototype_target_alpha") + + +func test_resolve_returns_null_target_when_no_lock() -> void: + # Arrange + var slots := _eight_empty_slots() + slots[1] = "prototype_guard" + var target_state := {"lockedTargetId": null, "validity": "none"} + + # Act + var outcome: Dictionary = Resolver.resolve(1, slots, target_state) + + # Assert + assert_that(bool(outcome.get(Resolver.KEY_OK, false))).is_true() + assert_that(outcome.get(Resolver.KEY_TARGET_ID, "sentinel")).is_null() + + +func test_resolve_returns_null_target_when_lock_empty_string() -> void: + # Arrange + var slots := _eight_empty_slots() + slots[2] = "prototype_dash" + var target_state := {"lockedTargetId": ""} + + # Act + var outcome: Dictionary = Resolver.resolve(2, slots, target_state) + + # Assert + assert_that(bool(outcome.get(Resolver.KEY_OK, false))).is_true() + assert_that(outcome.get(Resolver.KEY_TARGET_ID, "sentinel")).is_null() + + +func test_resolve_denies_empty_slot_without_ability() -> void: + # Arrange + var slots := _eight_empty_slots() + slots[3] = null + var target_state := {"lockedTargetId": "prototype_target_beta"} + + # Act + var outcome: Dictionary = Resolver.resolve(3, slots, target_state) + + # Assert + assert_that(bool(outcome.get(Resolver.KEY_OK, true))).is_false() + assert_that(str(outcome.get(Resolver.KEY_REASON, ""))).is_equal(Resolver.REASON_EMPTY_SLOT) + + +func test_resolve_denies_empty_string_ability() -> void: + # Arrange + var slots := _eight_empty_slots() + slots[4] = " " + var target_state := {} + + # Act + var outcome: Dictionary = Resolver.resolve(4, slots, target_state) + + # Assert + assert_that(bool(outcome.get(Resolver.KEY_OK, true))).is_false() + assert_that(str(outcome.get(Resolver.KEY_REASON, ""))).is_equal(Resolver.REASON_EMPTY_SLOT) + + +func test_resolve_denies_slot_index_out_of_range() -> void: + # Arrange + var slots := _eight_empty_slots() + slots[0] = "prototype_pulse" + var target_state := {} + + # Act + var high: Dictionary = Resolver.resolve(8, slots, target_state) + var low: Dictionary = Resolver.resolve(-1, slots, target_state) + + # Assert + assert_that(bool(high.get(Resolver.KEY_OK, true))).is_false() + assert_that(str(high.get(Resolver.KEY_REASON, ""))).is_equal(Resolver.REASON_INVALID_SLOT) + assert_that(bool(low.get(Resolver.KEY_OK, true))).is_false() + assert_that(str(low.get(Resolver.KEY_REASON, ""))).is_equal(Resolver.REASON_INVALID_SLOT) diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 7a0b789..cb557c2 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -49,7 +49,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E1.M1 | Ready | Prototype milestone **Done** ([E1.M1](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4)). Authoritative `PositionState` + **MoveCommand** over HTTP (JSON v1 snap); **in-memory** store by default, **Postgres** when configured ([NEO-8](../../plans/NEO-8-implementation-plan.md)); shared **NpgsqlDataSource** disposed on host shutdown ([NEO-13](../../plans/NEO-13-implementation-plan.md)); Godot sync + path-follow ([NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md)); **InteractionRequest** + horizontal range ([NEO-9](../../plans/NEO-9-implementation-plan.md)). Follow-on: prediction/reconciliation, Slice 1 telemetry, Protobuf wire per [contracts.md](contracts.md). | [NEO-6](../../plans/NEO-6-implementation-plan.md), [NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-8](../../plans/NEO-8-implementation-plan.md), [NEO-9](../../plans/NEO-9-implementation-plan.md), [NEO-10](../../plans/NEO-10-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md), [NEO-13](../../plans/NEO-13-implementation-plan.md); `server/NeonSprawl.Server/Game/PositionState/`, `Game/Interaction/`; [server README](../../../server/README.md) | | 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. **Still open:** cast request path, accept/deny integration UX, cooldown snapshot sync, and telemetry hooks from later E1.M4 stories. | [NEO-29](../../plans/NEO-29-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); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29) | +| 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). **Still open:** combat accept/deny integration UX, cooldown snapshot sync, and telemetry hooks from later E1.M4 stories. | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-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); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29) | --- diff --git a/docs/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index 880ff63..fee350a 100644 --- a/docs/decomposition/modules/module_dependency_register.md +++ b/docs/decomposition/modules/module_dependency_register.md @@ -21,7 +21,7 @@ Fleshed-out scope, contracts, and integration notes live in **per-module documen **E1.M3 note:** Decomposition expanded in [E1_M3_InteractionAndTargetingLayer.md](E1_M3_InteractionAndTargetingLayer.md) (E1.M1 vs E1.M3 boundary, contract sketches, Slice 3 alignment). **Partial (NEO-23):** versioned JSON v1 **`PlayerTargetStateResponse`** + **`POST …/target/select`** under `server/NeonSprawl.Server/Game/Targeting/` ([NEO-23](../../plans/NEO-23-implementation-plan.md); [server README — Targeting](../../../server/README.md#targeting-neo-23)) — throwaway HTTP spike per [contracts.md](contracts.md). **Partial (NEO-25):** versioned **`GET /game/world/interactables`** + `InteractablesListDtos` / `InteractablesWorldApi` in `Game/Interaction/`; Godot `interactables_catalog_client.gd` + `interactable_world_builder.gd` ([NEO-25](../../plans/NEO-25-implementation-plan.md); [server README — Interactable descriptors](../../../server/README.md#interactable-descriptors-neo-25)). **NEO-26 (prototype `SelectionEvent`):** Godot **`TargetSelectionClient.selection_event`** on **`lockedTargetId`** changes ([NEO-26](../../plans/NEO-26-implementation-plan.md)). **Follow-on / in progress:** richer multi-consumer **`InteractableDescriptor`** targeting on later Linear issues. **Precursor (E1.M1):** **`InteractionRequest`** + horizontal reach ([NEO-9](../../plans/NEO-9-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`). -**E1.M4 note:** Vertical-slice decomposition is defined in [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) with story slugs **E1M4-01** through **E1M4-05** (loadout contract, cast path, accept/deny UX, cooldown sync, telemetry hooks). **NEO-29** started implementation with `HotbarLoadout` v1 server APIs + persistence wiring and client hydration, so module status is **In Progress**; see [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md) and [NEO-29 plan](../../plans/NEO-29-implementation-plan.md). +**E1.M4 note:** Vertical-slice decomposition is defined in [E1M4-prototype-backlog.md](../../plans/E1M4-prototype-backlog.md) with story slugs **E1M4-01** through **E1M4-05** (loadout contract, cast path, accept/deny UX, cooldown sync, telemetry hooks). **NEO-29** started implementation with `HotbarLoadout` v1 server APIs + persistence wiring and client hydration; **NEO-31** added the prototype cast POST + client digit-key path and resolver tests — module status remains **In Progress**; see [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md), [NEO-29 plan](../../plans/NEO-29-implementation-plan.md), and [NEO-31 plan](../../plans/NEO-31-implementation-plan.md). ### Epic 2 — Skills and Progression Framework diff --git a/docs/plans/NEO-31-implementation-plan.md b/docs/plans/NEO-31-implementation-plan.md index 8b760e9..11d8259 100644 --- a/docs/plans/NEO-31-implementation-plan.md +++ b/docs/plans/NEO-31-implementation-plan.md @@ -59,7 +59,9 @@ | `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Route handler: validation, loadout cross-check, prototype response. | | `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | HTTP integration tests for accept/deny paths and payload shape. | | `client/scripts/ability_cast_client.gd` | Thin HTTP client for cast POST (injection pattern matches hotbar client). | -| `client/test/ability_cast_client_test.gd` | Mock-transport tests for payload formation and unbound-slot no-op. | +| `client/scripts/hotbar_cast_slot_resolver.gd` | Pure hotbar slot + cached target state → cast payload / deny reason (testable from `main.gd`). | +| `client/test/ability_cast_client_test.gd` | Mock-transport tests for payload formation and busy coalescing. | +| `client/test/hotbar_cast_slot_resolver_test.gd` | Resolver: `lockedTargetId` propagation, empty slot / OOB denies (acceptance criteria without booting `main.gd`). | | `docs/manual-qa/NEO-31.md` | Manual QA checklist (slot keys, bound vs unbound, target id from HUD lock). | | `bruno/neon-sprawl-server/ability-cast/Post hotbar bind slot0 pulse.bru` | POST hotbar bind slot 0 → `prototype_pulse` so cast happy/mismatch requests have deterministic loadout state. | | `bruno/neon-sprawl-server/ability-cast/Post cast happy.bru` | Happy-path `POST …/ability-cast` (run after preset when exercising alone). | @@ -73,7 +75,7 @@ |------|-----------| | `server/NeonSprawl.Server/Program.cs` | Register `MapAbilityCastApi()` (or named extension) alongside existing maps. | | `client/project.godot` | Add `hotbar_slot_1` … `hotbar_slot_8` (or compact equivalent) bound to digit keys for prototype. | -| `client/scripts/main.gd` | Wire input → hotbar snapshot + `TargetSelectionClient` + cast client; real `ability_cast_requested` hook site before submit. | +| `client/scripts/main.gd` | Wire input → `HotbarCastSlotResolver` + cast client; real `ability_cast_requested` hook site before submit. | | `client/README.md` | Document digit hotbar keys, cast endpoint, and manual QA link. | | `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | Snapshot + Linear link for E1M4-02. | @@ -82,7 +84,8 @@ | Suite | What it covers | |--------|----------------| | `AbilityCastApiTests.cs` | POST success when loadout matches; denies with stable `reasonCode` for bad player, wrong slot/ability, empty binding. | -| `ability_cast_client_test.gd` | Client builds JSON with `targetId` from injected target state; unbound slot does not call `request`; bound slot emits exactly one POST body. | +| `ability_cast_client_test.gd` | Client builds JSON with `targetId` from caller; busy coalescing. | +| `hotbar_cast_slot_resolver_test.gd` | Bound slot + `lockedTargetId` / empty lock / unbound / OOB — same rules `main.gd` uses before `request_cast`. | | Manual `docs/manual-qa/NEO-31.md` | Human verification of keys, logs, and HUD target id alignment. | | Bruno `ability-cast/*.bru` | Embedded `tests { … }` blocks assert status + JSON fields for quick regression (same style as `hotbar-loadout/Post loadout bind.bru`). | diff --git a/docs/reviews/2026-04-27-NEO-31.md b/docs/reviews/2026-04-27-NEO-31.md new file mode 100644 index 0000000..86a0476 --- /dev/null +++ b/docs/reviews/2026-04-27-NEO-31.md @@ -0,0 +1,45 @@ +# Code review — NEO-31 + +- **Date:** 2026-04-27 +- **Scope:** Branch `NEO-31-e1m4-02-input-to-abilitycastrequest-path` (`origin/main...HEAD`) plus current untracked Bruno/Godot UID files in the working tree. +- **Base:** `origin/main` + +## Verdict + +Approve with nits. **Follow-up:** blocking coordinator-test gap addressed in-repo (`hotbar_cast_slot_resolver.gd` + `hotbar_cast_slot_resolver_test.gd`, `documentation_and_implementation_alignment.md` + register note). + +## Summary + +NEO-31 adds the prototype `POST /game/players/{id}/ability-cast` route, client digit-key hotbar cast wiring, Bruno requests, manual QA, and E1.M4 documentation updates. The server API is small and well covered, and `dotnet test` passes. **Update:** client-side slot/target resolution is extracted to **`hotbar_cast_slot_resolver.gd`** with GdUnit coverage so unbound vs bound and `lockedTargetId` sourcing are automated without booting `main.gd`. + +## Documentation checked + +- `docs/plans/NEO-31-implementation-plan.md` — **matches** after follow-up (resolver + resolver tests listed). +- `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` — **matches** for the NEO-31 snapshot and Linear row update. +- `docs/decomposition/modules/module_dependency_register.md` — **matches** after follow-up (E1.M4 note mentions NEO-31). +- `docs/decomposition/modules/documentation_and_implementation_alignment.md` — **matches** after follow-up (E1.M4 row documents NEO-31 cast path landed; remaining open items are accept/deny UX, cooldown, telemetry). +- `docs/decomposition/modules/contracts.md` — **matches** for prototype scope. JSON/HTTP remains acceptable as an early spike with explicit `schemaVersion`. +- `docs/decomposition/modules/client_server_authority.md` — **matches** for this slice. The client sends cast intent, while the server validates player existence, hotbar binding, slot bounds, and known abilities; full combat authority remains deferred to E5.M1. + +## Blocking Issues + +1. ~~`client/test/ability_cast_client_test.gd` does not cover the behavior that lives in `client/scripts/main.gd`, so two plan acceptance criteria are only manually checked. The NEO-31 plan explicitly calls for client harness tests around "formation, send triggers, and unbound-slot behavior" plus target id propagation from a stub target state provider. Current tests verify `AbilityCastClient.request_cast()` serializes a provided `target_id`, but they do not exercise `_request_hotbar_cast_slot()`, do not prove empty slots avoid POSTs, and do not prove `lockedTargetId` is read from `TargetSelectionClient.cached_state()`. Add a focused GdUnit test around the coordinator path, or extract that small decision into a testable helper and cover bound, unbound, and locked-target cases.~~ **Done.** Added `client/scripts/hotbar_cast_slot_resolver.gd` + `client/test/hotbar_cast_slot_resolver_test.gd`; `main.gd` delegates resolution before `request_cast`. + +## Suggestions + +1. ~~Update `docs/decomposition/modules/documentation_and_implementation_alignment.md` for NEO-31. The module page now says NEO-31 landed, but the tracking table still says "Still open: cast request path"; this will mislead the next implementation/review pass.~~ **Done.** + +2. Consider making `AbilityCastClient.request_cast()` return whether a POST actually started, then log `ability_cast_requested` only on success. Today `client/scripts/main.gd` prints `ability_cast_requested` before calling the client, while `client/scripts/ability_cast_client.gd` silently returns when `_busy` is true or when the request fails to start. That can over-count cast requests in the future telemetry hook and conflicts with the plan wording that the hook sits immediately before a successful POST start. + +3. Consider adding a short `server/README.md` section for `POST /game/players/{id}/ability-cast`, matching the existing server-side endpoint documentation pattern used for hotbar loadout and targeting. The client README and Bruno collection cover manual usage, but the server API index currently has no NEO-31 entry. + +## Nits + +- Nit: The new `main.gd` variables `authority_base_url2` and `authority_player_id2` are clear enough, but `ability_base_url` / `ability_player_id` would be easier to scan if this setup grows. + +## Verification + +- `dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj` — passed: 86 tests. +- `git diff --check origin/main...HEAD && git diff --check` — passed. +- Cursor lints checked for the touched C# and GDScript files — no linter errors reported. +- Not run: GdUnit client suite or Bruno collection. Run `godot --headless --path client -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/ability_cast_client_test.gd` and the NEO-31 Bruno/manual checklist before merge. From cc1c6f711e6a96b2d7a80db906e09f3bb1d61a1a Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:44:40 -0400 Subject: [PATCH 09/12] NEO-31: request_cast bool for telemetry hook; document cast API in server README --- client/README.md | 2 +- client/scripts/ability_cast_client.gd | 12 ++++++--- client/scripts/main.gd | 21 +++++++++------- client/test/ability_cast_client_test.gd | 19 +++++++++++--- ...umentation_and_implementation_alignment.md | 2 +- docs/plans/NEO-31-implementation-plan.md | 2 +- docs/reviews/2026-04-27-NEO-31.md | 4 +-- server/README.md | 25 +++++++++++++++++++ 8 files changed, 65 insertions(+), 22 deletions(-) diff --git a/client/README.md b/client/README.md index b47d147..b366707 100644 --- a/client/README.md +++ b/client/README.md @@ -112,7 +112,7 @@ Full checklist: [`docs/manual-qa/NEO-25.md`](../manual-qa/NEO-25.md). - **`TargetSelectionClient`** emits **`selection_event(event: Dictionary)`** only when the server-acknowledged **`lockedTargetId`** changes (not on **`validity`-only** updates; those stay on **`target_state_changed`**). - Payload keys: **`previous`** ([String] or `null`), **`next`** (same), **`cause`** (`tab` \| `clear` \| `server_correction`), **`sequence`** (int from the new state). **`POST …/target/select`** from **`request_select_target_id`** (including the path used by Tab) uses cause **`tab`**; **`request_clear_target`** uses **`clear`**; every **`GET …/target`** completion uses **`server_correction`** (including boot sync). - **Slice 3 telemetry hook (NEO-27):** this selection transition maps to product event name **`target_changed`**; with **`log_selection_events`** enabled, Godot Output prints the `target_changed` token for manual verification. **TODO(E9.M1):** replace dev log with cataloged telemetry emit. -- **Ability cast hooks (NEO-27 + NEO-31):** digit keys **`hotbar_slot_1` … `hotbar_slot_8`** (defaults **1**–**8**) issue a prototype cast via `scripts/ability_cast_client.gd`. **`ability_cast_requested`** is printed from `main.gd` immediately before the POST; **`ability_cast_denied`** is logged as `push_warning` from the client on deny responses (TODO(E9.M1): telemetry). +- **Ability cast hooks (NEO-27 + NEO-31):** digit keys **`hotbar_slot_1` … `hotbar_slot_8`** (defaults **1**–**8**) issue a prototype cast via `scripts/ability_cast_client.gd`. **`ability_cast_requested`** is printed from `main.gd` only when **`request_cast`** returns **`true`** (HTTP POST successfully queued); **`ability_cast_denied`** is logged as `push_warning` from the client on deny responses (TODO(E9.M1): telemetry). - **E1.M4+:** connect to **`$TargetSelectionClient.selection_event`** (or your scene’s path to that node) without reshaping the payload. Full checklist: [`docs/manual-qa/NEO-26.md`](../docs/manual-qa/NEO-26.md). diff --git a/client/scripts/ability_cast_client.gd b/client/scripts/ability_cast_client.gd index 471a7e0..b3b3350 100644 --- a/client/scripts/ability_cast_client.gd +++ b/client/scripts/ability_cast_client.gd @@ -22,9 +22,12 @@ func _ready() -> void: ## [param target_id] server [code]lockedTargetId[/code] string, or [code]null[/code] if none. -func request_cast(slot_index: int, ability_id: String, target_id: Variant) -> void: +## Returns [code]true[/code] when the HTTP POST was **queued** ([method HTTPRequest.request] returned [code]OK[/code]); +## [code]false[/code] if already [member _busy], request failed to start, or client invalid — use for +## [code]ability_cast_requested[/code] dev / future telemetry so hooks match actual submit attempts. +func request_cast(slot_index: int, ability_id: String, target_id: Variant) -> bool: if _busy: - return + return false var tid: Variant = null if target_id is String: var s: String = target_id as String @@ -36,13 +39,14 @@ func request_cast(slot_index: int, ability_id: String, target_id: Variant) -> vo "abilityId": ability_id, "targetId": tid, } - _busy = true var url := "%s/game/players/%s/ability-cast" % [_base_root(), _player_path_segment()] var headers := PackedStringArray(["Content-Type: application/json"]) var err: Error = _http.request(url, headers, HTTPClient.METHOD_POST, JSON.stringify(payload)) if err != OK: push_warning("AbilityCastClient: POST failed to start (%s)" % err) - _busy = false + return false + _busy = true + return true func _base_root() -> String: diff --git a/client/scripts/main.gd b/client/scripts/main.gd index b35bd0a..141d23b 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -207,7 +207,8 @@ func _on_authoritative_position(world: Vector3, apply_as_snap: bool) -> void: ## (that is `authoritative_position_received`'s job on boot + move rejection). ## ## NEO-27 / NEO-31: `ability_cast_requested` is emitted as a dev [code]print[/code] from -## [method _request_hotbar_cast_slot] immediately before the cast HTTP POST starts. +## [method _request_hotbar_cast_slot] only after [code]request_cast[/code] on +## [code]AbilityCastClient[/code] returns [code]true[/code] (POST successfully queued on [code]HTTPRequest[/code]). ## [code]ability_cast_denied[/code] is surfaced via [code]push_warning[/code] from ## `ability_cast_client.gd` on deny responses (TODO(E9.M1): telemetry schema + ingest). func _on_authoritative_ack_for_hud(world: Vector3) -> void: @@ -367,15 +368,17 @@ func _request_hotbar_cast_slot(slot_index: int) -> void: var ability_id: String = outcome[HotbarCastSlotResolver.KEY_ABILITY_ID] as String var resolved_slot: int = int(outcome.get(HotbarCastSlotResolver.KEY_SLOT_INDEX, slot_index)) var target_id: Variant = outcome.get(HotbarCastSlotResolver.KEY_TARGET_ID, null) - var target_label: String = "null" - if target_id != null: - target_label = str(target_id) - print( - "ability_cast_requested slot=%d ability_id=%s targetId=%s" - % [resolved_slot, ability_id, target_label] - ) + var started: bool = false if is_instance_valid(_ability_cast_client) and _ability_cast_client.has_method("request_cast"): - _ability_cast_client.call("request_cast", resolved_slot, ability_id, target_id) + started = bool(_ability_cast_client.call("request_cast", resolved_slot, ability_id, target_id)) + if started: + var target_label: String = "null" + if target_id != null: + target_label = str(target_id) + print( + "ability_cast_requested slot=%d ability_id=%s targetId=%s" + % [resolved_slot, ability_id, target_label] + ) func _forward_interact_post(method: String) -> void: diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 5f23329..8b92f69 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -77,7 +77,8 @@ func test_request_cast_posts_expected_payload_with_null_target() -> void: var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') var c := _make_client(transport) - c.call("request_cast", 2, "prototype_guard", null) + var started: bool = bool(c.call("request_cast", 2, "prototype_guard", null)) + assert_that(started).is_true() assert_that(transport.last_url).contains("/ability-cast") assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST) var parsed: Variant = JSON.parse_string(transport.last_body) @@ -93,17 +94,27 @@ func test_request_cast_posts_target_id_string_when_set() -> void: var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') var c := _make_client(transport) - c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha") + assert_that(bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha"))).is_true() var parsed: Variant = JSON.parse_string(transport.last_body) var body: Dictionary = parsed assert_that(body.get("targetId")).is_equal("prototype_target_alpha") +func test_request_cast_returns_false_when_http_request_fails_to_start() -> void: + # Arrange: empty queue → transport.request returns ERR_UNAVAILABLE (same as failed start). + var transport := MockHttpTransport.new() + var c := _make_client(transport) + # Act + var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", null)) + # Assert + assert_that(started).is_false() + + func test_request_cast_while_busy_is_ignored() -> void: var transport := HoldFirstThenAutoTransport.new() var c := _make_client(transport) - c.call("request_cast", 0, "prototype_pulse", null) - c.call("request_cast", 1, "prototype_guard", null) + assert_that(bool(c.call("request_cast", 0, "prototype_pulse", null))).is_true() + assert_that(bool(c.call("request_cast", 1, "prototype_guard", null))).is_false() await get_tree().process_frame await get_tree().process_frame assert_that(transport.request_count).is_equal(1) diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index cb557c2..8e4396a 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -49,7 +49,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | E1.M1 | Ready | Prototype milestone **Done** ([E1.M1](https://linear.app/neon-sprawl/project/e1m1-inputandmovementruntime-20eb28dd3db4)). Authoritative `PositionState` + **MoveCommand** over HTTP (JSON v1 snap); **in-memory** store by default, **Postgres** when configured ([NEO-8](../../plans/NEO-8-implementation-plan.md)); shared **NpgsqlDataSource** disposed on host shutdown ([NEO-13](../../plans/NEO-13-implementation-plan.md)); Godot sync + path-follow ([NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md)); **InteractionRequest** + horizontal range ([NEO-9](../../plans/NEO-9-implementation-plan.md)). Follow-on: prediction/reconciliation, Slice 1 telemetry, Protobuf wire per [contracts.md](contracts.md). | [NEO-6](../../plans/NEO-6-implementation-plan.md), [NEO-7](../../plans/NEO-7-implementation-plan.md), [NEO-8](../../plans/NEO-8-implementation-plan.md), [NEO-9](../../plans/NEO-9-implementation-plan.md), [NEO-10](../../plans/NEO-10-implementation-plan.md), [NEO-11](../../plans/NEO-11-implementation-plan.md), [NEO-13](../../plans/NEO-13-implementation-plan.md); `server/NeonSprawl.Server/Game/PositionState/`, `Game/Interaction/`; [server README](../../../server/README.md) | | 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). **Still open:** combat accept/deny integration UX, cooldown snapshot sync, and telemetry hooks from later E1.M4 stories. | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-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); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29) | +| 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). **Still open:** combat accept/deny integration UX, cooldown snapshot sync, and telemetry hooks from later E1.M4 stories. | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-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); [server README — Hotbar loadout](../../../server/README.md#hotbar-loadout-neo-29), [Ability cast (NEO-31)](../../../server/README.md#ability-cast-neo-31) | --- diff --git a/docs/plans/NEO-31-implementation-plan.md b/docs/plans/NEO-31-implementation-plan.md index 11d8259..d00fd33 100644 --- a/docs/plans/NEO-31-implementation-plan.md +++ b/docs/plans/NEO-31-implementation-plan.md @@ -43,7 +43,7 @@ 2. **Server** — New `POST /game/players/{id}/ability-cast` (name fixed in implementation; document in plan **Decisions** if adjusted) mapped in `Program.cs`, implemented beside existing `Game/AbilityInput` types. Validate: player exists (same **player id** gate as hotbar APIs via `IPositionStateStore` or equivalent), slot in range, **non-empty ability** on that slot from **`IPlayerHotbarLoadoutStore`** matches request `abilityId`. Do **not** implement full combat engine; optionally shallow-check `targetId` against current lock store if low-cost, otherwise accept client-provided target for v1 and let NEO-28 tighten. -3. **Client** — New `AbilityCastClient` (or equivalent) mirroring `HotbarLoadoutClient`: `base_url`, `dev_player_id`, injectable HTTP, `request_cast(slot_index, ability_id, target_id_variant)` that POSTs JSON once. **`main.gd`** (or dedicated small coordinator): on new InputMap actions **`hotbar_slot_1` … `hotbar_slot_8`** (digits 1–8), resolve slot index, read **`HotbarState.slots_snapshot()`**; if empty, warn and return; else read **`TargetSelectionClient.cached_state()`** for `lockedTargetId`, build payload, call cast client. Replace NEO-27 **reserve-only** comment block with a real **`ability_cast_requested`** hook site (still `TODO(E9.M1)` for ingest) immediately **before** successful POST start, per `main.gd` notes. +3. **Client** — New `AbilityCastClient` (or equivalent) mirroring `HotbarLoadoutClient`: `base_url`, `dev_player_id`, injectable HTTP, **`request_cast(...) -> bool`** (true when `HTTPRequest.request` queued the POST). **`main.gd`**: digit keys **`hotbar_slot_1` … `hotbar_slot_8`**, **`HotbarCastSlotResolver`**, then cast client; **`ability_cast_requested`** dev print only when **`request_cast`** returns **`true`** (still `TODO(E9.M1)` for ingest). 4. **Tests** — **Server:** xUnit suite posting valid/invalid bodies (unknown player, empty slot, ability mismatch). **Client:** GdUnit suite with mock HTTP verifying URL/method/body for bound vs unbound paths and correct target id propagation from a stub target state provider. diff --git a/docs/reviews/2026-04-27-NEO-31.md b/docs/reviews/2026-04-27-NEO-31.md index 86a0476..c8114e8 100644 --- a/docs/reviews/2026-04-27-NEO-31.md +++ b/docs/reviews/2026-04-27-NEO-31.md @@ -29,9 +29,9 @@ NEO-31 adds the prototype `POST /game/players/{id}/ability-cast` route, client d 1. ~~Update `docs/decomposition/modules/documentation_and_implementation_alignment.md` for NEO-31. The module page now says NEO-31 landed, but the tracking table still says "Still open: cast request path"; this will mislead the next implementation/review pass.~~ **Done.** -2. Consider making `AbilityCastClient.request_cast()` return whether a POST actually started, then log `ability_cast_requested` only on success. Today `client/scripts/main.gd` prints `ability_cast_requested` before calling the client, while `client/scripts/ability_cast_client.gd` silently returns when `_busy` is true or when the request fails to start. That can over-count cast requests in the future telemetry hook and conflicts with the plan wording that the hook sits immediately before a successful POST start. +2. ~~Consider making `AbilityCastClient.request_cast()` return whether a POST actually started, then log `ability_cast_requested` only on success. Today `client/scripts/main.gd` prints `ability_cast_requested` before calling the client, while `client/scripts/ability_cast_client.gd` silently returns when `_busy` is true or when the request fails to start. That can over-count cast requests in the future telemetry hook and conflicts with the plan wording that the hook sits immediately before a successful POST start.~~ **Done.** `request_cast` → `bool`; `_busy` set only after `HTTPRequest.request` returns `OK`; `main.gd` prints only when `true`. -3. Consider adding a short `server/README.md` section for `POST /game/players/{id}/ability-cast`, matching the existing server-side endpoint documentation pattern used for hotbar loadout and targeting. The client README and Bruno collection cover manual usage, but the server API index currently has no NEO-31 entry. +3. ~~Consider adding a short `server/README.md` section for `POST /game/players/{id}/ability-cast`, matching the existing server-side endpoint documentation pattern used for hotbar loadout and targeting. The client README and Bruno collection cover manual usage, but the server API index currently has no NEO-31 entry.~~ **Done.** See [server README — Ability cast (NEO-31)](../../server/README.md#ability-cast-neo-31). ## Nits diff --git a/server/README.md b/server/README.md index 4ad782f..bd5a3dd 100644 --- a/server/README.md +++ b/server/README.md @@ -223,6 +223,31 @@ Prototype server-owned hotbar bindings are available at: Current prototype allowlist: `prototype_pulse`, `prototype_guard`, `prototype_dash`, `prototype_burst`. +## Ability cast (NEO-31) + +Prototype **cast intent** (no combat resolution yet — see E1.M4 / E5.M1 follow-on stories): + +- **`POST /game/players/{id}/ability-cast`** with `AbilityCastRequest` v1 (`schemaVersion`, `slotIndex` `0..7`, `abilityId`, optional nullable `targetId` mirroring client `lockedTargetId`) validates the player exists, the slot is bound in persisted hotbar loadout, and `abilityId` matches that binding and the prototype allowlist. Returns **`AbilityCastResponse` v1** JSON (`schemaVersion`, `accepted`, optional `reasonCode`). + +**HTTP status:** + +| Status | When | +|--------|------| +| **200** | Body parses as `AbilityCastResponse`; `accepted` is `true` or `false` with a stable `reasonCode` on deny. | +| **400** | Missing body or wrong `schemaVersion`. | +| **404** | Unknown player id (no position row / unknown player for this prototype). | + +**Deny `reasonCode` values (200, `accepted=false`):** + +| Code | Meaning | +|------|---------| +| `slot_out_of_bounds` | `slotIndex` outside `0..7`. | +| `slot_unbound` | No ability bound on that slot in stored loadout. | +| `loadout_mismatch` | `abilityId` does not match the ability bound on that slot. | +| `unknown_ability` | `abilityId` failed registry normalization (empty/garbage/off-list). | + +Implementation: `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs`, `AbilityCastDtos.cs`. + ## Solution From repo root: `dotnet build NeonSprawl.sln` and `dotnet test NeonSprawl.sln`. From af1fdef816a65b6153524c5e4fb448cc3ae6194c Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:45:48 -0400 Subject: [PATCH 10/12] NEO-31: single authority mirror for hotbar and ability cast clients --- client/scripts/main.gd | 15 +++++---------- client/test/ability_cast_client_test.gd | 1 + docs/reviews/2026-04-27-NEO-31.md | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 141d23b..76f9996 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -235,27 +235,22 @@ func _setup_hotbar_loadout_sync() -> void: _hotbar_client = load("res://scripts/hotbar_loadout_client.gd").new() _hotbar_client.name = "HotbarLoadoutClient" add_child(_hotbar_client) + _ability_cast_client = load("res://scripts/ability_cast_client.gd").new() + _ability_cast_client.name = "AbilityCastClient" + add_child(_ability_cast_client) if is_instance_valid(_authority): var authority_base_url: Variant = _authority.get("base_url") var authority_player_id: Variant = _authority.get("dev_player_id") if authority_base_url is String and not (authority_base_url as String).is_empty(): _hotbar_client.set("base_url", authority_base_url) + _ability_cast_client.set("base_url", authority_base_url) if authority_player_id is String and not (authority_player_id as String).is_empty(): _hotbar_client.set("dev_player_id", authority_player_id) + _ability_cast_client.set("dev_player_id", authority_player_id) if _hotbar_client.has_method("set_hotbar_state"): _hotbar_client.call("set_hotbar_state", _hotbar_state) if _hotbar_client.has_method("request_sync_from_server"): _hotbar_client.call("request_sync_from_server") - _ability_cast_client = load("res://scripts/ability_cast_client.gd").new() - _ability_cast_client.name = "AbilityCastClient" - add_child(_ability_cast_client) - if is_instance_valid(_authority): - var authority_base_url2: Variant = _authority.get("base_url") - var authority_player_id2: Variant = _authority.get("dev_player_id") - if authority_base_url2 is String and not (authority_base_url2 as String).is_empty(): - _ability_cast_client.set("base_url", authority_base_url2) - if authority_player_id2 is String and not (authority_player_id2 as String).is_empty(): - _ability_cast_client.set("dev_player_id", authority_player_id2) ## Combines cached server state (`_last_target_state`) with per-anchor horizontal diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 8b92f69..a8efc2c 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -1,5 +1,6 @@ extends GdUnitTestSuite +# main.gd sets base_url/dev_player_id on AbilityCastClient from the same authority block as HotbarLoadoutClient (NEO-31). const CastClient := preload("res://scripts/ability_cast_client.gd") diff --git a/docs/reviews/2026-04-27-NEO-31.md b/docs/reviews/2026-04-27-NEO-31.md index c8114e8..83460a1 100644 --- a/docs/reviews/2026-04-27-NEO-31.md +++ b/docs/reviews/2026-04-27-NEO-31.md @@ -35,7 +35,7 @@ NEO-31 adds the prototype `POST /game/players/{id}/ability-cast` route, client d ## Nits -- Nit: The new `main.gd` variables `authority_base_url2` and `authority_player_id2` are clear enough, but `ability_base_url` / `ability_player_id` would be easier to scan if this setup grows. +- ~~Nit: The new `main.gd` variables `authority_base_url2` and `authority_player_id2` are clear enough, but `ability_base_url` / `ability_player_id` would be easier to scan if this setup grows.~~ **Done.** One `_authority` read applies `base_url` / `dev_player_id` to both HTTP clients after both nodes exist. ## Verification From 18e55f717ae73466335c169743d3f42cb133759d Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:48:47 -0400 Subject: [PATCH 11/12] NEO-31: wrap long lines for gdlint pre-push --- client/scripts/ability_cast_client.gd | 8 +++++--- client/scripts/hotbar_cast_slot_resolver.gd | 12 +++++++++--- client/scripts/main.gd | 16 +++++++++++----- client/test/ability_cast_client_test.gd | 9 +++++++-- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/client/scripts/ability_cast_client.gd b/client/scripts/ability_cast_client.gd index b3b3350..de21fba 100644 --- a/client/scripts/ability_cast_client.gd +++ b/client/scripts/ability_cast_client.gd @@ -22,9 +22,11 @@ func _ready() -> void: ## [param target_id] server [code]lockedTargetId[/code] string, or [code]null[/code] if none. -## Returns [code]true[/code] when the HTTP POST was **queued** ([method HTTPRequest.request] returned [code]OK[/code]); -## [code]false[/code] if already [member _busy], request failed to start, or client invalid — use for -## [code]ability_cast_requested[/code] dev / future telemetry so hooks match actual submit attempts. +## Returns [code]true[/code] when the HTTP POST was **queued** +## ([method HTTPRequest.request] returned [code]OK[/code]); +## [code]false[/code] if already [member _busy], request failed to start, or client invalid — +## use for [code]ability_cast_requested[/code] dev / future telemetry so hooks match actual submit +## attempts. func request_cast(slot_index: int, ability_id: String, target_id: Variant) -> bool: if _busy: return false diff --git a/client/scripts/hotbar_cast_slot_resolver.gd b/client/scripts/hotbar_cast_slot_resolver.gd index 76ea168..7181c4f 100644 --- a/client/scripts/hotbar_cast_slot_resolver.gd +++ b/client/scripts/hotbar_cast_slot_resolver.gd @@ -1,7 +1,8 @@ extends Object ## Pure resolver for digit hotbar → cast intent (NEO-31). [code]main.gd[/code] delegates here so -## GdUnit can cover unbound slots and [code]lockedTargetId[/code] sourcing without booting the scene. +## GdUnit can cover unbound slots and [code]lockedTargetId[/code] sourcing without booting +## the scene. ## No [code]class_name[/code] — match [code]prototype_target_constants.gd[/code] preload pattern. const KEY_OK := "ok" @@ -16,14 +17,19 @@ const REASON_EMPTY_SLOT := "empty_slot" ## [param slots] snapshot from [code]HotbarState.slots_snapshot()[/code] (fixed length, entries ## [code]null[/code] or [String] ability ids). -## [param target_cached_state] from [code]TargetSelectionClient.cached_state()[/code] (may be empty). +## [param target_cached_state] from [code]TargetSelectionClient.cached_state()[/code] +## (may be empty). ## Returns [code]{ "ok": true, "slotIndex", "abilityId", "targetId" }[/code] or ## [code]{ "ok": false, "reason" }[/code]. static func resolve(slot_index: int, slots: Array, target_cached_state: Dictionary) -> Dictionary: if slot_index < 0 or slot_index >= slots.size(): return {KEY_OK: false, KEY_REASON: REASON_INVALID_SLOT} var ability_variant: Variant = slots[slot_index] - if ability_variant == null or not (ability_variant is String) or (ability_variant as String).strip_edges().is_empty(): + if ( + ability_variant == null + or not (ability_variant is String) + or (ability_variant as String).strip_edges().is_empty() + ): return {KEY_OK: false, KEY_REASON: REASON_EMPTY_SLOT} var ability_id: String = (ability_variant as String).strip_edges() var target_id: Variant = null diff --git a/client/scripts/main.gd b/client/scripts/main.gd index 76f9996..3ee0362 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -10,7 +10,8 @@ extends Node3D ## Prototype: two random short bumps (sibling StaticBody3D under NavigationRegion3D; see ## `random_floor_bumps.gd`) before nav bake. ## Prototype: `PhysicsRampTest*` (+X extension: west slab ends before the dip; -## `PhysicsRampTestFloorExtensionNorthFill` / `SouthFill` restore walkable floor past the dip in ±Z; +## `PhysicsRampTestFloorExtensionNorthFill` / `SouthFill` restore walkable floor past the dip +## in ±Z; ## block; gentle ramp on **+Z**; steep on **+X**), plus `PhysicsSmoothHillTest` / ## `PhysicsSmoothDipTest` (HeightMap collision). ## Steep ramp: **2 m / 1.5 m** rise/run (~53°) so slope exceeds `Player` `floor_max_angle` (~50°) @@ -208,7 +209,8 @@ func _on_authoritative_position(world: Vector3, apply_as_snap: bool) -> void: ## ## NEO-27 / NEO-31: `ability_cast_requested` is emitted as a dev [code]print[/code] from ## [method _request_hotbar_cast_slot] only after [code]request_cast[/code] on -## [code]AbilityCastClient[/code] returns [code]true[/code] (POST successfully queued on [code]HTTPRequest[/code]). +## [code]AbilityCastClient[/code] returns [code]true[/code] +## (POST successfully queued on [code]HTTPRequest[/code]). ## [code]ability_cast_denied[/code] is surfaced via [code]push_warning[/code] from ## `ability_cast_client.gd` on deny responses (TODO(E9.M1): telemetry schema + ingest). func _on_authoritative_ack_for_hud(world: Vector3) -> void: @@ -365,14 +367,18 @@ func _request_hotbar_cast_slot(slot_index: int) -> void: var target_id: Variant = outcome.get(HotbarCastSlotResolver.KEY_TARGET_ID, null) var started: bool = false if is_instance_valid(_ability_cast_client) and _ability_cast_client.has_method("request_cast"): - started = bool(_ability_cast_client.call("request_cast", resolved_slot, ability_id, target_id)) + started = bool( + _ability_cast_client.call("request_cast", resolved_slot, ability_id, target_id) + ) if started: var target_label: String = "null" if target_id != null: target_label = str(target_id) print( - "ability_cast_requested slot=%d ability_id=%s targetId=%s" - % [resolved_slot, ability_id, target_label] + ( + "ability_cast_requested slot=%d ability_id=%s targetId=%s" + % [resolved_slot, ability_id, target_label] + ) ) diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index a8efc2c..53dbdcc 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -1,6 +1,8 @@ extends GdUnitTestSuite -# main.gd sets base_url/dev_player_id on AbilityCastClient from the same authority block as HotbarLoadoutClient (NEO-31). +# main.gd sets base_url/dev_player_id on AbilityCastClient from the same authority block as +# HotbarLoadoutClient (NEO-31). + const CastClient := preload("res://scripts/ability_cast_client.gd") @@ -95,7 +97,10 @@ func test_request_cast_posts_target_id_string_when_set() -> void: var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') var c := _make_client(transport) - assert_that(bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha"))).is_true() + ( + assert_that(bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha"))) + . is_true() + ) var parsed: Variant = JSON.parse_string(transport.last_body) var body: Dictionary = parsed assert_that(body.get("targetId")).is_equal("prototype_target_alpha") From 479d1e18064d4ae9947c791da448218cf4491f7b Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 20:50:53 -0400 Subject: [PATCH 12/12] NEO-31: Bruno collection metadata, cast script UIDs, review verification notes --- bruno/neon-sprawl-server/ability-cast/folder.bru | 3 +++ bruno/neon-sprawl-server/collection.bru | 3 +++ bruno/neon-sprawl-server/core/folder.bru | 3 +++ client/scripts/ability_cast_client.gd.uid | 1 + client/test/ability_cast_client_test.gd.uid | 1 + docs/reviews/2026-04-27-NEO-31.md | 6 ++++-- 6 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 bruno/neon-sprawl-server/ability-cast/folder.bru create mode 100644 bruno/neon-sprawl-server/collection.bru create mode 100644 bruno/neon-sprawl-server/core/folder.bru create mode 100644 client/scripts/ability_cast_client.gd.uid create mode 100644 client/test/ability_cast_client_test.gd.uid diff --git a/bruno/neon-sprawl-server/ability-cast/folder.bru b/bruno/neon-sprawl-server/ability-cast/folder.bru new file mode 100644 index 0000000..59b3b47 --- /dev/null +++ b/bruno/neon-sprawl-server/ability-cast/folder.bru @@ -0,0 +1,3 @@ +meta { + name: ability-cast +} diff --git a/bruno/neon-sprawl-server/collection.bru b/bruno/neon-sprawl-server/collection.bru new file mode 100644 index 0000000..1ea72f1 --- /dev/null +++ b/bruno/neon-sprawl-server/collection.bru @@ -0,0 +1,3 @@ +meta { + name: Neon Sprawl Server +} diff --git a/bruno/neon-sprawl-server/core/folder.bru b/bruno/neon-sprawl-server/core/folder.bru new file mode 100644 index 0000000..4f3feb6 --- /dev/null +++ b/bruno/neon-sprawl-server/core/folder.bru @@ -0,0 +1,3 @@ +meta { + name: core +} diff --git a/client/scripts/ability_cast_client.gd.uid b/client/scripts/ability_cast_client.gd.uid new file mode 100644 index 0000000..a2e890e --- /dev/null +++ b/client/scripts/ability_cast_client.gd.uid @@ -0,0 +1 @@ +uid://bppqfpl3vskik diff --git a/client/test/ability_cast_client_test.gd.uid b/client/test/ability_cast_client_test.gd.uid new file mode 100644 index 0000000..cfd9163 --- /dev/null +++ b/client/test/ability_cast_client_test.gd.uid @@ -0,0 +1 @@ +uid://cpalxhytjdxpg diff --git a/docs/reviews/2026-04-27-NEO-31.md b/docs/reviews/2026-04-27-NEO-31.md index 83460a1..be488ed 100644 --- a/docs/reviews/2026-04-27-NEO-31.md +++ b/docs/reviews/2026-04-27-NEO-31.md @@ -41,5 +41,7 @@ NEO-31 adds the prototype `POST /game/players/{id}/ability-cast` route, client d - `dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj` — passed: 86 tests. - `git diff --check origin/main...HEAD && git diff --check` — passed. -- Cursor lints checked for the touched C# and GDScript files — no linter errors reported. -- Not run: GdUnit client suite or Bruno collection. Run `godot --headless --path client -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/ability_cast_client_test.gd` and the NEO-31 Bruno/manual checklist before merge. +- `godot --headless --path client -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/ability_cast_client_test.gd` — passed: 4 tests. +- `godot --headless --path client -s res://addons/gdUnit4/bin/GdUnitCmdTool.gd --ignoreHeadlessMode -a res://test/hotbar_cast_slot_resolver_test.gd` — passed: 6 tests. +- Cursor lints checked for the touched C#, GDScript, and docs files — no linter errors reported. +- Not run: Bruno collection / manual checklist `docs/manual-qa/NEO-31.md`.