Merge pull request #21 from ViPro-Technologies/NS-19-move-validation
NS-19 move validationpull/22/head
GitHub
commit
d064b745e2
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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`).
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -25,3 +25,4 @@ client/.godot/
|
|||
|
||||
# Content validation (optional local venv)
|
||||
.venv-content/
|
||||
/.venv-gd
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
||||
|
|
|
|||
|
|
@ -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<GamePositionOptions>`** (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<GamePositionOptions>` (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.
|
||||
|
|
@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// NS-16: integration tests for <c>POST /game/players/{{id}}/move</c> on <see cref="NeonSprawl.Server.Game.PositionState.PositionStateApi"/>.
|
||||
/// NS-16 / NS-19: integration tests for <c>POST /game/players/{{id}}/move</c> on <see cref="NeonSprawl.Server.Game.PositionState.PositionStateApi"/>.
|
||||
/// Isolated host per test (fresh in-memory store).
|
||||
/// </summary>
|
||||
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<GamePositionOptions>(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<MoveCommandRejectedResponse>();
|
||||
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<GamePositionOptions>(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<MoveCommandRejectedResponse>();
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 _));
|
||||
}
|
||||
}
|
||||
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,9 @@ public sealed class GamePositionOptions
|
|||
|
||||
/// <summary>World position for the dev player at process start.</summary>
|
||||
public DefaultPositionOptions DefaultPosition { get; set; } = new();
|
||||
|
||||
/// <summary>NS-19: per-command step limits and optional district bounds.</summary>
|
||||
public MovementValidationOptions MovementValidation { get; set; } = new();
|
||||
}
|
||||
|
||||
/// <summary>Default spawn coordinates for the configured dev player.</summary>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,9 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Stable <c>reasonCode</c> strings for <see cref="MoveCommandRejectedResponse"/> (telemetry / client logging).</summary>
|
||||
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";
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>HTTP JSON body when <c>POST …/move</c> is rejected by movement validation (NS-19).</summary>
|
||||
/// <remarks>
|
||||
/// <para><b><c>reasonCode</c> (v1):</b> <see cref="MoveCommandReasonCodes"/>.</para>
|
||||
/// </remarks>
|
||||
public sealed class MoveCommandRejectedResponse
|
||||
{
|
||||
public const int CurrentSchemaVersion = 1;
|
||||
|
||||
public int SchemaVersion { get; init; }
|
||||
|
||||
/// <summary>Non-empty stable code; see <see cref="MoveCommandReasonCodes"/>.</summary>
|
||||
public string ReasonCode { get; init; } = "";
|
||||
}
|
||||
|
|
@ -4,7 +4,7 @@ namespace NeonSprawl.Server.Game.PositionState;
|
|||
/// HTTP JSON body for <c>POST /game/players/{{id}}/move</c>. Wire contract v1 (JSON); migrate to protobuf later per project contracts docs.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para><b>Server rule (v1):</b> authoritative position is set to <see cref="Target"/> immediately (snap). <see cref="PositionStateResponse.Sequence"/> increments by one on each successful apply.</para>
|
||||
/// <para><b>Server rule (v1):</b> after NS-19 validation, authoritative position is set to <see cref="Target"/> (snap). <see cref="PositionStateResponse.Sequence"/> increments by one on each successful apply. Oversized steps return HTTP 400 with <see cref="MoveCommandRejectedResponse"/>.</para>
|
||||
/// <para>Example v1 payload:</para>
|
||||
/// <code>
|
||||
/// {"schemaVersion":1,"target":{"x":1.5,"y":0,"z":-2}}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,48 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Pure movement validation for <see cref="MoveCommandRequest"/> targets (NS-19).</summary>
|
||||
/// <remarks>
|
||||
/// <para><b>Precedence</b> when multiple rules fail: <see cref="MoveCommandReasonCodes.HorizontalStepExceeded"/>,
|
||||
/// then <see cref="MoveCommandReasonCodes.VerticalStepExceeded"/>, then <see cref="MoveCommandReasonCodes.OutOfBounds"/>.</para>
|
||||
/// </remarks>
|
||||
public static class MoveCommandValidation
|
||||
{
|
||||
/// <summary>Returns false and sets <paramref name="reasonCode"/> when the move must be rejected.</summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Per-command movement sanity limits (NS-19). Bound from <c>Game:MovementValidation</c>.</summary>
|
||||
public sealed class MovementValidationOptions
|
||||
{
|
||||
/// <summary>Max horizontal displacement on X/Z in one move (meters). Inclusive at the limit.</summary>
|
||||
public double MaxHorizontalStep { get; set; } = 18.0;
|
||||
|
||||
/// <summary>Max absolute delta on Y in one move (meters). Inclusive at the limit.</summary>
|
||||
public double MaxVerticalStep { get; set; } = 1.25;
|
||||
|
||||
/// <summary>When true, target must lie inside the axis-aligned district box (inclusive min/max).</summary>
|
||||
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;
|
||||
}
|
||||
|
|
@ -1,6 +1,8 @@
|
|||
using Microsoft.Extensions.Options;
|
||||
|
||||
namespace NeonSprawl.Server.Game.PositionState;
|
||||
|
||||
/// <summary>Maps HTTP routes for authoritative position read and move (NS-15 / NS-16).</summary>
|
||||
/// <summary>Maps HTTP routes for authoritative position read and move (NS-15 / NS-16 / NS-19).</summary>
|
||||
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<GamePositionOptions> 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();
|
||||
|
|
|
|||
|
|
@ -12,6 +12,11 @@
|
|||
"X": -5,
|
||||
"Y": 0.9,
|
||||
"Z": -5
|
||||
},
|
||||
"MovementValidation": {
|
||||
"MaxHorizontalStep": 18,
|
||||
"MaxVerticalStep": 1.25,
|
||||
"DistrictBoundsEnabled": false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue