From 7de8e7ecfd5a069ccaae140a181774e33bc40b0b Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 21:02:10 -0400 Subject: [PATCH 1/8] NEO-28: cast target authority, HUD feedback, tests + docs --- .../ability-cast/Post cast bad schema.bru | 2 +- .../ability-cast/Post cast happy.bru | 4 +- .../Post cast loadout mismatch.bru | 2 +- .../ability-cast/Post cast missing player.bru | 2 +- .../Post target select alpha before cast.bru | 27 +++ client/README.md | 10 +- client/scenes/main.tscn | 14 ++ client/scripts/ability_cast_client.gd | 10 +- client/scripts/main.gd | 23 ++- client/test/ability_cast_client_test.gd | 32 ++++ .../modules/E1_M4_AbilityInputScaffold.md | 4 +- .../modules/E5_M1_CombatRulesEngine.md | 4 + ...umentation_and_implementation_alignment.md | 2 +- docs/manual-qa/NEO-28.md | 24 +++ docs/manual-qa/NEO-31.md | 4 +- docs/plans/NEO-28-implementation-plan.md | 91 +++++++++ .../Game/AbilityInput/AbilityCastApiTests.cs | 175 +++++++++++++++++- .../Game/AbilityInput/AbilityCastApi.cs | 47 ++++- .../Game/AbilityInput/AbilityCastDtos.cs | 7 +- server/README.md | 6 +- 20 files changed, 460 insertions(+), 30 deletions(-) create mode 100644 bruno/neon-sprawl-server/ability-cast/Post target select alpha before cast.bru create mode 100644 docs/manual-qa/NEO-28.md create mode 100644 docs/plans/NEO-28-implementation-plan.md diff --git a/bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru b/bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru index cc55b1b..af7ef4c 100644 --- a/bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast bad schema.bru @@ -1,7 +1,7 @@ meta { name: POST ability cast bad schema type: http - seq: 22 + seq: 23 } post { 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 39ad219..49956c4 100644 --- a/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast happy.bru @@ -1,7 +1,7 @@ meta { name: POST ability cast happy type: http - seq: 21 + seq: 22 } post { @@ -15,7 +15,7 @@ body:json { "schemaVersion": 1, "slotIndex": 0, "abilityId": "prototype_pulse", - "targetId": null + "targetId": "prototype_target_alpha" } } diff --git a/bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru b/bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru index 81e0341..6537e0a 100644 --- a/bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru @@ -1,7 +1,7 @@ meta { name: POST ability cast loadout mismatch type: http - seq: 24 + seq: 25 } post { diff --git a/bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru b/bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru index a765d2a..34741f9 100644 --- a/bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru +++ b/bruno/neon-sprawl-server/ability-cast/Post cast missing player.bru @@ -1,7 +1,7 @@ meta { name: POST ability cast missing player type: http - seq: 23 + seq: 24 } post { diff --git a/bruno/neon-sprawl-server/ability-cast/Post target select alpha before cast.bru b/bruno/neon-sprawl-server/ability-cast/Post target select alpha before cast.bru new file mode 100644 index 0000000..d906269 --- /dev/null +++ b/bruno/neon-sprawl-server/ability-cast/Post target select alpha before cast.bru @@ -0,0 +1,27 @@ +meta { + name: POST target select alpha (cast preset) + type: http + seq: 21 +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/target/select + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "targetId": "prototype_target_alpha" + } +} + +tests { + test("status 200 lock applied", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.selectionApplied).to.equal(true); + expect(body.targetState.lockedTargetId).to.equal("prototype_target_alpha"); + }); +} diff --git a/client/README.md b/client/README.md index b366707..ab3af26 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` 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). +- **Ability cast hooks (NEO-27 + NEO-31 + NEO-28):** 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, and **NEO-28** mirrors the same outcome on the **`CastFeedbackLabel`** HUD line (`ability_cast_denied: ` or `Cast: accepted`) via **`AbilityCastClient.cast_result_received`** (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,12 +127,12 @@ 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) +## Ability cast request (NEO-31 + NEO-28) -- **`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. +- **`POST /game/players/{id}/ability-cast`** — JSON body `schemaVersion`, `slotIndex` (0–7), `abilityId`, **`targetId`** (must match server lock + prototype registry + range for `accepted: true`; see `server/README.md` and `AbilityCastApi.cs`). +- **Client:** `main.gd` reads `HotbarState.slots_snapshot()` and `TargetSelectionClient.cached_state()` (`lockedTargetId`) so cast payloads use **server-acknowledged** target context, not camera guessing. **Tab-lock a target** before expecting a successful cast at the default spawn. -Full checklist: [`docs/manual-qa/NEO-31.md`](../docs/manual-qa/NEO-31.md). +Full checklist: [`docs/manual-qa/NEO-31.md`](../docs/manual-qa/NEO-31.md) · cast deny/accept HUD: [`docs/manual-qa/NEO-28.md`](../docs/manual-qa/NEO-28.md). ### Manual check (NEO-24) diff --git a/client/scenes/main.tscn b/client/scenes/main.tscn index 8564075..ee4f6ad 100644 --- a/client/scenes/main.tscn +++ b/client/scenes/main.tscn @@ -1133,3 +1133,17 @@ theme_override_font_sizes/font_size = 15 text = "Target: — Validity: — Seq: —" + +[node name="CastFeedbackLabel" type="Label" parent="UICanvas" unique_id=9000005] +offset_left = 8.0 +offset_top = 318.0 +offset_right = 520.0 +offset_bottom = 372.0 +grow_horizontal = 0 +grow_vertical = 0 +theme_override_colors/font_color = Color(0.95, 0.88, 0.72, 1) +theme_override_colors/font_outline_color = Color(0.08, 0.08, 0.1, 1) +theme_override_constants/outline_size = 6 +theme_override_font_sizes/font_size = 15 +autowrap_mode = 3 +text = "Cast: —" diff --git a/client/scripts/ability_cast_client.gd b/client/scripts/ability_cast_client.gd index de21fba..6a6bc53 100644 --- a/client/scripts/ability_cast_client.gd +++ b/client/scripts/ability_cast_client.gd @@ -1,6 +1,9 @@ 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. +signal cast_result_received(accepted: bool, reason_code: String) + @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 @@ -76,8 +79,9 @@ func _on_request_completed( return var data: Dictionary = parsed var accepted: bool = bool(data.get("accepted", false)) + var reason_variant: Variant = data.get("reasonCode", "") + var reason: String = reason_variant as String if reason_variant is String else "" + cast_result_received.emit(accepted, reason) 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)). + # NEO-27 / NEO-31 / NEO-28: 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 3ee0362..d9e82b4 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -81,6 +81,7 @@ var _ability_cast_client: Node = null @onready var _move_reject_label: Label = $UICanvas/MoveRejectLabel @onready var _player_pos_label: Label = $UICanvas/PlayerPositionLabel @onready var _target_lock_label: Label = $UICanvas/TargetLockLabel +@onready var _cast_feedback_label: Label = $UICanvas/CastFeedbackLabel @onready var _target_client: Node = $TargetSelectionClient @onready var _interaction_client: Node = $InteractionRequestClient @onready var _floor: StaticBody3D = $World/NavigationRegion3D/Floor @@ -211,8 +212,9 @@ func _on_authoritative_position(world: Vector3, apply_as_snap: bool) -> void: ## [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). +## [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). func _on_authoritative_ack_for_hud(world: Vector3) -> void: _last_ack_world = world _have_last_ack = true @@ -249,6 +251,10 @@ func _setup_hotbar_loadout_sync() -> void: 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 _ability_cast_client.has_signal("cast_result_received"): + _ability_cast_client.connect( + "cast_result_received", Callable(self, "_on_cast_result_received") + ) 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"): @@ -288,6 +294,19 @@ func _render_target_lock_label(world: Vector3) -> void: _target_lock_label.text = "\n".join(lines) +func _on_cast_result_received(accepted: bool, reason_code: String) -> void: + if not is_instance_valid(_cast_feedback_label): + return + if accepted: + _cast_feedback_label.text = "Cast: accepted" + return + var rc := reason_code.strip_edges() + if rc.is_empty(): + _cast_feedback_label.text = "Cast: denied (no reasonCode)" + else: + _cast_feedback_label.text = "ability_cast_denied: %s" % rc + + func _on_move_rejected(reason_code: String) -> void: _authority_force_snap_next = true # Rejected stream: server state may differ; next snap is forced. diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 53dbdcc..1e6bb85 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -127,3 +127,35 @@ func test_request_cast_while_busy_is_ignored() -> void: var parsed: Variant = JSON.parse_string(transport.last_body) var body: Dictionary = parsed assert_that(int(body.get("slotIndex", -1))).is_equal(0) + + +func test_cast_result_received_emits_false_with_reason_on_denied() -> void: + var transport := MockHttpTransport.new() + transport.enqueue( + HTTPRequest.RESULT_SUCCESS, + 200, + '{"schemaVersion":1,"accepted":false,"reasonCode":"invalid_target"}' + ) + var c := _make_client(transport) + var got: Array = [] + c.connect("cast_result_received", func(accepted: bool, reason: String): got.append([accepted, reason])) + # Act + assert_that(bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha"))).is_true() + # Assert + assert_that(got.size()).is_equal(1) + assert_that(bool(got[0][0])).is_false() + assert_that(str(got[0][1])).is_equal("invalid_target") + + +func test_cast_result_received_emits_true_with_empty_reason_on_accepted() -> void: + var transport := MockHttpTransport.new() + transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') + var c := _make_client(transport) + var got: Array = [] + c.connect("cast_result_received", func(accepted: bool, reason: String): got.append([accepted, reason])) + # Act + assert_that(bool(c.call("request_cast", 0, "prototype_pulse", null))).is_true() + # Assert + assert_that(got.size()).is_equal(1) + assert_that(bool(got[0][0])).is_true() + assert_that(str(got[0][1])).is_equal("") diff --git a/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md b/docs/decomposition/modules/E1_M4_AbilityInputScaffold.md index aa9ba8a..a48356e 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). `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. `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. @@ -57,7 +57,7 @@ Parent epic: [Epic 1 — Core Player Runtime](https://linear.app/neon-sprawl/pro |------|--------| | **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** | [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-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 | diff --git a/docs/decomposition/modules/E5_M1_CombatRulesEngine.md b/docs/decomposition/modules/E5_M1_CombatRulesEngine.md index 722e3d4..f24c03f 100644 --- a/docs/decomposition/modules/E5_M1_CombatRulesEngine.md +++ b/docs/decomposition/modules/E5_M1_CombatRulesEngine.md @@ -49,6 +49,10 @@ Core tab-target combat resolution: valid actions against a locked target, hit re See Epic 5 **Slice 1 — Combat rules MVP**: target lock, basic attacks, 4–6 abilities, cooldowns/resources; telemetry `encounter_start`, `ability_used`, `player_death`, `enemy_defeat`. +## Prototype notes (NEO-28) + +Epic 1 **Slice 3** exercises a minimal cast accept/deny surface on **`POST /game/players/{id}/ability-cast`** before the full engine exists: after hotbar/loadout validation ([E1.M4](E1_M4_AbilityInputScaffold.md)), the server requires **`targetId`** to match the persisted combat lock, resolve in the prototype target registry, and sit within horizontal lock radius against authoritative **`PositionState`**. Deny codes **`invalid_target`** and **`out_of_range`** reuse the same strings as E1.M3 **`TargetState.validity`** for vocabulary alignment. **`accepted: true`** still does not imply damage, cooldown commit, or a full **`CombatResolution`** payload — those remain E5.M1 scope. + ## Risks and telemetry - Sluggish or opaque tab-target: prototype readability gate; instrument time-in-combat and deny reasons. diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 8e4396a..694948d 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), [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). **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) | --- diff --git a/docs/manual-qa/NEO-28.md b/docs/manual-qa/NEO-28.md new file mode 100644 index 0000000..264b7f1 --- /dev/null +++ b/docs/manual-qa/NEO-28.md @@ -0,0 +1,24 @@ +# NEO-28 — Manual QA checklist + +| Field | Value | +|-------|-------| +| Key | NEO-28 | +| Title | E1M4-03: Combat accept/deny integration + reason-code UX | +| Linear | https://linear.app/neon-sprawl/issue/NEO-28/e1m4-03-combat-acceptdeny-integration-reason-code-ux | +| Plan | `docs/plans/NEO-28-implementation-plan.md` | + +## 1) Server + Bruno + +- [ ] Start server: `cd server/NeonSprawl.Server && dotnet run`. +- [ ] `curl -s http://localhost:5253/health` returns JSON with `status: ok`. +- [ ] Happy path (same order as folder **seq**): **`Post hotbar bind slot0 pulse`** → **`Post target select alpha before cast`** → **`Post cast happy`** — cast **`accepted: true`**. +- [ ] **`Post cast happy`** alone (no target select in session) — expect **`accepted: false`**, **`reasonCode: invalid_target`** (or run `curl` with loadout + no lock). +- [ ] After lock + loadout, **`curl`** cast with **`targetId: prototype_target_beta`** while lock is alpha — **`invalid_target`**. +- [ ] After lock + loadout, move player far from alpha (`POST …/move-stream` with a distant position), then cast with alpha — **`out_of_range`**. + +## 2) Client — HUD + +- [ ] With server running, open main scene; hydrate hotbar (NEO-29); bind slot **0** to **`prototype_pulse`** if needed. +- [ ] **Without** Tab lock, press **1**: left HUD **`CastFeedbackLabel`** shows **`ability_cast_denied: invalid_target`** (or equivalent); Output may still show **`ability_cast_denied`** warning. +- [ ] **Tab** to lock **alpha**; press **1**: **`CastFeedbackLabel`** shows **`Cast: accepted`**; Output still prints **`ability_cast_requested`** when POST queued. +- [ ] **WASD** until target HUD shows **`Validity: out_of_range`** (soft lock); press **1**: **`CastFeedbackLabel`** shows **`ability_cast_denied: out_of_range`**. diff --git a/docs/manual-qa/NEO-31.md b/docs/manual-qa/NEO-31.md index 245a5b0..640a718 100644 --- a/docs/manual-qa/NEO-31.md +++ b/docs/manual-qa/NEO-31.md @@ -12,10 +12,10 @@ - [ ] 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/Post hotbar bind slot0 pulse` (seq **20**) then `ability-cast/Post cast happy` (seq **21**) — expect cast `accepted: true`. +- [ ] Run Bruno collection (or at least): `ability-cast/Post hotbar bind slot0 pulse` (seq **20**), **`ability-cast/Post target select alpha before cast`** (seq **21**), then **`ability-cast/Post cast happy`** (seq **22**) — 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"`. +- [ ] **`ability-cast/Post cast loadout mismatch`** (seq **25**): 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 diff --git a/docs/plans/NEO-28-implementation-plan.md b/docs/plans/NEO-28-implementation-plan.md new file mode 100644 index 0000000..343d8c8 --- /dev/null +++ b/docs/plans/NEO-28-implementation-plan.md @@ -0,0 +1,91 @@ +# NEO-28 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NEO-28 | +| **Title** | E1M4-03: Combat accept/deny integration + reason-code UX | +| **Linear** | [NEO-28](https://linear.app/neon-sprawl/issue/NEO-28/e1m4-03-combat-acceptdeny-integration-reason-code-ux) | +| **Slug** | E1M4-03 | +| **Git branch** | `NEO-28-combat-accept-deny-reason-ux` | +| **Parent context** | [E1.M4 prototype backlog](E1M4-prototype-backlog.md) · [E1.M4 — AbilityInputScaffold](../decomposition/modules/E1_M4_AbilityInputScaffold.md) | +| **Related modules** | [E1.M3 — InteractionAndTargetingLayer](../decomposition/modules/E1_M3_InteractionAndTargetingLayer.md), [E5.M1 — CombatRulesEngine](../decomposition/modules/E5_M1_CombatRulesEngine.md) | + +## Kickoff clarifications + +- **UX surface:** Asked which prototype surface should render denied cast reasons. User chose a **HUD/debug label**: add a small on-screen cast feedback/reason line near the existing target/hotbar debug UI. +- **Validation scope:** Asked how strict the prototype server-side cast denial contract should be. Recommendation was accepted: use the **stricter lock contract**. `POST /game/players/{id}/ability-cast` should validate the request target against the server-acknowledged lock, prototype target registry, and current server position; return `invalid_target` for missing/unknown/mismatch and `out_of_range` when the locked target is too far. +- **Linear blocker:** Linear showed NEO-28 blocked by NEO-31. User confirmed NEO-31 is merged and Done; kickoff removed the `blockedBy: NEO-31` relation from NEO-28 and left NEO-28 **In Progress**. + +## Goal, scope, and out-of-scope + +**Goal:** Consume authoritative cast accept/deny responses in the client cast path and make rejected casts visible through stable reason-code UX. + +**In scope** + +- Extend the current prototype ability-cast response contract with target-related denial reasons needed by E1.M4/E5.M1 integration. +- Server-side cast validation for target presence, registry membership, server lock match, and range against the authoritative position snapshot. +- Client-side handling for accepted and denied cast responses, including HUD/debug feedback for reason codes. +- Documentation of the denial contract and reason list so E1.M4 and E5.M1 do not drift. + +**Out of scope** + +- Rich combat VFX, animation, final combat log styling, damage/healing resolution, cooldown presentation, and final E5.M1 `CombatResolution` semantics. +- Production telemetry ingestion; keep existing `TODO(E9.M1)` hook notes. + +## Acceptance criteria checklist + +- [x] Denied casts render a player-visible reason path for at least `invalid_target` and `out_of_range`. +- [x] Successful casts transition to post-request state without conflicting local rollback behavior. +- [x] Denial contract and reason list are documented in plan/module docs to prevent drift. + +## Technical approach + +1. **Server contract** — Extend `AbilityCastApi` from NEO-31 rather than adding a parallel endpoint. Keep the v1 JSON shape (`schemaVersion`, `accepted`, optional `reasonCode`) and add stable target denial constants: `invalid_target` and `out_of_range`. + +2. **Server validation order** — Preserve existing request/loadout/ability validation first. After the loadout match succeeds, validate target authority: non-empty request `targetId`, known `PrototypeTargetRegistry` id, request target equals `IPlayerTargetLockStore` current lock, and target is within horizontal range of the authoritative `IPositionStateStore` snapshot. Return a JSON deny response with `accepted: false` for target denials; reserve transport status codes for malformed/missing-player cases already established by NEO-31. + +3. **Client response handling** — Make `AbilityCastClient` emit a signal for completed cast responses so `main.gd` can update a HUD/debug cast feedback label. Denies should display `ability_cast_denied: ` or equivalent player-visible text; accepts should clear or replace stale deny text with a lightweight accepted/post-request message. + +4. **Optimistic state reconciliation** — Keep the current prototype behavior conservative: `ability_cast_requested` only logs once the POST starts, and no local cooldown/rollback state is introduced in this story. On deny, the feedback label becomes the reconciliation surface; on accept, the client avoids any rollback behavior that would conflict with later cooldown work in NEO-32. + +5. **Documentation** — Update E1.M4 and E5.M1 docs with the prototype accept/deny contract, including the reason list and which reasons belong to hotbar/loadout validation versus combat target validation. + +## Files to add + +| Path | Rationale | +|------|-----------| +| `docs/manual-qa/NEO-28.md` | Manual QA checklist for accepted cast, `invalid_target`, `out_of_range`, and HUD/debug feedback visibility. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs` | Add target authority validation, reason constants, and `IPlayerTargetLockStore` / `PrototypeTargetRegistry` use. | +| `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs` | Update summary/comments if needed to describe accept/deny v1 target semantics. | +| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | Cover `invalid_target`, `out_of_range`, and strict lock-match behavior. | +| `client/scripts/ability_cast_client.gd` | Surface parsed accept/deny responses via signal instead of only `push_warning`. | +| `client/scripts/main.gd` | Add/update HUD/debug cast feedback label wiring and reconcile accepted/denied response display. | +| `client/README.md` | Document cast deny feedback behavior for manual prototype use. | +| `server/README.md` | Document the extended ability-cast denial contract and reason codes. | +| `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` | Update implementation snapshot, Linear row, and denial contract notes for E1M4-03. | +| `docs/decomposition/modules/E5_M1_CombatRulesEngine.md` | Document the prototype accept/deny handoff and deferred final `CombatResolution` scope. | +| `bruno/neon-sprawl-server/ability-cast/Post cast happy.bru` | Ensure existing happy path still asserts accepted response after target validation setup. | +| `bruno/neon-sprawl-server/ability-cast/Post cast loadout mismatch.bru` | Keep non-target denial regression aligned with response contract. | + +## Tests + +| Suite | What it covers | +|--------|----------------| +| `server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs` | Accepted cast with locked in-range target; `invalid_target` for missing/unknown/mismatched target; `out_of_range` when authoritative position is outside prototype lock radius; existing loadout denies remain stable. | +| `client/test/ability_cast_client_test.gd` | Response parsing and signal emission for accepted and denied JSON bodies, including `invalid_target` / `out_of_range`. | +| `client/test/hotbar_cast_slot_resolver_test.gd` | No required change expected unless target payload rules change; keep as regression for request formation from server-ack target state. | +| `docs/manual-qa/NEO-28.md` | Human verification of HUD/debug feedback, accepted cast display, and denial reason visibility. | +| Bruno `ability-cast/*.bru` | Add or adjust requests/tests if needed for target validation preconditions and stable response assertions. | + +## Open questions / risks + +- **Server lock setup in tests / Bruno:** Existing happy-path requests may need a target-select setup step before cast because the stricter contract requires a server lock. +- **Out-of-range reproduction:** Current target selection rejects out-of-range locks, so `out_of_range` cast tests may need to lock while in range, then move the authoritative position out of range before casting. +- **UI scene edit size:** Adding a HUD/debug label may touch `client/main.tscn`; keep it minimal and aligned with existing debug labels. diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs index 812d40b..156a4f1 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs @@ -2,6 +2,8 @@ using System.Net; using System.Net.Http.Json; using System.Text; using NeonSprawl.Server.Game.AbilityInput; +using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Targeting; using Xunit; namespace NeonSprawl.Server.Tests.Game.AbilityInput; @@ -20,13 +22,41 @@ public sealed class AbilityCastApiTests post.EnsureSuccessStatusCode(); } + private static async Task LockPrototypeTargetAlphaAsync(HttpClient client) + { + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/target/select", + new TargetSelectRequest + { + SchemaVersion = TargetSelectRequest.CurrentSchemaVersion, + TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, + }); + response.EnsureSuccessStatusCode(); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.True(body!.SelectionApplied); + } + + private static async Task TeleportDevPlayerAsync(HttpClient client, double x, double y, double z) + { + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/move-stream", + new MoveStreamRequest + { + SchemaVersion = MoveStreamRequest.CurrentSchemaVersion, + Targets = [new PositionVector { X = x, Y = y, Z = z }], + }); + response.EnsureSuccessStatusCode(); + } + [Fact] - public async Task PostAbilityCast_ShouldAccept_WhenLoadoutMatches() + public async Task PostAbilityCast_ShouldAccept_WhenLoadoutMatchesAndLockInRange() { // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); await BindSlot0PulseAsync(client); + await LockPrototypeTargetAlphaAsync(client); // Act var response = await client.PostAsJsonAsync( @@ -36,7 +66,7 @@ public sealed class AbilityCastApiTests SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, SlotIndex = 0, AbilityId = PrototypeAbilityRegistry.PrototypePulse, - TargetId = null, + TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, }); var body = await response.Content.ReadFromJsonAsync(); @@ -54,6 +84,7 @@ public sealed class AbilityCastApiTests await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); await BindSlot0PulseAsync(client); + await LockPrototypeTargetAlphaAsync(client); // Act var response = await client.PostAsJsonAsync( @@ -73,6 +104,146 @@ public sealed class AbilityCastApiTests Assert.True(body!.Accepted); } + [Fact] + public async Task PostAbilityCast_ShouldDenyInvalidTarget_WhenTargetIdMissing() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + await LockPrototypeTargetAlphaAsync(client); + + // Act + 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 + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonInvalidTarget, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenyInvalidTarget_WhenUnknownTargetId() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + await LockPrototypeTargetAlphaAsync(client); + + // Act + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + TargetId = "not_a_registered_combat_target", + }); + var body = await response.Content.ReadFromJsonAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonInvalidTarget, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenyInvalidTarget_WhenTargetDoesNotMatchServerLock() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + await LockPrototypeTargetAlphaAsync(client); + + // Act — server lock is alpha; client sends beta id + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + TargetId = PrototypeTargetRegistry.PrototypeTargetBetaId, + }); + var body = await response.Content.ReadFromJsonAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonInvalidTarget, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenyInvalidTarget_WhenNoServerLock() + { + // 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 + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, + }); + var body = await response.Content.ReadFromJsonAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonInvalidTarget, body.ReasonCode); + } + + [Fact] + public async Task PostAbilityCast_ShouldDenyOutOfRange_WhenAuthoritativePositionTooFarFromLock() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await BindSlot0PulseAsync(client); + await LockPrototypeTargetAlphaAsync(client); + await TeleportDevPlayerAsync(client, 50.0, 0.9, 50.0); + + // Act + var response = await client.PostAsJsonAsync( + "/game/players/dev-local-1/ability-cast", + new AbilityCastRequest + { + SchemaVersion = AbilityCastRequest.CurrentSchemaVersion, + SlotIndex = 0, + AbilityId = PrototypeAbilityRegistry.PrototypePulse, + TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, + }); + var body = await response.Content.ReadFromJsonAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); + Assert.False(body!.Accepted); + Assert.Equal(AbilityCastApi.ReasonOutOfRange, body.ReasonCode); + } + [Fact] public async Task PostAbilityCast_ShouldDenySlotUnbound_WhenSlotEmpty() { diff --git a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs index ba3cf6c..b136565 100644 --- a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs +++ b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs @@ -1,8 +1,10 @@ using NeonSprawl.Server.Game.PositionState; +using NeonSprawl.Server.Game.Targeting; +using NeonSprawl.Server.Game.World; namespace NeonSprawl.Server.Game.AbilityInput; -/// Maps prototype ability cast POST (NEO-31). +/// Maps prototype ability cast POST (NEO-31 + NEO-28 target authority). public static class AbilityCastApi { public const string ReasonSlotOutOfBounds = "slot_out_of_bounds"; @@ -10,18 +12,29 @@ public static class AbilityCastApi public const string ReasonLoadoutMismatch = "loadout_mismatch"; public const string ReasonUnknownAbility = "unknown_ability"; + /// Request target missing, unknown to the prototype registry, or not the server lock (NEO-28). + public const string ReasonInvalidTarget = TargetValidity.InvalidTarget; + + /// Locked prototype target is outside horizontal reach of authoritative position (NEO-28). + public const string ReasonOutOfRange = TargetValidity.OutOfRange; + public static WebApplication MapAbilityCastApi(this WebApplication app) { app.MapPost( "/game/players/{id}/ability-cast", - (string id, AbilityCastRequest? body, IPositionStateStore positions, IPlayerHotbarLoadoutStore store) => + ( + string id, + AbilityCastRequest? body, + IPositionStateStore positions, + IPlayerHotbarLoadoutStore store, + IPlayerTargetLockStore locks) => { if (body is null || body.SchemaVersion != AbilityCastRequest.CurrentSchemaVersion) { return Results.BadRequest(); } - if (!positions.TryGetPosition(id, out _)) + if (!positions.TryGetPosition(id, out var snap)) { return Results.NotFound(); } @@ -73,7 +86,33 @@ public static class AbilityCastApi }); } - // TargetId echoed for NEO-28 / E5.M1; server does not validate against lock store in NEO-31. + if (string.IsNullOrWhiteSpace(body.TargetId)) + { + return Results.Json( + new AbilityCastResponse { Accepted = false, ReasonCode = ReasonInvalidTarget }); + } + + var lookupKey = body.TargetId.Trim().ToLowerInvariant(); + if (!PrototypeTargetRegistry.TryGet(lookupKey, out var entry)) + { + return Results.Json( + new AbilityCastResponse { Accepted = false, ReasonCode = ReasonInvalidTarget }); + } + + var (lockIdLowercase, _) = locks.GetLockState(id); + if (string.IsNullOrEmpty(lockIdLowercase) || + !string.Equals(lookupKey, lockIdLowercase, StringComparison.Ordinal)) + { + return Results.Json( + new AbilityCastResponse { Accepted = false, ReasonCode = ReasonInvalidTarget }); + } + + if (!HorizontalReach.IsWithinHorizontalRadius(snap.X, snap.Z, entry.X, entry.Z, entry.LockRadius)) + { + return Results.Json( + new AbilityCastResponse { Accepted = false, ReasonCode = ReasonOutOfRange }); + } + return Results.Json(new AbilityCastResponse { Accepted = true }); }); diff --git a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs index ff08086..e34d56c 100644 --- a/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs +++ b/server/NeonSprawl.Server/Game/AbilityInput/AbilityCastDtos.cs @@ -16,12 +16,15 @@ public sealed class AbilityCastRequest [JsonPropertyName("abilityId")] public string? AbilityId { get; init; } - /// Optional; mirrors lockedTargetId from target state v1. + /// + /// Must match the server's current combat lock (same string as GET …/target lockedTargetId) for + /// accepted: true; NEO-28 adds invalid_target / out_of_range denies when it does not. + /// [JsonPropertyName("targetId")] public string? TargetId { get; init; } } -/// POST response for cast submit (prototype accept/deny only; NEO-31). +/// POST response for cast submit (prototype accept/deny; NEO-31 + NEO-28 target rules). public sealed class AbilityCastResponse { public const int CurrentSchemaVersion = 1; diff --git a/server/README.md b/server/README.md index bd5a3dd..9418d60 100644 --- a/server/README.md +++ b/server/README.md @@ -225,9 +225,9 @@ Current prototype allowlist: `prototype_pulse`, `prototype_guard`, `prototype_da ## Ability cast (NEO-31) -Prototype **cast intent** (no combat resolution yet — see E1.M4 / E5.M1 follow-on stories): +Prototype **cast intent** (no full combat resolution — see E5.M1). **NEO-28** adds server-side target lock + registry + range gates and documents `invalid_target` / `out_of_range` denies; the Godot client surfaces accept/deny on **`CastFeedbackLabel`**. -- **`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`). +- **`POST /game/players/{id}/ability-cast`** with `AbilityCastRequest` v1 (`schemaVersion`, `slotIndex` `0..7`, `abilityId`, `targetId`) validates the player exists, the slot is bound in persisted hotbar loadout, and `abilityId` matches that binding and the prototype allowlist. **NEO-28:** `targetId` must be **non-empty**, exist in **`PrototypeTargetRegistry`**, **match** the server's current combat lock (`IPlayerTargetLockStore`), and the lock must be **within horizontal reach** of the authoritative position snapshot (same XZ radius rule as targeting). Returns **`AbilityCastResponse` v1** JSON (`schemaVersion`, `accepted`, optional `reasonCode`). **HTTP status:** @@ -245,6 +245,8 @@ Prototype **cast intent** (no combat resolution yet — see E1.M4 / E5.M1 follow | `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). | +| `invalid_target` | `targetId` missing/empty, unknown prototype id, or not equal to the server's locked target id (NEO-28). | +| `out_of_range` | Locked target is outside horizontal reach of authoritative player position vs. prototype anchor (NEO-28). | Implementation: `server/NeonSprawl.Server/Game/AbilityInput/AbilityCastApi.cs`, `AbilityCastDtos.cs`. From 09e783a5e8a7b79608dc1cdd5bf976fbb4d9c6ad Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 21:46:37 -0400 Subject: [PATCH 2/8] NEO-28: AAA layout for cast tests (C# + GdUnit) --- client/test/ability_cast_client_test.gd | 20 ++++++++++++---- .../Game/AbilityInput/AbilityCastApiTests.cs | 24 +++++++++---------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 1e6bb85..29127c7 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -77,10 +77,13 @@ func _make_client(transport: Node) -> Node: func test_request_cast_posts_expected_payload_with_null_target() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') var c := _make_client(transport) + # Act var started: bool = bool(c.call("request_cast", 2, "prototype_guard", null)) + # Assert assert_that(started).is_true() assert_that(transport.last_url).contains("/ability-cast") assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST) @@ -94,20 +97,22 @@ func test_request_cast_posts_expected_payload_with_null_target() -> void: func test_request_cast_posts_target_id_string_when_set() -> void: + # Arrange 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() - ) + # Act + var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha")) + # Assert + assert_that(started).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). + # Arrange + # Empty queue → transport.request returns ERR_UNAVAILABLE (same as failed start). var transport := MockHttpTransport.new() var c := _make_client(transport) # Act @@ -117,12 +122,15 @@ func test_request_cast_returns_false_when_http_request_fails_to_start() -> void: func test_request_cast_while_busy_is_ignored() -> void: + # Arrange var transport := HoldFirstThenAutoTransport.new() var c := _make_client(transport) + # Act 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 assert_that(transport.request_count).is_equal(1) var parsed: Variant = JSON.parse_string(transport.last_body) var body: Dictionary = parsed @@ -130,6 +138,7 @@ func test_request_cast_while_busy_is_ignored() -> void: func test_cast_result_received_emits_false_with_reason_on_denied() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, @@ -148,6 +157,7 @@ func test_cast_result_received_emits_false_with_reason_on_denied() -> void: func test_cast_result_received_emits_true_with_empty_reason_on_accepted() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') var c := _make_client(transport) diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs index 156a4f1..b98c552 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/AbilityCastApiTests.cs @@ -68,9 +68,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.True(body!.Accepted); @@ -96,9 +96,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = "prototype_target_alpha", }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.True(body!.Accepted); @@ -123,9 +123,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = null, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -151,9 +151,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = "not_a_registered_combat_target", }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -169,7 +169,7 @@ public sealed class AbilityCastApiTests await BindSlot0PulseAsync(client); await LockPrototypeTargetAlphaAsync(client); - // Act — server lock is alpha; client sends beta id + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/ability-cast", new AbilityCastRequest @@ -179,9 +179,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = PrototypeTargetRegistry.PrototypeTargetBetaId, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -206,9 +206,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -235,9 +235,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -261,9 +261,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypeGuard, TargetId = null, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -288,9 +288,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypeBurst, TargetId = null, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -315,9 +315,9 @@ public sealed class AbilityCastApiTests AbilityId = PrototypeAbilityRegistry.PrototypePulse, TargetId = null, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); @@ -342,9 +342,9 @@ public sealed class AbilityCastApiTests AbilityId = "not_a_real_ability", TargetId = null, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.False(body!.Accepted); From 31f9b74d3f9612fe87b06d80248b7e9561adf882 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 21:52:39 -0400 Subject: [PATCH 3/8] chore: make AAA test layout mandatory in style rules and snippets --- .cursor/rules/code-review-agent.md | 4 +- .cursor/rules/csharp-style.md | 51 ++++++++++++++++--- .cursor/rules/gdscript-style.md | 16 ++++++ .cursor/rules/testing-expectations.md | 5 +- .vscode/csharp.code-snippets | 6 +++ AGENTS.md | 2 +- .../AAA_TEST_METHOD_TEMPLATE.cs.txt | 5 +- 7 files changed, 75 insertions(+), 14 deletions(-) diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md index cb2910d..8b67df4 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#; for C# tests, [AAA sections](csharp-style.md#unit-and-integration-tests-arrange-act-assert) on every **new or changed** `[Fact]` / `[Theory]` method. +- [csharp-style](csharp-style.md) / [gdscript-style](gdscript-style.md) — naming, layout, primary constructors for C#; for **tests**, full [C# AAA](csharp-style.md#unit-and-integration-tests-arrange-act-assert) and [GdUnit AAA](gdscript-style.md#gdunit-test-layout-aaa) on every **new or changed** test 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. **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. +5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. **C# (`*Tests.cs`):** full AAA per [csharp-style](csharp-style.md#unit-and-integration-tests-arrange-act-assert) — labels plus **blank line after each**, Act-only invocation(s), **verification reads in Assert**. **GdUnit (`client/test/`):** same AAA with **`# Arrange` / `# Act` / `# Assert`** per [gdscript-style](gdscript-style.md#gdunit-test-layout-aaa). Treat missing or partial AAA on **any touched** test method as **should fix** (or **blocking** if the change is test-heavy); only **nit** when the diff is a one-line fix inside a legacy method you did not own end-to-end **and** AAA was already complete. 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/csharp-style.md b/.cursor/rules/csharp-style.md index e92c436..8aeb37e 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -89,14 +89,49 @@ public async Task PostMove_ShouldReturnBadRequest_WhenSchemaVersionIsWrong() { ## Unit and integration tests (Arrange, Act, Assert) -- Structure every test method as **Arrange → Act → Assert (AAA)**. -- Use a **`// Arrange`**, **`// Act`**, and **`// Assert`** comment (or the same words in a brief block comment) so the three phases are obvious at a glance. -- **Arrange:** create SUT dependencies, inputs, HTTP clients/factories, and test data. No assertions on the outcome under test (guards on arrange setup are optional and rare). -- **Act:** invoke the single behavior under test. For integration tests, the Act block may include a short sequence of calls if that sequence *is* the behavior (e.g. POST then GET to verify persistence)—keep it one clear scenario per test. -- **Assert:** all expectations (`Assert.*`, etc.); read response bodies here when the read is for verification, not for driving the next call (otherwise fold those reads into Act). -- Multi-scenario files stay readable with **one AAA triple per `[Fact]` / `[Theory]`** method. -- **Write-time default:** use the workspace VS Code snippet prefix **`xut`** (method) or **`xutc`** (class) from `.vscode/csharp.code-snippets` when authoring tests so AAA sections are present from the first draft. -- **Template reference:** `server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`. +**Mandatory for every new or changed test method** in `*Tests.cs` (`[Fact]` / `[Theory]` bodies). Do not ship partial AAA (“comments present but body read still in Act,” missing blank lines, etc.). Agents and humans must treat this as **merge-ready** layout, not a suggestion. + +### Required layout + +1. **Phase labels** — Each test method contains **`// Arrange`**, **`// Act`**, and **`// Assert`** exactly once, in that order, each on its own line (same indentation as the test body). Use these exact words so grep and review stay consistent. + +2. **Blank line after each label** — Insert a **blank line** immediately after `// Arrange`, after `// Act`, and after `// Assert` before the first statement of that phase. **Required** (not optional): it matches the template, satisfies [code-review-agent](code-review-agent.md) expectations, and makes phases obvious in diff view. + +3. **Arrange** — Factories, `HttpClient`, queued mock transports, DTOs, seeds, and other setup. Do **not** assert the **outcome under test** in Arrange (rare guards on arrange-only helpers are acceptable). + +4. **Act** — Invoke the **behavior under test** only (e.g. one `PostAsJsonAsync`, `GetAsync`, or SUT call). If the scenario *is* a short sequence (e.g. POST then GET to prove persistence), keep the whole sequence in Act with **no** `Assert.*` between those calls. + +5. **Assert** — All `Assert.*` (and any other outcome checks). **HTTP / deserialization:** when the next step is verifying the response (status already observed, body for assertions), perform **`ReadFromJsonAsync`** / similar **reads here**, not in Act beside the POST. Reads that **drive** the next call belong in Act, not Assert. + +6. **One AAA triple per test method** — Each `[Fact]` or `[Theory]` case gets its own Arrange/Act/Assert; do not share a single Assert block across unrelated scenarios in one method. + +### Authoring defaults + +- Use VS Code snippet **`xut`** (method) or **`xutc`** (class) from `.vscode/csharp.code-snippets`, or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`** (includes blank lines after labels). + +### Example (minimal HTTP integration) + +```csharp +[Fact] +public async Task PostExample_ShouldReturnOk_WhenBodyValid() +{ + // Arrange + + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var request = new { schemaVersion = 1 }; + + // Act + + var response = await client.PostAsJsonAsync("/game/example", request); + + // Assert + + var body = await response.Content.ReadFromJsonAsync(); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.NotNull(body); +} +``` ## Tooling diff --git a/.cursor/rules/gdscript-style.md b/.cursor/rules/gdscript-style.md index 205d24c..aff34ee 100644 --- a/.cursor/rules/gdscript-style.md +++ b/.cursor/rules/gdscript-style.md @@ -62,6 +62,22 @@ CI also enforces **`gdlint`** and **`gdformat`** (see `gdlintrc` and `.github/wo - Install the repo’s local **pre-push** hook from the repo root to enforce those checks automatically: `./scripts/install-git-hooks.sh` +## GdUnit test layout (AAA) + +**Mandatory** for every **new or changed** test function in **`client/test/**/*.gd`** (GdUnit suites, `test_*` methods). + +1. **`# Arrange`**, **`# Act`**, and **`# Assert`** — each appears **once** per test, in that order, as a full-line comment at the same indentation as the test body. + +2. **Blank line after each label** — Required: put a blank line immediately after `# Arrange`, after `# Act`, and after `# Assert` before the first line of that phase (same rule as [C# AAA](csharp-style.md#unit-and-integration-tests-arrange-act-assert)). + +3. **Arrange** — Transports, clients, `enqueue`/`connect` setup, test data. + +4. **Act** — The call(s) under test only (e.g. `request_cast`, `call` into SUT). No `assert_that` / `assert_*` in Act. + +5. **Assert** — All expectations; parse or inspect results here when parsing exists only to support assertions. + +`gdlint` / `gdformat` do not enforce AAA text; **review and agents** do. Match the spirit of server **`xut`** snippets: three labeled phases, blank lines, one scenario per test. + ## Comments - Use `#` comments; document non-obvious **why**, not what the next line literally does. diff --git a/.cursor/rules/testing-expectations.md b/.cursor/rules/testing-expectations.md index c2d69e5..5eb2f1f 100644 --- a/.cursor/rules/testing-expectations.md +++ b/.cursor/rules/testing-expectations.md @@ -9,8 +9,8 @@ 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 (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`. +- **Layout (mandatory for new/changed tests):** every **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **AAA**: **`// Arrange`**, **`// Act`**, **`// Assert`** on their own lines, a **blank line after each label**, **Act** limited to the behavior under test, and **response-body reads for verification in Assert** (not mixed into Act with the HTTP call). See [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). Treat anything less as **incomplete** for merge. +- **At-write-time default:** start new tests with snippet **`xut`** / **`xutc`** from `.vscode/csharp.code-snippets` or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`** so blank-line AAA is present from the first keystroke. - **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. @@ -26,6 +26,7 @@ alwaysApply: true ## Godot client (GDScript) - **Harness (NEO-12):** **GdUnit4** lives under **`client/addons/gdUnit4/`**; suites under **`client/test/`**. **`client/README.md`** documents local and headless runs; **`.github/workflows/gdscript.yml`** runs **gdlint**, **gdformat**, and **headless GdUnit** when `client/scripts/`, `client/test/`, `client/addons/`, or `client/project.godot` change. +- **GdUnit tests (`client/test/**/*.gd`):** every **new or changed** test function must use the same **AAA** discipline as C#: **`# Arrange`**, **`# Act`**, **`# Assert`** each on its own line, in order, with a **blank line after each label** before that phase’s statements. Act holds only the invocation(s) under test; assertions (and any parsing done purely to assert) live under Assert. See [gdscript-style — GdUnit test layout (AAA)](gdscript-style.md#gdunit-test-layout-aaa). - **Production script changes:** Any PR that **adds or changes** GDScript under **`client/scripts/`** (and any other **application** `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** tests under **`client/test/`** in the same change set. If a change is **not** reasonably testable, say so in the PR (or story plan) with a short reason—do not skip tests silently. - **Scenes / assets / manual checks:** Scene tweaks, visuals, and flows that unit tests do not cover still deserve **manual verification** notes in the PR when behavior could regress. diff --git a/.vscode/csharp.code-snippets b/.vscode/csharp.code-snippets index 106f6f5..950f8b7 100644 --- a/.vscode/csharp.code-snippets +++ b/.vscode/csharp.code-snippets @@ -8,12 +8,15 @@ "public async Task ${1:MethodName}_Should${2:ExpectedOutcome}_When${3:Scenario}()", "{", " // Arrange", + "", " ${4:// setup}", "", " // Act", + "", " ${5:// execute}", "", " // Assert", + "", " ${6:// verify}", "}" ] @@ -33,12 +36,15 @@ " public async Task ${3:MethodName}_Should${4:ExpectedOutcome}_When${5:Scenario}()", " {", " // Arrange", + "", " ${6:// setup}", "", " // Act", + "", " ${7:// execute}", "", " // Assert", + "", " ${8:// verify}", " }", "}" diff --git a/AGENTS.md b/AGENTS.md index 5ae8dc5..af78752 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). **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). +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). **Automated tests (AAA, mandatory):** **C#** — every new or changed **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **Arrange → Act → Assert**: the three **`// Arrange` / `// Act` / `// Assert`** labels, a **blank line after each label**, **Act** = behavior under test only, **Assert** = all expectations including **`ReadFromJsonAsync`** (etc.) for verification — [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), [testing-expectations](.cursor/rules/testing-expectations.md); snippets **`xut`** / **`xutc`**. **GdUnit (`client/test/`)** — same AAA with **`# Arrange` / `# Act` / `# Assert`** and blank lines after each — [gdscript-style](.cursor/rules/gdscript-style.md#gdunit-test-layout-aaa). **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). diff --git a/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt b/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt index 34032f1..0c5122f 100644 --- a/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt +++ b/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt @@ -2,11 +2,14 @@ public async Task MethodName_ShouldExpectedOutcome_WhenScenario() { // Arrange + // Create factory/client/dependencies and test data. // Act + // Invoke exactly one behavior/scenario under test. // Assert - // Verify outcome(s) with Assert.* calls. + + // Verify outcome(s) with Assert.* calls; read HTTP/deserialized bodies here when verifying responses. } From 7050bc12da6cb916e38728c66c4cd0ab8ff3049c Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 21:54:02 -0400 Subject: [PATCH 4/8] =?UTF-8?q?chore:=20AAA=20rules=20=E2=80=94=20no=20req?= =?UTF-8?q?uired=20blank=20line=20after=20phase=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .cursor/rules/code-review-agent.md | 2 +- .cursor/rules/csharp-style.md | 19 +++++++------------ .cursor/rules/gdscript-style.md | 12 +++++------- .cursor/rules/testing-expectations.md | 4 ++-- .vscode/csharp.code-snippets | 6 ------ AGENTS.md | 2 +- .../AAA_TEST_METHOD_TEMPLATE.cs.txt | 3 --- 7 files changed, 16 insertions(+), 32 deletions(-) diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md index 8b67df4..a2ec9de 100644 --- a/.cursor/rules/code-review-agent.md +++ b/.cursor/rules/code-review-agent.md @@ -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. **C# (`*Tests.cs`):** full AAA per [csharp-style](csharp-style.md#unit-and-integration-tests-arrange-act-assert) — labels plus **blank line after each**, Act-only invocation(s), **verification reads in Assert**. **GdUnit (`client/test/`):** same AAA with **`# Arrange` / `# Act` / `# Assert`** per [gdscript-style](gdscript-style.md#gdunit-test-layout-aaa). Treat missing or partial AAA on **any touched** test method as **should fix** (or **blocking** if the change is test-heavy); only **nit** when the diff is a one-line fix inside a legacy method you did not own end-to-end **and** AAA was already complete. +5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. **C# (`*Tests.cs`):** full AAA per [csharp-style](csharp-style.md#unit-and-integration-tests-arrange-act-assert) — phase labels, Act-only invocation(s), **verification reads in Assert**. **GdUnit (`client/test/`):** same AAA with **`# Arrange` / `# Act` / `# Assert`** per [gdscript-style](gdscript-style.md#gdunit-test-layout-aaa). Treat missing or partial AAA on **any touched** test method as **should fix** (or **blocking** if the change is test-heavy); only **nit** when the diff is a one-line fix inside a legacy method you did not own end-to-end **and** AAA was already complete. 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/csharp-style.md b/.cursor/rules/csharp-style.md index 8aeb37e..4f3e4b9 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -89,25 +89,23 @@ public async Task PostMove_ShouldReturnBadRequest_WhenSchemaVersionIsWrong() { ## Unit and integration tests (Arrange, Act, Assert) -**Mandatory for every new or changed test method** in `*Tests.cs` (`[Fact]` / `[Theory]` bodies). Do not ship partial AAA (“comments present but body read still in Act,” missing blank lines, etc.). Agents and humans must treat this as **merge-ready** layout, not a suggestion. +**Mandatory for every new or changed test method** in `*Tests.cs` (`[Fact]` / `[Theory]` bodies). Do not ship partial AAA (e.g. comments present but body read still in Act). Agents and humans must treat this as **merge-ready** layout, not a suggestion. ### Required layout -1. **Phase labels** — Each test method contains **`// Arrange`**, **`// Act`**, and **`// Assert`** exactly once, in that order, each on its own line (same indentation as the test body). Use these exact words so grep and review stay consistent. +1. **Phase labels** — Each test method contains **`// Arrange`**, **`// Act`**, and **`// Assert`** exactly once, in that order, each on its own line (same indentation as the test body). Use these exact words so grep and review stay consistent. **No blank line is required** immediately after those comments; the first statement of each phase may follow on the next line. -2. **Blank line after each label** — Insert a **blank line** immediately after `// Arrange`, after `// Act`, and after `// Assert` before the first statement of that phase. **Required** (not optional): it matches the template, satisfies [code-review-agent](code-review-agent.md) expectations, and makes phases obvious in diff view. +2. **Arrange** — Factories, `HttpClient`, queued mock transports, DTOs, seeds, and other setup. Do **not** assert the **outcome under test** in Arrange (rare guards on arrange-only helpers are acceptable). -3. **Arrange** — Factories, `HttpClient`, queued mock transports, DTOs, seeds, and other setup. Do **not** assert the **outcome under test** in Arrange (rare guards on arrange-only helpers are acceptable). +3. **Act** — Invoke the **behavior under test** only (e.g. one `PostAsJsonAsync`, `GetAsync`, or SUT call). If the scenario *is* a short sequence (e.g. POST then GET to prove persistence), keep the whole sequence in Act with **no** `Assert.*` between those calls. -4. **Act** — Invoke the **behavior under test** only (e.g. one `PostAsJsonAsync`, `GetAsync`, or SUT call). If the scenario *is* a short sequence (e.g. POST then GET to prove persistence), keep the whole sequence in Act with **no** `Assert.*` between those calls. +4. **Assert** — All `Assert.*` (and any other outcome checks). **HTTP / deserialization:** when the next step is verifying the response (status already observed, body for assertions), perform **`ReadFromJsonAsync`** / similar **reads here**, not in Act beside the POST. Reads that **drive** the next call belong in Act, not Assert. -5. **Assert** — All `Assert.*` (and any other outcome checks). **HTTP / deserialization:** when the next step is verifying the response (status already observed, body for assertions), perform **`ReadFromJsonAsync`** / similar **reads here**, not in Act beside the POST. Reads that **drive** the next call belong in Act, not Assert. - -6. **One AAA triple per test method** — Each `[Fact]` or `[Theory]` case gets its own Arrange/Act/Assert; do not share a single Assert block across unrelated scenarios in one method. +5. **One AAA triple per test method** — Each `[Fact]` or `[Theory]` case gets its own Arrange/Act/Assert; do not share a single Assert block across unrelated scenarios in one method. ### Authoring defaults -- Use VS Code snippet **`xut`** (method) or **`xutc`** (class) from `.vscode/csharp.code-snippets`, or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`** (includes blank lines after labels). +- Use VS Code snippet **`xut`** (method) or **`xutc`** (class) from `.vscode/csharp.code-snippets`, or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`**. ### Example (minimal HTTP integration) @@ -116,17 +114,14 @@ public async Task PostMove_ShouldReturnBadRequest_WhenSchemaVersionIsWrong() { public async Task PostExample_ShouldReturnOk_WhenBodyValid() { // Arrange - await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); var request = new { schemaVersion = 1 }; // Act - var response = await client.PostAsJsonAsync("/game/example", request); // Assert - var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); diff --git a/.cursor/rules/gdscript-style.md b/.cursor/rules/gdscript-style.md index aff34ee..9106fb0 100644 --- a/.cursor/rules/gdscript-style.md +++ b/.cursor/rules/gdscript-style.md @@ -66,17 +66,15 @@ CI also enforces **`gdlint`** and **`gdformat`** (see `gdlintrc` and `.github/wo **Mandatory** for every **new or changed** test function in **`client/test/**/*.gd`** (GdUnit suites, `test_*` methods). -1. **`# Arrange`**, **`# Act`**, and **`# Assert`** — each appears **once** per test, in that order, as a full-line comment at the same indentation as the test body. +1. **`# Arrange`**, **`# Act`**, and **`# Assert`** — each appears **once** per test, in that order, as a full-line comment at the same indentation as the test body. **No blank line is required** after those comments; the next statement may follow immediately. -2. **Blank line after each label** — Required: put a blank line immediately after `# Arrange`, after `# Act`, and after `# Assert` before the first line of that phase (same rule as [C# AAA](csharp-style.md#unit-and-integration-tests-arrange-act-assert)). +2. **Arrange** — Transports, clients, `enqueue`/`connect` setup, test data. -3. **Arrange** — Transports, clients, `enqueue`/`connect` setup, test data. +3. **Act** — The call(s) under test only (e.g. `request_cast`, `call` into SUT). No `assert_that` / `assert_*` in Act. -4. **Act** — The call(s) under test only (e.g. `request_cast`, `call` into SUT). No `assert_that` / `assert_*` in Act. +4. **Assert** — All expectations; parse or inspect results here when parsing exists only to support assertions. -5. **Assert** — All expectations; parse or inspect results here when parsing exists only to support assertions. - -`gdlint` / `gdformat` do not enforce AAA text; **review and agents** do. Match the spirit of server **`xut`** snippets: three labeled phases, blank lines, one scenario per test. +`gdlint` / `gdformat` do not enforce AAA text; **review and agents** do. Match the spirit of server **`xut`** snippets: three labeled phases, one scenario per test. ## Comments diff --git a/.cursor/rules/testing-expectations.md b/.cursor/rules/testing-expectations.md index 5eb2f1f..51106c2 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 (mandatory for new/changed tests):** every **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **AAA**: **`// Arrange`**, **`// Act`**, **`// Assert`** on their own lines, a **blank line after each label**, **Act** limited to the behavior under test, and **response-body reads for verification in Assert** (not mixed into Act with the HTTP call). See [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). Treat anything less as **incomplete** for merge. +- **Layout (mandatory for new/changed tests):** every **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **AAA**: **`// Arrange`**, **`// Act`**, **`// Assert`** on their own lines, **Act** limited to the behavior under test, and **response-body reads for verification in Assert** (not mixed into Act with the HTTP call). A blank line after each phase comment is **not** required. See [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). Treat anything less as **incomplete** for merge. - **At-write-time default:** start new tests with snippet **`xut`** / **`xutc`** from `.vscode/csharp.code-snippets` or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`** so blank-line AAA is present from the first keystroke. - **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. @@ -26,7 +26,7 @@ alwaysApply: true ## Godot client (GDScript) - **Harness (NEO-12):** **GdUnit4** lives under **`client/addons/gdUnit4/`**; suites under **`client/test/`**. **`client/README.md`** documents local and headless runs; **`.github/workflows/gdscript.yml`** runs **gdlint**, **gdformat**, and **headless GdUnit** when `client/scripts/`, `client/test/`, `client/addons/`, or `client/project.godot` change. -- **GdUnit tests (`client/test/**/*.gd`):** every **new or changed** test function must use the same **AAA** discipline as C#: **`# Arrange`**, **`# Act`**, **`# Assert`** each on its own line, in order, with a **blank line after each label** before that phase’s statements. Act holds only the invocation(s) under test; assertions (and any parsing done purely to assert) live under Assert. See [gdscript-style — GdUnit test layout (AAA)](gdscript-style.md#gdunit-test-layout-aaa). +- **GdUnit tests (`client/test/**/*.gd`):** every **new or changed** test function must use the same **AAA** discipline as C#: **`# Arrange`**, **`# Act`**, **`# Assert`** each on its own line, in order. A blank line after each label is **not** required. Act holds only the invocation(s) under test; assertions (and any parsing done purely to assert) live under Assert. See [gdscript-style — GdUnit test layout (AAA)](gdscript-style.md#gdunit-test-layout-aaa). - **Production script changes:** Any PR that **adds or changes** GDScript under **`client/scripts/`** (and any other **application** `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** tests under **`client/test/`** in the same change set. If a change is **not** reasonably testable, say so in the PR (or story plan) with a short reason—do not skip tests silently. - **Scenes / assets / manual checks:** Scene tweaks, visuals, and flows that unit tests do not cover still deserve **manual verification** notes in the PR when behavior could regress. diff --git a/.vscode/csharp.code-snippets b/.vscode/csharp.code-snippets index 950f8b7..106f6f5 100644 --- a/.vscode/csharp.code-snippets +++ b/.vscode/csharp.code-snippets @@ -8,15 +8,12 @@ "public async Task ${1:MethodName}_Should${2:ExpectedOutcome}_When${3:Scenario}()", "{", " // Arrange", - "", " ${4:// setup}", "", " // Act", - "", " ${5:// execute}", "", " // Assert", - "", " ${6:// verify}", "}" ] @@ -36,15 +33,12 @@ " public async Task ${3:MethodName}_Should${4:ExpectedOutcome}_When${5:Scenario}()", " {", " // Arrange", - "", " ${6:// setup}", "", " // Act", - "", " ${7:// execute}", "", " // Assert", - "", " ${8:// verify}", " }", "}" diff --git a/AGENTS.md b/AGENTS.md index af78752..854468d 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). **Automated tests (AAA, mandatory):** **C#** — every new or changed **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **Arrange → Act → Assert**: the three **`// Arrange` / `// Act` / `// Assert`** labels, a **blank line after each label**, **Act** = behavior under test only, **Assert** = all expectations including **`ReadFromJsonAsync`** (etc.) for verification — [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), [testing-expectations](.cursor/rules/testing-expectations.md); snippets **`xut`** / **`xutc`**. **GdUnit (`client/test/`)** — same AAA with **`# Arrange` / `# Act` / `# Assert`** and blank lines after each — [gdscript-style](.cursor/rules/gdscript-style.md#gdunit-test-layout-aaa). **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). **Automated tests (AAA, mandatory):** **C#** — every new or changed **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **Arrange → Act → Assert**: the three **`// Arrange` / `// Act` / `// Assert`** labels, **Act** = behavior under test only, **Assert** = all expectations including **`ReadFromJsonAsync`** (etc.) for verification (no blank line required after the phase comments) — [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), [testing-expectations](.cursor/rules/testing-expectations.md); snippets **`xut`** / **`xutc`**. **GdUnit (`client/test/`)** — same AAA with **`# Arrange` / `# Act` / `# Assert`** — [gdscript-style](.cursor/rules/gdscript-style.md#gdunit-test-layout-aaa). **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). diff --git a/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt b/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt index 0c5122f..c00c10a 100644 --- a/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt +++ b/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt @@ -2,14 +2,11 @@ public async Task MethodName_ShouldExpectedOutcome_WhenScenario() { // Arrange - // Create factory/client/dependencies and test data. // Act - // Invoke exactly one behavior/scenario under test. // Assert - // Verify outcome(s) with Assert.* calls; read HTTP/deserialized bodies here when verifying responses. } From e6ccc4453c8ea460456fe8a4d32e712424840c50 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 22:09:35 -0400 Subject: [PATCH 5/8] chore: Apply Arrange/Act/Assert across server and client tests. Split HTTP verification reads into Assert in C# API and integration tests. Add # Arrange/# Act/# Assert to GdUnit suites and keep Act free of assertions where casts and busy guards are under test. --- client/test/ability_cast_client_test.gd | 12 ++- client/test/camera_state_test.gd | 9 +- client/test/hotbar_loadout_client_test.gd | 18 ++++ .../test/interactables_catalog_client_test.gd | 9 +- .../test/interaction_request_client_test.gd | 15 ++++ client/test/isometric_follow_camera_test.gd | 86 +++++++++++++++---- client/test/occlusion_policy_test.gd | 51 ++++++++--- client/test/player_idle_support_test.gd | 31 ++++++- client/test/player_locomotion_wasd_test.gd | 42 +++++++++ client/test/player_test.gd | 55 +++++++++++- client/test/position_authority_client_test.gd | 24 ++++++ client/test/selection_event_client_test.gd | 15 ++++ .../test/target_refresh_on_locomotion_test.gd | 11 ++- client/test/target_selection_client_test.gd | 51 +++++++++++ client/test/zoom_band_config_test.gd | 78 +++++++++++++---- .../AbilityInput/HotbarLoadoutApiTests.cs | 16 ++-- ...otbarLoadoutPersistenceIntegrationTests.cs | 3 +- .../Interaction/InteractablesWorldApiTests.cs | 3 +- .../Game/Interaction/InteractionApiTests.cs | 65 ++++++++------ .../Game/PositionState/MoveCommandApiTests.cs | 21 ++--- .../Game/PositionState/MoveStreamApiTests.cs | 26 +++--- .../PostgresPositionStateIntegrationTests.cs | 9 +- .../Game/Targeting/TargetingApiTests.cs | 68 ++++++++------- 23 files changed, 569 insertions(+), 149 deletions(-) diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 29127c7..1f4970b 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -126,11 +126,13 @@ func test_request_cast_while_busy_is_ignored() -> void: var transport := HoldFirstThenAutoTransport.new() var c := _make_client(transport) # Act - 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() + var first_started: bool = bool(c.call("request_cast", 0, "prototype_pulse", null)) + var second_started: bool = bool(c.call("request_cast", 1, "prototype_guard", null)) await get_tree().process_frame await get_tree().process_frame # Assert + assert_that(first_started).is_true() + assert_that(second_started).is_false() assert_that(transport.request_count).is_equal(1) var parsed: Variant = JSON.parse_string(transport.last_body) var body: Dictionary = parsed @@ -149,8 +151,9 @@ func test_cast_result_received_emits_false_with_reason_on_denied() -> void: var got: Array = [] c.connect("cast_result_received", func(accepted: bool, reason: String): got.append([accepted, reason])) # Act - assert_that(bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha"))).is_true() + var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha")) # Assert + assert_that(started).is_true() assert_that(got.size()).is_equal(1) assert_that(bool(got[0][0])).is_false() assert_that(str(got[0][1])).is_equal("invalid_target") @@ -164,8 +167,9 @@ func test_cast_result_received_emits_true_with_empty_reason_on_accepted() -> voi var got: Array = [] c.connect("cast_result_received", func(accepted: bool, reason: String): got.append([accepted, reason])) # Act - assert_that(bool(c.call("request_cast", 0, "prototype_pulse", null))).is_true() + var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", null)) # Assert + assert_that(started).is_true() assert_that(got.size()).is_equal(1) assert_that(bool(got[0][0])).is_true() assert_that(str(got[0][1])).is_equal("") diff --git a/client/test/camera_state_test.gd b/client/test/camera_state_test.gd index e70f23a..0499b96 100644 --- a/client/test/camera_state_test.gd +++ b/client/test/camera_state_test.gd @@ -5,17 +5,24 @@ const CameraStateScript := preload("res://scripts/camera_state.gd") func test_default_yaw_is_zero() -> void: + # Arrange var s = CameraStateScript.new() - assert_that(s.yaw).is_equal(0.0) + # Act + var yaw: float = s.yaw + # Assert + assert_that(yaw).is_equal(0.0) func test_fields_round_trip() -> void: + # Arrange var s = CameraStateScript.new() + # Act s.follow_target_path = NodePath("../Player") s.distance = 18.5 s.zoom_band_index = 2 s.focus_world = Vector3(1.0, 2.0, 3.0) s.yaw = 0.12 + # Assert assert_that(s.follow_target_path).is_equal(NodePath("../Player")) assert_that(s.distance).is_equal(18.5) assert_that(s.zoom_band_index).is_equal(2) diff --git a/client/test/hotbar_loadout_client_test.gd b/client/test/hotbar_loadout_client_test.gd index 0178da6..4cf358d 100644 --- a/client/test/hotbar_loadout_client_test.gd +++ b/client/test/hotbar_loadout_client_test.gd @@ -95,6 +95,7 @@ func _sample_slots_json() -> String: func test_sync_get_applies_slots_to_hotbar_state() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, _sample_slots_json()) var c := _make_client(transport) @@ -102,7 +103,9 @@ func test_sync_get_applies_slots_to_hotbar_state() -> void: auto_free(state) add_child(state) c.call("set_hotbar_state", state) + # Act c.call("request_sync_from_server") + # Assert var slots: Array = state.call("slots_snapshot") assert_that(slots.size()).is_equal(8) assert_that(slots[0]).is_equal("prototype_pulse") @@ -111,6 +114,7 @@ func test_sync_get_applies_slots_to_hotbar_state() -> void: func test_bind_slot_posts_expected_payload() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, @@ -121,7 +125,9 @@ func test_bind_slot_posts_expected_payload() -> void: ) ) var c := _make_client(transport) + # Act c.call("request_bind_slot", 3, "prototype_burst") + # Assert assert_that(transport.last_url).contains("/hotbar-loadout") assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST) var parsed: Variant = JSON.parse_string(transport.last_body) @@ -135,6 +141,7 @@ func test_bind_slot_posts_expected_payload() -> void: func test_denied_update_emits_reason_signal() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, @@ -146,17 +153,22 @@ func test_denied_update_emits_reason_signal() -> void: ) var c := _make_client(transport) monitor_signals(c) + # Act c.call("request_bind_slot", 1, "not_real") + # Assert assert_signal(c).is_emitted("loadout_update_denied") func test_bind_slot_while_busy_is_ignored() -> void: + # Arrange var transport := HoldFirstThenAutoTransport.new() var c := _make_client(transport) + # Act c.call("request_bind_slot", 1, "prototype_pulse") c.call("request_bind_slot", 2, "prototype_guard") await get_tree().process_frame await get_tree().process_frame + # Assert assert_that(transport.request_count).is_equal(1) var parsed: Variant = JSON.parse_string(transport.last_body) assert_that(parsed is Dictionary).is_true() @@ -165,18 +177,24 @@ func test_bind_slot_while_busy_is_ignored() -> void: func test_http_failure_does_not_mutate_cached_loadout() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_CANT_CONNECT, 0, "") var c := _make_client(transport) + # Act c.call("request_sync_from_server") + # Assert var cached: Dictionary = c.call("cached_loadout") assert_that(cached.is_empty()).is_true() func test_non_json_error_response_keeps_cached_loadout_empty() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 500, "oops") var c := _make_client(transport) + # Act c.call("request_sync_from_server") + # Assert var cached: Dictionary = c.call("cached_loadout") assert_that(cached.is_empty()).is_true() diff --git a/client/test/interactables_catalog_client_test.gd b/client/test/interactables_catalog_client_test.gd index bf2e70d..2ff3268 100644 --- a/client/test/interactables_catalog_client_test.gd +++ b/client/test/interactables_catalog_client_test.gd @@ -6,6 +6,7 @@ const _CATALOG_SCRIPT := preload("res://scripts/interactables_catalog_client.gd" func test_parse_catalog_json_orders_and_fields() -> void: + # Arrange var json := ( '{"schemaVersion":1,"interactables":[' + '{"interactableId":"prototype_resource_node_alpha","kind":"resource_node",' @@ -14,7 +15,9 @@ func test_parse_catalog_json_orders_and_fields() -> void: + '"anchor":{"x":0,"y":0.5,"z":0},"interactionRadius":3}' + "]}" ) + # Act var rows: Variant = _CATALOG_SCRIPT.parse_catalog_json(json) + # Assert assert_that(rows is Array).is_true() var arr: Array = rows assert_that(arr.size()).is_equal(2) @@ -29,5 +32,9 @@ func test_parse_catalog_json_orders_and_fields() -> void: func test_parse_catalog_json_returns_empty_on_garbage() -> void: - var rows2: Variant = _CATALOG_SCRIPT.parse_catalog_json("[]") + # Arrange + var garbage := "[]" + # Act + var rows2: Variant = _CATALOG_SCRIPT.parse_catalog_json(garbage) + # Assert assert_that((rows2 as Array).is_empty()).is_true() diff --git a/client/test/interaction_request_client_test.gd b/client/test/interaction_request_client_test.gd index 57f5128..86980bc 100644 --- a/client/test/interaction_request_client_test.gd +++ b/client/test/interaction_request_client_test.gd @@ -66,41 +66,56 @@ func _make_ix(transport: Node) -> Node: func test_post_terminal_body_contains_prototype_terminal() -> void: + # Arrange var transport := MockHttpTransport.new() var ix := _make_ix(transport) + # Act ix.call("_post_interact", "prototype_terminal") + # Assert assert_that(transport.last_body).contains("prototype_terminal") assert_that(transport.last_url).contains("/interact") func test_post_resource_body_contains_resource_node_id() -> void: + # Arrange var transport := MockHttpTransport.new() var ix := _make_ix(transport) + # Act ix.call("_post_interact", "prototype_resource_node_alpha") + # Assert assert_that(transport.last_body).contains("prototype_resource_node_alpha") func test_post_interact_terminal_public_entrypoint() -> void: + # Arrange var transport := MockHttpTransport.new() var ix := _make_ix(transport) + # Act ix.call("post_interact_terminal") + # Assert assert_that(transport.last_body).contains("prototype_terminal") func test_post_interact_resource_public_entrypoint() -> void: + # Arrange var transport := MockHttpTransport.new() var ix := _make_ix(transport) + # Act ix.call("post_interact_resource") + # Assert assert_that(transport.last_body).contains("prototype_resource_node_alpha") func test_last_wins_second_press_before_first_completes() -> void: + # Arrange var transport := HoldFirstThenAutoTransport.new() var ix := _make_ix(transport) + # Act ix.call("_post_interact", "prototype_terminal") ix.call("_post_interact", "prototype_resource_node_alpha") await get_tree().process_frame await get_tree().process_frame + # Assert assert_that(transport.bodies.size()).is_equal(2) assert_that(transport.bodies[0]).contains("prototype_terminal") assert_that(transport.bodies[1]).contains("prototype_resource_node_alpha") diff --git a/client/test/isometric_follow_camera_test.gd b/client/test/isometric_follow_camera_test.gd index 83fde92..c709394 100644 --- a/client/test/isometric_follow_camera_test.gd +++ b/client/test/isometric_follow_camera_test.gd @@ -7,78 +7,134 @@ const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd") func test_desired_eye_matches_previous_static_camera_frame() -> void: + # Arrange var focus := Vector3(-5.0, 0.9, -5.0) + var want := Vector3(12.0, 10.0, 12.0) + # Act var eye: Vector3 = IsoScript.desired_eye_world( focus, 25.709, deg_to_rad(20.693), deg_to_rad(45.0) ) - var want := Vector3(12.0, 10.0, 12.0) + # Assert assert_that(eye.distance_to(want)).is_less(0.06) func test_effective_follow_distance_fallback_when_config_null() -> void: - assert_that(IsoScript.effective_follow_distance(null, 3, 12.5)).is_equal(12.5) + # Arrange + # Act + var d: float = IsoScript.effective_follow_distance(null, 3, 12.5) + # Assert + assert_that(d).is_equal(12.5) func test_effective_follow_distance_uses_config_when_valid() -> void: + # Arrange var cfg = ZoomBandConfigScript.new() cfg.band_distances = PackedFloat32Array([10.0, 20.0, 30.0]) - assert_that(IsoScript.effective_follow_distance(cfg, 1, 99.0)).is_equal(20.0) + # Act + var d: float = IsoScript.effective_follow_distance(cfg, 1, 99.0) + # Assert + assert_that(d).is_equal(20.0) func test_effective_follow_distance_clamps_band_index() -> void: + # Arrange var cfg = ZoomBandConfigScript.new() cfg.band_distances = PackedFloat32Array([10.0, 20.0]) - assert_that(IsoScript.effective_follow_distance(cfg, 99, 1.0)).is_equal(20.0) - assert_that(IsoScript.effective_follow_distance(cfg, -3, 1.0)).is_equal(10.0) + # Act + var high: float = IsoScript.effective_follow_distance(cfg, 99, 1.0) + var low: float = IsoScript.effective_follow_distance(cfg, -3, 1.0) + # Assert + assert_that(high).is_equal(20.0) + assert_that(low).is_equal(10.0) func test_effective_follow_distance_empty_config_bands_falls_back() -> void: + # Arrange var cfg = ZoomBandConfigScript.new() cfg.band_distances = PackedFloat32Array() - assert_that(IsoScript.effective_follow_distance(cfg, 0, 7.5)).is_equal(7.5) + # Act + var d: float = IsoScript.effective_follow_distance(cfg, 0, 7.5) + # Assert + assert_that(d).is_equal(7.5) func test_effective_follow_distance_fallback_when_band_non_positive() -> void: + # Arrange var cfg = ZoomBandConfigScript.new() cfg.band_distances = PackedFloat32Array([10.0, 0.0, 30.0]) - assert_that(IsoScript.effective_follow_distance(cfg, 2, 99.0)).is_equal(99.0) + # Act + var d: float = IsoScript.effective_follow_distance(cfg, 2, 99.0) + # Assert + assert_that(d).is_equal(99.0) func test_occlusion_policy_is_valid_null() -> void: - assert_that(IsoScript.occlusion_policy_is_valid(null)).is_false() + # Arrange + # Act + var ok: bool = IsoScript.occlusion_policy_is_valid(null) + # Assert + assert_that(ok).is_false() func test_occlusion_policy_is_valid_wrong_script() -> void: + # Arrange var wrong = ZoomBandConfigScript.new() - assert_that(IsoScript.occlusion_policy_is_valid(wrong)).is_false() + # Act + var ok: bool = IsoScript.occlusion_policy_is_valid(wrong) + # Assert + assert_that(ok).is_false() func test_occlusion_policy_is_valid_correct_script() -> void: + # Arrange var p = OcclusionPolicyScript.new() - assert_that(IsoScript.occlusion_policy_is_valid(p)).is_true() + # Act + var ok: bool = IsoScript.occlusion_policy_is_valid(p) + # Assert + assert_that(ok).is_true() func test_occlusion_policy_is_valid_false_when_disabled() -> void: + # Arrange var p = OcclusionPolicyScript.new() p.enabled = false - assert_that(IsoScript.occlusion_policy_is_valid(p)).is_false() + # Act + var ok: bool = IsoScript.occlusion_policy_is_valid(p) + # Assert + assert_that(ok).is_false() func test_occluder_override_key_is_valid_null() -> void: - assert_that(IsoScript.occluder_override_key_is_valid(null)).is_false() + # Arrange + # Act + var ok: bool = IsoScript.occluder_override_key_is_valid(null) + # Assert + assert_that(ok).is_false() func test_occluder_override_key_is_valid_non_node() -> void: - assert_that(IsoScript.occluder_override_key_is_valid(42)).is_false() + # Arrange + # Act + var ok: bool = IsoScript.occluder_override_key_is_valid(42) + # Assert + assert_that(ok).is_false() func test_occluder_override_key_is_valid_live_node3d() -> void: + # Arrange var n := Node3D.new() - assert_that(IsoScript.occluder_override_key_is_valid(n)).is_true() + # Act + var ok: bool = IsoScript.occluder_override_key_is_valid(n) + # Assert + assert_that(ok).is_true() n.free() func test_occluder_override_key_is_valid_false_after_free() -> void: + # Arrange var n := Node3D.new() n.free() - assert_that(IsoScript.occluder_override_key_is_valid(n)).is_false() + # Act + var ok: bool = IsoScript.occluder_override_key_is_valid(n) + # Assert + assert_that(ok).is_false() diff --git a/client/test/occlusion_policy_test.gd b/client/test/occlusion_policy_test.gd index c5c59ae..3d0c8e4 100644 --- a/client/test/occlusion_policy_test.gd +++ b/client/test/occlusion_policy_test.gd @@ -5,39 +5,68 @@ const OcclusionPolicyScript := preload("res://scripts/occlusion_policy.gd") func test_defaults_are_valid() -> void: + # Arrange var p = OcclusionPolicyScript.new() - assert_that(p.is_valid()).is_true() + # Act + var ok: bool = p.is_valid() + # Assert + assert_that(ok).is_true() func test_defaults_match_spec() -> void: + # Arrange var p = OcclusionPolicyScript.new() - assert_that(p.enabled).is_true() - assert_that(p.fade_alpha).is_equal(0.25) - assert_that(p.occluder_group).is_equal("occluder") - assert_that(p.occluder_collision_mask).is_equal(1) - assert_that(p.max_occluder_cast_depth).is_equal(4) - assert_that(p.occluder_count_log_threshold).is_equal(0) + # Act + var enabled: bool = p.enabled + var fade: float = p.fade_alpha + var group: String = p.occluder_group + var mask: int = p.occluder_collision_mask + var depth: int = p.max_occluder_cast_depth + var threshold: int = p.occluder_count_log_threshold + # Assert + assert_that(enabled).is_true() + assert_that(fade).is_equal(0.25) + assert_that(group).is_equal("occluder") + assert_that(mask).is_equal(1) + assert_that(depth).is_equal(4) + assert_that(threshold).is_equal(0) func test_is_valid_false_when_disabled() -> void: + # Arrange var p = OcclusionPolicyScript.new() p.enabled = false - assert_that(p.is_valid()).is_false() + # Act + var ok: bool = p.is_valid() + # Assert + assert_that(ok).is_false() func test_is_valid_true_when_fade_alpha_zero() -> void: + # Arrange var p = OcclusionPolicyScript.new() p.fade_alpha = 0.0 - assert_that(p.is_valid()).is_true() + # Act + var ok: bool = p.is_valid() + # Assert + assert_that(ok).is_true() func test_is_valid_false_when_fade_alpha_negative() -> void: + # Arrange var p = OcclusionPolicyScript.new() p.fade_alpha = -0.1 - assert_that(p.is_valid()).is_false() + # Act + var ok: bool = p.is_valid() + # Assert + assert_that(ok).is_false() func test_is_valid_true_at_full_opaque() -> void: + # Arrange var p = OcclusionPolicyScript.new() p.fade_alpha = 1.0 - assert_that(p.is_valid()).is_true() + # Act + var ok: bool = p.is_valid() + # Assert + assert_that(ok).is_true() diff --git a/client/test/player_idle_support_test.gd b/client/test/player_idle_support_test.gd index 2221afa..2800c5f 100644 --- a/client/test/player_idle_support_test.gd +++ b/client/test/player_idle_support_test.gd @@ -5,71 +5,98 @@ const PLAYER_SCRIPT: Script = preload("res://scripts/player.gd") func test_idle_support_is_stable_on_flat_floor_without_wall_contacts() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP] + # Act var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, Vector3.UP, slide_normals, 0) + # Assert assert_that(stable).is_true() func test_idle_support_is_stable_false_when_floor_is_not_flat_enough() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP] # Must be steeper than [member STABLE_IDLE_FLOOR_MIN_UP_DOT] (0.998) *and* below # [member IDLE_SLOPE_STABLE_MIN_UP_DOT] (0.88), or the ramp short-circuit treats support as stable. var tilted_floor: Vector3 = Vector3(0.5, 0.82, 0.0).normalized() + # Act + var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, tilted_floor, slide_normals, 0) + # Assert assert_that(tilted_floor.dot(Vector3.UP)).is_less( PLAYER_SCRIPT.get("IDLE_SLOPE_STABLE_MIN_UP_DOT") ) - var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, tilted_floor, slide_normals, 0) assert_that(stable).is_false() func test_idle_support_is_stable_min_flat_up_dot_parameter() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP] var floor_n: Vector3 = Vector3(0.05, 0.99875, 0.0).normalized() + # Act var strict: bool = PLAYER_SCRIPT.idle_support_is_stable(true, floor_n, slide_normals, 0, 0.999) var holdish: bool = PLAYER_SCRIPT.idle_support_is_stable(true, floor_n, slide_normals, 0, 0.992) + # Assert assert_that(strict).is_false() assert_that(holdish).is_true() func test_idle_support_is_stable_true_when_flat_floor_has_extra_contacts() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP, Vector3(0.0, 0.6, 0.8).normalized()] + # Act var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, Vector3.UP, slide_normals, 0) + # Assert assert_that(stable).is_true() func test_idle_support_is_stable_true_when_loose_ticks_and_ridged_but_floor_still_level() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP, Vector3(1.0, 0.0, 0.0)] + # Act var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, Vector3.UP, slide_normals, 12) + # Assert assert_that(stable).is_true() func test_idle_support_is_stable_false_when_loose_ticks_and_ridged_and_shallow_floor() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP, Vector3(1.0, 0.0, 0.0)] # Normal must be outside the ramp-stable band [IDLE_SLOPE_STABLE_MIN_UP_DOT, IDLE_RIM) or # `idle_ridged_stair_lip_only` is false and the ramp short-circuit returns stable. Use a tread # tilt above the rim dot but still below the ridged hold threshold (0.992). var floor_n: Vector3 = Vector3(0.14, 0.98, 0.0).normalized() + # Act + var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, floor_n, slide_normals, 12) + # Assert var up_dot: float = floor_n.dot(Vector3.UP) assert_that(up_dot).is_greater_equal(PLAYER_SCRIPT.get("IDLE_RIM_MIN_FLOOR_UP_DOT") as float) assert_that(up_dot).is_less(PLAYER_SCRIPT.get("STABLE_IDLE_FLOOR_MIN_UP_DOT") as float) - var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, floor_n, slide_normals, 12) assert_that(stable).is_false() func test_idle_support_is_stable_true_when_loose_ticks_but_flat_open_support() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP] + # Act var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, Vector3.UP, slide_normals, 12) + # Assert assert_that(stable).is_true() func test_idle_support_is_stable_true_when_loose_ticks_and_shallow_seam_not_ridged() -> void: + # Arrange var n2: Vector3 = Vector3(0.0, 0.42, sqrt(1.0 - 0.42 * 0.42)).normalized() var slide_normals: Array[Vector3] = [Vector3.UP, n2] + # Act var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(true, Vector3.UP, slide_normals, 12) + # Assert assert_that(stable).is_true() func test_idle_support_is_stable_false_when_not_on_floor() -> void: + # Arrange var slide_normals: Array[Vector3] = [Vector3.UP] + # Act var stable: bool = PLAYER_SCRIPT.idle_support_is_stable(false, Vector3.UP, slide_normals, 0) + # Assert assert_that(stable).is_false() diff --git a/client/test/player_locomotion_wasd_test.gd b/client/test/player_locomotion_wasd_test.gd index ebfedcf..dfed995 100644 --- a/client/test/player_locomotion_wasd_test.gd +++ b/client/test/player_locomotion_wasd_test.gd @@ -7,108 +7,150 @@ const FLOOR_RAY_FEET_INVALID: float = -1.0e9 func test_evaluate_floor_ray_hit_y_rejects_hit_too_far_below_feet() -> void: + # Arrange var feet_y := 0.0 var hit_y := feet_y - 0.2 var n := Vector3.UP + # Act var out: float = LocomotionWasdScript.evaluate_floor_ray_hit_y(feet_y, hit_y, n, 0.108, 0.42) + # Assert assert_that(out).is_equal(FLOOR_RAY_FEET_INVALID) func test_evaluate_floor_ray_hit_y_rejects_shallow_floor_normal() -> void: + # Arrange var feet_y := 0.0 var hit_y := 0.0 # Up component ~0.37 so dot(Vector3.UP, n) is below min_floor_up_dot (0.42). var n := Vector3(0.85, 0.35, 0.0).normalized() + # Act var out: float = LocomotionWasdScript.evaluate_floor_ray_hit_y(feet_y, hit_y, n, 0.108, 0.42) + # Assert assert_that(out).is_equal(FLOOR_RAY_FEET_INVALID) func test_evaluate_floor_ray_hit_y_accepts_valid_hit() -> void: + # Arrange var feet_y := 0.0 var hit_y := -0.04 var n := Vector3.UP + # Act var out: float = LocomotionWasdScript.evaluate_floor_ray_hit_y(feet_y, hit_y, n, 0.108, 0.42) + # Assert assert_that(out).is_equal(hit_y) func test_median_feet_y_from_samples_empty_is_invalid() -> void: + # Arrange var empty: Array[float] = [] + # Act var out: float = LocomotionWasdScript.median_feet_y_from_samples(empty) + # Assert assert_that(out).is_equal(FLOOR_RAY_FEET_INVALID) func test_median_feet_y_from_samples_single() -> void: + # Arrange var samples: Array[float] = [0.12] + # Act var out: float = LocomotionWasdScript.median_feet_y_from_samples(samples) + # Assert assert_that(out).is_equal(0.12) func test_median_feet_y_from_samples_three_sorted_middle() -> void: + # Arrange var samples: Array[float] = [0.3, 0.1, 0.2] + # Act var out: float = LocomotionWasdScript.median_feet_y_from_samples(samples) + # Assert assert_that(out).is_equal(0.2) func test_median_feet_y_from_samples_two_uses_lower_index() -> void: + # Arrange # Matches runtime: mid index (n-1)>>1 picks the smaller of two sorted values. var samples: Array[float] = [0.5, 0.1] + # Act var out: float = LocomotionWasdScript.median_feet_y_from_samples(samples) + # Assert assert_that(out).is_equal(0.1) func test_xz_after_micro_slip_projection_zero_wish_unchanged() -> void: + # Arrange var anchor := Vector2.ZERO var player_xz := Vector2(0.02, 0.03) + # Act var out: Vector2 = LocomotionWasdScript.xz_after_micro_slip_projection( anchor, player_xz, Vector2.ZERO, 0.032 ) + # Assert assert_that(out).is_equal(player_xz) func test_xz_after_micro_slip_projection_large_perp_unchanged() -> void: + # Arrange var anchor := Vector2.ZERO var player_xz := Vector2(0.0, 0.2) var wish := Vector2(1.0, 0.0) + # Act var out: Vector2 = LocomotionWasdScript.xz_after_micro_slip_projection( anchor, player_xz, wish, 0.032 ) + # Assert assert_that(out).is_equal(player_xz) func test_xz_after_micro_slip_projection_nudges_small_perp() -> void: + # Arrange var anchor := Vector2.ZERO var player_xz := Vector2(0.0, 0.01) var wish := Vector2(1.0, 0.0) + # Act var out: Vector2 = LocomotionWasdScript.xz_after_micro_slip_projection( anchor, player_xz, wish, 0.032 ) + # Assert assert_that(out.x).is_equal(0.0) assert_that(out.y).is_equal(0.0) func test_horizontal_velocity_aligned_to_wish_zero_wish_returns_velocity() -> void: + # Arrange + # Act var out: Vector2 = LocomotionWasdScript.horizontal_velocity_aligned_to_wish( Vector2.ZERO, Vector2(3.0, 4.0), 5.0 ) + # Assert assert_that(out).is_equal(Vector2(3.0, 4.0)) func test_horizontal_velocity_aligned_to_wish_clamps_negative_along_to_zero() -> void: + # Arrange + # Act var out: Vector2 = LocomotionWasdScript.horizontal_velocity_aligned_to_wish( Vector2(1.0, 0.0), Vector2(-2.0, 0.0), 5.0 ) + # Assert assert_that(out).is_equal(Vector2.ZERO) func test_horizontal_velocity_aligned_to_wish_caps_speed() -> void: + # Arrange + # Act var out: Vector2 = LocomotionWasdScript.horizontal_velocity_aligned_to_wish( Vector2(1.0, 0.0), Vector2(10.0, 0.0), 5.0 ) + # Assert assert_that(out).is_equal(Vector2(5.0, 0.0)) func test_horizontal_velocity_aligned_to_wish_preserves_wish_direction() -> void: + # Arrange + # Act var out: Vector2 = LocomotionWasdScript.horizontal_velocity_aligned_to_wish( Vector2(0.0, 1.0), Vector2(0.0, 2.0), 5.0 ) + # Assert assert_that(out).is_equal(Vector2(0.0, 2.0)) diff --git a/client/test/player_test.gd b/client/test/player_test.gd index f87d5c5..5f8d611 100644 --- a/client/test/player_test.gd +++ b/client/test/player_test.gd @@ -16,19 +16,25 @@ func _make_player() -> CharacterBody3D: func test_snap_to_server_resets_goal_position_and_velocity() -> void: + # Arrange var p := _make_player() var target := Vector3(1.0, 0.9, -2.0) + # Act p.snap_to_server(target) + # Assert assert_that(p.global_position).is_equal(target) assert_that(p.velocity).is_equal(Vector3.ZERO) assert_that(p.get("_has_walk_goal")).is_false() func test_set_authoritative_nav_goal_sets_goal_and_target() -> void: + # Arrange var p := _make_player() p.set("_idle_anchor_active", true) var goal := Vector3(10.0, 0.0, 3.0) + # Act p.set_authoritative_nav_goal(goal) + # Assert assert_that(p.get("_has_walk_goal")).is_true() assert_that(p.get("_idle_anchor_active")).is_false() assert_that(p.get("_auth_walk_goal")).is_equal(goal) @@ -37,24 +43,33 @@ func test_set_authoritative_nav_goal_sets_goal_and_target() -> void: func test_set_locomotion_wish_world_xz_normalizes_horizontal() -> void: + # Arrange var p := _make_player() + # Act p.call("set_locomotion_wish_world_xz", Vector3(3.0, 9.0, 4.0)) + # Assert var w: Vector3 = p.get("_locomotion_wish_world_xz") as Vector3 assert_that(w.y).is_equal(0.0) assert_that(absf(w.length_squared() - 1.0)).is_less(0.0001) func test_set_locomotion_wish_world_xz_zero_for_near_zero() -> void: + # Arrange var p := _make_player() + # Act p.call("set_locomotion_wish_world_xz", Vector3(0.0, 1.0, 0.0)) + # Assert assert_that(p.get("_locomotion_wish_world_xz") as Vector3).is_equal(Vector3.ZERO) func test_clear_nav_goal_clears_velocity_and_nav_target() -> void: + # Arrange var p := _make_player() p.velocity = Vector3(1.0, 0.0, 0.0) p.set_authoritative_nav_goal(Vector3(5.0, 0.0, 5.0)) + # Act p.clear_nav_goal() + # Assert assert_that(p.velocity).is_equal(Vector3.ZERO) assert_that(p.get("_has_walk_goal")).is_false() assert_that(p.get("_idle_anchor_active")).is_false() @@ -63,59 +78,83 @@ func test_clear_nav_goal_clears_velocity_and_nav_target() -> void: func test_nav_goal_lifecycle_resets_vert_route_latch() -> void: + # Arrange var p := _make_player() + # Act p.set_authoritative_nav_goal(Vector3(1.0, 0.0, 2.0)) p.set("_walk_vert_route_latched", true) p.clear_nav_goal() + # Assert assert_that(p.get("_walk_vert_route_latched")).is_false() + # Act p.set("_walk_vert_route_latched", true) p.set_authoritative_nav_goal(Vector3(3.0, 0.0, 4.0)) + # Assert assert_that(p.get("_walk_vert_route_latched")).is_false() + # Act p.set("_walk_vert_route_latched", true) p.snap_to_server(Vector3(0.0, 0.9, 0.0)) + # Assert assert_that(p.get("_walk_vert_route_latched")).is_false() func test_hold_idle_anchor_sets_anchor_first_then_clamps_xz() -> void: + # Arrange var p := _make_player() p.global_position = Vector3(1.0, 0.5, 2.0) + # Act p.call("_hold_idle_anchor") + # Assert assert_that(p.get("_idle_anchor_active")).is_true() assert_that(p.get("_idle_anchor_xz")).is_equal(Vector2(1.0, 2.0)) - + # Act p.global_position = Vector3(1.2, 0.7, 2.3) p.call("_hold_idle_anchor") + # Assert assert_that(p.global_position).is_equal(Vector3(1.0, 0.5, 2.0)) func test_snap_to_server_clears_idle_anchor() -> void: + # Arrange var p := _make_player() p.set("_idle_anchor_active", true) p.set("_idle_anchor_xz", Vector2(9.0, 9.0)) + # Act p.snap_to_server(Vector3(3.0, 0.9, 4.0)) + # Assert assert_that(p.get("_idle_anchor_active")).is_false() func test_capsule_feet_y_uses_body_origin_minus_half_height() -> void: + # Arrange + # Act var feet_y: float = PLAYER_SCRIPT.capsule_feet_y(0.545678, 0.5) + # Assert assert_that(absf(feet_y - 0.045678)).is_less(0.000001) func test_vertical_arrival_error_uses_capsule_feet_height() -> void: + # Arrange var goal_y := 0.045678 var body_origin_y := 0.545678 + # Act var err: float = PLAYER_SCRIPT.vertical_arrival_error(goal_y, body_origin_y, 0.5) + # Assert assert_that(absf(err)).is_less(0.000001) func test_vertical_arrival_error_stays_large_for_origin_height_only_match() -> void: + # Arrange var goal_y := 0.545678 var body_origin_y := 0.545678 + # Act var err: float = PLAYER_SCRIPT.vertical_arrival_error(goal_y, body_origin_y, 0.5) + # Assert assert_that(absf(err - 0.5)).is_less(0.000001) func test_arrival_vert_err_large_for_step_goal_from_floor() -> void: + # Arrange # Player at floor height (body Y=0.9). Goal at step surface (Y=0.3). # Using actual capsule bottom (CAPSULE_HALF_HEIGHT + PLAYER_CAPSULE_RADIUS) the error # must exceed VERT_ARRIVE_EPS so the arrival check does NOT fire prematurely. @@ -128,11 +167,14 @@ func test_arrival_vert_err_large_for_step_goal_from_floor() -> void: ) as float ) + # Act var err: float = PLAYER_SCRIPT.vertical_arrival_error(goal_y, body_y, full_half) + # Assert assert_that(err).is_greater(PLAYER_SCRIPT.get("VERT_ARRIVE_EPS") as float) func test_arrival_vert_err_zero_when_standing_on_step() -> void: + # Arrange # Player body centred at goal_y + total_half_height (i.e. actually standing on the step). # Error must be within VERT_ARRIVE_EPS so arrival fires correctly once the player is up. var goal_y := 0.3 @@ -144,20 +186,29 @@ func test_arrival_vert_err_zero_when_standing_on_step() -> void: as float ) var body_y: float = goal_y + full_half + # Act var err: float = PLAYER_SCRIPT.vertical_arrival_error(goal_y, body_y, full_half) + # Assert assert_that(err).is_less_equal(PLAYER_SCRIPT.get("VERT_ARRIVE_EPS") as float) func test_flat_floor_goal_is_not_below_feet_height_margin() -> void: + # Arrange var goal_y := 0.0 var body_origin_y := 0.4 var descend_margin := PLAYER_SCRIPT.get("DESCEND_GOAL_Y_MARGIN") as float + # Act var below_feet_margin: bool = ( goal_y < PLAYER_SCRIPT.capsule_feet_y(body_origin_y, 0.5) - descend_margin ) + # Assert assert_that(below_feet_margin).is_false() func test_step_assist_max_surface_delta_matches_navigation_mesh_agent_max_climb() -> void: + # Arrange # Keep in sync with NavigationMesh `agent_max_climb` in `scenes/main.tscn`. - assert_that(PLAYER_SCRIPT.get("WALK_STEP_ASSIST_MAX_SURFACE_DELTA") as float).is_equal(0.35) + # Act + var delta: float = PLAYER_SCRIPT.get("WALK_STEP_ASSIST_MAX_SURFACE_DELTA") as float + # Assert + assert_that(delta).is_equal(0.35) diff --git a/client/test/position_authority_client_test.gd b/client/test/position_authority_client_test.gd index cbd3f19..89acc39 100644 --- a/client/test/position_authority_client_test.gd +++ b/client/test/position_authority_client_test.gd @@ -57,64 +57,83 @@ func _make_client(http_transport: Node) -> Node: func test_sync_boot_get_200_emits_snap() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"position":{"x":1,"y":0.9,"z":-5}}') var c := _make_client(transport) monitor_signals(c) + # Act c.sync_from_server() + # Assert assert_signal(c).is_emitted("authoritative_position_received", Vector3(1.0, 0.9, -5.0), true) # NEO-24: boot sync also emits the cooldown-throttled heartbeat. func test_sync_boot_get_200_emits_authoritative_ack() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"position":{"x":1,"y":0.9,"z":-5}}') var c := _make_client(transport) monitor_signals(c) + # Act c.sync_from_server() + # Assert assert_signal(c).is_emitted("authoritative_ack", Vector3(1.0, 0.9, -5.0)) func test_sync_boot_get_200_malformed_position_does_not_emit() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"position":"invalid"}') var c := _make_client(transport) monitor_signals(c) + # Act c.sync_from_server() + # Assert assert_signal(c).wait_until(400).is_not_emitted("authoritative_position_received") func test_move_stream_post_400_emits_move_rejected_then_boot_get() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 400, '{"reasonCode":"vertical_step_exceeded"}') transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"position":{"x":0,"y":0.9,"z":0}}') var c := _make_client(transport) monitor_signals(c) + # Act c.submit_stream_targets([Vector3(1.0, 2.0, 3.0)]) + # Assert assert_signal(c).is_emitted("move_rejected", "vertical_step_exceeded") assert_signal(c).is_emitted("authoritative_position_received", Vector3(0.0, 0.9, 0.0), true) func test_move_stream_post_400_without_reason_emits_unknown() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 400, "{}") transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"position":{"x":1,"y":0.9,"z":1}}') var c := _make_client(transport) monitor_signals(c) + # Act c.submit_stream_targets([Vector3.ZERO]) + # Assert assert_signal(c).is_emitted("move_rejected", "unknown") assert_signal(c).is_emitted("authoritative_position_received", Vector3(1.0, 0.9, 1.0), true) func test_second_sync_while_busy_is_ignored() -> void: + # Arrange var transport := HangingHttpTransport.new() var c := _make_client(transport) + # Act c.sync_from_server() c.sync_from_server() + # Assert assert_that(transport.request_count).is_equal(1) func test_move_stream_post_200_does_not_emit_position() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, @@ -126,13 +145,16 @@ func test_move_stream_post_200_does_not_emit_position() -> void: ) var c := _make_client(transport) monitor_signals(c) + # Act c.submit_stream_targets([Vector3(-4.9, 0.9, -5.0)]) + # Assert assert_signal(c).wait_until(400).is_not_emitted("authoritative_position_received") # NEO-24: move-stream 200 must emit `authoritative_ack` so a held target lock can re-validate # during normal locomotion (without re-introducing the snap rubber-band). func test_move_stream_post_200_emits_authoritative_ack() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, @@ -144,5 +166,7 @@ func test_move_stream_post_200_emits_authoritative_ack() -> void: ) var c := _make_client(transport) monitor_signals(c) + # Act c.submit_stream_targets([Vector3(-4.9, 0.9, -5.0)]) + # Assert assert_signal(c).is_emitted("authoritative_ack", Vector3(-4.9, 0.9, -5.0)) diff --git a/client/test/selection_event_client_test.gd b/client/test/selection_event_client_test.gd index 178458b..f0e36a9 100644 --- a/client/test/selection_event_client_test.gd +++ b/client/test/selection_event_client_test.gd @@ -71,6 +71,7 @@ func _select_response_json( func test_tab_lock_change_emits_tab() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -78,7 +79,9 @@ func test_tab_lock_change_emits_tab() -> void: var c := _make_client(transport) var events: Array = [] c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate())) + # Act c.request_tab_next() + # Assert assert_that(events.size()).is_equal(1) var last: Dictionary = events[0] as Dictionary assert_that(last.get("cause", "")).is_equal("tab") @@ -88,6 +91,7 @@ func test_tab_lock_change_emits_tab() -> void: func test_clear_lock_change_emits_clear() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -96,8 +100,10 @@ func test_clear_lock_change_emits_clear() -> void: var c := _make_client(transport) var events: Array = [] c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate())) + # Act c.request_tab_next() c.request_clear_target() + # Assert assert_that(events.size()).is_equal(2) assert_that(int((events[0] as Dictionary).get("sequence", -1))).is_equal(1) assert_that((events[1] as Dictionary).get("cause", "")).is_equal("clear") @@ -107,6 +113,7 @@ func test_clear_lock_change_emits_clear() -> void: func test_get_lock_id_change_emits_server_correction() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -115,8 +122,10 @@ func test_get_lock_id_change_emits_server_correction() -> void: var c := _make_client(transport) var events: Array = [] c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate())) + # Act c.request_tab_next() c.request_sync_from_server() + # Assert assert_that(events.size()).is_equal(2) var last: Dictionary = events[1] as Dictionary assert_that(last.get("cause", "")).is_equal("server_correction") @@ -126,6 +135,7 @@ func test_get_lock_id_change_emits_server_correction() -> void: func test_get_validity_only_change_emits_nothing() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -136,14 +146,17 @@ func test_get_validity_only_change_emits_nothing() -> void: var c := _make_client(transport) var events: Array = [] c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate())) + # Act c.request_tab_next() c.request_sync_from_server() + # Assert assert_that(events.size()).is_equal(1) assert_that((events[0] as Dictionary).get("cause", "")).is_equal("tab") assert_that(int((events[0] as Dictionary).get("sequence", -1))).is_equal(1) func test_denial_without_lock_id_change_emits_nothing() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, @@ -153,5 +166,7 @@ func test_denial_without_lock_id_change_emits_nothing() -> void: var c := _make_client(transport) var events: Array = [] c.selection_event.connect(func(ev: Dictionary) -> void: events.append(ev.duplicate())) + # Act c.request_select_target_id(ALPHA_ID) + # Assert assert_that(events.size()).is_equal(0) diff --git a/client/test/target_refresh_on_locomotion_test.gd b/client/test/target_refresh_on_locomotion_test.gd index 1d642f3..f4f81e6 100644 --- a/client/test/target_refresh_on_locomotion_test.gd +++ b/client/test/target_refresh_on_locomotion_test.gd @@ -83,6 +83,7 @@ func _target_state_json(locked_id: String, validity: String, sequence: int) -> S func test_move_stream_200_triggers_target_refresh_while_locked() -> void: + # Arrange var auth_transport := MockHttpTransport.new() var target_transport := MockHttpTransport.new() @@ -111,15 +112,16 @@ func test_move_stream_200_triggers_target_refresh_while_locked() -> void: # or handler without updating both ends. authority.connect("authoritative_ack", Callable(target_client, "on_authoritative_ack")) - # Establish the lock first. + # Act target_client.request_select_target_id(ALPHA_ID) + # Assert assert_that(target_client.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID) var get_count_baseline: int = target_transport.request_count - # Simulate WASD locomotion: authority POSTs move-stream, server echoes 200. + # Act authority.submit_stream_targets([Vector3(-20.0, 0.9, -20.0)]) - # The target client must have issued exactly one refresh GET in response to the ack. + # Assert var new_requests: int = target_transport.request_count - get_count_baseline assert_that(new_requests).is_equal(1) assert_that(target_transport.last_url).contains("/game/players/dev-local-1/target") @@ -130,6 +132,7 @@ func test_move_stream_200_triggers_target_refresh_while_locked() -> void: func test_move_stream_200_does_not_refresh_without_lock() -> void: + # Arrange var auth_transport := MockHttpTransport.new() var target_transport := MockHttpTransport.new() @@ -143,6 +146,8 @@ func test_move_stream_200_does_not_refresh_without_lock() -> void: var target_client := _make_target_client(target_transport) authority.connect("authoritative_ack", Callable(target_client, "on_authoritative_ack")) + # Act authority.submit_stream_targets([Vector3.ZERO]) + # Assert assert_that(target_transport.request_count).is_equal(0) diff --git a/client/test/target_selection_client_test.gd b/client/test/target_selection_client_test.gd index f8fa9e8..27ad8e3 100644 --- a/client/test/target_selection_client_test.gd +++ b/client/test/target_selection_client_test.gd @@ -83,11 +83,14 @@ func _select_response_json( func test_sync_get_200_emits_state() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(null, "none")) var c := _make_client(transport) monitor_signals(c) + # Act c.request_sync_from_server() + # Assert assert_signal(c).is_emitted("target_state_changed") var state: Dictionary = c.cached_state() assert_that(state.get("validity", "")).is_equal("none") @@ -95,13 +98,16 @@ func test_sync_get_200_emits_state() -> void: func test_tab_from_no_lock_selects_first_id() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) ) var c := _make_client(transport) monitor_signals(c) + # Act c.request_tab_next() + # Assert assert_that(transport.last_url).contains("/target/select") assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST) assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID) @@ -110,6 +116,7 @@ func test_tab_from_no_lock_selects_first_id() -> void: func test_tab_from_alpha_requests_beta() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -118,13 +125,16 @@ func test_tab_from_alpha_requests_beta() -> void: HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, BETA_ID, "ok", 2) ) var c := _make_client(transport) + # Act c.request_tab_next() c.request_tab_next() + # Assert assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID) assert_that(c.cached_state().get("lockedTargetId")).is_equal(BETA_ID) func test_tab_wraps_from_beta_to_alpha() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -136,14 +146,17 @@ func test_tab_wraps_from_beta_to_alpha() -> void: HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 3) ) var c := _make_client(transport) + # Act c.request_tab_next() c.request_tab_next() c.request_tab_next() + # Assert assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID) assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID) func test_denial_reflects_authoritative_target_state() -> void: + # Arrange var transport := MockHttpTransport.new() # Server denies: selectionApplied=false, lockedTargetId=null, reasonCode=out_of_range. transport.enqueue( @@ -152,7 +165,9 @@ func test_denial_reflects_authoritative_target_state() -> void: _select_response_json(false, null, "none", 0, "out_of_range") ) var c := _make_client(transport) + # Act c.request_select_target_id(ALPHA_ID) + # Assert var state: Dictionary = c.cached_state() assert_that(bool(state.get("selectionApplied", true))).is_false() assert_that(state.get("reasonCode", "")).is_equal("out_of_range") @@ -160,16 +175,20 @@ func test_denial_reflects_authoritative_target_state() -> void: func test_clear_issues_post_without_target_id() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, null, "none", 4)) var c := _make_client(transport) + # Act c.request_clear_target() + # Assert assert_that(transport.last_url).contains("/target/select") assert_that(transport.last_method).is_equal(HTTPClient.METHOD_POST) assert_that(transport.last_body).not_contains("targetId") func test_authoritative_ack_while_locked_fires_refresh_get() -> void: + # Arrange var transport := MockHttpTransport.new() # Step 1: lock alpha via POST. transport.enqueue( @@ -180,22 +199,28 @@ func test_authoritative_ack_while_locked_fires_refresh_get() -> void: HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(ALPHA_ID, "out_of_range", 1) ) var c := _make_client(transport) + # Act c.request_select_target_id(ALPHA_ID) var count_before_ack: int = transport.request_count c.on_authoritative_ack(Vector3.ZERO) + # Assert assert_that(transport.request_count).is_equal(count_before_ack + 1) assert_that(c.cached_state().get("validity")).is_equal("out_of_range") assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID) func test_authoritative_ack_without_lock_fires_nothing() -> void: + # Arrange var transport := MockHttpTransport.new() var c := _make_client(transport) + # Act c.on_authoritative_ack(Vector3.ZERO) + # Assert assert_that(transport.request_count).is_equal(0) func test_two_acks_within_cooldown_collapse_to_one_get() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -204,10 +229,12 @@ func test_two_acks_within_cooldown_collapse_to_one_get() -> void: HTTPRequest.RESULT_SUCCESS, 200, _target_state_json(ALPHA_ID, "out_of_range", 1) ) var c := _make_client(transport) + # Act c.request_select_target_id(ALPHA_ID) var count_before: int = transport.request_count c.on_authoritative_ack(Vector3.ZERO) c.on_authoritative_ack(Vector3.ZERO) + # Assert assert_that(transport.request_count).is_equal(count_before + 1) @@ -232,6 +259,7 @@ func _make_player(pos: Vector3) -> Node3D: func test_select_post_kicks_freshness_stream_before_posting() -> void: + # Arrange var transport := MockHttpTransport.new() transport.enqueue( HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) @@ -242,7 +270,9 @@ func test_select_post_kicks_freshness_stream_before_posting() -> void: add_child(authority) var player := _make_player(Vector3(1.5, 0.5, -2.25)) c.set_freshness_kick(authority, player) + # Act c.request_select_target_id(ALPHA_ID) + # Assert # Exactly one freshness submit, matching the pre-POST capsule position. assert_that(authority.submit_count).is_equal(1) assert_that(authority.last_batch.size()).is_equal(1) @@ -252,6 +282,7 @@ func test_select_post_kicks_freshness_stream_before_posting() -> void: func test_select_post_includes_position_hint_when_player_wired() -> void: + # Arrange # NEO-24 follow-up #5: the select POST body must carry the live capsule position as # `positionHint` so the server's range check can bypass any stale stored snap. var transport := MockHttpTransport.new() @@ -266,7 +297,9 @@ func test_select_post_includes_position_hint_when_player_wired() -> void: # drift): 3.0 → "3", 0.5 → "0.5", 5.0 → "5". Re-parse the JSON for precise assertions. var player := _make_player(Vector3(3.0, 0.5, 5.0)) c.set_freshness_kick(authority, player) + # Act c.request_select_target_id(BETA_ID) + # Assert var parsed: Variant = JSON.parse_string(transport.last_body) assert_that(parsed is Dictionary).is_true() var body: Dictionary = parsed @@ -279,6 +312,7 @@ func test_select_post_includes_position_hint_when_player_wired() -> void: func test_select_post_without_freshness_wiring_is_a_noop() -> void: + # Arrange # Regression: tests that construct the client without `set_freshness_kick(...)` must # still POST normally, *without* a `positionHint` (hint is opt-in per wiring). var transport := MockHttpTransport.new() @@ -286,7 +320,9 @@ func test_select_post_without_freshness_wiring_is_a_noop() -> void: HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) ) var c := _make_client(transport) + # Act c.request_select_target_id(ALPHA_ID) + # Assert assert_that(transport.last_url).contains("/target/select") # No `positionHint` key in the body when no player is wired — re-parse so the assertion # is robust against JSON whitespace / key ordering. @@ -297,6 +333,7 @@ func test_select_post_without_freshness_wiring_is_a_noop() -> void: func test_tab_from_no_lock_skips_out_of_range_and_picks_beta() -> void: + # Arrange # Player stands next to beta (well outside alpha's ring). Without the range-aware # tab cycle the server denies with `out_of_range` on alpha — see NEO-24 follow-up #3. var transport := MockHttpTransport.new() @@ -310,12 +347,15 @@ func test_tab_from_no_lock_skips_out_of_range_and_picks_beta() -> void: # Near beta anchor (3, 3), ~1.4 m horizontal — inside 6 m. Far from alpha at (-3, -3). var player := _make_player(Vector3(4.0, 0.5, 4.0)) c.set_freshness_kick(authority, player) + # Act c.request_tab_next() + # Assert assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID) assert_that(c.cached_state().get("lockedTargetId")).is_equal(BETA_ID) func test_tab_from_no_lock_with_nothing_in_range_falls_back_to_first_id() -> void: + # Arrange # No anchors are in range → cycle-order fallback. Server owns the denial reason. var transport := MockHttpTransport.new() transport.enqueue( @@ -329,12 +369,15 @@ func test_tab_from_no_lock_with_nothing_in_range_falls_back_to_first_id() -> voi add_child(authority) var player := _make_player(Vector3(20.0, 0.5, 20.0)) c.set_freshness_kick(authority, player) + # Act c.request_tab_next() + # Assert assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID) assert_that(c.cached_state().get("reasonCode", "")).is_equal("out_of_range") func test_tab_from_locked_alpha_at_origin_swaps_to_beta_when_both_in_range() -> void: + # Arrange # With alpha (-3,-3) and beta (3,3) sharing r=6, origin is in both rings. Locked to # alpha, Tab should request beta (normal cycle behavior; range-aware skip-current # does not interfere when the next id is also in range). @@ -351,13 +394,16 @@ func test_tab_from_locked_alpha_at_origin_swaps_to_beta_when_both_in_range() -> add_child(authority) var player := _make_player(Vector3(0.0, 0.5, 0.0)) c.set_freshness_kick(authority, player) + # Act c.request_tab_next() c.request_tab_next() + # Assert assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID) assert_that(c.cached_state().get("lockedTargetId")).is_equal(BETA_ID) func test_tab_when_only_current_lock_is_in_range_falls_back_to_cycle_for_denial() -> void: + # Arrange # Locked to alpha, only alpha is in range — Tab should express a *swap* intent and # fall through to the plain cycle so the server denies with `out_of_range` (NEO-23 # soft-lock rule). If the picker returned alpha instead, the user would see no @@ -378,14 +424,17 @@ func test_tab_when_only_current_lock_is_in_range_falls_back_to_cycle_for_denial( # Near alpha (-3, -3), ~1.4 m in — beta (3, 3) is ~8.5 m out. var player := _make_player(Vector3(-2.0, 0.5, -2.0)) c.set_freshness_kick(authority, player) + # Act c.request_select_target_id(ALPHA_ID) c.request_tab_next() + # Assert assert_that(transport.last_body).contains('"targetId":"%s"' % BETA_ID) assert_that(c.cached_state().get("reasonCode", "")).is_equal("out_of_range") assert_that(c.cached_state().get("lockedTargetId")).is_equal(ALPHA_ID) func test_tab_without_player_ref_still_uses_plain_cycle_order() -> void: + # Arrange # Regression: headless unit tests and any client that skips `set_freshness_kick(...)` # must keep the pre-existing cycle-order semantics. Alpha is requested from no-lock. var transport := MockHttpTransport.new() @@ -393,5 +442,7 @@ func test_tab_without_player_ref_still_uses_plain_cycle_order() -> void: HTTPRequest.RESULT_SUCCESS, 200, _select_response_json(true, ALPHA_ID, "ok", 1) ) var c := _make_client(transport) + # Act c.request_tab_next() + # Assert assert_that(transport.last_body).contains('"targetId":"%s"' % ALPHA_ID) diff --git a/client/test/zoom_band_config_test.gd b/client/test/zoom_band_config_test.gd index eb8d0fe..617b253 100644 --- a/client/test/zoom_band_config_test.gd +++ b/client/test/zoom_band_config_test.gd @@ -5,65 +5,109 @@ const ZoomBandConfigScript := preload("res://scripts/zoom_band_config.gd") func test_empty_bands_band_count_zero() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array() - assert_that(c.band_count()).is_equal(0) + # Act + var n: int = c.band_count() + # Assert + assert_that(n).is_equal(0) func test_empty_bands_clamp_index_zero() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array() - assert_that(c.clamp_index(0)).is_equal(0) - assert_that(c.clamp_index(99)).is_equal(0) + # Act + var a: int = c.clamp_index(0) + var b: int = c.clamp_index(99) + # Assert + assert_that(a).is_equal(0) + assert_that(b).is_equal(0) func test_empty_bands_distance_at_zero() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array() - assert_that(c.distance_at(0)).is_equal(0.0) + # Act + var d: float = c.distance_at(0) + # Assert + assert_that(d).is_equal(0.0) func test_single_band_clamp_always_zero() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array([42.0]) - assert_that(c.clamp_index(-1)).is_equal(0) - assert_that(c.clamp_index(0)).is_equal(0) - assert_that(c.clamp_index(9)).is_equal(0) + # Act + var a: int = c.clamp_index(-1) + var b: int = c.clamp_index(0) + var cc: int = c.clamp_index(9) + # Assert + assert_that(a).is_equal(0) + assert_that(b).is_equal(0) + assert_that(cc).is_equal(0) func test_multi_band_clamp_and_distance_at() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array([10.0, 20.0, 30.0]) - assert_that(c.clamp_index(-5)).is_equal(0) - assert_that(c.clamp_index(1)).is_equal(1) - assert_that(c.clamp_index(99)).is_equal(2) - assert_that(c.distance_at(1)).is_equal(20.0) - assert_that(c.distance_at(100)).is_equal(30.0) + # Act + var i0: int = c.clamp_index(-5) + var i1: int = c.clamp_index(1) + var i2: int = c.clamp_index(99) + var d1: float = c.distance_at(1) + var d2: float = c.distance_at(100) + # Assert + assert_that(i0).is_equal(0) + assert_that(i1).is_equal(1) + assert_that(i2).is_equal(2) + assert_that(d1).is_equal(20.0) + assert_that(d2).is_equal(30.0) func test_default_index_clamped_via_clamp_index() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array([1.0, 2.0, 3.0]) c.default_band_index = 99 - assert_that(c.clamp_index(c.default_band_index)).is_equal(2) + # Act + var idx: int = c.clamp_index(c.default_band_index) + # Assert + assert_that(idx).is_equal(2) func test_all_band_distances_positive_vacuous_when_empty() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array() - assert_that(c.all_band_distances_positive()).is_true() + # Act + var ok: bool = c.all_band_distances_positive() + # Assert + assert_that(ok).is_true() func test_all_band_distances_positive_true_when_all_positive() -> void: + # Arrange var c = ZoomBandConfigScript.new() c.band_distances = PackedFloat32Array([1.0, 2.5, 10.0]) - assert_that(c.all_band_distances_positive()).is_true() + # Act + var ok: bool = c.all_band_distances_positive() + # Assert + assert_that(ok).is_true() func test_all_band_distances_positive_false_when_zero_or_negative() -> void: + # Arrange var c0 = ZoomBandConfigScript.new() c0.band_distances = PackedFloat32Array([10.0, 0.0, 20.0]) - assert_that(c0.all_band_distances_positive()).is_false() var cn = ZoomBandConfigScript.new() cn.band_distances = PackedFloat32Array([5.0, -1.0]) - assert_that(cn.all_band_distances_positive()).is_false() + # Act + var ok0: bool = c0.all_band_distances_positive() + var okn: bool = cn.all_band_distances_positive() + # Assert + assert_that(ok0).is_false() + assert_that(okn).is_false() diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutApiTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutApiTests.cs index 4dda54c..93f8ad5 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutApiTests.cs @@ -17,9 +17,9 @@ public sealed class HotbarLoadoutApiTests // Act var response = await client.GetAsync("/game/players/dev-local-1/hotbar-loadout"); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); Assert.Equal(HotbarLoadoutResponse.CurrentSchemaVersion, body!.SchemaVersion); @@ -47,10 +47,10 @@ public sealed class HotbarLoadoutApiTests new HotbarSlotBindingJson { SlotIndex = 3, AbilityId = PrototypeAbilityRegistry.PrototypeBurst }, ], }); - var postBody = await post.Content.ReadFromJsonAsync(); - var get = await client.GetFromJsonAsync("/game/players/dev-local-1/hotbar-loadout"); // Assert + var postBody = await post.Content.ReadFromJsonAsync(); + var get = await client.GetFromJsonAsync("/game/players/dev-local-1/hotbar-loadout"); Assert.Equal(HttpStatusCode.OK, post.StatusCode); Assert.NotNull(postBody); Assert.True(postBody!.Updated); @@ -77,9 +77,9 @@ public sealed class HotbarLoadoutApiTests SchemaVersion = HotbarLoadoutUpdateRequest.CurrentSchemaVersion, Slots = [new HotbarSlotBindingJson { SlotIndex = 8, AbilityId = PrototypeAbilityRegistry.PrototypePulse }], }); - var body = await post.Content.ReadFromJsonAsync(); // Assert + var body = await post.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, post.StatusCode); Assert.NotNull(body); Assert.False(body!.Updated); @@ -101,9 +101,9 @@ public sealed class HotbarLoadoutApiTests SchemaVersion = HotbarLoadoutUpdateRequest.CurrentSchemaVersion, Slots = [new HotbarSlotBindingJson { SlotIndex = -1, AbilityId = PrototypeAbilityRegistry.PrototypePulse }], }); - var body = await post.Content.ReadFromJsonAsync(); // Assert + var body = await post.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, post.StatusCode); Assert.NotNull(body); Assert.False(body!.Updated); @@ -125,9 +125,9 @@ public sealed class HotbarLoadoutApiTests SchemaVersion = HotbarLoadoutUpdateRequest.CurrentSchemaVersion, Slots = [new HotbarSlotBindingJson { SlotIndex = 2, AbilityId = "missing_ability" }], }); - var body = await post.Content.ReadFromJsonAsync(); // Assert + var body = await post.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, post.StatusCode); Assert.NotNull(body); Assert.False(body!.Updated); @@ -153,9 +153,9 @@ public sealed class HotbarLoadoutApiTests new HotbarSlotBindingJson { SlotIndex = 1, AbilityId = PrototypeAbilityRegistry.PrototypeGuard }, ], }); - var body = await post.Content.ReadFromJsonAsync(); // Assert + var body = await post.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, post.StatusCode); Assert.NotNull(body); Assert.False(body!.Updated); @@ -177,9 +177,9 @@ public sealed class HotbarLoadoutApiTests SchemaVersion = HotbarLoadoutUpdateRequest.CurrentSchemaVersion, Slots = [new HotbarSlotBindingJson { SlotIndex = 5, AbilityId = " PROTOTYPE_DASH " }], }); - var body = await post.Content.ReadFromJsonAsync(); // Assert + var body = await post.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, post.StatusCode); Assert.NotNull(body); Assert.True(body!.Updated); diff --git a/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs index 0ce6f24..1ea4c61 100644 --- a/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/AbilityInput/HotbarLoadoutPersistenceIntegrationTests.cs @@ -45,12 +45,11 @@ public sealed class HotbarLoadoutPersistenceIntegrationTests(PostgresIntegration postStatus = post.StatusCode; } + // Assert await using var secondFactory = new PostgresWebApplicationFactory(); using var secondClient = secondFactory.CreateClient(); var loadout = await secondClient.GetFromJsonAsync( "/game/players/dev-local-1/hotbar-loadout"); - - // Assert Assert.Equal(HttpStatusCode.OK, postStatus); Assert.NotNull(loadout); Assert.Equal(PrototypeAbilityRegistry.PrototypePulse, loadout!.Slots[1].AbilityId); diff --git a/server/NeonSprawl.Server.Tests/Game/Interaction/InteractablesWorldApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Interaction/InteractablesWorldApiTests.cs index 5431be4..830468c 100644 --- a/server/NeonSprawl.Server.Tests/Game/Interaction/InteractablesWorldApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Interaction/InteractablesWorldApiTests.cs @@ -10,7 +10,8 @@ public class InteractablesWorldApiTests { [Fact] public async Task GetInteractables_ShouldReturnOrderedDescriptors_WithSchemaV1() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); diff --git a/server/NeonSprawl.Server.Tests/Game/Interaction/InteractionApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Interaction/InteractionApiTests.cs index 303ab37..a229e4f 100644 --- a/server/NeonSprawl.Server.Tests/Game/Interaction/InteractionApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Interaction/InteractionApiTests.cs @@ -21,10 +21,9 @@ public class InteractionApiTests SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, Target = new PositionVector { X = 3, Y = 0, Z = 0 }, }; - - // Act var moveResponse = await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/interact", new InteractionRequest @@ -32,9 +31,9 @@ public class InteractionApiTests SchemaVersion = InteractionRequest.CurrentSchemaVersion, InteractableId = PrototypeInteractableRegistry.PrototypeTerminalId, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, moveResponse.StatusCode); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); @@ -43,7 +42,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldAllow_WhenPlayerInHorizontalRange() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -52,10 +52,7 @@ public class InteractionApiTests SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, Target = new PositionVector { X = 0, Y = 0.9, Z = 0 }, }; - // Act var moveResp = await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); - // Assert - Assert.Equal(HttpStatusCode.OK, moveResp.StatusCode); var req = new InteractionRequest { @@ -63,8 +60,11 @@ public class InteractionApiTests InteractableId = PrototypeInteractableRegistry.PrototypeTerminalId, }; + // Act var response = await client.PostAsJsonAsync("/game/players/dev-local-1/interact", req); + // Assert + Assert.Equal(HttpStatusCode.OK, moveResp.StatusCode); Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadFromJsonAsync(); Assert.NotNull(body); @@ -77,7 +77,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldDenyOutOfRange_WhenPlayerSeededFarFromTerminal() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -100,7 +101,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldDenyUnknownInteractable_WhenIdNotInRegistry() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -132,8 +134,6 @@ public class InteractionApiTests SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, Target = new PositionVector { X = 0, Y = 0, Z = 0 }, }; - - // Act var moveResponse = await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); var req = new InteractionRequest @@ -142,10 +142,11 @@ public class InteractionApiTests InteractableId = " PROTOTYPE_TERMINAL ", }; + // Act var response = await client.PostAsJsonAsync("/game/players/dev-local-1/interact", req); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, moveResponse.StatusCode); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); @@ -154,7 +155,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldReturnBadRequest_WhenSchemaVersionWrong() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -173,7 +175,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldReturnBadRequest_WhenInteractableIdMissing() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -191,7 +194,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldReturnBadRequest_WhenInteractableIdWhitespaceOnly() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -210,7 +214,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldReturnNotFound_WhenPlayerUnknown() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -229,12 +234,12 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldReflectNewPosition_AfterMove() - { // Arrange - + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); - // Act + // Act var far = await client.PostAsJsonAsync( "/game/players/dev-local-1/interact", new InteractionRequest @@ -242,18 +247,25 @@ public class InteractionApiTests SchemaVersion = InteractionRequest.CurrentSchemaVersion, InteractableId = PrototypeInteractableRegistry.PrototypeTerminalId, }); - var farBody = await far.Content.ReadFromJsonAsync(); + // Assert + Assert.Equal(HttpStatusCode.OK, far.StatusCode); + var farBody = await far.Content.ReadFromJsonAsync(); Assert.False(farBody!.Allowed); Assert.Equal(InteractionApi.ReasonOutOfRange, farBody.ReasonCode); + // Act var move = new MoveCommandRequest { SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, Target = new PositionVector { X = 0, Y = 0, Z = 0 }, }; - Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync("/game/players/dev-local-1/move", move)).StatusCode); + var moveResponse = await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); + // Assert + Assert.Equal(HttpStatusCode.OK, moveResponse.StatusCode); + + // Act var near = await client.PostAsJsonAsync( "/game/players/dev-local-1/interact", new InteractionRequest @@ -261,6 +273,9 @@ public class InteractionApiTests SchemaVersion = InteractionRequest.CurrentSchemaVersion, InteractableId = PrototypeInteractableRegistry.PrototypeTerminalId, }); + + // Assert + Assert.Equal(HttpStatusCode.OK, near.StatusCode); var nearBody = await near.Content.ReadFromJsonAsync(); Assert.True(nearBody!.Allowed); } @@ -276,10 +291,9 @@ public class InteractionApiTests SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, Target = new PositionVector { X = 12, Y = 0.9, Z = -6 }, }; - - // Act var moveResponse = await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/interact", new InteractionRequest @@ -287,9 +301,9 @@ public class InteractionApiTests SchemaVersion = InteractionRequest.CurrentSchemaVersion, InteractableId = PrototypeInteractableRegistry.PrototypeResourceNodeAlphaId, }); - var body = await response.Content.ReadFromJsonAsync(); // Assert + var body = await response.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, moveResponse.StatusCode); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); @@ -299,7 +313,8 @@ public class InteractionApiTests [Fact] public async Task PostInteract_ShouldDenyOutOfRange_ForResourceNode_WhenPlayerFar() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs index e393286..583ac24 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs @@ -25,18 +25,18 @@ public class MoveCommandApiTests SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, Target = new PositionVector { X = 1.5, Y = 0.0, Z = 2.25 }, }; + var beforeMove = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); // Act - var beforeMove = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); var postResponse = await client.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); - var postBody = await postResponse.Content.ReadFromJsonAsync(); - var afterMove = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); // Assert Assert.NotNull(beforeMove); var seq0 = beforeMove!.Sequence; Assert.Equal(HttpStatusCode.OK, postResponse.StatusCode); + var postBody = await postResponse.Content.ReadFromJsonAsync(); + var afterMove = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); Assert.NotNull(postBody); Assert.Equal(PositionStateResponse.CurrentSchemaVersion, postBody!.SchemaVersion); Assert.Equal("dev-local-1", postBody.PlayerId); @@ -110,7 +110,8 @@ public class MoveCommandApiTests [Fact] public async Task PostMove_ShouldReturn400WithReason_WhenHorizontalStepTooLarge() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory().WithWebHostBuilder(b => { @@ -130,10 +131,9 @@ public class MoveCommandApiTests Target = new PositionVector { X = 0, Y = 0.9, Z = 0 }, }; // Act - var response = await client.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); - // Assert + // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); var body = await response.Content.ReadFromJsonAsync(); Assert.NotNull(body); @@ -143,7 +143,8 @@ public class MoveCommandApiTests [Fact] public async Task PostMove_ShouldReturn400WithReason_WhenVerticalStepTooLarge() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory().WithWebHostBuilder(b => { @@ -159,10 +160,9 @@ public class MoveCommandApiTests Target = new PositionVector { X = -5, Y = 2.0, Z = -5 }, }; // Act - var response = await client.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); - // Assert + // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); var body = await response.Content.ReadFromJsonAsync(); Assert.NotNull(body); @@ -171,7 +171,8 @@ public class MoveCommandApiTests [Fact] public async Task PostMove_ShouldReturn400WithoutReasonBody_WhenSchemaInvalid() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveStreamApiTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveStreamApiTests.cs index 38cf073..7b0e893 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveStreamApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveStreamApiTests.cs @@ -12,7 +12,8 @@ public sealed class MoveStreamApiTests { [Fact] public async Task PostMoveStream_ShouldApplyChainAndIncrementSequence_WhenTwoSmallSteps() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory().WithWebHostBuilder(b => { @@ -26,12 +27,7 @@ public sealed class MoveStreamApiTests }); }); var client = factory.CreateClient(); - // Act var before = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); - // Assert - Assert.NotNull(before); - var seq0 = before!.Sequence; - var body = new MoveStreamRequest { SchemaVersion = MoveStreamRequest.CurrentSchemaVersion, @@ -42,9 +38,13 @@ public sealed class MoveStreamApiTests ], }; + // Act var postResponse = await client.PostAsJsonAsync("/game/players/dev-local-1/move-stream", body); - var postBody = await postResponse.Content.ReadFromJsonAsync(); + // Assert + Assert.NotNull(before); + var seq0 = before!.Sequence; + var postBody = await postResponse.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, postResponse.StatusCode); Assert.NotNull(postBody); Assert.Equal(seq0 + 2, postBody!.Sequence); @@ -55,7 +55,8 @@ public sealed class MoveStreamApiTests [Fact] public async Task PostMoveStream_ShouldReturnNotFound_WhenPlayerIsUnknown() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -74,7 +75,8 @@ public sealed class MoveStreamApiTests [Fact] public async Task PostMoveStream_ShouldReturn400_WhenTargetsEmpty() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -92,7 +94,8 @@ public sealed class MoveStreamApiTests [Fact] public async Task PostMoveStream_ShouldReturn400WithReason_WhenSecondStepExceedsHorizontalLimit() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory().WithWebHostBuilder(b => { @@ -116,10 +119,9 @@ public sealed class MoveStreamApiTests ], }; // Act - var response = await client.PostAsJsonAsync("/game/players/dev-local-1/move-stream", body); - // Assert + // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); var rej = await response.Content.ReadFromJsonAsync(); Assert.NotNull(rej); diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs index cd73678..b0a6ef1 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs @@ -24,16 +24,16 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, Target = new PositionVector { X = 2, Y = 0.5, Z = 3.5 }, }; + var before = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); // Act - var before = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); var post = await client.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); - var postBody = await post.Content.ReadFromJsonAsync(); - var after = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); // Assert Assert.NotNull(before); Assert.Equal(HttpStatusCode.OK, post.StatusCode); + var postBody = await post.Content.ReadFromJsonAsync(); + var after = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); Assert.NotNull(postBody); Assert.NotNull(after); Assert.Equal(PositionStateResponse.CurrentSchemaVersion, postBody!.SchemaVersion); @@ -69,11 +69,10 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar persisted = await post.Content.ReadFromJsonAsync(); } + // Assert await using var secondFactory = new PostgresWebApplicationFactory(); using var secondClient = secondFactory.CreateClient(); var after = await secondClient.GetFromJsonAsync("/game/players/dev-local-1/position"); - - // Assert Assert.Equal(HttpStatusCode.OK, postStatus); Assert.NotNull(persisted); Assert.NotNull(after); diff --git a/server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs index 57d1e51..22d8668 100644 --- a/server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Targeting/TargetingApiTests.cs @@ -12,7 +12,8 @@ public class TargetingApiTests { [Fact] public async Task GetTarget_ShouldReturnNone_WhenNoLock() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -32,7 +33,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldApplyLock_WhenInRange() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -59,7 +61,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldDenyUnknownTarget_WithReasonCode() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -85,7 +88,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldDenyOutOfRange_WhenBetaFarFromSpawn() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -172,23 +176,21 @@ public class TargetingApiTests // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); - // Act - var before = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); + + // Act + var selectResponse = await client.PostAsJsonAsync( + "/game/players/dev-local-1/target/select", + new TargetSelectRequest + { + SchemaVersion = TargetSelectRequest.CurrentSchemaVersion, + TargetId = PrototypeTargetRegistry.PrototypeTargetBetaId, + PositionHint = new PositionVector { X = 3.0, Y = 0.5, Z = 5.0 }, + }); + // Assert Assert.NotNull(before); - - Assert.Equal( - HttpStatusCode.OK, - (await client.PostAsJsonAsync( - "/game/players/dev-local-1/target/select", - new TargetSelectRequest - { - SchemaVersion = TargetSelectRequest.CurrentSchemaVersion, - TargetId = PrototypeTargetRegistry.PrototypeTargetBetaId, - PositionHint = new PositionVector { X = 3.0, Y = 0.5, Z = 5.0 }, - })).StatusCode); - + Assert.Equal(HttpStatusCode.OK, selectResponse.StatusCode); var after = await client.GetFromJsonAsync("/game/players/dev-local-1/position"); Assert.NotNull(after); Assert.Equal(before!.Position.X, after!.Position.X); @@ -220,9 +222,9 @@ public class TargetingApiTests $"{{\"schemaVersion\":{TargetSelectRequest.CurrentSchemaVersion}}}", Encoding.UTF8, "application/json")); - var body = await clear.Content.ReadFromJsonAsync(); // Assert + var body = await clear.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, setResponse.StatusCode); Assert.Equal(HttpStatusCode.OK, clear.StatusCode); Assert.NotNull(body); @@ -256,9 +258,9 @@ public class TargetingApiTests var moveResponse = await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); var get = await client.GetAsync("/game/players/dev-local-1/target"); - var body = await get.Content.ReadFromJsonAsync(); // Assert + var body = await get.Content.ReadFromJsonAsync(); Assert.Equal(HttpStatusCode.OK, selectResponse.StatusCode); Assert.Equal(HttpStatusCode.OK, moveResponse.StatusCode); Assert.Equal(HttpStatusCode.OK, get.StatusCode); @@ -269,7 +271,8 @@ public class TargetingApiTests [Fact] public async Task GetTarget_ShouldReturnNotFound_WhenPlayerUnknown() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -282,7 +285,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldReturnNotFound_WhenPlayerUnknown() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -302,7 +306,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldReturnBadRequest_WhenSchemaVersionWrong() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -322,7 +327,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldReturnBadRequest_WhenTargetIdWhitespaceOnly() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -342,7 +348,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldBeCaseInsensitive_ForTargetId() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -366,7 +373,8 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldStayIdempotent_WhenSameTargetReselect() - { // Arrange + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); @@ -389,11 +397,10 @@ public class TargetingApiTests [Fact] public async Task PostSelect_ShouldDenyOutOfRange_WhenReselectingSameLockWhileOutOfRange() - { // Arrange - + { + // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); - // Act await client.PostAsJsonAsync( "/game/players/dev-local-1/target/select", @@ -410,6 +417,7 @@ public class TargetingApiTests }; await client.PostAsJsonAsync("/game/players/dev-local-1/move", move); + // Act var response = await client.PostAsJsonAsync( "/game/players/dev-local-1/target/select", new TargetSelectRequest @@ -417,8 +425,8 @@ public class TargetingApiTests SchemaVersion = TargetSelectRequest.CurrentSchemaVersion, TargetId = PrototypeTargetRegistry.PrototypeTargetAlphaId, }); - // Assert + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadFromJsonAsync(); Assert.NotNull(body); From c3acbdb373b0263eebd0cec07a596591bb193a8f Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 22:13:11 -0400 Subject: [PATCH 6/8] NEO-28: Add reviewability guideline; mark NEO-28 review suggestions done. Document preferring separate branches/commits for mechanical test-only churn vs ticket-scoped changes. Strike through resolved bullets in the saved review file per project convention. --- .cursor/rules/commit-and-review.md | 9 +++++++ docs/reviews/2026-04-27-NEO-28.md | 43 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 docs/reviews/2026-04-27-NEO-28.md diff --git a/.cursor/rules/commit-and-review.md b/.cursor/rules/commit-and-review.md index f0c5cae..7bc7b84 100644 --- a/.cursor/rules/commit-and-review.md +++ b/.cursor/rules/commit-and-review.md @@ -35,6 +35,15 @@ When the user asks to **open**, **create**, or **file** a pull request (or clear 7. **Upstream** — If **`create_pull_request`** fails because **`head`** is not on **`origin`**, push first (**only** as allowed under **Never push** above—i.e. the same user message asked to open the PR or explicitly to push). 8. **Fallback** — If the GitHub MCP is unavailable or the call fails after push, give **`https://github.com/{owner}/{repo}/pull/new/{head}`** and tell the user to set the PR title to **`NEO-123:`** … . Do not switch to `gh` as an agent fallback. +### Ticket scope vs mechanical churn (reviewability) + +When a story would otherwise mix **ticket-scoped gameplay/API changes** with **broad, cross-suite edits that do not change runtime behavior** (for example: Arrange/Act/Assert layout-only updates, mass snippet or rule doc tweaks), **prefer** one of: + +- A **separate branch or pull request** for the mechanical churn (often `chore:`), based on or merged separately from the feature branch; or +- **Separate commits** on the story branch so history and blame separate behavioral changes from test-only or doc-only batches. + +That keeps review and bisect focused on product risk. + ## Commit message format when a Linear issue applies - Any commit that is **part of implementing or delivering a Linear issue or task** must put the **Linear issue id first** in the subject line, then **`:`**, then the summary (e.g. `NEO-8: persist position state in PostgreSQL`). diff --git a/docs/reviews/2026-04-27-NEO-28.md b/docs/reviews/2026-04-27-NEO-28.md new file mode 100644 index 0000000..aed7734 --- /dev/null +++ b/docs/reviews/2026-04-27-NEO-28.md @@ -0,0 +1,43 @@ +# Code review — NEO-28 + +- **Date:** 2026-04-27 +- **Scope:** Branch `NEO-28-combat-accept-deny-reason-ux` (`origin/main...HEAD`). +- **Base:** `origin/main` + +## Verdict + +Approve with nits. + +## Summary + +This branch lands the NEO-28 cast accept/deny integration end to end: server-side target authority checks (`invalid_target`, `out_of_range`), client-side cast result handling, HUD feedback, and documentation/manual-QA updates. The server path remains authoritative and aligned with existing targeting/position rules, and targeted API tests pass for the expanded deny matrix. The branch also includes broad AAA-layout cleanup and rule updates across existing tests, which is useful but increases review surface area relative to the feature change. + +## Documentation checked + +- `docs/plans/NEO-28-implementation-plan.md` — **matches** (scope, acceptance criteria, and file touchpoints align with implementation). +- `docs/decomposition/modules/module_dependency_register.md` — **matches** (E1.M4 remains In Progress; note references NEO-28). +- `docs/decomposition/modules/E1_M4_AbilityInputScaffold.md` — **matches** (implementation snapshot and Linear row reflect NEO-28 behavior). +- `docs/decomposition/modules/E5_M1_CombatRulesEngine.md` — **matches** (prototype cast accept/deny staging documented as pre-engine behavior). +- `docs/decomposition/modules/documentation_and_implementation_alignment.md` — **matches** (E1.M4 row includes NEO-28 landed state and remaining scope). +- `docs/decomposition/modules/client_server_authority.md` — **N/A for changes** (no conflict observed with server-authoritative cast validation). +- `docs/decomposition/modules/contracts.md` — **N/A for changes** (existing prototype JSON contract approach remains consistent). + +Implementation tracking table/register updates are already included in this branch; no additional status correction appears required for merge. + +## Blocking issues + +None. + +## Suggestions + +1. ~~Consider splitting broad AAA/rule-only churn from ticket-scoped gameplay/API changes in future branches so reviewers can isolate behavior risk more quickly.~~ **Done.** Guideline added under [commit-and-review.md](../../.cursor/rules/commit-and-review.md) (“Ticket scope vs mechanical churn”). + +## Nits + +- ~~Nit: The branch diff is large for a single story because it mixes feature work with cross-suite formatting/rule edits; this is acceptable here but raises cognitive load for future audits and blame.~~ **Done.** Same guideline documents the preferred split for future work; acknowledged for this merge. + +## Verification + +- `dotnet test server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj --filter "FullyQualifiedName~AbilityCastApiTests"` — passed (14/14). +- `git diff --check origin/main...HEAD` — passed. +- Not run: headless Godot/GdUnit suite and manual checklist in `docs/manual-qa/NEO-28.md`. From 0bce6ed74c92f04fcd07bcb1845071327022f7df Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 22:14:47 -0400 Subject: [PATCH 7/8] NEO-28: Wrap long lines for gdlint (ability cast + tests). --- client/scripts/ability_cast_client.gd | 3 ++- client/test/ability_cast_client_test.gd | 12 ++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/client/scripts/ability_cast_client.gd b/client/scripts/ability_cast_client.gd index 6a6bc53..4cc8b57 100644 --- a/client/scripts/ability_cast_client.gd +++ b/client/scripts/ability_cast_client.gd @@ -1,7 +1,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-28: emits [signal cast_result_received] when the response body is JSON +## [AbilityCastResponse] v1. signal cast_result_received(accepted: bool, reason_code: String) @export var base_url: String = "http://127.0.0.1:5253" diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 1f4970b..25df6b7 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -149,7 +149,11 @@ func test_cast_result_received_emits_false_with_reason_on_denied() -> void: ) var c := _make_client(transport) var got: Array = [] - c.connect("cast_result_received", func(accepted: bool, reason: String): got.append([accepted, reason])) + c.connect( + "cast_result_received", + func(accepted: bool, reason: String) -> void: + got.append([accepted, reason]) + ) # Act var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha")) # Assert @@ -165,7 +169,11 @@ func test_cast_result_received_emits_true_with_empty_reason_on_accepted() -> voi transport.enqueue(HTTPRequest.RESULT_SUCCESS, 200, '{"schemaVersion":1,"accepted":true}') var c := _make_client(transport) var got: Array = [] - c.connect("cast_result_received", func(accepted: bool, reason: String): got.append([accepted, reason])) + c.connect( + "cast_result_received", + func(accepted: bool, reason: String) -> void: + got.append([accepted, reason]) + ) # Act var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", null)) # Assert From bbfa761aee6affe0c19114bd925fa6f1aa0da9fb Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 22:15:00 -0400 Subject: [PATCH 8/8] NEO-28: gdformat ability_cast_client_test.gd after connect() wrap. --- client/test/ability_cast_client_test.gd | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/test/ability_cast_client_test.gd b/client/test/ability_cast_client_test.gd index 25df6b7..7c62429 100644 --- a/client/test/ability_cast_client_test.gd +++ b/client/test/ability_cast_client_test.gd @@ -151,8 +151,7 @@ func test_cast_result_received_emits_false_with_reason_on_denied() -> void: var got: Array = [] c.connect( "cast_result_received", - func(accepted: bool, reason: String) -> void: - got.append([accepted, reason]) + func(accepted: bool, reason: String) -> void: got.append([accepted, reason]) ) # Act var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", "prototype_target_alpha")) @@ -171,8 +170,7 @@ func test_cast_result_received_emits_true_with_empty_reason_on_accepted() -> voi var got: Array = [] c.connect( "cast_result_received", - func(accepted: bool, reason: String) -> void: - got.append([accepted, reason]) + func(accepted: bool, reason: String) -> void: got.append([accepted, reason]) ) # Act var started: bool = bool(c.call("request_cast", 0, "prototype_pulse", null))