neon-sprawl/docs/plans/NEO-31-implementation-plan.md

96 lines
9.0 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters!

This file contains ambiguous Unicode characters that may be confused with others in your current locale. If your use case is intentional and legitimate, you can safely ignore this warning. Use the Escape button to highlight these characters.

# 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
- [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
1. **Contract (v1 JSON)** — Add a small versioned request DTO aligned with hotbar/target naming: `schemaVersion`, `slotIndex` (07), `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(...) -> 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.
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).
## 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/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). |
| `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
| 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 → `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. |
## 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 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`). |
## 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).