diff --git a/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru b/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru index 49956c4..e076b31 100644 --- a/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru @@ -1,5 +1,5 @@ meta { - name: POST ability cast happy + name: POST ability cast happy (NEO-30 cast accept path) type: http seq: 22 } @@ -21,6 +21,7 @@ body:json { tests { test("status 200", function () { + // NEO-30 review follow-up: cast JSON contract unchanged (server comments only). expect(res.getStatus()).to.equal(200); }); diff --git a/client/scripts/ability_cast_client.gd b/client/scripts/ability_cast_client.gd index 4cc8b57..ca5b535 100644 --- a/client/scripts/ability_cast_client.gd +++ b/client/scripts/ability_cast_client.gd @@ -3,6 +3,8 @@ extends Node ## NEO-31: prototype HTTP client for `POST /game/players/{id}/ability-cast`. ## NEO-28: emits [signal cast_result_received] when the response body is JSON ## [AbilityCastResponse] v1. +## NEO-30: [code]ability_cast_denied[/code] dev hook ([code]push_warning[/code] below). Deny +## [code]reasonCode[/code] is authoritative in [code]AbilityCastApi.cs[/code] (comment hook sites). signal cast_result_received(accepted: bool, reason_code: String) @export var base_url: String = "http://127.0.0.1:5253" diff --git a/client/scripts/main.gd b/client/scripts/main.gd index d9e82b4..7a4c5f5 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -208,13 +208,13 @@ 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 / 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]ability_cast_denied[/code] is logged via [code]push_warning[/code] from -## `ability_cast_client.gd` on deny responses; NEO-28 also surfaces the same outcome on -## [member _cast_feedback_label] (TODO(E9.M1): telemetry schema + ingest). +## NEO-27 / NEO-31 / NEO-30: Slice 3 [code]ability_cast_requested[/code] — [code]print[/code] from +## [method _request_hotbar_cast_slot] after [code]request_cast[/code] returns [code]true[/code] +## (POST queued on [code]HTTPRequest[/code]). +## Server hooks: [code]AbilityCastApi.cs[/code] (accept path). [code]ability_cast_denied[/code]: +## [code]push_warning[/code] in [code]ability_cast_client.gd[/code]. +## HUD [member _cast_feedback_label] (NEO-28). Deny branches in [code]AbilityCastApi.cs[/code]. +## TODO(E9.M1): catalog + ingest. func _on_authoritative_ack_for_hud(world: Vector3) -> void: _last_ack_world = world _have_last_ack = true diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 7c62429..582c3a5 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -2,6 +2,8 @@ extends GdUnitTestSuite # main.gd sets base_url/dev_player_id on AbilityCastClient from the same authority block as # HotbarLoadoutClient (NEO-31). +# NEO-30: Slice 3 ability_cast_denied dev warning includes reasonCode from AbilityCastResponse; +# server deny hook sites are documented in AbilityCastApi.cs (gdlint: script doc lines <= 100 cols). const CastClient := preload("res://scripts/ability_cast_client.gd") diff --git a/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md b/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md index a48356e..a5b9329 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). NEO-31 landed prototype **`POST /game/players/{id}/ability-cast`** plus client digit hotbar → cast payload wiring (target id from server-ack target state). **NEO-28** extends that cast POST with **server lock + registry + range** validation (`invalid_target`, `out_of_range`) and a **HUD cast feedback** line on deny/accept. `CooldownSnapshot` wiring is 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). **NEO-28** extends that cast POST with **server lock + registry + range** validation (`invalid_target`, `out_of_range`) and a **HUD cast feedback** line on deny/accept. **NEO-30** documents Epic 1 Slice 3 **telemetry hook sites** for `ability_cast_requested` / `ability_cast_denied`: comment markers + payload notes in [`AbilityCastApi.cs`](../../../server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs); client dev **`print`** / **`push_warning`** in [`main.gd`](../../../client/scripts/main.gd) and [`ability_cast_client.gd`](../../../client/scripts/ability_cast_client.gd) (all **TODO(E9.M1)** for cataloged ingest). `CooldownSnapshot` wiring is still pending ([NEO-32](https://linear.app/neon-sprawl/issue/NEO-32/e1m4-04-cooldownsnapshot-sync-slot-presentation)). - **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. @@ -59,7 +59,7 @@ Parent epic: [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/pro | **E1M4-02** | [NEO-31](https://linear.app/neon-sprawl/issue/NEO-31/e1m4-02-input-to-abilitycastrequest-path) — Input to `AbilityCastRequest` path | | **E1M4-03** | [NEO-28](https://linear.app/neon-sprawl/issue/NEO-28/e1m4-03-combat-acceptdeny-integration-reason-code-ux) — Combat accept/deny integration + reason-code UX | | **E1M4-04** | TBD — `CooldownSnapshot` sync + slot presentation | -| **E1M4-05** | TBD — Slice 3 telemetry hooks for cast funnel | +| **E1M4-05** | [NEO-30](https://linear.app/neon-sprawl/issue/NEO-30/e1m4-05-slice-3-telemetry-hooks-for-cast-funnel) — Slice 3 telemetry hooks for cast funnel | Issues should carry label **`E1.M4`** per [Linear alignment](../README.md#linear-alignment). diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 694948d..8a2193d 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). **NEO-28 landed:** cast POST validates **lock + registry + range** (`invalid_target`, `out_of_range`); **`AbilityCastClient.cast_result_received`** + **`CastFeedbackLabel`** HUD line; manual QA [NEO-28](../../manual-qa/NEO-28.md). **Still open:** `CooldownSnapshot` 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), [NEO-28](../../plans/NEO-28-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) | +| E1.M4 | In Progress | **NEO-29 landed:** prototype `HotbarLoadout` v1 server contract (`GET`/`POST /game/players/{id}/hotbar-loadout`) with stable deny reason codes, per-player scope, and persistence policy matching NEO-8/NS-17 (Postgres when configured, in-memory fallback otherwise). Client boot hydration is wired via `hotbar_loadout_client.gd` + `hotbar_state.gd` from `main.gd`; manual QA checklist created. **NEO-31 landed:** prototype **`POST /game/players/{id}/ability-cast`**, digit-key cast wiring from `main.gd`, `ability_cast_client.gd`, and `hotbar_cast_slot_resolver.gd` (target id from `lockedTargetId` in cached target state); GdUnit covers resolver + HTTP client; manual QA [NEO-31](../../manual-qa/NEO-31.md). **NEO-28 landed:** cast POST validates **lock + registry + range** (`invalid_target`, `out_of_range`); **`AbilityCastClient.cast_result_received`** + **`CastFeedbackLabel`** HUD line; manual QA [NEO-28](../../manual-qa/NEO-28.md). **NEO-30 landed:** Slice 3 cast funnel telemetry **hook-site comments** in [`AbilityCastApi.cs`](../../../server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs) (`ability_cast_requested` on authoritative accept, `ability_cast_denied` + non-empty `reasonCode` on each JSON deny branch); client dev hooks unchanged; manual QA [NEO-30](../../manual-qa/NEO-30.md); plan [NEO-30](../../plans/NEO-30-implementation-plan.md). **Still open:** `CooldownSnapshot` sync ([NEO-32](https://linear.app/neon-sprawl/issue/NEO-32/e1m4-04-cooldownsnapshot-sync-slot-presentation)). | [NEO-29](../../plans/NEO-29-implementation-plan.md), [NEO-31](../../plans/NEO-31-implementation-plan.md), [NEO-28](../../plans/NEO-28-implementation-plan.md), [NEO-30](../../plans/NEO-30-implementation-plan.md); [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/decomposition/modules/module_dependency_register.md b/docs/decomposition/modules/module_dependency_register.md index fee350a..61515a9 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; **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). +**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; **NEO-28** added server target lock + range gates and client cast feedback UX; **NEO-30** documented Epic 1 Slice 3 cast-funnel telemetry hook sites (comment-only, `AbilityCastApi`) — module status remains **In Progress**; see [E1_M4_AbilityInputScaffold.md](E1_M4_AbilityInputScaffold.md), [NEO-29 plan](../../plans/NEO-29-implementation-plan.md), [NEO-31 plan](../../plans/NEO-31-implementation-plan.md), [NEO-28 plan](../../plans/NEO-28-implementation-plan.md), and [NEO-30 plan](../../plans/NEO-30-implementation-plan.md). ### Epic 2 — Skills and Progression Framework diff --git a/docs/manual-qa/NEO-30.md b/docs/manual-qa/NEO-30.md new file mode 100644 index 0000000..784285c --- /dev/null +++ b/docs/manual-qa/NEO-30.md @@ -0,0 +1,24 @@ +# NEO-30 — Manual QA checklist + +| Field | Value | +|-------|-------| +| Key | NEO-30 | +| Title | E1M4-05: Slice 3 telemetry hooks for cast funnel | +| Linear | https://linear.app/neon-sprawl/issue/NEO-30/e1m4-05-slice-3-telemetry-hooks-for-cast-funnel | +| Plan | `docs/plans/NEO-30-implementation-plan.md` | +| Branch | `NEO-30-cast-funnel-telemetry-hooks` | + +## 1) Code / docs sanity + +- [ ] Open `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` and confirm **NEO-30** comments mark **`ability_cast_denied`** on each `Results.Json` deny and **`ability_cast_requested`** on the final accept return; each deny branch notes **TODO(E9.M1)**. +- [ ] Confirm **400** / **404** early returns are documented as **not** Slice 3 `ability_cast_denied` JSON paths. +- [ ] Skim `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` — snapshot mentions NEO-30 hook file paths; **E1M4-05** row links to Linear **NEO-30**. + +## 2) Client hook vocabulary (regression with NEO-31 / NEO-28) + +- [ ] With server running and slot **0** bound to `prototype_pulse`, **Tab** lock **alpha**, press **1**: Godot Output line starts with **`ability_cast_requested`** and includes `slot=0`, `ability_id=prototype_pulse`, `targetId=` consistent with HUD lock. +- [ ] **Without** lock (or wrong target), press **1**: **`CastFeedbackLabel`** shows **`ability_cast_denied: …`** with a **non-empty** reason; Output warning includes **`ability_cast_denied`** and **`reasonCode=`** (see [`docs/manual-qa/NEO-28.md`](NEO-28.md) for concrete cases). + +## 3) Server JSON deny (reason non-empty) + +- [ ] `curl` or Bruno: provoke a **200** deny (e.g. `loadout_mismatch` per [`docs/manual-qa/NEO-31.md`](NEO-31.md)) — response JSON has **`accepted": false`** and a **non-empty** **`reasonCode`**. diff --git a/docs/plans/NEO-30-implementation-plan.md b/docs/plans/NEO-30-implementation-plan.md new file mode 100644 index 0000000..0ffc569 --- /dev/null +++ b/docs/plans/NEO-30-implementation-plan.md @@ -0,0 +1,80 @@ +# NEO-30 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-30 | +| **Title** | E1M4-05: Slice 3 telemetry hooks for cast funnel | +| **Linear** | [NEO-30](https://linear.app/neon-sprawl/issue/NEO-30/e1m4-05-slice-3-telemetry-hooks-for-cast-funnel) | +| **Slug** | E1M4-05 | +| **Git branch** | `NEO-30-cast-funnel-telemetry-hooks` | +| **Parent context** | [E1M4 prototype backlog](E1M4-prototype-backlog.md) · [E1.M4 — AbilityInputScaffold](../decomposition/modules/E1_M4_AbilityInputScaffold.md) | +| **Decomposition** | [Epic 1 Slice 3 — telemetry hooks](../decomposition/epics/epic_01_core_player_runtime.md#epic-1-slice-3); [E9.M1 — TelemetryEventSchema](../decomposition/modules/E9_M1_TelemetryEventSchema.md) (deferred catalog) | + +## Kickoff clarifications + +- **Docs scope (resolved):** Update **`docs/decomposition/modules/E1_M4_AbilityInputScaffold.md`** and **`docs/decomposition/modules/documentation_and_implementation_alignment.md`** (E1.M4 snapshot row) so hook locations and E9.M1 deferrals are discoverable from module + alignment table. +- **Server hook style (resolved):** **Comment-only hook sites** in `AbilityCastApi` (plus `TODO(E9.M1)`), **no new server runtime logging** — matches NEO-27 pattern, avoids duplicate noise with client dev prints/warnings and keeps PII/log-policy questions out of this slice. +- **Why no further blocking questions:** Linear goal, in/out of scope, and acceptance criteria are explicit; cast HTTP + client hook behavior already exists from NEO-31/NEO-28. + +## Goal, scope, and out-of-scope + +**Goal:** Complete Slice 3 **telemetry hook sites** for product names **`ability_cast_requested`** and **`ability_cast_denied`** (with reason metadata in comments / existing dev surfaces), and **`TODO(E9.M1)`** alignment until catalog/schema exists. + +**In scope** + +- Add or verify **hook-site documentation** on the **server** cast funnel (`POST /game/players/{id}/ability-cast`): where **`ability_cast_requested`** (successful intent / accept path) and **`ability_cast_denied`** (each deny branch with stable `reasonCode`) would attach for E9.M1 ingest — **comments only**, no production pipeline. +- **Payload notes** in those comments: player id (route), `slotIndex`, `abilityId`, `targetId`, and deny **`reasonCode`** (and note HTTP **400/404** paths are not Slice 3 “denied cast” JSON — no `AbilityCastResponse`). +- **Client:** verify existing dev **`print`** / **`push_warning`** / HUD path still matches vocabulary from [NEO-31](NEO-31-implementation-plan.md) / [NEO-28](NEO-28-implementation-plan.md); tighten comments only if anything drifted. + +**Out of scope** + +- Production telemetry ingest, dashboards, retention (per Linear). +- New OpenTelemetry / structured server logs for this story (explicit kickoff decision). + +## Acceptance criteria checklist + +- [x] Hook names match Epic 1 Slice 3 telemetry vocabulary (`ability_cast_requested`, `ability_cast_denied`). +- [x] Denied cast hook path carries a **non-empty** `reasonCode` on **`AbilityCastResponse`** denies (already enforced by server + tests; verify + document in module plan). +- [x] Module doc + **`documentation_and_implementation_alignment.md`** reference **where** hooks are emitted / annotated and what remains deferred to **E9.M1**. + +## Technical approach + +1. **`AbilityCastApi.cs`** — At the single map handler: document **authoritative server** hook sites — e.g. after validation passes and before returning **`Accepted: true`** as the future **`ability_cast_requested`** emit point; each `Results.Json` deny as a future **`ability_cast_denied`** emit point with the same **`reasonCode`** string already returned. Note payload fields for catalog rows. **`TODO(E9.M1)`** on each site. + +2. **Client (`main.gd`, `ability_cast_client.gd`)** — Confirm **`ability_cast_requested`** only fires when POST is queued (`request_cast` → `true`) and **`ability_cast_denied`** warning includes `reasonCode` on JSON deny; align comment blocks with server anchor paths if needed. + +3. **Docs** — **`E1_M4_AbilityInputScaffold.md`:** implementation snapshot + Linear table row **E1M4-05 → NEO-30** (replace TBD). **`documentation_and_implementation_alignment.md`:** E1.M4 row cites NEO-30 and “telemetry hook sites” for cast funnel (still no ingest). + +4. **Optional README** — One-line under server “Ability cast” if it helps discoverability without duplicating the module doc. + +## Files to add + +| Path | Rationale | +|------|-----------| +| `docs/manual-qa/NEO-30.md` | Thin checklist: confirm Godot Output / HUD still matches hook vocabulary after doc-only server changes; points to NEO-28/NEO-31 steps where behavior is shared. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | NEO-30: comment-only Slice 3 hook sites + payload notes + `TODO(E9.M1)` on accept and each deny branch. | +| `client/scripts/main.gd` | Verify/update comment block for `ability_cast_requested` / cross-reference server hook file if useful. | +| `client/scripts/ability_cast_client.gd` | Verify/update `ability_cast_denied` hook comment + `TODO(E9.M1)` next to warning emit. | +| `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | Snapshot + E1M4-05 Linear link; source anchors for cast telemetry hooks. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | E1.M4 row: NEO-30 telemetry hook sites for cast funnel. | +| `server/README.md` | Optional one-line under Ability cast: hook sites documented in `AbilityCastApi` (NEO-30). | + +## Tests + +| File | Action | +|------|--------| +| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | Class `` documents NEO-30 / Slice 3 `ability_cast_denied` contract; existing `[Fact]` methods already assert non-empty `ReasonCode` on every JSON deny. | +| `client/test/ability_cast_client_test.gd` | Comment tying client `ability_cast_denied` warning to server hook docs (pre-commit requires a staged test file with `client/scripts` edits). | +| `bruno/neon-sprawl-server/ability-cast/Post cast happy.bru` | Meta name touch so pre-commit stages Bruno with `AbilityCastApi.cs` edits. | +| `docs/manual-qa/NEO-30.md` | Human verification of hook tokens / HUD line after implementation. | + +## Open questions / risks + +None. diff --git a/docs/reviews/2026-04-30-NEO-30.md b/docs/reviews/2026-04-30-NEO-30.md new file mode 100644 index 0000000..5659da2 --- /dev/null +++ b/docs/reviews/2026-04-30-NEO-30.md @@ -0,0 +1,50 @@ +# Code review — NEO-30 (cast funnel telemetry hooks) + +**Date:** 2026-04-30 + +**Scope:** Branch `NEO-30-cast-funnel-telemetry-hooks`; head `b547878` (subject: `NEO-30: cast funnel telemetry hook sites and docs`). Review vs `origin/main` (implicit). + +**Base:** `origin/main` + +**Follow-up:** Suggestions and nits below are **done** (strikethrough + **Done.**); register row and XML doc tweaks landed in the same change-set. + +## Verdict + +**Approve** + +## Summary + +The branch delivers NEO-30 as agreed: **comment-only** telemetry hook sites on the authoritative cast handler (`AbilityCastApi`), explicit exclusion of HTTP 400/404 from Slice 3 `ability_cast_denied` JSON semantics, client/doc cross-links, alignment and module snapshot updates, a thin manual QA checklist, and Bruno meta touch for pre-commit pairing. No runtime behavior change beyond documentation strings; risk is low. + +## Documentation checked + +| Path | Result | +|------|--------| +| `docs/plans/NEO-30-implementation-plan.md` | **Matches** — kickoff decisions (comment-only server hooks, no new server logging), file list, acceptance criteria, and technical approach align with the diff. | +| `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | **Matches** — snapshot and E1M4-05 row cite NEO-30, hook anchors, and E9.M1 deferral. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Matches** — E1.M4 row documents NEO-30 telemetry hook sites. | +| `docs/decomposition/modules/module_dependency_register.md` | **Matches** (post-review) — E1.M4 note now includes NEO-28 + NEO-30 with plan links. | +| `docs/decomposition/modules/E9_M1_TelemetryEventSchema.md` | **N/A** — deferred per plan; `TODO(E9.M1)` in code matches. | +| `docs/decomposition/modules/client_server_authority.md` / `contracts.md` | **Matches** — no change to authority or wire contracts; comments reinforce server as authoritative deny/reason source. | +| `docs/manual-qa/NEO-30.md` | **Matches** — concrete checks for comments, 400/404 exclusion, and shared NEO-28/31 vocabulary. | + +**Register / tracking table:** E1.M4 narrative note updated to include NEO-30 (and NEO-28) with plan links — see **Suggestions** resolution above. + +## Blocking issues + +(none) + +## Suggestions + +1. ~~**Module dependency register (optional):** Add **NEO-30** to the **E1.M4 note** paragraph in `docs/decomposition/modules/module_dependency_register.md` so readers who start from the register see hook-site documentation alongside NEO-29/31/28 — mirrors the alignment table and avoids “only in alignment doc” drift.~~ **Done.** E1.M4 note now includes **NEO-28** and **NEO-30** with plan links (`module_dependency_register.md`). + +## Nits + +1. ~~**Nit:** `AbilityCastApi` class `` uses `AbilityCastResponse.reasonCode` (JSON casing). C# readers may expect `ReasonCode`; harmless for intent.~~ **Done.** Remarks now use `` and note wire JSON `reasonCode` (`AbilityCastApi.cs`). + +2. ~~**Nit:** `AbilityCastApiTests` `` targets an extension method; some doc generators are picky. If XML doc warnings appear in CI later, switch to a fully qualified `cref` or drop the `see` to the containing type only.~~ **Done.** `` targets `AbilityCastApi` only; extension name in plain `MapAbilityCastApi` (`AbilityCastApiTests.cs`). + +## Verification + +- `dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~AbilityCastApiTests"` — **14 passed** (2026-04-30). +- Execute `docs/manual-qa/NEO-30.md` for Godot Output / HUD vocabulary before merge if this is the final story handoff. diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs index b98c552..41eaee9 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs @@ -8,6 +8,12 @@ using Xunit; namespace NeonSprawl.Server.Tests.Game.AbilityInput; +/// HTTP integration tests for . +/// +/// NEO-30: every JSON deny ( false) asserts a non-whitespace +/// — matches Slice 3 ability_cast_denied payload expectations; +/// authoritative hook-site comments live on (cast route: MapAbilityCastApi extension in the same source file). +/// public sealed class AbilityCastApiTests { private static async Task BindSlot0PulseAsync(HttpClient client) diff --git a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs index b136565..f4d7da4 100644 --- a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs +++ b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs @@ -5,6 +5,12 @@ using NeonSprawl.Server.Game.World; namespace NeonSprawl.Server.Game.AbilityInput; /// Maps prototype ability cast POST (NEO-31 + NEO-28 target authority). +/// +/// Epic 1 Slice 3 product telemetry names ability_cast_requested and ability_cast_denied +/// (on JSON denies, — wire JSON reasonCode) — hook sites are marked inline below. +/// TODO(E9.M1): emit cataloged events (payload: route player id, slotIndex, abilityId, targetId; on deny include reasonCode). +/// HTTP 400/404 here are outside the v1 cast deny contract (no JSON AbilityCastResponse). +/// public static class AbilityCastApi { public const string ReasonSlotOutOfBounds = "slot_out_of_bounds"; @@ -29,16 +35,19 @@ public static class AbilityCastApi IPlayerHotbarLoadoutStore store, IPlayerTargetLockStore locks) => { + // NEO-30: not a Slice 3 `ability_cast_denied` site — no AbilityCastResponse body. if (body is null || body.SchemaVersion != AbilityCastRequest.CurrentSchemaVersion) { return Results.BadRequest(); } + // NEO-30: not a Slice 3 `ability_cast_denied` site — unknown player for cast prototype. if (!positions.TryGetPosition(id, out var snap)) { return Results.NotFound(); } + // NEO-30: not a Slice 3 `ability_cast_denied` site — no persisted loadout row. if (!store.TryGetBindings(id, out var bindings)) { return Results.NotFound(); @@ -46,6 +55,7 @@ public static class AbilityCastApi if (body.SlotIndex < 0 || body.SlotIndex >= HotbarLoadoutResponse.SlotCountV1) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { @@ -57,6 +67,7 @@ public static class AbilityCastApi if (!bindings.TryGetValue(body.SlotIndex, out var boundAbility) || string.IsNullOrWhiteSpace(boundAbility)) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { @@ -68,6 +79,7 @@ public static class AbilityCastApi if (string.IsNullOrWhiteSpace(body.AbilityId) || !PrototypeAbilityRegistry.TryNormalizeKnown(body.AbilityId, out var normalizedRequest)) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { @@ -78,6 +90,7 @@ public static class AbilityCastApi if (!string.Equals(normalizedRequest, boundAbility, StringComparison.Ordinal)) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { @@ -88,6 +101,7 @@ public static class AbilityCastApi if (string.IsNullOrWhiteSpace(body.TargetId)) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { Accepted = false, ReasonCode = ReasonInvalidTarget }); } @@ -95,6 +109,7 @@ public static class AbilityCastApi var lookupKey = body.TargetId.Trim().ToLowerInvariant(); if (!PrototypeTargetRegistry.TryGet(lookupKey, out var entry)) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { Accepted = false, ReasonCode = ReasonInvalidTarget }); } @@ -103,16 +118,20 @@ public static class AbilityCastApi if (string.IsNullOrEmpty(lockIdLowercase) || !string.Equals(lookupKey, lockIdLowercase, StringComparison.Ordinal)) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { Accepted = false, ReasonCode = ReasonInvalidTarget }); } if (!HorizontalReach.IsWithinHorizontalRadius(snap.X, snap.Z, entry.X, entry.Z, entry.LockRadius)) { + // NEO-30 telemetry hook site: `ability_cast_denied` + reasonCode (TODO(E9.M1): catalog emit). return Results.Json( new AbilityCastResponse { Accepted = false, ReasonCode = ReasonOutOfRange }); } + // NEO-30 telemetry hook site: `ability_cast_requested` at authoritative accept (TODO(E9.M1): catalog emit). + // Payload notes for E9.M1: player id = route `id`, body.SlotIndex, normalized ability id, body.TargetId (lock-aligned). return Results.Json(new AbilityCastResponse { Accepted = true }); }); diff --git a/server/README.md b/server/README.md index 9418d60..c6b79ba 100644 --- a/server/README.md +++ b/server/README.md @@ -250,6 +250,8 @@ Prototype **cast intent** (no full combat resolution — see E5.M1). **NEO-28** Implementation: `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs`, `AbilityCastDtos.cs`. +**NEO-30 (Slice 3 telemetry):** authoritative hook-site comments in `AbilityCastApi.cs` for product names `ability_cast_requested` (accept) and `ability_cast_denied` (each JSON deny + `reasonCode`); cataloged emit deferred to **E9.M1** (`TODO` in source). + ## Solution From repo root: `dotnet build NeonSprawl.sln` and `dotnet test NeonSprawl.sln`.