diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index e41ea2c..92a16c7 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -55,6 +55,34 @@ public sealed class OrderService(IOrderStore store, ILogger log) - Use **`///` XML doc comments** on public APIs (types and members) when behavior or contracts are not obvious. +## Test project layout (`*.Tests`) + +- **Mirror the server project:** place test types under the same relative path as the production code they exercise (e.g. `NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` → `NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs`). +- Use a namespace that matches the folder tree under the test assembly root, e.g. `NeonSprawl.Server.Tests.Game.PositionState` for files in `Game/PositionState/`. + +## Test method naming convention + +- Use **`MethodName_ShouldExpectedOutcome_WhenScenario`** (three segments, **`PascalCase`** inside each segment, separated by **underscores**). +- **`MethodName`:** the behavior or entry under test (SUT method, HTTP operation, or short feature verb)—e.g. `GetPosition`, `PostMove`, `TryApplyMoveTarget`. +- **`ShouldExpectedOutcome`:** the outcome the test proves—e.g. `ShouldReturnNotFound`, `ShouldPersistTargetAndIncrementSequence`. +- **`WhenScenario`:** the condition or inputs that trigger it—e.g. `WhenPlayerIsUnknown`, `WhenDevPlayerPostsValidMove`. +- Omit redundant words when the scenario is already obvious; keep names readable in test runners and failure output. + +```csharp +// Examples +public async Task GetPosition_ShouldReturnNotFound_WhenPlayerIsUnknown() { … } +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. + ## Tooling - Prefer fixes that satisfy built-in **.NET analyzers** / `dotnet format` (if adopted) rather than fighting IDE warnings without reason. diff --git a/.cursor/rules/gdscript-style.md b/.cursor/rules/gdscript-style.md index 3490cce..f816b65 100644 --- a/.cursor/rules/gdscript-style.md +++ b/.cursor/rules/gdscript-style.md @@ -33,6 +33,7 @@ Follow the [Godot GDScript style guide](https://docs.godotengine.org/en/stable/t - **`class_name`** only when the type must be referenced globally; otherwise anonymous `extends` is fine. - Order loosely: `extends` → `class_name` → `signals` → `enums` → `const` → `export/@export` → `onready` → other vars → `_ready` / lifecycle → public methods → private helpers. - Virtual overrides (`_ready`, `_process`, `_physics_process`, etc.): keep small; extract helpers with leading `_`. +- **Scene / main script shape:** For `client/`, follow [godot-client-script-organization](godot-client-script-organization.md) — thin `main.gd`, split picking, networking, and similar concerns into child scripts. ## Comments diff --git a/.cursor/rules/godot-client-script-organization.md b/.cursor/rules/godot-client-script-organization.md new file mode 100644 index 0000000..2f78b5a --- /dev/null +++ b/.cursor/rules/godot-client-script-organization.md @@ -0,0 +1,38 @@ +--- +description: Godot client — keep main.gd thin; split scripts by concern (Neon Sprawl). +globs: "client/**/*.gd" +alwaysApply: false +--- + +# Godot client script organization (Neon Sprawl) + +Avoid a single **monolithic `main.gd`**. Treat the main scene root as a **composer**: wire nodes, `@export` references, and signals—not every subsystem. + +## Where logic lives + +- **`main.gd` (scene root):** Bootstrapping only—attach children, connect signals, optional high-level toggles. If a block grows past ~50 lines or a clear concern appears, **extract** it. +- **Avatar / props:** Keep behavior on the node that owns it (e.g. `player.gd` on `CharacterBody3D`). +- **Cross-cutting features:** Prefer a **child `Node`** (or `Node3D`) with its own script, e.g. ground picking → `ground_pick.gd`, server HTTP sync → `position_authority_client.gd`. Use **signals** or small public methods so `main` does not call into deep implementation details. + +## `class_name` vs headless / CI + +**`class_name`** registers global types only after Godot has imported the project (`.godot/`). That folder is **gitignored** here, so **`godot --headless --path client`** on a fresh clone **fails** if `main.gd` annotates types with `class_name` aliases. Prefer **`Node` / `Node3D` + `connect("signal", …)`** and **`call("method", …)`** for thin composition from the scene root unless the project guarantees a prior editor import. + +## Autoloads + +Use **sparingly** (e.g. shared config many scenes need). Default to **scene-local nodes** until duplication proves an autoload. + +## Granularity + +- **One obvious concern per script** (picking vs network vs locomotion)—not one file per tiny helper. +- **New work:** add `res://scripts/.gd` (or a subfolder when a feature set grows) rather than extending `main.gd` by default. + +## Examples + +```text +✅ main.gd: _ready() → get_node, connect ground_pick.target_selected to _on_target +✅ ground_pick.gd: raycast + walkable check → emit target_selected(Vector3) +❌ main.gd: 300 lines of HTTP + ray math + player tuning in one file +``` + +Aligns with [GDScript style](gdscript-style.md) (small virtual overrides, extract `_` helpers). diff --git a/.cursor/rules/testing-expectations.md b/.cursor/rules/testing-expectations.md index 2ac2742..cfc5b5d 100644 --- a/.cursor/rules/testing-expectations.md +++ b/.cursor/rules/testing-expectations.md @@ -9,6 +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:** use **Arrange / Act / Assert** in every test method; see [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). +- **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. diff --git a/AGENTS.md b/AGENTS.md index 9c5804d..44fabc4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,4 +6,4 @@ 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/`; short chat pointer | -Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). +Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). diff --git a/client/README.md b/client/README.md index 254ac1d..09b21b1 100644 --- a/client/README.md +++ b/client/README.md @@ -5,21 +5,37 @@ Open this **`client/`** directory as a project in **Godot 4.6** (4.x compatible) - Main scene: `scenes/main.tscn` (bootstrap `scripts/main.gd`). - Networking will use **WebSocket** or **TCP** to the C# server; authoritative logic stays on the server per [`docs/architecture/tech_stack.md`](../docs/architecture/tech_stack.md). -## Movement prototype (NS-14) +## Script organization (repo policy) -The main scene includes a **client-only** click-to-move demo: +Do not grow an all-in-one **`main.gd`**. The main scene root should **compose** child nodes and scripts; put picking, server sync, and similar features in **dedicated scripts** (typically under `scripts/`) and connect via **signals** or small APIs. Use **Autoloads** only when several scenes truly need the same global. Full guidance for contributors and tooling: [`.cursor/rules/godot-client-script-organization.md`](../.cursor/rules/godot-client-script-organization.md). + +## Authoritative movement (NS-16) + +With the game server running ([`server/README.md`](../server/README.md)), the client sends a **`MoveCommand`** over HTTP and **snaps** the avatar to the server’s **`PositionState`** after a follow-up **`GET`**. + +- **Scripts:** `scripts/ground_pick.gd` (walkable pick + `target_chosen`), `scripts/position_authority_client.gd` (`PositionAuthorityClient`: POST move, GET verify), thin `scripts/main.gd` wiring. +- **Inspector:** select **`PositionAuthorityClient`** on the main scene to set **`base_url`** (default `http://127.0.0.1:5253`) and **`dev_player_id`** (default `dev-local-1`, must match server `Game:DevPlayerId`). +- On startup, **`sync_from_server()`** runs once so the capsule matches the server before you click. + +### Manual check (NS-16) + +1. From repo root: `cd server/NeonSprawl.Server && dotnet run` (note the URL/port, usually `http://localhost:5253`). +2. If the port differs, set **`base_url`** on **`PositionAuthorityClient`** accordingly (e.g. `http://127.0.0.1:5253`). +3. Open the client in Godot and run the main scene (**F5**). The player should jump to the server’s default position (e.g. origin). +4. **Left-click** the floor: the client POSTs the target, then GETs position; the avatar **snaps** to the authoritative coordinates (no client-side walk to the click). + +If the server is **down**, boot **`GET`** fails silently (check Output for warnings); clicks while a request is in flight are ignored until it finishes. + +## Movement prototype (NS-14, legacy local steering) + +The **`CharacterBody3D`** still uses horizontal steering + **`move_and_slide()`** in **`player.gd`**, but **NS-16** drives placement via **`snap_to_server()`** after each successful sync, so you will mostly see **snapping**, not walking to the click. Obstacle sliding from NS-14 applies only if local goals diverge from server snaps in future work. -- **Left-click** walkable ground (the large floor) to move the capsule avatar; **WASD is not required**. -- Movement is **direct horizontal steering** plus **`move_and_slide()`**: the capsule walks toward the click and **slides along** the center crate instead of pathfinding around it. (A hand-authored `NavigationMesh` was not reliable across Godot versions; **NavigationAgent3D** can return later for routed paths.) - The avatar is on **physics layer 2** with **mask 1** (floor + obstacle on layer 1); the pick ray uses **mask 1** so clicks pass through the avatar and hit the floor. -This behavior is **temporary**: when authoritative movement and `MoveCommand` / `PositionState` exist, the client will follow server state instead of driving navigation locally. +### Manual check (NS-14 remnants) -### Manual check - -1. Run the main scene (**F5**). -2. Click the floor: the avatar walks to the point. -3. Click behind the center crate: the avatar **slides** against the crate (no nav mesh path around it in this build). +1. Run the main scene (**F5**) **with server running** for NS-16 behavior above. +2. Without the server, startup sync does not apply authoritative position; behavior is undefined for this repo’s intended demo—**run the server** for NS-16. ### Clicks still ignored? @@ -30,3 +46,18 @@ In the **Game** dock, **Input** must be active (not the **2D** / **3D** scene-pi 1. Install [Godot 4.x](https://godotengine.org/download). 2. In the project manager, **Import** and select `client/project.godot`. 3. Press **F5** to run the main scene. + +## Godot CLI (Linux smoke test) + +For agents and CI, use the **official Linux x86_64 editor binary** (matches **`config/features`** `4.6` in `project.godot`): + +1. Download [Godot_v4.6-stable_linux.x86_64.zip](https://github.com/godotengine/godot/releases/download/4.6-stable/Godot_v4.6-stable_linux.x86_64.zip) from the [4.6-stable release](https://github.com/godotengine/godot/releases/tag/4.6-stable). +2. Unzip and put the binary on your **`PATH`**, e.g. `~/.local/opt/godot-4.6-stable/` and `ln -s …/Godot_v4.6-stable_linux.x86_64 ~/.local/bin/godot`. +3. Ensure **`~/.local/bin`** is on **`PATH`**, then from **`client/`**: + +```bash +godot --version +godot --headless --path . --quit-after 5 +``` + +A clean checkout has **`client/.godot/`** gitignored, so scripts intentionally avoid **`class_name`** global types that require a full editor import before headless can resolve them. diff --git a/client/scenes/main.tscn b/client/scenes/main.tscn index 2a86b14..c9cdd7c 100644 --- a/client/scenes/main.tscn +++ b/client/scenes/main.tscn @@ -2,6 +2,8 @@ [ext_resource type="Script" uid="uid://bh04b3iify0hd" path="res://scripts/main.gd" id="1_main"] [ext_resource type="Script" uid="uid://1jimgt3d4bjj" path="res://scripts/player.gd" id="2_player"] +[ext_resource type="Script" path="res://scripts/ground_pick.gd" id="3_ground"] +[ext_resource type="Script" path="res://scripts/position_authority_client.gd" id="4_auth"] [sub_resource type="BoxMesh" id="BoxMesh_floor"] size = Vector3(20, 0.2, 20) @@ -70,3 +72,9 @@ shape = SubResource("CapsuleShape3D_player") [node name="MeshInstance3D" type="MeshInstance3D" parent="Player" unique_id=2027034386] mesh = SubResource("CapsuleMesh_player") + +[node name="GroundPick" type="Node3D" parent="." unique_id=2500001] +script = ExtResource("3_ground") + +[node name="PositionAuthorityClient" type="Node" parent="." unique_id=2500002] +script = ExtResource("4_auth") diff --git a/client/scripts/ground_pick.gd b/client/scripts/ground_pick.gd new file mode 100644 index 0000000..c388777 --- /dev/null +++ b/client/scripts/ground_pick.gd @@ -0,0 +1,49 @@ +extends Node3D + +## NS-16: walkable ground ray pick. Emits world target for MoveCommand; camera wired from main. +## (No `class_name` so headless/CI can load the project before `.godot` import exists.) + +signal target_chosen(world: Vector3) + +## Fallback when `Viewport.get_camera_3d()` is null (e.g. some editor setups). Set from `main.gd` in `_ready`. +var fallback_camera: Camera3D + + +func _input(event: InputEvent) -> void: + if event is InputEventMouseButton: + var mb := event as InputEventMouseButton + if mb.button_index == MOUSE_BUTTON_LEFT and mb.pressed: + _try_pick(get_viewport().get_mouse_position()) + + +func _try_pick(screen_pos: Vector2) -> void: + var cam: Camera3D = get_viewport().get_camera_3d() + if cam == null: + cam = fallback_camera + if cam == null: + return + + var origin: Vector3 = cam.project_ray_origin(screen_pos) + var ray_dir: Vector3 = cam.project_ray_normal(screen_pos) + var to: Vector3 = origin + ray_dir * 2000.0 + var query := PhysicsRayQueryParameters3D.create(origin, to) + query.collision_mask = 1 + var space: PhysicsDirectSpaceState3D = get_world_3d().direct_space_state + var hit: Dictionary = space.intersect_ray(query) + if hit.is_empty(): + return + var collider: Variant = hit.get("collider") + if not _collider_is_walkable(collider): + return + var hit_pos: Variant = hit.get("position") + if hit_pos is Vector3: + target_chosen.emit(hit_pos as Vector3) + + +func _collider_is_walkable(collider: Variant) -> bool: + var n: Node = collider as Node + while n: + if n.is_in_group("walkable"): + return true + n = n.get_parent() + return false diff --git a/client/scripts/ground_pick.gd.uid b/client/scripts/ground_pick.gd.uid new file mode 100644 index 0000000..8116d33 --- /dev/null +++ b/client/scripts/ground_pick.gd.uid @@ -0,0 +1 @@ +uid://v0xpntk6uumr diff --git a/client/scripts/main.gd b/client/scripts/main.gd index e52f950..3f28654 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -1,45 +1,23 @@ extends Node3D -## NS-14: client-only click-to-move. Provisional until authoritative movement sync (server / MoveCommand). +## NS-16: composes ground pick + server authority; see `ground_pick.gd` / `position_authority_client.gd`. @onready var _camera: Camera3D = $World/Camera3D @onready var _player: CharacterBody3D = $Player +@onready var _ground_pick: Node3D = $GroundPick +@onready var _authority: Node = $PositionAuthorityClient -func _input(event: InputEvent) -> void: - if event is InputEventMouseButton: - var mb := event as InputEventMouseButton - if mb.button_index == MOUSE_BUTTON_LEFT and mb.pressed: - _try_set_move_target(get_viewport().get_mouse_position()) +func _ready() -> void: + _ground_pick.set("fallback_camera", _camera) + _ground_pick.connect("target_chosen", Callable(self, "_on_target_chosen")) + _authority.connect("authoritative_position_received", Callable(self, "_on_authoritative_position")) + _authority.call("sync_from_server") - -func _try_set_move_target(screen_pos: Vector2) -> void: - var cam: Camera3D = get_viewport().get_camera_3d() - if cam == null: - cam = _camera - var origin: Vector3 = cam.project_ray_origin(screen_pos) - var ray_dir: Vector3 = cam.project_ray_normal(screen_pos) - var to: Vector3 = origin + ray_dir * 2000.0 - var query := PhysicsRayQueryParameters3D.create(origin, to) - query.collision_mask = 1 - var space: PhysicsDirectSpaceState3D = get_world_3d().direct_space_state - var hit: Dictionary = space.intersect_ray(query) - if hit.is_empty(): - return - var collider: Variant = hit.get("collider") - if not _collider_is_walkable(collider): - return - var hit_pos: Variant = hit.get("position") - if hit_pos is Vector3: - _player.set_move_goal(hit_pos as Vector3) +func _on_target_chosen(world: Vector3) -> void: + _authority.call("submit_move_target", world) - -func _collider_is_walkable(collider: Variant) -> bool: - var n: Node = collider as Node - while n: - if n.is_in_group("walkable"): - return true - n = n.get_parent() - return false +func _on_authoritative_position(world: Vector3) -> void: + _player.snap_to_server(world) diff --git a/client/scripts/player.gd b/client/scripts/player.gd index 2c42033..8a9db56 100644 --- a/client/scripts/player.gd +++ b/client/scripts/player.gd @@ -19,6 +19,13 @@ func set_move_goal(world_pos: Vector3) -> void: _goal.y = global_position.y +## NS-16: snap to authoritative server position; clears velocity so `move_and_slide` does not fight the sync. +func snap_to_server(world_pos: Vector3) -> void: + global_position = world_pos + velocity = Vector3.ZERO + _goal = world_pos + + func _physics_process(_delta: float) -> void: var to_goal: Vector3 = _goal - global_position to_goal.y = 0.0 diff --git a/client/scripts/position_authority_client.gd b/client/scripts/position_authority_client.gd new file mode 100644 index 0000000..085ff46 --- /dev/null +++ b/client/scripts/position_authority_client.gd @@ -0,0 +1,105 @@ +extends Node + +## NS-16: POST MoveCommand then GET PositionState; emits authoritative world position for the avatar. +## (No `class_name` so headless/CI can load the project before `.godot` import exists.) + +signal authoritative_position_received(world: Vector3) + +@export var base_url: String = "http://127.0.0.1:5253" +@export var dev_player_id: String = "dev-local-1" + +var _http: HTTPRequest +var _busy: bool = false + +enum _Phase { BOOT_GET, POST_MOVE, VERIFY_GET } +var _phase: _Phase = _Phase.BOOT_GET + + +func _ready() -> void: + _http = HTTPRequest.new() + add_child(_http) + _http.request_completed.connect(_on_request_completed) + + +func sync_from_server() -> void: + if _busy: + return + _busy = true + _phase = _Phase.BOOT_GET + _request_get() + + +func submit_move_target(world: Vector3) -> void: + if _busy: + return + _busy = true + _phase = _Phase.POST_MOVE + var url := "%s/game/players/%s/move" % [_base_root(), _player_path_segment()] + var payload := { + "schemaVersion": 1, + "target": {"x": world.x, "y": world.y, "z": world.z}, + } + var body := JSON.stringify(payload) + var headers := PackedStringArray(["Content-Type: application/json"]) + var err: Error = _http.request(url, headers, HTTPClient.METHOD_POST, body) + if err != OK: + push_warning("PositionAuthorityClient: POST failed to start (%s)" % err) + _busy = false + + +func _base_root() -> String: + return base_url.strip_edges().rstrip("/") + + +func _player_path_segment() -> String: + # ASCII-safe ids (e.g. dev-local-1); use percent-encoding if ids gain reserved URL characters. + return dev_player_id.strip_edges() + + +func _request_get() -> void: + var url := "%s/game/players/%s/position" % [_base_root(), _player_path_segment()] + var err: Error = _http.request(url) + if err != OK: + push_warning("PositionAuthorityClient: GET failed to start (%s)" % err) + _busy = false + + +func _on_request_completed(_result: int, response_code: int, _headers: PackedStringArray, body: PackedByteArray) -> void: + if _result != HTTPRequest.RESULT_SUCCESS: + _busy = false + return + + var text := body.get_string_from_utf8() + + match _phase: + _Phase.BOOT_GET: + _busy = false + if response_code == 200: + _emit_position_from_response(text) + _Phase.POST_MOVE: + if response_code != 200: + _busy = false + return + _phase = _Phase.VERIFY_GET + _request_get() + _Phase.VERIFY_GET: + _busy = false + if response_code == 200: + _emit_position_from_response(text) + + +func _emit_position_from_response(json_text: String) -> void: + var parsed: Variant = JSON.parse_string(json_text) + if parsed == null: + return + if not parsed is Dictionary: + return + var data: Dictionary = parsed + var pos_variant: Variant = data.get("position", null) + if not pos_variant is Dictionary: + return + var pos: Dictionary = pos_variant + var x: float = float(pos.get("x", 0.0)) + var y: float = float(pos.get("y", 0.0)) + var z: float = float(pos.get("z", 0.0)) + authoritative_position_received.emit(Vector3(x, y, z)) diff --git a/client/scripts/position_authority_client.gd.uid b/client/scripts/position_authority_client.gd.uid new file mode 100644 index 0000000..9266167 --- /dev/null +++ b/client/scripts/position_authority_client.gd.uid @@ -0,0 +1 @@ +uid://ds5fkbscljkxi diff --git a/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md b/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md index f1e6b43..a1757d2 100644 --- a/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md +++ b/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md @@ -33,8 +33,8 @@ Contract readiness is tracked in the [module dependency register](module_depende ## Implementation snapshot -- **Done (spike):** Server-side authoritative **read** API for `PositionState` (JSON over HTTP, versioned payload); in-memory store. See [NS-15 implementation plan](../../plans/NS-15-implementation-plan.md), `server/NeonSprawl.Server/Game/PositionState/`, and [server README — Position state](../../../server/README.md#position-state-ns-15). -- **Not yet:** `MoveCommand` ingestion, client sync, persistence, full movement loop per Epic 1 Slice 1. +- **Done (prototype):** Server-side authoritative **`PositionState`** read + **`MoveCommand`** apply (HTTP JSON v1, snap-to-target, `sequence` increments); in-memory store; Godot client submits move and snaps to server after GET ([NS-15](../../plans/NS-15-implementation-plan.md), [NS-16](../../plans/NS-16-implementation-plan.md), `server/NeonSprawl.Server/Game/PositionState/`, `client/scripts/`). +- **Not yet:** Persistence, prediction/reconciliation, full Epic 1 Slice 1 movement loop and telemetry. - **Alignment:** [Documentation and implementation alignment](documentation_and_implementation_alignment.md). ## Module dependencies diff --git a/docs/decomposition/modules/documentation_and_implementation_alignment.md b/docs/decomposition/modules/documentation_and_implementation_alignment.md index 7a24879..a6aec40 100644 --- a/docs/decomposition/modules/documentation_and_implementation_alignment.md +++ b/docs/decomposition/modules/documentation_and_implementation_alignment.md @@ -45,7 +45,7 @@ Rows appear when work starts; default for unlisted modules is **Planned** / not | Module | Register status | Snapshot | Plans / pointers | |--------|-----------------|----------|-------------------| -| E1.M1 | In Progress | Authoritative **read** `PositionState` over HTTP (JSON spike); in-memory store; no `MoveCommand` / client sync yet. | [NS-15 plan](../../plans/NS-15-implementation-plan.md); code: `server/NeonSprawl.Server/Game/PositionState/`; [server README — Position state](../../../server/README.md#position-state-ns-15) | +| E1.M1 | In Progress | Authoritative `PositionState` + **MoveCommand** over HTTP (JSON v1 snap); in-memory store; Godot client sync (NS-16). Persistence / prediction / full slice still open. | [NS-15](../../plans/NS-15-implementation-plan.md), [NS-16](../../plans/NS-16-implementation-plan.md); `server/NeonSprawl.Server/Game/PositionState/`; [server README — Position / move](../../../server/README.md) | --- diff --git a/docs/plans/NS-15-implementation-plan.md b/docs/plans/NS-15-implementation-plan.md index e8f5dbe..f5d3e43 100644 --- a/docs/plans/NS-15-implementation-plan.md +++ b/docs/plans/NS-15-implementation-plan.md @@ -62,7 +62,7 @@ | Path | Purpose | |------|--------| | `server/NeonSprawl.Server.Tests/NeonSprawl.Server.Tests.csproj` | xUnit + `Microsoft.AspNetCore.Mvc.Testing` test project. | -| `server/NeonSprawl.Server.Tests/PositionStateApiTests.cs` (or similar) | Integration test(s) for `GET /game/players/{id}/position`. | +| `server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs` | Integration test(s) for `GET /game/players/{id}/position`. | | `server/NeonSprawl.Server/...` DTOs / contracts | e.g. `PositionStateResponse.cs` (or folder `Game/PositionState/`) — versioned JSON shape. | | `server/NeonSprawl.Server/...` `IPositionStateStore` + in-memory implementation | Authority + seeding from options. | | `server/NeonSprawl.Server/...PositionStateServiceCollectionExtensions.cs` (or `Game/DependencyInjection/…`) | `AddPositionStateStore(IServiceCollection, IConfiguration)` (or options binding); **all** registration for this feature, not `Program.cs`. | @@ -115,7 +115,7 @@ Jira NS-15 asks for stable JSON **field names** documented for reviewers outside | `position.x` | number | X coordinate. | | `position.y` | number | Y coordinate. | | `position.z` | number | Z coordinate. | -| `sequence` | number (integer) | Ordering hint for future sync; v1 is `0`. | +| `sequence` | number (integer) | Increments on each successful move apply ([NS-16](NS-16-implementation-plan.md)); `0` until first move for the seeded dev player. | **v1 omissions:** `facing` is not included (see Open questions). JSON uses **camelCase** property names (ASP.NET Core default serialization). diff --git a/docs/plans/NS-16-implementation-plan.md b/docs/plans/NS-16-implementation-plan.md new file mode 100644 index 0000000..8327234 --- /dev/null +++ b/docs/plans/NS-16-implementation-plan.md @@ -0,0 +1,127 @@ +# NS-16 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NS-16 | +| **Title** | E1.M1: Submit MoveCommand → server updates PositionState | +| **Jira** | [NS-16](https://neon-sprawl.atlassian.net/browse/NS-16) | +| **Parent context** | [NS-10 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NS-10) | + +## Goal, scope, and out-of-scope + +**Goal:** End-to-end **intent → authority**: the client sends a move destination; the server updates in-memory state; the client can read back authoritative `PositionState`. + +**In scope** + +- **Server:** HTTP endpoint accepting a **MoveCommand** JSON payload: at minimum **target position** + **player id** (route and/or body; dev session using configured `Game:DevPlayerId` is sufficient). +- **Server rule (document in plan + XML on types):** apply movement with a single clear rule. **Planned v1 rule:** **snap** authoritative position to the command’s target immediately (no fixed-step simulation). Increment **`sequence`** on each successful apply so consumers can detect updates (NS-15 left `sequence` at `0`; this story advances semantics). +- **Client:** Build on [NS-14](NS-14-implementation-plan.md) click-to-move: add a minimal **HTTP** client to **submit** the command and **confirm** state via **`GET`** (poll or follow-up GET after POST — both satisfy “read back”). +- **Wire format:** JSON for the prototype (explicit throwaway per [`docs/decomposition/modules/contracts.md`](../decomposition/modules/contracts.md) until protobuf). + +**Out of scope (per Jira)** + +- Prediction / reconciliation, PostgreSQL, speed anti-cheat. + +**Dependencies** + +- [NS-15](NS-15-implementation-plan.md): in-memory `PositionState` **read** API and store. +- [NS-14](NS-14-implementation-plan.md): click-to-move scaffold. + +## Acceptance criteria checklist + +- [ ] After sending a command, subsequent **`GET /game/players/{id}/position`** reflects the new authoritative position. +- [ ] Happy path demo: click in Godot → HTTP request → server → **displayed** avatar position matches server (snapping acceptable; no smooth interpolation required). +- [ ] Request/response shapes documented as **v1 `MoveCommand`** and **`PositionState`** (XML on DTOs + `server/README.md` / PR blurb; **reuse** existing `PositionStateResponse` shape for successful command responses where practical so one authoritative snapshot type stays consistent). + +## Technical approach + +1. **MoveCommand (v1 JSON)** + - New request DTO (e.g. `MoveCommandRequest`) with top-level **`schemaVersion`** (`1`) and **`target`**: object with **`x`**, **`y`**, **`z`** (same numeric style as `PositionVector`). + - **Endpoint:** `POST /game/players/{id}/move` with JSON body; `{id}` identifies the player (trimmed, case-insensitive match to store, consistent with NS-15 `GET`). + - **Responses:** **200** + body same shape as **`PositionStateResponse`** after apply; **404** if player unknown; **400** if `schemaVersion` ≠ 1 or payload invalid / missing `target`. + +2. **Store mutation** + - Extend **`IPositionStateStore`** with an apply method (e.g. `TryApplyMoveTarget`) that updates position and increments **`sequence`**, using **`ConcurrentDictionary`**-safe update (e.g. `TryUpdate` loop) consistent with `InMemoryPositionStateStore`. + +3. **Routing** + - Add the new route inside existing **`PositionStateApi.MapPositionStateApi()`** (or equivalent dedicated class); keep **`Program.cs`** limited to extension calls only. + +4. **Tests** (`NeonSprawl.Server.Tests`) + - Integration: `POST` move for dev player → **200**, body position matches target and **`sequence`** increased; then **`GET`** matches. + - Unknown player → **404**; bad `schemaVersion` → **400**. + +5. **Godot client** — **Split scripts by concern** (required repo policy: [godot-client-script-organization.md](../../.cursor/rules/godot-client-script-organization.md)); do **not** grow a monolithic `main.gd` with pick + HTTP + wiring. + - **`ground_pick.gd`** on a new **`Node3D`** child (e.g. `GroundPick`): owns walkable **ray pick** (logic moved out of NS-14 `main.gd`). Exposes **`target_chosen(Vector3)`** when the user clicks valid ground. **`main.gd`** sets **`fallback_camera`** (or equivalent) in `_ready`. + - **`position_authority_client.gd`** on a new **`Node`** child: `@export` **base URL** (default `http://127.0.0.1:5253`, match `launchSettings.json`) and **dev player id** (`dev-local-1`). Creates **`HTTPRequest`** in **`_ready`** via **`add_child`** (not as a node hand-placed in `main.tscn`). Flow: **`POST`** `MoveCommand`, then **`GET`** position (tests must show **GET** reflects apply). Emits **`authoritative_position_received(Vector3)`** so **`main`** only snaps the player. + - **`main.gd`:** Thin composer only—connect signals, call **`sync_from_server()`** on run, delegate pick and HTTP to the two child scripts. + - Align **client README** with NS-14 note: server authority path replaces pure local steering for this demo. + +6. **Documentation** + - **`server/README.md`:** new subsection for move submission: method, path, sample **`curl`**, v1 snap rule, link to DTO XML. + - **`client/README.md`:** how to run server + client for NS-16 demo, exported vars, expectation of snap-to-server. + +## Files to add + +| Path | Purpose | +|------|--------| +| `server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs` | Versioned JSON request DTO + XML contract blurb for v1 `MoveCommand`. | +| `client/scripts/ground_pick.gd` | Walkable ground ray pick + **`target_chosen`** signal; **moved out of `main.gd`** per client script organization policy. | +| `client/scripts/position_authority_client.gd` | HTTP **`POST`** move + **`GET`** position; **`HTTPRequest`** created in **`_ready`** (`add_child`); config exports; signal when authoritative position is known. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs` | Declare authoritative apply API. | +| `server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs` | Implement snap + sequence increment. | +| `server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` | Map `POST /game/players/{id}/move`. | +| `server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs` | XML: clarify **`sequence`** increments after moves (post–NS-15). | +| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs` | `POST` + `GET` integration and error cases. | +| `server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs` | Existing `GET` integration tests (same feature folder). | +| `client/scripts/main.gd` | **Thin composer:** add/wire `GroundPick` + `PositionAuthorityClient`, connect signals, snap player to server (remove inlined pick + HTTP). | +| `client/scenes/main.tscn` | Add child nodes that attach **`ground_pick.gd`** and **`position_authority_client.gd`** only. **Do not** add **`HTTPRequest`** to the scene—the authority script owns and instantiates it. | +| `client/scripts/player.gd` | Only if needed (e.g. **`velocity = ZERO`** on snap, or helper **`snap_to(world_pos)`**); prefer minimal changes. | +| `server/README.md` | Move command + curl + v1 behavior. | +| `client/README.md` | NS-16 runbook and authority note. | + +## Tests + +- **Automated:** xUnit integration tests via `WebApplicationFactory` — `POST` then `GET` equality, `404` / `400` as above. +- **Manual:** Godot F5 with server running; click floor; avatar snaps to server-reported position. +- **No new client automated test harness** in scope unless one already exists (none planned for NS-16). + +## Pull request description (Jira AC) — draft for merge + +Paste when opening the PR (adjust if shapes differ slightly in implementation). + +**MoveCommand v1 — request** + +- **Endpoint:** `POST /game/players/{id}/move` +- **Body (example):** + +```json +{ + "schemaVersion": 1, + "target": { "x": 1.5, "y": 0, "z": -2.0 } +} +``` + +| JSON property | Type (v1) | Meaning | +|---------------|-----------|---------| +| `schemaVersion` | integer | Must be `1` for v1. | +| `target` | object | Destination world position. | +| `target.x` / `y` / `z` | number | Coordinates (snap applied server-side). | + +**PositionState v1 — response** (unchanged from NS-15; **`sequence`** increments after each successful move) + +- Same as NS-15 **`GET`** response body; successful **`POST`** returns the updated snapshot. + +**Server rule:** position is set to **`target`** immediately (**snap**); **`sequence`** increases by one per successful apply. + +## Open questions / risks + +- **Concurrent clicks from the client:** minimal approach is last-write-wins on the server; optional client-side “in flight” flag to reduce spam. +- **Axis / height:** server stores full **`y`** from the command; client should send the same **`y`** as the ray hit (or document if server flattens — plan is **no flatten** unless playtest demands). +- **WebSocket:** explicitly deferred; HTTP only for this story. diff --git a/docs/reviews/2026-03-30-NS-16.md b/docs/reviews/2026-03-30-NS-16.md new file mode 100644 index 0000000..a0a2278 --- /dev/null +++ b/docs/reviews/2026-03-30-NS-16.md @@ -0,0 +1,54 @@ +# Code review: NS-16 (MoveCommand → PositionState) + +**Date:** 2026-03-30 +**Scope:** Branch `NS-16-move-command-position-state` vs `origin/main` (working tree / uncommitted changes included at review time). +**Issue:** [NS-16](https://neon-sprawl.atlassian.net/browse/NS-16) — E1.M1: Submit MoveCommand → server updates PositionState +**Base:** `origin/main` + +## Verdict + +**Approve** (after removing machine-specific `launchSettings.json` entries and refreshing decomposition docs; see below). + +## Summary + +The branch delivers NS-16 end-to-end: **`POST /game/players/{id}/move`** with versioned **`MoveCommandRequest`**, thread-safe snap + **`sequence`** in **`InMemoryPositionStateStore`**, **`PositionStateResponse`** on success, integration tests under **`Game/PositionState/`** with **`MethodName_ShouldExpectedOutcome_WhenScenario`** naming and AAA layout, and a Godot client split into **`ground_pick.gd`**, **`position_authority_client.gd`**, and a thin **`main.gd`**. **`UseAppHost=false`** for Debug avoids Linux apphost probing **`/usr/share/dotnet`** when .NET 10 lives under **`~/.dotnet`**. Overall risk is **low** for a prototype slice; there is **no auth** on move (acceptable for the story). Repo rules and READMEs were extended for tests, Godot layout, and Linux/Rider notes. + +## Documentation checked + +| Document | Result | +|----------|--------| +| `docs/plans/NS-16-implementation-plan.md` | **Matches** — endpoint, snap rule, client split, tests, README expectations align with implementation. | +| `docs/plans/NS-15-implementation-plan.md` | **Partially matches** — PR table row for **`sequence`** was stale (“v1 is 0” only); **updated** in this review pass to reference NS-16 semantics. | +| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Partially matched → updated** — snapshot still said “Not yet: MoveCommand”; **updated** to reflect NS-16. | +| `docs/decomposition/modules/documentation_and_implementation_alignment.md` | **Partially matched → updated** — E1.M1 row still said no MoveCommand/client sync; **updated**. | +| `docs/decomposition/modules/module_dependency_register.md` | **N/A** — no row text change required for this review beyond alignment table (register Status unchanged **In Progress**). | +| `docs/decomposition/modules/contracts.md` | **Matches** — JSON spike / throwaway until protobuf acknowledged in code comments. | +| `docs/decomposition/modules/client_server_authority.md` | **Matches** — server owns **`PositionState`**; client sends intent via HTTP. | + +## Blocking issues + +1. **`launchSettings.json` contained user-local `DOTNET_ROOT` and `PATH`** (`/home/don/.dotnet`, …) — unsuitable for shared repo and redundant with **Debug `UseAppHost=false`**. **Removed** as part of closing this review so the branch is safe to merge. + +*(No remaining blockers after that change.)* + +## Suggestions + +1. **Jira / plan checkboxes** — `docs/plans/NS-16-implementation-plan.md` acceptance bullets are still unchecked; tick them when you close NS-16 or paste PR proof in Jira. +2. **`PositionAuthorityClient` boot failure** — if the initial GET fails, the client fails quietly (warnings only); acceptable for the spike; consider a single user-visible error later. +3. **Observability** — server does not log failed moves or bad payloads; fine for prototype; add when debugging multiplayer. + +## Nits + +*(None remaining.)* + +**Resolved after review:** + +- NS-16 plan now states explicitly that scripts are **split per client policy**, and **`HTTPRequest`** is created in **`position_authority_client._ready`** (not placed in **`main.tscn`**). +- **`PositionAuthorityClient`** reuses **`_request_get()`** for the post-POST verify GET instead of duplicating the position URL. + +## Verification + +- `dotnet build NeonSprawl.sln -c Release` +- `dotnet test NeonSprawl.sln -c Release` +- Godot: `godot --headless --path client --quit-after 5` (with Godot 4.6 on `PATH`) +- Manual: server `dotnet run`, client F5, click floor → snap to server position diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs new file mode 100644 index 0000000..58e3f51 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs @@ -0,0 +1,109 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text; +using Microsoft.AspNetCore.Mvc.Testing; +using NeonSprawl.Server.Game.PositionState; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.PositionState; + +/// +/// NS-16: integration tests for POST /game/players/{{id}}/move on . +/// Isolated host per test (fresh in-memory store). +/// +public class MoveCommandApiTests +{ + [Fact] + public async Task PostMove_ShouldPersistTargetAndIncrementSequence_WhenDevPlayerPostsValidMove() + { + // Arrange + using var factory = new WebApplicationFactory(); + var client = factory.CreateClient(); + var cmd = new MoveCommandRequest + { + SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, + Target = new PositionVector { X = 1.5, Y = -0.5, Z = 2.25 }, + }; + + // 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); + Assert.NotNull(postBody); + Assert.Equal(PositionStateResponse.CurrentSchemaVersion, postBody!.SchemaVersion); + Assert.Equal("dev-local-1", postBody.PlayerId); + Assert.Equal(seq0 + 1, postBody.Sequence); + Assert.Equal(1.5, postBody.Position.X); + Assert.Equal(-0.5, postBody.Position.Y); + Assert.Equal(2.25, postBody.Position.Z); + + Assert.NotNull(afterMove); + Assert.Equal(postBody.Position.X, afterMove!.Position.X); + Assert.Equal(postBody.Position.Y, afterMove.Position.Y); + Assert.Equal(postBody.Position.Z, afterMove.Position.Z); + Assert.Equal(postBody.Sequence, afterMove.Sequence); + } + + [Fact] + public async Task PostMove_ShouldReturnNotFound_WhenPlayerIsUnknown() + { + // Arrange + using var factory = new WebApplicationFactory(); + var client = factory.CreateClient(); + var cmd = new MoveCommandRequest + { + SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, + Target = new PositionVector { X = 0, Y = 0, Z = 0 }, + }; + + // Act + var response = await client.PostAsJsonAsync("/game/players/unknown-player/move", cmd); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task PostMove_ShouldReturnBadRequest_WhenSchemaVersionIsWrong() + { + // Arrange + using var factory = new WebApplicationFactory(); + var client = factory.CreateClient(); + var cmd = new MoveCommandRequest + { + SchemaVersion = 999, + Target = new PositionVector { X = 0, Y = 0, Z = 0 }, + }; + + // Act + var response = await client.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Fact] + public async Task PostMove_ShouldReturnBadRequest_WhenTargetIsMissing() + { + // Arrange + using var factory = new WebApplicationFactory(); + var client = factory.CreateClient(); + var content = new StringContent( + "{\"schemaVersion\":1}", + Encoding.UTF8, + "application/json"); + + // Act + var response = await client.PostAsync("/game/players/dev-local-1/move", content); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } +} diff --git a/server/NeonSprawl.Server.Tests/PositionStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs similarity index 58% rename from server/NeonSprawl.Server.Tests/PositionStateApiTests.cs rename to server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs index 6386375..d46e9bf 100644 --- a/server/NeonSprawl.Server.Tests/PositionStateApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs @@ -4,18 +4,24 @@ using Microsoft.AspNetCore.Mvc.Testing; using NeonSprawl.Server.Game.PositionState; using Xunit; -namespace NeonSprawl.Server.Tests; +namespace NeonSprawl.Server.Tests.Game.PositionState; +/// Integration tests for GET routes. public class PositionStateApiTests(WebApplicationFactory factory) : IClassFixture> { private readonly HttpClient _client = factory.CreateClient(); [Fact] - public async Task GetPosition_dev_player_returns_versioned_json_with_xyz() + public async Task GetPosition_ShouldReturnVersionedPayloadAtOrigin_WhenDevPlayerRequested() { - var response = await _client.GetAsync("/game/players/dev-local-1/position"); + // Arrange + const string url = "/game/players/dev-local-1/position"; + // Act + var response = await _client.GetAsync(url); + + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadFromJsonAsync(); Assert.NotNull(body); @@ -28,10 +34,15 @@ public class PositionStateApiTests(WebApplicationFactory factory) } [Fact] - public async Task GetPosition_dev_player_case_insensitive_path_returns_200() + public async Task GetPosition_ShouldReturnOk_WhenPathIdDiffersOnlyByCase() { - var response = await _client.GetAsync("/game/players/DEV-LOCAL-1/position"); + // Arrange + const string url = "/game/players/DEV-LOCAL-1/position"; + // Act + var response = await _client.GetAsync(url); + + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); var body = await response.Content.ReadFromJsonAsync(); Assert.NotNull(body); @@ -41,9 +52,15 @@ public class PositionStateApiTests(WebApplicationFactory factory) } [Fact] - public async Task GetPosition_unknown_player_returns_404() + public async Task GetPosition_ShouldReturnNotFound_WhenPlayerIsUnknown() { - var response = await _client.GetAsync("/game/players/unknown-player/position"); + // Arrange + const string url = "/game/players/unknown-player/position"; + + // Act + var response = await _client.GetAsync(url); + + // Assert Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } } diff --git a/server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs b/server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs index 055c623..91ecf58 100644 --- a/server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs +++ b/server/NeonSprawl.Server/Game/PositionState/IPositionStateStore.cs @@ -1,8 +1,14 @@ namespace NeonSprawl.Server.Game.PositionState; -/// In-memory source of truth for player positions (NS-15 spike). +/// In-memory source of truth for player positions (NS-15 read API, NS-16 move apply). public interface IPositionStateStore { /// Returns false if the player is unknown (HTTP 404). is trimmed; matching is ordinal case-insensitive. bool TryGetPosition(string playerId, out PositionSnapshot snapshot); + + /// + /// Sets authoritative position to the given coordinates (v1 snap) and increments . + /// Returns false if the player is unknown. + /// + bool TryApplyMoveTarget(string playerId, double x, double y, double z, out PositionSnapshot snapshot); } diff --git a/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs index ba69008..9bf93d0 100644 --- a/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs +++ b/server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs @@ -30,4 +30,30 @@ public sealed class InMemoryPositionStateStore : IPositionStateStore return _positions.TryGetValue(key, out snapshot); } + + public bool TryApplyMoveTarget(string playerId, double x, double y, double z, out PositionSnapshot snapshot) + { + var key = playerId?.Trim(); + if (string.IsNullOrEmpty(key)) + { + snapshot = default; + return false; + } + + while (true) + { + if (!_positions.TryGetValue(key, out var previous)) + { + snapshot = default; + return false; + } + + var next = new PositionSnapshot(x, y, z, previous.Sequence + 1); + if (_positions.TryUpdate(key, next, previous)) + { + snapshot = next; + return true; + } + } + } } diff --git a/server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs b/server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs new file mode 100644 index 0000000..bc2f8b3 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs @@ -0,0 +1,23 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// +/// HTTP JSON body for POST /game/players/{{id}}/move. Wire contract v1 (JSON); migrate to protobuf later per project contracts docs. +/// +/// +/// Server rule (v1): authoritative position is set to immediately (snap). increments by one on each successful apply. +/// Example v1 payload: +/// +/// {"schemaVersion":1,"target":{"x":1.5,"y":0,"z":-2}} +/// +/// +public sealed class MoveCommandRequest +{ + /// Schema version for this command shape; must match for the handler to accept the body. + public const int CurrentSchemaVersion = 1; + + /// Contract version; must equal . + public int SchemaVersion { get; init; } + + /// World-space destination the server applies (v1: exact snap). + public PositionVector? Target { get; init; } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs b/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs index a56c634..2e2a41a 100644 --- a/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs +++ b/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs @@ -1,6 +1,6 @@ namespace NeonSprawl.Server.Game.PositionState; -/// Maps HTTP routes for authoritative position read API. +/// Maps HTTP routes for authoritative position read and move (NS-15 / NS-16). public static class PositionStateApi { public static WebApplication MapPositionStateApi(this WebApplication app) @@ -22,6 +22,32 @@ public static class PositionStateApi return Results.Json(body); }); + app.MapPost( + "/game/players/{id}/move", + (string id, MoveCommandRequest? body, IPositionStateStore store) => + { + if (body is null) + return Results.BadRequest(); + if (body.SchemaVersion != MoveCommandRequest.CurrentSchemaVersion) + return Results.BadRequest(); + + var target = body.Target; + if (target is null) + return Results.BadRequest(); + + if (!store.TryApplyMoveTarget(id, target.X, target.Y, target.Z, out var snap)) + return Results.NotFound(); + + var response = new PositionStateResponse + { + SchemaVersion = PositionStateResponse.CurrentSchemaVersion, + PlayerId = id, + Position = new PositionVector { X = snap.X, Y = snap.Y, Z = snap.Z }, + Sequence = snap.Sequence, + }; + return Results.Json(response); + }); + return app; } } diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs b/server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs index b9a0ad5..30246b2 100644 --- a/server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs +++ b/server/NeonSprawl.Server/Game/PositionState/PositionStateResponse.cs @@ -1,11 +1,11 @@ namespace NeonSprawl.Server.Game.PositionState; /// -/// HTTP JSON body for GET /game/players/{{id}}/position. Consumers should read -/// before interpreting other fields. +/// HTTP JSON body for GET /game/players/{{id}}/position and successful POST /game/players/{{id}}/move. +/// Consumers should read before interpreting other fields. /// /// -/// Example v1 payload: +/// Example v1 payload (initial seed; increases after each successful move, NS-16): /// /// {"schemaVersion":1,"playerId":"dev-local-1","position":{"x":0,"y":0,"z":0},"sequence":0} /// @@ -24,6 +24,6 @@ public sealed class PositionStateResponse /// Authoritative world position. public required PositionVector Position { get; init; } - /// Monotonic-ish ordering hint for future sync; v1 is always 0. + /// Increments by one on each successful move apply (NS-16); 0 for the seeded dev player until the first move. public ulong Sequence { get; init; } } diff --git a/server/NeonSprawl.Server/NeonSprawl.Server.csproj b/server/NeonSprawl.Server/NeonSprawl.Server.csproj index b13f202..7110b71 100644 --- a/server/NeonSprawl.Server/NeonSprawl.Server.csproj +++ b/server/NeonSprawl.Server/NeonSprawl.Server.csproj @@ -8,4 +8,10 @@ NeonSprawl.Server + + + false + + diff --git a/server/NeonSprawl.Server/Properties/launchSettings.json b/server/NeonSprawl.Server/Properties/launchSettings.json index 0d1b63a..7a8e7ca 100644 --- a/server/NeonSprawl.Server/Properties/launchSettings.json +++ b/server/NeonSprawl.Server/Properties/launchSettings.json @@ -1,4 +1,4 @@ -{ +{ "$schema": "http://json.schemastore.org/launchsettings.json", "iisSettings": { "windowsAuthentication": false, diff --git a/server/README.md b/server/README.md index d3fd831..677cf4b 100644 --- a/server/README.md +++ b/server/README.md @@ -2,6 +2,10 @@ ASP.NET Core **.NET 10** host for authoritative simulation (prototype: HTTP + health + in-memory position read API). +## Prerequisites + +- **.NET 10 SDK** (matches `TargetFramework` in `NeonSprawl.Server.csproj`). Check with `dotnet --list-sdks`. + ## Run ```bash @@ -30,7 +34,25 @@ Sample response: Unknown player ids return **404**. This endpoint is a **spike** until durable persistence and full sync exist. -For a **PR/Jira-ready** contract blurb (formatted JSON + field table), see the [NS-15 implementation plan — Pull request description](../../docs/plans/NS-15-implementation-plan.md#pull-request-description-jira-ac). +## Move command (NS-16) + +**`POST /game/players/{id}/move`** with JSON body applies a v1 **MoveCommand**: authoritative position **snaps** to `target` immediately; `sequence` in responses increases by one per successful apply. + +Request body (example): + +```bash +curl -s -X POST http://localhost:5253/game/players/dev-local-1/move \ + -H "Content-Type: application/json" \ + -d '{"schemaVersion":1,"target":{"x":1.5,"y":0,"z":-2}}' +``` + +- **200** — body matches **`PositionStateResponse`** (same shape as `GET` … `/position`). +- **400** — missing/invalid body or `schemaVersion` ≠ `1`. +- **404** — unknown player id. + +See XML on `MoveCommandRequest` and `PositionStateResponse` in the server project. + +For a **PR/Jira-ready** contract blurb (formatted JSON + field table), see the [NS-15 implementation plan — Pull request description](../../docs/plans/NS-15-implementation-plan.md#pull-request-description-jira-ac) (GET shape) and [NS-16 implementation plan](../../docs/plans/NS-16-implementation-plan.md#pull-request-description-jira-ac--draft-for-merge) (MoveCommand + sequence semantics). ## Solution @@ -39,3 +61,11 @@ From repo root: `dotnet build NeonSprawl.sln` and `dotnet test NeonSprawl.sln`. ## CI Pull requests targeting `main` run build and tests via [`.github/workflows/dotnet.yml`](../.github/workflows/dotnet.yml) on GitHub Actions (`ubuntu-latest` runner, **Release** configuration). + +## Linux + Rider: “only 8.x in /usr/share/dotnet” + +If the debugger prints **`.NET location: /usr/share/dotnet`** and **cannot find `Microsoft.NETCore.App` 10.x** while you have .NET 10 under **`~/.dotnet`**: + +- **`launchSettings.json` `environmentVariables`** (e.g. `DOTNET_ROOT`) often do **not** apply to the **native apphost** Rider launches first; the host probes `/usr/share/dotnet` before your app starts. +- This repo sets **`false`** for **Debug** builds so the IDE runs **`dotnet …/NeonSprawl.Server.dll`** using the **.NET CLI** you configured in the toolchain (**`/home/don/.dotnet/dotnet`**), which then loads the 10.x shared runtime from the same install. +- Alternatives: install [.NET 10 into the default location](https://learn.microsoft.com/en-us/dotnet/core/install/linux) so `/usr/share/dotnet` has 10.x, or add **`DOTNET_ROOT=/home/don/.dotnet`** in **Run → Edit Configurations → Environment variables** (IDE-level, not only `launchSettings.json`).