diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md index e4fbb1b..67a6eaa 100644 --- a/.cursor/rules/code-review-agent.md +++ b/.cursor/rules/code-review-agent.md @@ -62,7 +62,7 @@ Work through what applies to the diff (skip irrelevant sections briefly). - **Scope** — branch name, PR link, issue key, and/or `git` range the user asked to review (or “working tree / unstaged” if that was the scope) - **Base** — e.g. `origin/main` or commit SHA if stated or inferable 4. **Body:** the full review using the **Output format** sections below (verdict through verification). This is the canonical copy; the chat response may be a short summary plus **path to the file** (e.g. `docs/reviews/2026-03-29-NS-15.md`). -5. **Commits:** follow [commit-and-review](commit-and-review.md)—write the file to disk **uncommitted** unless the user explicitly asks to commit it. +5. **Commits:** Follow [commit-and-review](commit-and-review.md) — you may **commit** the review file at your discretion; **never** `git push`. In the markdown file, use **normal fenced code blocks** for code snippets and **backtick file paths** (e.g. ``server/Program.cs``). Do **not** use IDE-only line-number code citations in the saved document—they do not render on GitHub or in plain Markdown viewers. diff --git a/.cursor/rules/commit-and-review.md b/.cursor/rules/commit-and-review.md index 8b8c633..ef1c767 100644 --- a/.cursor/rules/commit-and-review.md +++ b/.cursor/rules/commit-and-review.md @@ -1,31 +1,31 @@ --- -description: Never git commit without explicit user instruction; user reviews before any commit +description: Commits at agent discretion on story work; never git push; PR text without tool boilerplate alwaysApply: true --- # Commits and review (Neon Sprawl) -## No commits without explicit instruction +## When the agent may commit -- Do **not** run **`git commit`** (or equivalent, e.g. committing via tools) unless the user **explicitly** asks—clear wording such as “commit this”, “make a commit”, or “commit with message …”. -- Phrases like “implement X”, “begin work”, or “open a PR” are **not** implicit permission to commit. -- **Default after edits:** leave changes **uncommitted** (working tree or staged only if the user asked to stage). Summarize what changed and where so the user can review in the Git / diff UI, then wait for commit instructions. +- You **may** run **`git commit`** at your **discretion** while **working on a Jira story** (or other ticketed work): logical checkpoints, end of a coherent change, plan-only commits on the story branch, implementation batches, test additions, etc. +- Use judgment: **small, coherent commits** are easier to review than one huge dump; match [jira-git-naming](jira-git-naming.md) for message format when a ticket applies. +- If the user is **not** on a story branch and the change is **exploratory** or **ambiguous**, prefer leaving the working tree uncommitted and summarizing until scope is clear—unless they asked you to commit. -## Review before commit +## Never push -- The user should **review** the diff before anything is committed. The agent’s job is to make the changes visible (uncommitted) and explain them; the user decides when to commit. -- If the user asks to commit, use a message that matches repo conventions; still follow [git workflow](git-workflow.md) (branch vs `main`, doc-only vs code). - -## Commit message format when a Jira story applies - -- Any commit that is **part of implementing or delivering a Jira story or task** must put the **Jira issue key first** in the subject line, then **`:`**, then the summary (e.g. `NS-17: persist position state in PostgreSQL`). -- Infer the key from the active branch name, the issue under discussion, or Jira context. Full rules (multi-issue commits, `chore:` when there is no ticket) are in [jira-git-naming](jira-git-naming.md). +- Do **not** run **`git push`**, **`git push --force`**, or any command that updates **remote** refs. The user publishes branches and opens PRs. +- Do not configure remotes or credentials to bypass this. ## Pull request and push descriptions - Do **not** add **“Made-with: Cursor”**, **“Generated with Cursor”**, tool co-author lines, or similar AI/IDE boilerplate to **PR descriptions**, **GitHub merge/squash commit bodies** you draft, or other **remote-facing** narrative unless the user explicitly requests it. - Keep PR text to scope, verification, and project-required contract snippets (e.g. from `docs/plans/`). +## Commit message format when a Jira story applies + +- Any commit that is **part of implementing or delivering a Jira story or task** must put the **Jira issue key first** in the subject line, then **`:`**, then the summary (e.g. `NS-17: persist position state in PostgreSQL`). +- Infer the key from the active branch name, the issue under discussion, or Jira context. Full rules (multi-issue commits, `chore:` when there is no ticket) are in [jira-git-naming](jira-git-naming.md). + ## Scope -- Applies to **all** commits the agent might make, including **documentation-only** changes (e.g. `docs/plans/`, README), not only application source. +- Applies to **all** commits the agent might make, including **documentation-only** changes (e.g. `docs/plans/`, `docs/reviews/`, README), not only application source. diff --git a/.cursor/rules/docs-review-agent.md b/.cursor/rules/docs-review-agent.md index 9cf9c7b..81e8439 100644 --- a/.cursor/rules/docs-review-agent.md +++ b/.cursor/rules/docs-review-agent.md @@ -50,7 +50,7 @@ Work through what applies (skip irrelevant briefly). 2. **Filename:** `YYYY-MM-DD-{slug}.md` — use session **Today’s date** when known; slug from topic or ticket (kebab-case). 3. **Preamble:** Date, **Scope** (paths or “full game-design pass”), **Base** branch or “as of date” if relevant. 4. **Body:** Use **Output format** below. For design reviews, use **Documentation checked** instead of only “plan + modules”: list each consulted path and **matches / partially matches / conflicts / N/A**. -5. **Commits:** Write the review file **uncommitted** unless the user asks to commit it. Follow [commit-and-review](commit-and-review.md) — no tool-attribution boilerplate in PR text. +5. **Commits:** Follow [commit-and-review](commit-and-review.md) — you may **commit** the review file at your discretion (e.g. with a `chore:` or ticket-prefixed message if it maps to a story); **never** `git push`. No tool-attribution boilerplate in PR text. In the saved file, use **normal fenced code blocks** and **backtick paths** — not IDE line-number citations (GitHub-friendly). diff --git a/.cursor/rules/git-workflow.md b/.cursor/rules/git-workflow.md index d785cd2..d7db2b6 100644 --- a/.cursor/rules/git-workflow.md +++ b/.cursor/rules/git-workflow.md @@ -5,7 +5,7 @@ alwaysApply: true # Git workflow (Neon Sprawl) -**Agent:** Never run `git commit` without explicit user instruction; the user reviews diffs first. See [commit-and-review](commit-and-review.md). +**Agent:** You may **`git commit`** at your discretion while on story/ticket work; do **not** **`git push`**. See [commit-and-review](commit-and-review.md). - **Beginning work on a new Jira story** — Create a **new branch** as soon as story work starts (planning, implementation, or both). **Branch names must start with the Jira key** (e.g. `NS-15-position-state-api`); see [jira-git-naming](jira-git-naming.md). Stay on that branch for everything scoped to that issue, including **`docs/plans/{KEY}-implementation-plan.md`**, until the story is merged. Do not put ticketed story plans only on `main` while implementation lives on a branch. diff --git a/.cursor/rules/jira-git-naming.md b/.cursor/rules/jira-git-naming.md index 96cf699..065f0bd 100644 --- a/.cursor/rules/jira-git-naming.md +++ b/.cursor/rules/jira-git-naming.md @@ -28,4 +28,4 @@ When suggesting or creating a branch for story **NS-14**, use something like **` ## Agent behavior - When creating a **branch** or **commit** for tracked work, **infer the key** from the current story (e.g. user says NS-14), the branch already in use, or the Jira issue fetched via MCP. -- Follow [git workflow](git-workflow.md) (branch vs `main`) and [commit-and-review](commit-and-review.md) (no commit until the user asks). +- Follow [git workflow](git-workflow.md) (branch vs `main`) and [commit-and-review](commit-and-review.md) (commits at discretion on story work; **never** `git push`). diff --git a/.cursor/rules/story-kickoff.md b/.cursor/rules/story-kickoff.md index 7acce45..436dcb9 100644 --- a/.cursor/rules/story-kickoff.md +++ b/.cursor/rules/story-kickoff.md @@ -36,18 +36,20 @@ When the user starts work on a **Jira story** (e.g. issue key `NS-14`, phrases l ## 4. Planning document - Add **`docs/plans/{JIRA_KEY}-implementation-plan.md`** (example: `docs/plans/NS-14-implementation-plan.md`). Create `docs/plans/` if it does not exist. -- **Do not** `git commit` the plan unless the user explicitly asks; see [commit-and-review](commit-and-review.md). When the user commits plan-only changes for this story, commit on the **story branch** from step 1b, not `main`, per [git workflow](git-workflow.md). +- **Commit** the plan on the **story branch** from step 1b when it is ready (at agent discretion per [commit-and-review](commit-and-review.md)); do not put ticketed plan-only work only on `main` while implementation stays on a branch—see [git workflow](git-workflow.md). -**Required sections** in that file: +**Required sections** in that file (do **not** ship a plan that omits any of these): - Story reference (key, title, link if available) - Goal, scope, and out-of-scope (from Jira) - Acceptance criteria checklist (from Jira) - Technical approach (concise) -- **Files to add** (paths) -- **Files to modify** (paths and one-line rationale each) -- **Tests** — what will be added or changed; if none, say why (e.g. no harness yet, manual verification only) -- Open questions / risks (if any) +- **Files to add** — **mandatory:** explicit list of new file paths (or state “none” with one line why) +- **Files to modify** — **mandatory:** explicit list of paths **with a one-line rationale each** (or state “none” with one line why) +- **Tests** — **mandatory:** explicit list of test files to **add** or **change**, and what each will cover; if truly no automated tests, say why (e.g. no harness yet) and what manual verification replaces them +- Open questions / risks (if none, write “None.”) + +These three lists (**files to add**, **files to modify**, **tests**) must **always** be generated during kickoff—they are not optional prose and must not be left implicit inside “Technical approach” only. ## 5. After the plan diff --git a/.gitignore b/.gitignore index 198003a..dab7423 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,4 @@ client/.godot/ # Content validation (optional local venv) .venv-content/ +/.venv-gd diff --git a/AGENTS.md b/AGENTS.md index 9bdc9f1..c9d0ef6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,6 +7,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/`; 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`; use when working in **documents** or @ mention | -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). **PR / push text:** no “Made-with: Cursor” boilerplate — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). +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). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **PR / push text:** no “Made-with: Cursor” boilerplate (same file). **Commits tied to a Jira issue** must start the subject with the **issue key** and a colon (e.g. `NS-17: …`). Branch naming and exceptions (`chore:` when there is no ticket) — [`.cursor/rules/jira-git-naming.md`](.cursor/rules/jira-git-naming.md). diff --git a/client/scenes/main.tscn b/client/scenes/main.tscn index fbec954..6d31397 100644 --- a/client/scenes/main.tscn +++ b/client/scenes/main.tscn @@ -39,6 +39,39 @@ size = Vector3(0.22, 0.35, 0.22) [sub_resource type="StandardMaterial3D" id="Mat_marker_base"] albedo_color = Color(0.15, 0.4, 1, 1) +[sub_resource type="BoxMesh" id="BoxMesh_ns19_bump_a"] +size = Vector3(1, 0.15, 1) + +[sub_resource type="BoxShape3D" id="BoxShape3D_ns19_bump_a"] +size = Vector3(1, 0.15, 1) + +[sub_resource type="BoxMesh" id="BoxMesh_ns19_bump_b"] +size = Vector3(0.85, 0.12, 0.85) + +[sub_resource type="BoxShape3D" id="BoxShape3D_ns19_bump_b"] +size = Vector3(0.85, 0.12, 0.85) + +[sub_resource type="BoxMesh" id="BoxMesh_ns19_pedestal"] +size = Vector3(1.5, 2.5, 1.5) + +[sub_resource type="BoxShape3D" id="BoxShape3D_ns19_pedestal"] +size = Vector3(1.5, 2.5, 1.5) + +[sub_resource type="BoxMesh" id="BoxMesh_ns19_farpad"] +size = Vector3(2.5, 0.12, 2.5) + +[sub_resource type="BoxShape3D" id="BoxShape3D_ns19_farpad"] +size = Vector3(2.5, 0.12, 2.5) + +[sub_resource type="StandardMaterial3D" id="Mat_ns19_bump"] +albedo_color = Color(0.22, 0.52, 0.32, 1) + +[sub_resource type="StandardMaterial3D" id="Mat_ns19_pedestal"] +albedo_color = Color(0.92, 0.42, 0.12, 1) + +[sub_resource type="StandardMaterial3D" id="Mat_ns19_farpad"] +albedo_color = Color(0.32, 0.38, 0.55, 1) + [node name="Main" type="Node3D" unique_id=1358372723] script = ExtResource("1_main") @@ -84,6 +117,52 @@ transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -0.95, 0.22, -1.05) mesh = SubResource("BoxMesh_marker") surface_material_override/0 = SubResource("Mat_marker_base") +[node name="NS19BumpA" type="StaticBody3D" parent="World" unique_id=1900001 groups=["walkable"]] +transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -3.4, 0.075, -2.6) +collision_layer = 1 + +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NS19BumpA" unique_id=1900002] +mesh = SubResource("BoxMesh_ns19_bump_a") +surface_material_override/0 = SubResource("Mat_ns19_bump") + +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NS19BumpA" unique_id=1900003] +shape = SubResource("BoxShape3D_ns19_bump_a") + +[node name="NS19BumpB" type="StaticBody3D" parent="World" unique_id=1900011 groups=["walkable"]] +transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -1.85, 0.06, -4.1) +collision_layer = 1 + +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NS19BumpB" unique_id=1900012] +mesh = SubResource("BoxMesh_ns19_bump_b") +surface_material_override/0 = SubResource("Mat_ns19_bump") + +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NS19BumpB" unique_id=1900013] +shape = SubResource("BoxShape3D_ns19_bump_b") + +[node name="NS19RejectPedestal" type="StaticBody3D" parent="World" unique_id=1900021 groups=["walkable"]] +transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 7.5, 0, -6.5) +collision_layer = 1 + +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NS19RejectPedestal" unique_id=1900022] +transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 1.25, 0) +mesh = SubResource("BoxMesh_ns19_pedestal") +surface_material_override/0 = SubResource("Mat_ns19_pedestal") + +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NS19RejectPedestal" unique_id=1900023] +transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 1.25, 0) +shape = SubResource("BoxShape3D_ns19_pedestal") + +[node name="NS19FarPad" type="StaticBody3D" parent="World" unique_id=1900031 groups=["walkable"]] +transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 9, 0.06, 9) +collision_layer = 1 + +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NS19FarPad" unique_id=1900032] +mesh = SubResource("BoxMesh_ns19_farpad") +surface_material_override/0 = SubResource("Mat_ns19_farpad") + +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NS19FarPad" unique_id=1900033] +shape = SubResource("BoxShape3D_ns19_farpad") + [node name="Obstacle" type="StaticBody3D" parent="World" unique_id=1638845763] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 6, 0, 5) collision_layer = 1 @@ -116,3 +195,18 @@ script = ExtResource("4_auth") [node name="InteractionRequestClient" type="Node" parent="." unique_id=2500003] script = ExtResource("5_ix") + +[node name="UICanvas" type="CanvasLayer" parent="." unique_id=9000001] + +[node name="MoveRejectLabel" type="Label" parent="UICanvas" unique_id=9000002] +anchors_preset = 12 +anchor_left = 0.0 +anchor_top = 1.0 +anchor_right = 1.0 +anchor_bottom = 1.0 +offset_left = 12.0 +offset_top = -44.0 +offset_right = -12.0 +offset_bottom = -8.0 +grow_horizontal = 2 +autowrap_mode = 3 diff --git a/client/scripts/ground_pick.gd b/client/scripts/ground_pick.gd index fd2b819..7bef4c8 100644 --- a/client/scripts/ground_pick.gd +++ b/client/scripts/ground_pick.gd @@ -1,10 +1,15 @@ extends Node3D ## NS-16: walkable ground ray pick. Emits world target for MoveCommand; camera wired from main. +## NS-19: ignores hits on **steep** surfaces (e.g. pedestal **walls**) so players cannot chain small +## vertical steps up a vertical face; only **floor-like** normals (high dot with UP) count. ## (No `class_name` so headless/CI can load the project before `.godot` import exists.) signal target_chosen(world: Vector3) +## Minimum `hit_normal.dot(Vector3.UP)` to accept a pick (0 = vertical wall, 1 = flat floor). +const MIN_WALKABLE_NORMAL_DOT_UP: float = 0.82 + ## Fallback when `Viewport.get_camera_3d()` is null (e.g. some editor setups). Set from `main.gd` ## in `_ready`. var fallback_camera: Camera3D @@ -36,6 +41,12 @@ func _try_pick(screen_pos: Vector2) -> void: var collider: Variant = hit.get("collider") if not _collider_is_walkable(collider): return + var hit_normal: Variant = hit.get("normal", Vector3.ZERO) + if hit_normal is not Vector3: + return + var nrm: Vector3 = (hit_normal as Vector3).normalized() + if nrm.dot(Vector3.UP) < MIN_WALKABLE_NORMAL_DOT_UP: + return var hit_pos: Variant = hit.get("position") if hit_pos is Vector3: target_chosen.emit(hit_pos as Vector3) diff --git a/client/scripts/main.gd b/client/scripts/main.gd index f488d6d..d218071 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -3,11 +3,17 @@ extends Node3D ## NS-16: composes ground pick + server authority; see `ground_pick.gd` / ## `position_authority_client.gd`. NS-18: interaction POST + radius glow preview (see child nodes). +const MOVE_REJECT_MSG_SECONDS: float = 4.0 + +## Bump on each rejection so older one-shot timers do not clear a newer message. +var _move_reject_msg_token: int = 0 + @onready var _camera: Camera3D = $World/Camera3D @onready var _player: CharacterBody3D = $Player @onready var _ground_pick: Node3D = $GroundPick @onready var _authority: Node = $PositionAuthorityClient @onready var _radius_preview: Node3D = $World/InteractionMarkers +@onready var _move_reject_label: Label = $UICanvas/MoveRejectLabel func _ready() -> void: @@ -16,6 +22,7 @@ func _ready() -> void: _authority.connect( "authoritative_position_received", Callable(self, "_on_authoritative_position") ) + _authority.connect("move_rejected", Callable(self, "_on_move_rejected")) _authority.call("sync_from_server") if _radius_preview.has_method("setup_player"): _radius_preview.call("setup_player", _player) @@ -27,3 +34,16 @@ func _on_target_chosen(world: Vector3) -> void: func _on_authoritative_position(world: Vector3) -> void: _player.snap_to_server(world) + + +func _on_move_rejected(reason_code: String) -> void: + push_warning("Move rejected: %s" % reason_code) + _move_reject_msg_token += 1 + var token: int = _move_reject_msg_token + _move_reject_label.text = "Move rejected: %s" % reason_code + get_tree().create_timer(MOVE_REJECT_MSG_SECONDS).timeout.connect( + func(): + if token == _move_reject_msg_token: + _move_reject_label.text = "", + CONNECT_ONE_SHOT + ) diff --git a/client/scripts/position_authority_client.gd b/client/scripts/position_authority_client.gd index 049e8e3..3f0df7a 100644 --- a/client/scripts/position_authority_client.gd +++ b/client/scripts/position_authority_client.gd @@ -1,10 +1,11 @@ extends Node ## NS-16: POST MoveCommand then GET PositionState; emits authoritative world position for the -## avatar. +## avatar. NS-19: failed validation (HTTP 400 + rejection body) emits `move_rejected`. ## (No `class_name` so headless/CI can load the project before `.godot` import exists.) signal authoritative_position_received(world: Vector3) +signal move_rejected(reason_code: String) enum Phase { BOOT_GET, POST_MOVE, VERIFY_GET } @@ -80,7 +81,14 @@ func _on_request_completed( if response_code == 200: _emit_position_from_response(text) Phase.POST_MOVE: + if response_code == 400: + _emit_move_rejection_if_present(text) + _busy = false + return if response_code != 200: + push_warning( + "PositionAuthorityClient: move POST unexpected code %s" % response_code + ) _busy = false return _phase = Phase.VERIFY_GET @@ -91,6 +99,17 @@ func _on_request_completed( _emit_position_from_response(text) +func _emit_move_rejection_if_present(json_text: String) -> void: + var parsed: Variant = JSON.parse_string(json_text) + if parsed != null and parsed is Dictionary: + var data: Dictionary = parsed + var code_variant: Variant = data.get("reasonCode", "") + if code_variant is String: + move_rejected.emit(code_variant as String) + return + move_rejected.emit("unknown") + + func _emit_position_from_response(json_text: String) -> void: var parsed: Variant = JSON.parse_string(json_text) if parsed == null: diff --git a/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md b/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md index c40aa63..5fd2062 100644 --- a/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md +++ b/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md @@ -33,7 +33,7 @@ Contract readiness is tracked in the [module dependency register](module_depende ## Implementation snapshot -- **Done (prototype):** Server-side authoritative **`PositionState`** read + **`MoveCommand`** apply (HTTP JSON v1, snap-to-target, `sequence` increments); default **in-memory** store; optional **PostgreSQL** persistence when `ConnectionStrings:NeonSprawl` is set ([NS-17](../../plans/NS-17-implementation-plan.md)); 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/`). **`InteractionRequest`** + server-side horizontal range check ([NS-18](../../plans/NS-18-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`, [server README — Interaction](../../../server/README.md#interaction-ns-18)). See [server README — Position persistence](../../../server/README.md#position-persistence-ns-17). +- **Done (prototype):** Server-side authoritative **`PositionState`** read + **`MoveCommand`** apply (HTTP JSON v1, snap-to-target, `sequence` increments); **NS-19** server-side step + optional district bounds validation with **`reasonCode`** rejections ([NS-19](../../plans/NS-19-implementation-plan.md), `MoveCommandValidation`, [server README — Move command](../../../server/README.md#move-command-ns-16-ns-19)); default **in-memory** store; optional **PostgreSQL** persistence when `ConnectionStrings:NeonSprawl` is set ([NS-17](../../plans/NS-17-implementation-plan.md)); 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/`). **`InteractionRequest`** + server-side horizontal range check ([NS-18](../../plans/NS-18-implementation-plan.md); `Game/Interaction/`, `Game/World/HorizontalReach.cs`, [server README — Interaction](../../../server/README.md#interaction-ns-18)). See [server README — Position persistence](../../../server/README.md#position-persistence-ns-17). - **Not yet:** Prediction/reconciliation, full Epic 1 Slice 1 movement loop and telemetry. - **Alignment:** [Documentation and implementation alignment](documentation_and_implementation_alignment.md). diff --git a/docs/plans/NS-19-implementation-plan.md b/docs/plans/NS-19-implementation-plan.md new file mode 100644 index 0000000..9aa2dae --- /dev/null +++ b/docs/plans/NS-19-implementation-plan.md @@ -0,0 +1,127 @@ +# NS-19 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NS-19 | +| **Title** | E1.M1: Server movement validation (speed + bounds) | +| **Jira** | [NS-19](https://neon-sprawl.atlassian.net/browse/NS-19) | +| **Parent context** | [NS-1 — Epic 1 — Core Player Runtime](https://neon-sprawl.atlassian.net/browse/NS-1) · [NS-10 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NS-10) | +| **Decomposition** | [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) (`MoveCommand` / `PositionState` authority) | + +**Delivery:** Ship **`docs/plans/NS-19-implementation-plan.md`** on the **same branch / PR** as the NS-19 implementation (plan + code pushed together). + +## Goal, scope, and out-of-scope + +**Goal:** Server-side **sanity rules** on `MoveCommand` so impossible steps (teleport, out-of-world targets) are **not** applied to authoritative `PositionState`. Failures return a **stable `reasonCode`** for logs and future telemetry. + +**Design alignment — floor play (XZ emphasis)** + +Match [NS-18](NS-18-implementation-plan.md): prototype gameplay is **on the floor**. **Primary** move cap should be **horizontal displacement on X/Z** (same spirit as `HorizontalReach`). Optionally cap **|ΔY|** separately so small floor bumps are allowed but vertical teleports are not. Document the exact rule in code and server README. + +**In scope** + +- **Configurable limits** under the existing `Game` options section (extend [`GamePositionOptions`](../../server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs) or add a nested `MovementValidationOptions` bound from `appsettings` / env): at minimum **max horizontal step** (XZ) and **max absolute vertical delta** per command; tunable defaults suitable for the current Godot click-to-move stride. Defaults must sit **between** the **small bump** and **tall platform** heights described below so manual QA is obvious without editing numbers every run. +- **Godot manual-test props** in [`main.tscn`](../../client/scenes/main.tscn): **(1)** one or more **low bumps**—`StaticBody3D` nodes in the **`walkable`** group (same as [`ground_pick.gd`](../../client/scripts/ground_pick.gd) expects) with a **shallow** top surface (e.g. **0.08–0.2 m** above the main floor) so a normal click on the bump keeps **|ΔY|** **under** `MaxVerticalStep` and the server **accepts**; **(2)** a **tall “reject” pedestal**—also **`walkable`**, with its **clickable top** high enough (e.g. **≥ 1.5–2 m** above floor, exact value vs. default `MaxVerticalStep` documented in README) so a click from the floor sends a target the server **rejects** with **`vertical_step_exceeded`**; **(3)** optional **far pad** at similar **Y** to spawn but at **horizontal distance** past `MaxHorizontalStep` to exercise **`horizontal_step_exceeded`**. Use distinct materials (e.g. muted green bumps, warning stripe pedestal) so testers can see what to click. +- **Optional axis-aligned “district” bounds** (min/max per axis or min/max XZ + Y range): when **enabled**, reject targets **outside** the box; when **disabled**, skip the check (default for minimal friction in local dev). +- **`POST /game/players/{id}/move`:** after resolving the **current** authoritative position, run validation **before** `TryApplyMoveTarget`. On violation, **do not** mutate position or sequence. +- **Error payload:** JSON body with **`schemaVersion`** (`1`) and **`reasonCode`** (non-empty, stable string). **HTTP status** — prefer **400** for “valid request shape but move **rejected**” vs **404** for unknown player (keep **404** unchanged); finalize in PR if needed. + +**Out of scope (per Jira)** + +- Navmesh, full anti-cheat, client prediction / reconciliation. + +**Dependencies** + +- **`MoveCommand` endpoint** and stores ([NS-15](NS-15-implementation-plan.md), [NS-16](NS-16-implementation-plan.md), [NS-17](NS-17-implementation-plan.md)) — **done**. + +## Policy — reject (locked) + +Invalid moves are **rejected**: return the error payload; **do not** change authoritative position or sequence. Client can log **`reasonCode`** or show a short label. **Clamp** (silent or partial apply) is **out of scope** for NS-19; a future story would need explicit product rules if we ever add it. + +## Acceptance criteria checklist + +- [x] **Oversized** move (horizontal and/or vertical per configured rules) is **rejected**; client can observe failure (existing logging and/or minimal UI). +- [x] Rules implemented in **testable** pure logic (e.g. static `MoveCommandValidation` or small service) with **unit tests** on the math and edge cases (at limit, just inside, just outside; floating-point safe). +- [x] **Constants / thresholds** live in **one configuration surface** (options pattern bound to `Game:`), not scattered magic numbers. +- [x] **Integration tests** on `PositionStateApi` / HTTP: valid move still **200** + `PositionStateResponse`; invalid move returns the agreed status + **`reasonCode`**; unknown player still **404**. +- [x] **Godot scene:** **`walkable`** bumps (small ΔY) and tall **`walkable`** pedestal (and optional far pad) per §5b; manual pass confirms bump **accepts** vs pedestal **rejects** with expected **`reasonCode`**. + +## Technical approach + +### 1. Validation helper (C#) + +- Inputs: **from** `PositionSnapshot` (or x,y,z + sequence if only coords matter), **to** `PositionVector` target, **options** snapshot. +- Outputs: `bool ok` + **`reasonCode`** when not ok. +- **Horizontal step:** \( \sqrt{(x_t-x_f)^2 + (z_t-z_f)^2} \) compared to **`MaxHorizontalStep`** (or chosen name). +- **Vertical:** \( |y_t - y_f| \) compared to **`MaxVerticalStep`**. +- **Bounds:** when enabled, require `min.X <= x_t <= max.X` (and Y, Z similarly) — document inclusive vs exclusive boundaries (recommend **inclusive** min/max for a closed district box). + +### 2. Wire-up in `PositionStateApi` + +- Inject **`IOptions`** (or a dedicated options type) into the minimal API delegate via method injection / scoped helper already used elsewhere in the app. +- Flow: validate body (existing) → **`TryGetPosition`** → if missing **404** → **validate move** → if fail return **error JSON** → else **`TryApplyMoveTarget`** → **200** + response. + +### 3. Response DTO for failures + +- New type e.g. **`MoveCommandRejectedResponse`** with `schemaVersion`, `reasonCode`. +- **Stable `reasonCode` values (v1 draft — finalize in PR):** e.g. `horizontal_step_exceeded`, `vertical_step_exceeded`, `out_of_bounds` (names snake_case, stable for telemetry). + +### 4. Configuration defaults + +- Document in **`server/README.md`** (Position / move section): keys, units (world meters), and example `appsettings.Development.json` overrides. +- Defaults should allow normal prototype play from current spawn / interactable layout; tighten in tests to force rejections without flaky timing. + +### 5. Client (minimal) + +- If the Godot client treats every **200** as success today, add a **branch** for **400** (or chosen code): log **`reasonCode`** and optionally show a **Label** or status text — enough to satisfy “client can surface failure” without building full UI. + +### 5b. Scene props for manual movement validation + +- Only surfaces in group **`walkable`** receive ray picks; the **main floor** already uses this. Add bumps and the tall pedestal as **`walkable`** `StaticBody3D` (or parent in group) so clicks produce world targets. **Do not** mark the existing opaque **`Obstacle`** as walkable unless we explicitly want clicks on its sides/top (default: leave non-walkable so it stays visual/collision clutter only, or repurpose it with a **thin walkable cap** if we want a second tall reject target). +- After placing nodes, document in **`server/README.md`** the **intended** default `MaxVerticalStep` / `MaxHorizontalStep` relative to those props (e.g. “bumps ~0.15 m, pedestal top ~2 m above floor”). + +### 6. Documentation hygiene + +- After merge: add one line to [E1_M1_InputAndMovementRuntime.md](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) **implementation snapshot** (movement validation + link to this plan). + +## Files to add + +| Path | Purpose | +|------|---------| +| `server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs` | Pure validation API: current snapshot + target + options → pass/fail + `reasonCode` (horizontal step, vertical step, optional bounds). | +| `server/NeonSprawl.Server/Game/PositionState/MoveCommandRejectedResponse.cs` | JSON DTO for failed moves: `schemaVersion`, `reasonCode`. | +| `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandValidationTests.cs` | Unit tests for validation math (at limit, inside, outside; float-safe; bounds on/off). | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs` | Add nested **`MovementValidation`** (or sibling options type still bound under `Game:`): max horizontal step, max vertical delta, district bounds enable + min/max vector (or per-axis), defaults for prototype. | +| `server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs` | After `TryGetPosition`, run validation before `TryApplyMoveTarget`; on failure return **400** + `MoveCommandRejectedResponse`; inject `IOptions` (or `IOptionsSnapshot<>`). | +| `server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs` | Only if binding shape changes (e.g. new subsection); ensure new options properties bind from configuration. | +| `server/NeonSprawl.Server/appsettings.json` and/or `appsettings.Development.json` | Document example keys for movement limits and optional bounds (sane defaults for click-to-move). | +| `server/README.md` | Document move validation, config keys, rejection status/body, and `reasonCode` values. | +| `server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs` | Optional: small helper or documented pattern to override `Game` options with **strict** limits for HTTP tests without relying on production defaults. | +| `client/scripts/position_authority_client.gd` | Handle non-**200** from move POST: log **`reasonCode`** (and optional user-visible hint); do not emit authoritative snap as success. | +| `client/scenes/main.tscn` | Add **walkable** low bumps + tall **walkable** pedestal (and optional far pad) with meshes/materials for manual NS-19 validation; keep floor/interactable layout coherent. | +| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot: NS-19 move validation + link to this plan. | + +## Tests + +| Action | Path | What to cover | +|--------|------|----------------| +| **Add** | `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandValidationTests.cs` | Horizontal step: at `MaxHorizontalStep`, epsilon inside, epsilon outside; vertical `|ΔY|` same pattern; bounds disabled → no reject on extreme coords; bounds enabled → `out_of_bounds`; order of checks if multiple fail (document fixed precedence in code + test one case). | +| **Change** | `server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs` | Existing happy-path moves still **200** under new defaults; add cases: step too large → **400** + body contains expected `reasonCode`; unknown player still **404**; schema/body errors unchanged **400** (no `reasonCode` vs rejection body—document chosen behavior). | +| **Optional** | `server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs` | **No change expected** — validation runs before store write; skip unless a regression appears. | + +**Manual (required for props):** With server defaults documented for the scene: **(1)** Click **onto a small bump** from nearby floor → move **accepted**, player reconciles to server position. **(2)** Click **tall pedestal top** from floor → **`vertical_step_exceeded`** (or chosen code), log/UI visible, position unchanged. **(3)** If a far pad is added → **`horizontal_step_exceeded`** from a standing position that keeps vertical sane. **(4)** Optionally lower `MaxVerticalStep` in dev config until a bump **rejects** to confirm thresholds feel right. + +## Open questions / risks + +None. + +## PR / review + +Paste or adapt when opening the PR; align **`reasonCode`** strings and HTTP status with final DTOs (**reject-only** behavior). Cross-check [server README](../../server/README.md) and [E1.M1](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) for consistency. diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs index 4c772c4..4f4ab9a 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs @@ -1,13 +1,15 @@ using System.Net; using System.Net.Http.Json; using System.Text; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; using NeonSprawl.Server.Game.PositionState; using Xunit; namespace NeonSprawl.Server.Tests.Game.PositionState; /// -/// NS-16: integration tests for POST /game/players/{{id}}/move on . +/// NS-16 / NS-19: integration tests for POST /game/players/{{id}}/move on . /// Isolated host per test (fresh in-memory store). /// public class MoveCommandApiTests @@ -21,7 +23,7 @@ public class MoveCommandApiTests var cmd = new MoveCommandRequest { SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, - Target = new PositionVector { X = 1.5, Y = -0.5, Z = 2.25 }, + Target = new PositionVector { X = 1.5, Y = 0.0, Z = 2.25 }, }; // Act @@ -40,7 +42,7 @@ public class MoveCommandApiTests 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(0.0, postBody.Position.Y); Assert.Equal(2.25, postBody.Position.Z); Assert.NotNull(afterMove); @@ -105,4 +107,69 @@ public class MoveCommandApiTests // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); } + + [Fact] + public async Task PostMove_ShouldReturn400WithReason_WhenHorizontalStepTooLarge() + { + await using var factory = new InMemoryWebApplicationFactory().WithWebHostBuilder(b => + { + b.ConfigureTestServices(services => + { + services.PostConfigure(o => o.MovementValidation.MaxHorizontalStep = 1.0); + }); + }); + var client = factory.CreateClient(); + var cmd = new MoveCommandRequest + { + SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, + Target = new PositionVector { X = 0, Y = 0.9, Z = 0 }, + }; + + var response = await client.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal(MoveCommandRejectedResponse.CurrentSchemaVersion, body!.SchemaVersion); + Assert.Equal(MoveCommandReasonCodes.HorizontalStepExceeded, body.ReasonCode); + } + + [Fact] + public async Task PostMove_ShouldReturn400WithReason_WhenVerticalStepTooLarge() + { + await using var factory = new InMemoryWebApplicationFactory().WithWebHostBuilder(b => + { + b.ConfigureTestServices(services => + { + services.PostConfigure(o => o.MovementValidation.MaxVerticalStep = 0.5); + }); + }); + var client = factory.CreateClient(); + var cmd = new MoveCommandRequest + { + SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, + Target = new PositionVector { X = -5, Y = 2.0, Z = -5 }, + }; + + var response = await client.PostAsJsonAsync("/game/players/dev-local-1/move", cmd); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal(MoveCommandReasonCodes.VerticalStepExceeded, body!.ReasonCode); + } + + [Fact] + public async Task PostMove_ShouldReturn400WithoutReasonBody_WhenSchemaInvalid() + { + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + var response = await client.PostAsync( + "/game/players/dev-local-1/move", + new StringContent("{\"schemaVersion\":999,\"target\":{\"x\":0,\"y\":0,\"z\":0}}", Encoding.UTF8, "application/json")); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var text = await response.Content.ReadAsStringAsync(); + Assert.DoesNotContain("reasonCode", text, StringComparison.OrdinalIgnoreCase); + } } diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandValidationTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandValidationTests.cs new file mode 100644 index 0000000..7614c42 --- /dev/null +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandValidationTests.cs @@ -0,0 +1,90 @@ +using NeonSprawl.Server.Game.PositionState; +using Xunit; + +namespace NeonSprawl.Server.Tests.Game.PositionState; + +public class MoveCommandValidationTests +{ + private static MovementValidationOptions Rules(double maxH, double maxV, bool bounds = false) => + new() + { + MaxHorizontalStep = maxH, + MaxVerticalStep = maxV, + DistrictBoundsEnabled = bounds, + DistrictMinX = 0, + DistrictMaxX = 10, + DistrictMinY = 0, + DistrictMaxY = 10, + DistrictMinZ = 0, + DistrictMaxZ = 10, + }; + + [Fact] + public void TryValidate_ShouldAllow_WhenHorizontalExactlyAtMax() + { + var from = new PositionSnapshot(0, 0, 0, 0); + var to = new PositionVector { X = 5, Y = 0, Z = 0 }; + Assert.True(MoveCommandValidation.TryValidate(from, to, Rules(5, 10), out var code)); + Assert.Equal("", code); + } + + [Fact] + public void TryValidate_ShouldRejectHorizontal_WhenJustOutsideMax() + { + var from = new PositionSnapshot(0, 0, 0, 0); + var to = new PositionVector { X = 5 + 1e-6, Y = 0, Z = 0 }; + Assert.False(MoveCommandValidation.TryValidate(from, to, Rules(5, 10), out var code)); + Assert.Equal(MoveCommandReasonCodes.HorizontalStepExceeded, code); + } + + [Fact] + public void TryValidate_ShouldAllow_WhenVerticalExactlyAtMax() + { + var from = new PositionSnapshot(0, 1, 0, 0); + var to = new PositionVector { X = 0, Y = 2.5, Z = 0 }; + Assert.True(MoveCommandValidation.TryValidate(from, to, Rules(10, 1.5), out _)); + } + + [Fact] + public void TryValidate_ShouldRejectVertical_WhenJustOutsideMax() + { + var from = new PositionSnapshot(0, 1, 0, 0); + var to = new PositionVector { X = 0, Y = 2.5 + 1e-9, Z = 0 }; + Assert.False(MoveCommandValidation.TryValidate(from, to, Rules(10, 1.5), out var code)); + Assert.Equal(MoveCommandReasonCodes.VerticalStepExceeded, code); + } + + [Fact] + public void TryValidate_ShouldPreferHorizontalReason_WhenBothWouldFail() + { + var from = new PositionSnapshot(0, 0, 0, 0); + var to = new PositionVector { X = 100, Y = 100, Z = 0 }; + Assert.False(MoveCommandValidation.TryValidate(from, to, Rules(1, 1), out var code)); + Assert.Equal(MoveCommandReasonCodes.HorizontalStepExceeded, code); + } + + [Fact] + public void TryValidate_ShouldIgnoreBounds_WhenDistrictDisabled() + { + var from = new PositionSnapshot(0, 0, 0, 0); + var to = new PositionVector { X = -50, Y = 0, Z = 0 }; + Assert.True(MoveCommandValidation.TryValidate(from, to, Rules(100, 100, bounds: false), out _)); + } + + [Fact] + public void TryValidate_ShouldRejectOutOfBounds_WhenDistrictEnabled() + { + var from = new PositionSnapshot(5, 5, 5, 0); + var to = new PositionVector { X = 5, Y = 5, Z = 11 }; + Assert.False(MoveCommandValidation.TryValidate(from, to, Rules(20, 20, bounds: true), out var code)); + Assert.Equal(MoveCommandReasonCodes.OutOfBounds, code); + } + + [Fact] + public void TryValidate_ShouldAllowOnBoundsSurface_WhenDistrictEnabled() + { + var from = new PositionSnapshot(5, 5, 5, 0); + var to = new PositionVector { X = 10, Y = 10, Z = 10 }; + Assert.True(MoveCommandValidation.TryValidate(from, to, Rules(20, 20, bounds: true), out _)); + } +} diff --git a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs index 8b2555a..5878722 100644 --- a/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs @@ -22,7 +22,7 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar var cmd = new MoveCommandRequest { SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, - Target = new PositionVector { X = 2, Y = -1, Z = 3.5 }, + Target = new PositionVector { X = 2, Y = 0.5, Z = 3.5 }, }; // Act @@ -39,7 +39,7 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar Assert.Equal(PositionStateResponse.CurrentSchemaVersion, postBody!.SchemaVersion); Assert.Equal(before!.Sequence + 1, postBody.Sequence); Assert.Equal(2, postBody.Position.X); - Assert.Equal(-1, postBody.Position.Y); + Assert.Equal(0.5, postBody.Position.Y); Assert.Equal(3.5, postBody.Position.Z); Assert.Equal(postBody.Sequence, after!.Sequence); Assert.Equal(postBody.Position.X, after.Position.X); @@ -55,7 +55,7 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar var cmd = new MoveCommandRequest { SchemaVersion = MoveCommandRequest.CurrentSchemaVersion, - Target = new PositionVector { X = 10, Y = 20, Z = 30 }, + Target = new PositionVector { X = 7, Y = 0.8, Z = 7 }, }; HttpStatusCode postStatus = default; @@ -77,9 +77,9 @@ public sealed class PostgresPositionStateIntegrationTests(PostgresIntegrationHar Assert.Equal(HttpStatusCode.OK, postStatus); Assert.NotNull(persisted); Assert.NotNull(after); - Assert.Equal(10, after!.Position.X); - Assert.Equal(20, after.Position.Y); - Assert.Equal(30, after.Position.Z); + Assert.Equal(7, after!.Position.X); + Assert.Equal(0.8, after.Position.Y); + Assert.Equal(7, after.Position.Z); Assert.Equal(1ul, after.Sequence); } diff --git a/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs index a4642cc..aacc85b 100644 --- a/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs +++ b/server/NeonSprawl.Server/Game/PositionState/GamePositionOptions.cs @@ -12,6 +12,9 @@ public sealed class GamePositionOptions /// World position for the dev player at process start. public DefaultPositionOptions DefaultPosition { get; set; } = new(); + + /// NS-19: per-command step limits and optional district bounds. + public MovementValidationOptions MovementValidation { get; set; } = new(); } /// Default spawn coordinates for the configured dev player. diff --git a/server/NeonSprawl.Server/Game/PositionState/MoveCommandReasonCodes.cs b/server/NeonSprawl.Server/Game/PositionState/MoveCommandReasonCodes.cs new file mode 100644 index 0000000..4cb4eb4 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/MoveCommandReasonCodes.cs @@ -0,0 +1,9 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// Stable reasonCode strings for (telemetry / client logging). +public static class MoveCommandReasonCodes +{ + public const string HorizontalStepExceeded = "horizontal_step_exceeded"; + public const string VerticalStepExceeded = "vertical_step_exceeded"; + public const string OutOfBounds = "out_of_bounds"; +} diff --git a/server/NeonSprawl.Server/Game/PositionState/MoveCommandRejectedResponse.cs b/server/NeonSprawl.Server/Game/PositionState/MoveCommandRejectedResponse.cs new file mode 100644 index 0000000..2957a32 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/MoveCommandRejectedResponse.cs @@ -0,0 +1,15 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// HTTP JSON body when POST …/move is rejected by movement validation (NS-19). +/// +/// reasonCode (v1): . +/// +public sealed class MoveCommandRejectedResponse +{ + public const int CurrentSchemaVersion = 1; + + public int SchemaVersion { get; init; } + + /// Non-empty stable code; see . + public string ReasonCode { get; init; } = ""; +} diff --git a/server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs b/server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs index bc2f8b3..697a264 100644 --- a/server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs +++ b/server/NeonSprawl.Server/Game/PositionState/MoveCommandRequest.cs @@ -4,7 +4,7 @@ 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. +/// Server rule (v1): after NS-19 validation, authoritative position is set to (snap). increments by one on each successful apply. Oversized steps return HTTP 400 with . /// Example v1 payload: /// /// {"schemaVersion":1,"target":{"x":1.5,"y":0,"z":-2}} diff --git a/server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs b/server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs new file mode 100644 index 0000000..0dc1c86 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs @@ -0,0 +1,48 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// Pure movement validation for targets (NS-19). +/// +/// Precedence when multiple rules fail: , +/// then , then . +/// +public static class MoveCommandValidation +{ + /// Returns false and sets when the move must be rejected. + public static bool TryValidate( + in PositionSnapshot from, + PositionVector to, + MovementValidationOptions rules, + out string reasonCode) + { + reasonCode = ""; + + var dx = to.X - from.X; + var dz = to.Z - from.Z; + var horizontal = Math.Sqrt(dx * dx + dz * dz); + if (horizontal > rules.MaxHorizontalStep) + { + reasonCode = MoveCommandReasonCodes.HorizontalStepExceeded; + return false; + } + + var dy = Math.Abs(to.Y - from.Y); + if (dy > rules.MaxVerticalStep) + { + reasonCode = MoveCommandReasonCodes.VerticalStepExceeded; + return false; + } + + if (rules.DistrictBoundsEnabled) + { + if (to.X < rules.DistrictMinX || to.X > rules.DistrictMaxX || + to.Y < rules.DistrictMinY || to.Y > rules.DistrictMaxY || + to.Z < rules.DistrictMinZ || to.Z > rules.DistrictMaxZ) + { + reasonCode = MoveCommandReasonCodes.OutOfBounds; + return false; + } + } + + return true; + } +} diff --git a/server/NeonSprawl.Server/Game/PositionState/MovementValidationOptions.cs b/server/NeonSprawl.Server/Game/PositionState/MovementValidationOptions.cs new file mode 100644 index 0000000..51111e1 --- /dev/null +++ b/server/NeonSprawl.Server/Game/PositionState/MovementValidationOptions.cs @@ -0,0 +1,21 @@ +namespace NeonSprawl.Server.Game.PositionState; + +/// Per-command movement sanity limits (NS-19). Bound from Game:MovementValidation. +public sealed class MovementValidationOptions +{ + /// Max horizontal displacement on X/Z in one move (meters). Inclusive at the limit. + public double MaxHorizontalStep { get; set; } = 18.0; + + /// Max absolute delta on Y in one move (meters). Inclusive at the limit. + public double MaxVerticalStep { get; set; } = 1.25; + + /// When true, target must lie inside the axis-aligned district box (inclusive min/max). + public bool DistrictBoundsEnabled { get; set; } + + public double DistrictMinX { get; set; } = -12.0; + public double DistrictMaxX { get; set; } = 12.0; + public double DistrictMinY { get; set; } = -2.0; + public double DistrictMaxY { get; set; } = 24.0; + public double DistrictMinZ { get; set; } = -12.0; + public double DistrictMaxZ { get; set; } = 12.0; +} diff --git a/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs b/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs index 7a3c72e..ce3ca25 100644 --- a/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs +++ b/server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs @@ -1,6 +1,8 @@ +using Microsoft.Extensions.Options; + namespace NeonSprawl.Server.Game.PositionState; -/// Maps HTTP routes for authoritative position read and move (NS-15 / NS-16). +/// Maps HTTP routes for authoritative position read and move (NS-15 / NS-16 / NS-19). public static class PositionStateApi { public static WebApplication MapPositionStateApi(this WebApplication app) @@ -26,7 +28,7 @@ public static class PositionStateApi app.MapPost( "/game/players/{id}/move", - (string id, MoveCommandRequest? body, IPositionStateStore store) => + (string id, MoveCommandRequest? body, IPositionStateStore store, IOptions gameOptions) => { if (body is null || body.SchemaVersion != MoveCommandRequest.CurrentSchemaVersion) { @@ -39,6 +41,22 @@ public static class PositionStateApi return Results.BadRequest(); } + if (!store.TryGetPosition(id, out var current)) + { + return Results.NotFound(); + } + + var rules = gameOptions.Value.MovementValidation; + if (!MoveCommandValidation.TryValidate(current, target, rules, out var reasonCode)) + { + var rejected = new MoveCommandRejectedResponse + { + SchemaVersion = MoveCommandRejectedResponse.CurrentSchemaVersion, + ReasonCode = reasonCode, + }; + return Results.Json(rejected, statusCode: StatusCodes.Status400BadRequest); + } + if (!store.TryApplyMoveTarget(id, target.X, target.Y, target.Z, out var snap)) { return Results.NotFound(); diff --git a/server/NeonSprawl.Server/appsettings.json b/server/NeonSprawl.Server/appsettings.json index a65a4d8..e09c7f9 100644 --- a/server/NeonSprawl.Server/appsettings.json +++ b/server/NeonSprawl.Server/appsettings.json @@ -12,6 +12,11 @@ "X": -5, "Y": 0.9, "Z": -5 + }, + "MovementValidation": { + "MaxHorizontalStep": 18, + "MaxVerticalStep": 1.25, + "DistrictBoundsEnabled": false } } } diff --git a/server/README.md b/server/README.md index 9ca7918..d62b873 100644 --- a/server/README.md +++ b/server/README.md @@ -59,10 +59,21 @@ Sample response (default spawn matches Godot capsule and NS-18 walk-in range dem Unknown player ids return **404**. Full zone sync / replication is still out of scope for these spikes. -## Move command (NS-16) +## Move command (NS-16, NS-19) **`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. +**NS-19 validation (reject-only):** Before applying, the server checks the move against **`Game:MovementValidation`**. Limits use **horizontal** distance on **X/Z** only and **|ΔY|** separately (see `MoveCommandValidation`). Unknown players return **404** before validation. + +| Config key | Meaning | Default (see `appsettings.json`) | +|------------|---------|-----------------------------------| +| `Game:MovementValidation:MaxHorizontalStep` | Max XZ displacement per command (m); inclusive at limit | `18` | +| `Game:MovementValidation:MaxVerticalStep` | Max absolute Y delta per command (m); inclusive | `1.25` | +| `Game:MovementValidation:DistrictBoundsEnabled` | Axis-aligned box on **target** (inclusive min/max) | `false` | +| `Game:MovementValidation:DistrictMinX` … `DistrictMaxZ` | District extents when enabled | ±12 / Y −2…24 | + +**Manual QA (Godot `main.tscn`):** **Green bumps** (~0.15 m rise) stay under default `MaxVerticalStep` from the dev spawn; **orange reject pedestal** top is ~2.5 m above floor so a floor-level click yields **`vertical_step_exceeded`**; **far pad** is beyond default horizontal reach for **`horizontal_step_exceeded`**. The client’s **`ground_pick.gd`** only accepts ray hits whose surface **normal** is mostly **upward** (`MIN_WALKABLE_NORMAL_DOT_UP`), so **vertical faces** on the pedestal do not emit move targets—this complements server per-step limits and prevents “stair-stepping” up walls. + Request body (example): ```bash @@ -72,12 +83,12 @@ curl -s -X POST http://localhost:5253/game/players/dev-local-1/move \ ``` - **200** — body matches **`PositionStateResponse`** (same shape as `GET` … `/position`). -- **400** — missing/invalid body or `schemaVersion` ≠ `1`. +- **400** — malformed command (missing/invalid body or wrong `schemaVersion`) **or** move rejected by NS-19 rules. Rejection responses are JSON **`MoveCommandRejectedResponse`**: `schemaVersion` (`1`) + **`reasonCode`** (`horizontal_step_exceeded`, `vertical_step_exceeded`, `out_of_bounds`). Malformed requests return **400** with **no** rejection body. - **404** — unknown player id. -See XML on `MoveCommandRequest` and `PositionStateResponse` in the server project. +See XML on `MoveCommandRequest`, `PositionStateResponse`, and `MoveCommandRejectedResponse` 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). +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). NS-19: [NS-19 implementation plan](../../docs/plans/NS-19-implementation-plan.md). ## Interaction (NS-18)