NEO-31: hotbar cast resolver tests, doc alignment, review follow-up
parent
eac95f0c27
commit
e88e59f3a6
|
|
@ -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,
|
||||
}
|
||||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
@ -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) |
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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`). |
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue