diff --git a/client/README.md b/client/README.md index e8f576f..d217da7 100644 --- a/client/README.md +++ b/client/README.md @@ -9,20 +9,25 @@ Open this **`client/`** directory as a project in **Godot 4.6** (4.x compatible) Do not grow an all-in-one **`main.gd`**. The main scene root should **compose** child nodes and scripts; put picking, server sync, and similar features in **dedicated scripts** (typically under `scripts/`) and connect via **signals** or small APIs. Use **Autoloads** only when several scenes truly need the same global. Full guidance for contributors and tooling: [`.cursor/rules/godot-client-script-organization.md`](../.cursor/rules/godot-client-script-organization.md). -## Authoritative movement (NS-16) +## Authoritative movement (NS-16, NS-23) -With the game server running ([`server/README.md`](../server/README.md)), the client sends a **`MoveCommand`** over HTTP and **snaps** the avatar to the server’s **`PositionState`** after a follow-up **`GET`**. +With the game server running ([`server/README.md`](../server/README.md)), each valid floor click sends a **`MoveCommand`** (**`POST`**) and a follow-up **`GET`** for **`PositionState`**. The server still **snaps** authority to the target (NS-16/19); the client **moves** toward that verified position using **`NavigationAgent3D`** + a **baked mesh** instead of teleporting on the **`GET`** (NS-23). **Boot** `sync_from_server()` **snaps** once so spawn matches the server. -- **Scripts:** `scripts/ground_pick.gd` (walkable pick + `target_chosen`), `scripts/position_authority_client.gd` (`PositionAuthorityClient`: POST move, GET verify), thin `scripts/main.gd` wiring. +**Tradeoff (prototype):** `player.gd` often steers **straight in xz** toward the goal when the pick’s **Y** is below the capsule origin (smooth **stepped bumps**). **Automatic routing around tall obstacles on one click is not guaranteed** — use **several clicks** to go around e.g. the gray **`Obstacle`** when needed. + +**Known issue:** visible **idle / rest jitter** on the avatar — follow-up in [NS-24](https://neon-sprawl.atlassian.net/browse/NS-24) (Tech Debt). + +- **Scripts:** `scripts/ground_pick.gd` (walkable pick + `target_chosen`), `scripts/position_authority_client.gd` (`PositionAuthorityClient`: POST move, GET verify; second signal arg = boot snap vs nav goal), `scripts/player.gd` (path-follow), thin `scripts/main.gd` (nav bake on first frame, then wiring). +- **Scene:** `scenes/main.tscn` — walkable **`StaticBody3D`** geometry lives under **`World/NavigationRegion3D`**. `main.gd` calls **`bake_navigation_mesh(false)`** (main-thread bake) after one **`process_frame`**, then waits two **`physics_frame`**s so **`NavigationServer3D`** has a map before agents query paths. After moving floor or obstacles, re-bake in the editor if needed. - **Inspector:** select **`PositionAuthorityClient`** on the main scene to set **`base_url`** (default `http://127.0.0.1:5253`) and **`dev_player_id`** (default `dev-local-1`, must match server `Game:DevPlayerId`). -- On startup, **`sync_from_server()`** runs once so the capsule matches the server before you click. -### Manual check (NS-16) +### Manual check (NS-16 + NS-23) 1. From repo root: `cd server/NeonSprawl.Server && dotnet run` (note the URL/port, usually `http://localhost:5253`). 2. If the port differs, set **`base_url`** on **`PositionAuthorityClient`** accordingly (e.g. `http://127.0.0.1:5253`). -3. Open the client in Godot and run the main scene (**F5**). The player should jump to the server’s default position (e.g. **(-5, 0.9, -5)** per `Game:DefaultPosition` / NS-18 walk demo). -4. **Left-click** the floor: the client POSTs the target, then GETs position; the avatar **snaps** to the authoritative coordinates (no client-side walk to the click). +3. Open the client in Godot and run the main scene (**F5**). The player should **snap** to the server’s default position (e.g. **(-5, 0.9, -5)** per `Game:DefaultPosition` / NS-18 walk demo). +4. **Left-click** the floor: the client **POST**s the target, then **GET**s position; the capsule **walks** toward the authoritative target (may follow nav waypoints or bee-line in xz per the tradeoff above). NS-19 reject clicks still show the reject label and do **not** start a path. +5. Click the **far pad** or **pedestal top** (from spawn) to confirm **`horizontal_step_exceeded`** / **`vertical_step_exceeded`** behavior is unchanged. If the server is **down**, boot **`GET`** fails silently (check Output for warnings); clicks while a request is in flight are ignored until it finishes. @@ -39,16 +44,16 @@ The main scene includes a **prototype terminal** at the map center (same world * 2. **F5** in Godot: default spawn is out of range of the terminal; markers should stay **dim**. **Click-move** toward the center until markers **brighten** (within **3** m on the floor plane). 3. Press **E** (input action **`interact`** in `project.godot`): Output should show **`allowed=true`** when markers glow, **`allowed=false`** with **`reasonCode=out_of_range`** when dim (if you walk back out). Interaction uses **`_input`**, not `_unhandled_input`, so keys register reliably in the embedded **Game** dock; click the game view if the editor had focus elsewhere. -## Movement prototype (NS-14, legacy local steering) +## Movement prototype (NS-14 → NS-23) -The **`CharacterBody3D`** still uses horizontal steering + **`move_and_slide()`** in **`player.gd`**, but **NS-16** drives placement via **`snap_to_server()`** after each successful sync, so you will mostly see **snapping**, not walking to the click. Obstacle sliding from NS-14 applies only if local goals diverge from server snaps in future work. +**`player.gd`** uses **`NavigationAgent3D.get_next_path_position()`** + **`move_and_slide()`** for horizontal motion when following the mesh; it may steer **directly at the goal** in xz when the descend bypass applies (NS-23). **`snap_to_server()`** remains for **boot** (and would apply for any future hard reconcile). - The avatar is on **physics layer 2** with **mask 1** (floor + obstacle on layer 1); the pick ray uses **mask 1** so clicks pass through the avatar and hit the floor. -### Manual check (NS-14 remnants) +### Manual check (remnants) -1. Run the main scene (**F5**) **with server running** for NS-16 behavior above. -2. Without the server, startup sync does not apply authoritative position; behavior is undefined for this repo’s intended demo—**run the server** for NS-16. +1. Run the main scene (**F5**) **with server running** for movement behavior above. +2. Without the server, startup sync does not apply authoritative position; behavior is undefined for this repo’s intended demo—**run the server** for the demo path. ### Clicks still ignored? diff --git a/client/scenes/main.tscn b/client/scenes/main.tscn index 6d31397..7a25b54 100644 --- a/client/scenes/main.tscn +++ b/client/scenes/main.tscn @@ -33,6 +33,9 @@ size = Vector3(0.9, 1, 0.4) [sub_resource type="StandardMaterial3D" id="Mat_terminal"] albedo_color = Color(0.28, 0.38, 0.45, 1) +[sub_resource type="BoxShape3D" id="BoxShape3D_terminal"] +size = Vector3(0.9, 1, 0.4) + [sub_resource type="BoxMesh" id="BoxMesh_marker"] size = Vector3(0.22, 0.35, 0.22) @@ -42,14 +45,14 @@ 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="ConvexPolygonShape3D" id="Convex_ns19a_frustum"] +points = PackedVector3Array(-0.72, -0.075, -0.72, 0.72, -0.075, -0.72, 0.72, -0.075, 0.72, -0.72, -0.075, 0.72, -0.5, 0.075, -0.5, 0.5, 0.075, -0.5, 0.5, 0.075, 0.5, -0.5, 0.075, 0.5) + +[sub_resource type="ConvexPolygonShape3D" id="Convex_ns19b_frustum"] +points = PackedVector3Array(-0.575, -0.06, -0.575, 0.575, -0.06, -0.575, 0.575, -0.06, 0.575, -0.575, -0.06, 0.575, -0.425, 0.06, -0.425, 0.425, 0.06, -0.425, 0.425, 0.06, 0.425, -0.425, 0.06, 0.425) [sub_resource type="BoxMesh" id="BoxMesh_ns19_pedestal"] size = Vector3(1.5, 2.5, 1.5) @@ -72,6 +75,17 @@ 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) +[sub_resource type="NavigationMesh" id="NavigationMesh_district"] +geometry_parsed_geometry_type = 2 +geometry_collision_mask = 1 +geometry_source_geometry_mode = 0 +cell_size = 0.15 +cell_height = 0.15 +agent_radius = 0.4 +agent_height = 1.0 +agent_max_climb = 0.35 +border_size = 0.15 + [node name="Main" type="Node3D" unique_id=1358372723] script = ExtResource("1_main") @@ -86,24 +100,31 @@ transform = Transform3D(0.819152, -0.40558, 0.40558, 0, 0.707107, 0.707107, -0.5 current = true fov = 50.0 -[node name="Floor" type="StaticBody3D" parent="World" unique_id=1800743112 groups=["walkable"]] +[node name="NavigationRegion3D" type="NavigationRegion3D" parent="World" unique_id=8880001] +navigation_mesh = SubResource("NavigationMesh_district") + +[node name="Floor" type="StaticBody3D" parent="World/NavigationRegion3D" unique_id=1800743112 groups=["walkable"]] collision_layer = 1 -[node name="MeshInstance3D" type="MeshInstance3D" parent="World/Floor" unique_id=152652175] +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/Floor" unique_id=152652175] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, -0.1, 0) mesh = SubResource("BoxMesh_floor") -[node name="CollisionShape3D" type="CollisionShape3D" parent="World/Floor" unique_id=409532142] +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/Floor" unique_id=409532142] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, -0.1, 0) shape = SubResource("BoxShape3D_floor") -[node name="PrototypeTerminal" type="StaticBody3D" parent="World" unique_id=1700001] +[node name="PrototypeTerminal" type="StaticBody3D" parent="World/NavigationRegion3D" unique_id=1700001 groups=["walkable"]] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0.5, 0) +collision_layer = 1 -[node name="MeshInstance3D" type="MeshInstance3D" parent="World/PrototypeTerminal" unique_id=1700002] +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/PrototypeTerminal" unique_id=1700002] mesh = SubResource("BoxMesh_terminal") surface_material_override/0 = SubResource("Mat_terminal") +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/PrototypeTerminal" unique_id=1700006] +shape = SubResource("BoxShape3D_terminal") + [node name="InteractionMarkers" type="Node3D" parent="World" unique_id=1700003] script = ExtResource("6_rad") @@ -117,74 +138,85 @@ 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"]] +[node name="NS19BumpA" type="StaticBody3D" parent="World/NavigationRegion3D" unique_id=1900001 groups=["walkable", "ns19_bump"]] 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] +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/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="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/NS19BumpA" unique_id=1900003] +shape = SubResource("Convex_ns19a_frustum") -[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) +[node name="NS19BumpB" type="StaticBody3D" parent="World/NavigationRegion3D" unique_id=1900011 groups=["walkable", "ns19_bump"]] +transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -1.85, 0.06, -4.85) collision_layer = 1 -[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NS19BumpB" unique_id=1900012] +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/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="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/NS19BumpB" unique_id=1900013] +shape = SubResource("Convex_ns19b_frustum") -[node name="NS19RejectPedestal" type="StaticBody3D" parent="World" unique_id=1900021 groups=["walkable"]] +[node name="NS19RejectPedestal" type="StaticBody3D" parent="World/NavigationRegion3D" 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] +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/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] +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/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"]] +[node name="NS19FarPad" type="StaticBody3D" parent="World/NavigationRegion3D" 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] +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/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] +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/NS19FarPad" unique_id=1900033] shape = SubResource("BoxShape3D_ns19_farpad") -[node name="Obstacle" type="StaticBody3D" parent="World" unique_id=1638845763] +[node name="Obstacle" type="StaticBody3D" parent="World/NavigationRegion3D" unique_id=1638845763] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 6, 0, 5) collision_layer = 1 -[node name="MeshInstance3D" type="MeshInstance3D" parent="World/Obstacle" unique_id=750473628] +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/Obstacle" unique_id=750473628] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 1, 0) mesh = SubResource("BoxMesh_obstacle") -[node name="CollisionShape3D" type="CollisionShape3D" parent="World/Obstacle" unique_id=1344302688] +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/Obstacle" unique_id=1344302688] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 1, 0) shape = SubResource("BoxShape3D_obstacle") -[node name="Player" type="CharacterBody3D" parent="." unique_id=352931696] +[node name="Player" type="CharacterBody3D" parent="World/NavigationRegion3D" unique_id=352931696] transform = Transform3D(1, 0, 0, 0, 1, 0, 0, 0, 1, -5, 0.9, -5) collision_layer = 2 collision_mask = 1 +floor_max_angle = 0.872665 +floor_snap_length = 0.28 +safe_margin = 0.045 script = ExtResource("2_player") -[node name="CollisionShape3D" type="CollisionShape3D" parent="Player" unique_id=1695755590] +[node name="CollisionShape3D" type="CollisionShape3D" parent="World/NavigationRegion3D/Player" unique_id=1695755590] shape = SubResource("CapsuleShape3D_player") -[node name="MeshInstance3D" type="MeshInstance3D" parent="Player" unique_id=2027034386] +[node name="NavigationAgent3D" type="NavigationAgent3D" parent="World/NavigationRegion3D/Player" unique_id=8880002] +avoidance_enabled = false +path_desired_distance = 0.35 +target_desired_distance = 0.35 +path_height_offset = 0.0 +radius = 0.4 +height = 1.0 + +[node name="MeshInstance3D" type="MeshInstance3D" parent="World/NavigationRegion3D/Player" unique_id=2027034386] mesh = SubResource("CapsuleMesh_player") [node name="GroundPick" type="Node3D" parent="." unique_id=2500001] diff --git a/client/scripts/ground_pick.gd b/client/scripts/ground_pick.gd index 7bef4c8..3322ed7 100644 --- a/client/scripts/ground_pick.gd +++ b/client/scripts/ground_pick.gd @@ -1,14 +1,23 @@ 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. +## NS-19: reject vertical faces (pedestal). Stepped ray: steep walkable hits (bump slopes) advance +## along the ray so the next hit can be flat floor beyond — fixes “stuck” when leaving plateau. ## (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 +const MIN_WALKABLE_NORMAL_DOT_UP: float = 0.64 + +## Nudge past a steep walkable triangle so the ray can hit ground behind (meters along view ray). +const STEEP_PICK_THROUGH: float = 0.14 + +## Max ray segments (steep walkable → advance) before giving up. +const MAX_PICK_SEGMENTS: int = 24 + +## Below this dot, treat as wall — do not step through (pedestal sides stay non-pickable). +const WALL_NORMAL_DOT_CUTOFF: float = 0.09 ## Fallback when `Viewport.get_camera_3d()` is null (e.g. some editor setups). Set from `main.gd` ## in `_ready`. @@ -29,27 +38,45 @@ func _try_pick(screen_pos: Vector2) -> void: if cam == null: return - var origin: Vector3 = cam.project_ray_origin(screen_pos) var ray_dir: Vector3 = cam.project_ray_normal(screen_pos) - var to: Vector3 = origin + ray_dir * 2000.0 - var query := PhysicsRayQueryParameters3D.create(origin, to) - query.collision_mask = 1 + if ray_dir.length_squared() < 1e-12: + return + ray_dir = ray_dir.normalized() + var from: Vector3 = cam.project_ray_origin(screen_pos) var space: PhysicsDirectSpaceState3D = get_world_3d().direct_space_state - var hit: Dictionary = space.intersect_ray(query) - if hit.is_empty(): - return - var collider: Variant = hit.get("collider") - if not _collider_is_walkable(collider): - return - var hit_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) + const RAY_LEN: float = 2000.0 + var chosen: Vector3 = Vector3.ZERO + var have_chosen: bool = false + + for _i in range(MAX_PICK_SEGMENTS): + var to: Vector3 = from + ray_dir * RAY_LEN + var query := PhysicsRayQueryParameters3D.create(from, to) + query.collision_mask = 1 + var hit: Dictionary = space.intersect_ray(query) + if hit.is_empty(): + break + if not _collider_is_walkable(hit.get("collider")): + break + var hit_normal: Variant = hit.get("normal", Vector3.ZERO) + if hit_normal is not Vector3: + break + var nrm: Vector3 = (hit_normal as Vector3).normalized() + var ndot: float = nrm.dot(Vector3.UP) + if ndot >= MIN_WALKABLE_NORMAL_DOT_UP: + var pos_var: Variant = hit.get("position") + if pos_var is Vector3: + chosen = pos_var as Vector3 + have_chosen = true + break + if ndot <= WALL_NORMAL_DOT_CUTOFF: + break + var hp_var: Variant = hit.get("position") + if hp_var is not Vector3: + break + from = (hp_var as Vector3) + ray_dir * STEEP_PICK_THROUGH + + if have_chosen: + target_chosen.emit(chosen) func _collider_is_walkable(collider: Variant) -> bool: diff --git a/client/scripts/main.gd b/client/scripts/main.gd index d218071..6c8aaf1 100644 --- a/client/scripts/main.gd +++ b/client/scripts/main.gd @@ -2,6 +2,7 @@ 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). +## NS-23: bakes `NavigationRegion3D` on startup; boot snap vs nav goal from authority signal. const MOVE_REJECT_MSG_SECONDS: float = 4.0 @@ -9,7 +10,8 @@ const MOVE_REJECT_MSG_SECONDS: float = 4.0 var _move_reject_msg_token: int = 0 @onready var _camera: Camera3D = $World/Camera3D -@onready var _player: CharacterBody3D = $Player +@onready var _nav_region: NavigationRegion3D = $World/NavigationRegion3D +@onready var _player: CharacterBody3D = $World/NavigationRegion3D/Player @onready var _ground_pick: Node3D = $GroundPick @onready var _authority: Node = $PositionAuthorityClient @onready var _radius_preview: Node3D = $World/InteractionMarkers @@ -17,6 +19,12 @@ var _move_reject_msg_token: int = 0 func _ready() -> void: + await get_tree().process_frame + # Default `bake_navigation_mesh()` bakes on a background thread; sync bake ensures the map + # exists before any `NavigationAgent3D` queries (see Godot navigation_using_navigationagents). + _nav_region.bake_navigation_mesh(false) + await get_tree().physics_frame + await get_tree().physics_frame _ground_pick.set("fallback_camera", _camera) _ground_pick.connect("target_chosen", Callable(self, "_on_target_chosen")) _authority.connect( @@ -32,11 +40,16 @@ func _on_target_chosen(world: Vector3) -> void: _authority.call("submit_move_target", world) -func _on_authoritative_position(world: Vector3) -> void: - _player.snap_to_server(world) +func _on_authoritative_position(world: Vector3, apply_as_snap: bool) -> void: + if apply_as_snap: + _player.snap_to_server(world) + else: + _player.set_authoritative_nav_goal(world) func _on_move_rejected(reason_code: String) -> void: + # Rejected POST never reached VERIFY_GET, so no new nav target was applied; do not clear an + # in-flight path from a prior successful move. push_warning("Move rejected: %s" % reason_code) _move_reject_msg_token += 1 var token: int = _move_reject_msg_token diff --git a/client/scripts/player.gd b/client/scripts/player.gd index 48edee7..2d5078e 100644 --- a/client/scripts/player.gd +++ b/client/scripts/player.gd @@ -1,37 +1,110 @@ extends CharacterBody3D -## NS-14: click-to-move using horizontal steering + move_and_slide (not NavigationAgent3D). -## Obstacles are handled by physics sliding, not pathfinding — see client README. +## NS-23: Follow server move target on walkable geometry. +## +## Horizontal velocity toward nav waypoint or goal only; `velocity.y` stays 0. Height along ramps +## and steps comes from `move_and_slide` + floor_* on CharacterBody3D. Move **legality** (step, +## distance, bounds) is server MoveCommand only — not reimplemented here. +## +## Nav detours except when goal Y is below us: then xz toward goal (waypoints sit under bump rims). +## +## Idle: skip `move_and_slide()` when on floor and velocity is zero — repeated slide + floor snap +## causes visible micro-vibration at rest. const MOVE_SPEED: float = 5.0 const ARRIVE_EPS: float = 0.35 +const VERT_ARRIVE_EPS: float = 0.055 +const DIRECT_APPROACH_RADIUS: float = 0.85 +## Descend bypass: compares auth goal Y (often floor **surface** from pick) to `global_position.y` +## (**mid-capsule**). True for most floor clicks — smooth bumps; skips nav (bee-line xz), so +## obstacles may need multi-click. Nav waypoints under bump rims gave almost no horizontal speed. +const DESCEND_GOAL_Y_MARGIN: float = 0.06 -var _goal: Vector3 +var _has_walk_goal: bool = false +var _auth_walk_goal: Vector3 = Vector3.ZERO + +@onready var _nav_agent: NavigationAgent3D = $NavigationAgent3D func _ready() -> void: - _goal = global_position + _nav_agent.set_target_position(global_position) -func set_move_goal(world_pos: Vector3) -> void: - _goal = world_pos - _goal.y = global_position.y +func set_authoritative_nav_goal(world_pos: Vector3) -> void: + _auth_walk_goal = world_pos + _has_walk_goal = true + _nav_agent.set_target_position(world_pos) + + +func clear_nav_goal() -> void: + velocity = Vector3.ZERO + _has_walk_goal = false + _nav_agent.set_target_position(global_position) -## NS-16: snap to authoritative server position; clears velocity so `move_and_slide` does not fight -## the sync. func snap_to_server(world_pos: Vector3) -> void: global_position = world_pos velocity = Vector3.ZERO - _goal = world_pos + _has_walk_goal = false + _nav_agent.set_target_position(world_pos) + + +func _set_horizontal_velocity_toward(point: Vector3) -> void: + var pos := global_position + var dh: Vector3 = Vector3(point.x - pos.x, 0.0, point.z - pos.z) + if dh.length_squared() < 1e-10: + dh = Vector3(1.0, 0.0, 0.0) + velocity = dh.normalized() * MOVE_SPEED + velocity.y = 0.0 + + +## `is_on_floor()` comes from the last `move_and_slide()`. Skipping slide while idle reuses that +## stale floor state — OK for static floors; revisit if walkable geometry moves. +func _skip_move_when_idle_on_floor() -> bool: + return is_on_floor() and velocity.length_squared() < 1e-12 func _physics_process(_delta: float) -> void: - var to_goal: Vector3 = _goal - global_position - to_goal.y = 0.0 - if to_goal.length() <= ARRIVE_EPS: + if not _has_walk_goal: velocity = Vector3.ZERO + if _skip_move_when_idle_on_floor(): + return + move_and_slide() + return + + var full_to_goal: Vector3 = _auth_walk_goal - global_position + var horiz_dist: float = Vector2(full_to_goal.x, full_to_goal.z).length() + var vert_err: float = absf(full_to_goal.y) + if horiz_dist <= ARRIVE_EPS and vert_err <= VERT_ARRIVE_EPS: + velocity = Vector3.ZERO + _has_walk_goal = false + _nav_agent.set_target_position(global_position) + if _skip_move_when_idle_on_floor(): + return + move_and_slide() + return + + var nav_map: RID = _nav_agent.get_navigation_map() + if NavigationServer3D.map_get_iteration_id(nav_map) == 0: + _set_horizontal_velocity_toward(_auth_walk_goal) + move_and_slide() + return + + if _auth_walk_goal.y < global_position.y - DESCEND_GOAL_Y_MARGIN: + _set_horizontal_velocity_toward(_auth_walk_goal) + move_and_slide() + return + + if horiz_dist <= DIRECT_APPROACH_RADIUS: + _set_horizontal_velocity_toward(_auth_walk_goal) else: - velocity = to_goal.normalized() * MOVE_SPEED - velocity.y = 0.0 + var next_path_position: Vector3 = _nav_agent.get_next_path_position() + var to_next_h: Vector3 = Vector3( + next_path_position.x - global_position.x, 0.0, next_path_position.z - global_position.z + ) + if to_next_h.length_squared() > 0.0025: + _set_horizontal_velocity_toward(next_path_position) + else: + _set_horizontal_velocity_toward(_auth_walk_goal) + move_and_slide() diff --git a/client/scripts/position_authority_client.gd b/client/scripts/position_authority_client.gd index 3f0df7a..a8b832a 100644 --- a/client/scripts/position_authority_client.gd +++ b/client/scripts/position_authority_client.gd @@ -2,9 +2,10 @@ extends Node ## NS-16: POST MoveCommand then GET PositionState; emits authoritative world position for the ## avatar. NS-19: failed validation (HTTP 400 + rejection body) emits `move_rejected`. +## NS-23: second signal arg — `true` = boot snap, `false` = post-move (nav goal, no teleport). ## (No `class_name` so headless/CI can load the project before `.godot` import exists.) -signal authoritative_position_received(world: Vector3) +signal authoritative_position_received(world: Vector3, apply_as_snap: bool) signal move_rejected(reason_code: String) enum Phase { BOOT_GET, POST_MOVE, VERIFY_GET } @@ -79,7 +80,7 @@ func _on_request_completed( Phase.BOOT_GET: _busy = false if response_code == 200: - _emit_position_from_response(text) + _emit_position_from_response(text, true) Phase.POST_MOVE: if response_code == 400: _emit_move_rejection_if_present(text) @@ -96,7 +97,7 @@ func _on_request_completed( Phase.VERIFY_GET: _busy = false if response_code == 200: - _emit_position_from_response(text) + _emit_position_from_response(text, false) func _emit_move_rejection_if_present(json_text: String) -> void: @@ -110,7 +111,7 @@ func _emit_move_rejection_if_present(json_text: String) -> void: move_rejected.emit("unknown") -func _emit_position_from_response(json_text: String) -> void: +func _emit_position_from_response(json_text: String, apply_as_snap: bool) -> void: var parsed: Variant = JSON.parse_string(json_text) if parsed == null: return @@ -124,4 +125,4 @@ func _emit_position_from_response(json_text: String) -> void: var x: float = float(pos.get("x", 0.0)) var y: float = float(pos.get("y", 0.0)) var z: float = float(pos.get("z", 0.0)) - authoritative_position_received.emit(Vector3(x, y, z)) + authoritative_position_received.emit(Vector3(x, y, z), apply_as_snap) diff --git a/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md b/docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md index 5fd2062..607e12b 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); **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). +- **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 **`POST`/`GET`** move flow ([NS-15](../../plans/NS-15-implementation-plan.md), [NS-16](../../plans/NS-16-implementation-plan.md), `server/NeonSprawl.Server/Game/PositionState/`, `client/scripts/`). **NS-23** — client **`NavigationRegion3D` / `NavigationAgent3D`** for click-to-move visuals while server authority unchanged; **single-click obstacle detours not guaranteed** (see plan tradeoff) ([NS-23](../../plans/NS-23-implementation-plan.md); [client README](../../../client/README.md#authoritative-movement-ns-16-ns-23)). **`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-16-implementation-plan.md b/docs/plans/NS-16-implementation-plan.md index 8327234..dabe1e5 100644 --- a/docs/plans/NS-16-implementation-plan.md +++ b/docs/plans/NS-16-implementation-plan.md @@ -54,7 +54,7 @@ 5. **Godot client** — **Split scripts by concern** (required repo policy: [godot-client-script-organization.md](../../.cursor/rules/godot-client-script-organization.md)); do **not** grow a monolithic `main.gd` with pick + HTTP + wiring. - **`ground_pick.gd`** on a new **`Node3D`** child (e.g. `GroundPick`): owns walkable **ray pick** (logic moved out of NS-14 `main.gd`). Exposes **`target_chosen(Vector3)`** when the user clicks valid ground. **`main.gd`** sets **`fallback_camera`** (or equivalent) in `_ready`. - - **`position_authority_client.gd`** on a new **`Node`** child: `@export` **base URL** (default `http://127.0.0.1:5253`, match `launchSettings.json`) and **dev player id** (`dev-local-1`). Creates **`HTTPRequest`** in **`_ready`** via **`add_child`** (not as a node hand-placed in `main.tscn`). Flow: **`POST`** `MoveCommand`, then **`GET`** position (tests must show **GET** reflects apply). Emits **`authoritative_position_received(Vector3)`** so **`main`** only snaps the player. + - **`position_authority_client.gd`** on a new **`Node`** child: `@export` **base URL** (default `http://127.0.0.1:5253`, match `launchSettings.json`) and **dev player id** (`dev-local-1`). Creates **`HTTPRequest`** in **`_ready`** via **`add_child`** (not as a node hand-placed in `main.tscn`). Flow: **`POST`** `MoveCommand`, then **`GET`** position (tests must show **GET** reflects apply). Emits **`authoritative_position_received(Vector3, bool)`** (NS-23: second arg = boot snap vs nav goal) so **`main`** snaps on boot and path-follows after move verify. - **`main.gd`:** Thin composer only—connect signals, call **`sync_from_server()`** on run, delegate pick and HTTP to the two child scripts. - Align **client README** with NS-14 note: server authority path replaces pure local steering for this demo. diff --git a/docs/plans/NS-23-implementation-plan.md b/docs/plans/NS-23-implementation-plan.md new file mode 100644 index 0000000..821ab2f --- /dev/null +++ b/docs/plans/NS-23-implementation-plan.md @@ -0,0 +1,126 @@ +# NS-23 — Implementation plan + +## Story reference + +| Field | Value | +|--------|--------| +| **Key** | NS-23 | +| **Title** | E1.M1: Client path-follow baseline (navigation + visible locomotion) | +| **Jira** | [NS-23](https://neon-sprawl.atlassian.net/browse/NS-23) | +| **Parent context** | [NS-10 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NS-10) · [NS-1 — Epic 1](https://neon-sprawl.atlassian.net/browse/NS-1) · Slice 1 — Movement and position sync | +| **Decomposition** | [E1.M1 — InputAndMovementRuntime](../decomposition/modules/E1_M1_InputAndMovementRuntime.md) | + +**Delivery:** Ship **`docs/plans/NS-23-implementation-plan.md`** on the **same branch / PR** as the NS-23 implementation (plan + code together), matching the NS-19 pattern. + +## Goal, scope, and out-of-scope + +**Goal:** Deliver **visible** click-to-move: the avatar **moves** toward the verified server target using **`NavigationAgent3D`** + a **baked mesh** where applicable, instead of teleporting on the verifying **`GET`**. Aligns with E1.M1: path-follow baseline under server authority. + +**In scope** + +- **Godot:** `NavigationRegion3D` (baked mesh) over the prototype walkable floor in `main.tscn` (obstacle geometry included in bake for future use); **`NavigationAgent3D`** on the player driving **`CharacterBody3D`** motion (`move_and_slide` toward next path position when the client is following the mesh — Godot 4.x pattern). +- **Pick integration:** Reuse **`ground_pick.gd`** (`walkable` group, upward normal); clicked point becomes **`NavigationAgent3D.target_position`** (after any needed Y / agent height adjustment consistent with current capsule). +- **Server authority (unchanged contract):** One **`MoveCommand`** per successful pick with the **same world target** as today ([NS-16](NS-16-implementation-plan.md)); server **v1 snap** + **[NS-19](NS-19-implementation-plan.md)** validation unchanged. + +**Out of scope (per Jira)** + +- Full prediction/reconciliation, MMO-grade netcode, server-side nav mesh validation. +- Replacing **`InteractionRequest`** / range rules. + +## Locked — client nav vs server authority (this slice) + +| Topic | Decision | +|--------|----------| +| **When to POST** | **Once per click**, immediately with the **picked destination** (same JSON as today). No waypoint streaming to the server in this story. | +| **Server truth** | Still **snap to target** on **200** after validation; **`GET /position`** after POST remains the reconciliation read (existing `PositionAuthorityClient` flow). | +| **Straight-line vs path** | NS-19 validates **displacement** from **current authoritative position** to **target** (horizontal + vertical limits). It does **not** check path clearance through geometry. **Client** may follow nav waypoints when the descend bypass is inactive; **automatic obstacle detours are not required** for this slice (see **Locked — prototype movement tradeoff**). | +| **After successful move** | **Do not** teleport the avatar on the verifying **`GET`** when the player is expected to walk: apply the authoritative position as a **navigation goal** and let the agent complete the path; **initial boot** `sync_from_server` remains a **hard snap** so spawn matches the server. | +| **POST 400** | Existing behavior: emit **`move_rejected`**, show NS-19 UX; **do not** start (or clear) a nav path to that target. | +| **Distance limits** | Default **`MaxHorizontalStep`** (e.g. 18 m) remains compatible with **single-command** long clicks across the prototype; if a future scene needs shorter steps, chain commands or tune limits in a later story. | + +## Locked — prototype movement tradeoff + +| Topic | Decision | +|--------|----------| +| **Uneven surfaces vs obstacles** | The client uses a **descend bypass** in `player.gd`: when the authoritative goal **Y** is below the body origin (typical for floor picks vs mid-capsule), steering goes **straight in xz** toward the goal instead of following nav waypoints first. This keeps **stepped bumps** smooth; it does **not** guarantee routing around tall **static obstacles** on a **single** click. | +| **Player expectation** | **Multi-click** (or chained moves) is acceptable to navigate around geometry the client would otherwise bee-line into. **No** automatic obstacle navigation is required for NS-23 closure. | +| **Residual polish** | Visible idle jitter at rest is tracked as tech debt — [NS-24](https://neon-sprawl.atlassian.net/browse/NS-24) (not a blocker for this story). | + +## Acceptance criteria checklist + +- [x] Clicking a valid floor target produces **continuous motion** toward the verified target without sliding through geometry in the common case; **navmesh waypoints** are used when the descend bypass does not apply. **Obstacle detours on one click are not required** (tradeoff above). +- [x] Authoritative position still **converges** with the server (`MoveCommand` + `GET`); no regression on NS-19 rejection UX (`reasonCode` / label timeout). +- [x] Short note in **`server/README.md`** and/or **client script header**: client nav vs server authority for this slice. + +## Technical approach + +1. **`main.tscn` / navigation** + - Add **`NavigationRegion3D`** covering walkable floor; include obstacle **`StaticBody3D`** geometry in baking as required by Godot 4 so paths wrap obstacles. + - Bake after layout changes; document any **editor step** in `client/README.md` if bake is not fully reproducible from scene alone. + +2. **Player / agent** + - Attach **`NavigationAgent3D`**; replace NS-14-style **direct steering toward `_goal`** with **path following** (velocity toward `get_next_path_position()` or equivalent, `move_and_slide`). + - Keep **`snap_to_server`** for **boot** and any **hard reconcile** case you explicitly document (default: boot only). + +3. **Wiring (`main.gd` / `PositionAuthorityClient`)** + - On **`target_chosen`**: call **`submit_move_target`** as today (POST → GET). + - Distinguish **authoritative position from boot** vs **after successful move** so **`main.gd`** snaps on boot but sets **nav target** (no snap) after verify when implementing path-follow. + - Optional small API: second signal, enum on existing signal, or `sync_from_server`-only snap path — pick the smallest change that stays readable per [godot-client-script-organization](../../.cursor/rules/godot-client-script-organization.md). + +4. **Documentation** + - **`client/README.md`:** Movement section: nav agent + server authority, and the **uneven-surface vs obstacle** tradeoff (multi-click around obstacles). + - **`server/README.md`:** Server validates **end target** and **step**; **client** nav is **presentational** only (no server navmesh); obstacle detours not guaranteed. + +## Resolved — agent shape vs capsule (was risk) + +Prototype player collider in [`main.tscn`](../../client/scenes/main.tscn): **`CapsuleShape3D_player`** — **`radius = 0.4`**, **`height = 1.0`**. + +| Setting | Locked choice | +|--------|----------------| +| **`NavigationAgent3D.radius` / `height`** | Match the **physics** capsule (**0.4** / **1.0**) so nav clearance matches what `move_and_slide` actually sweeps. | +| **Navmesh bake agent** | In **`NavigationRegion3D`** bake settings, use the **same** radius/height (or Godot’s equivalent **agent** parameters) so baked polygons match runtime pathfinding. | +| **Corner clipping vs “stuck”** | If the mesh clips into box corners, **increase** agent radius slightly and **rebake** (wider corridor on the mesh) before shrinking physics. Do not silently shrink the capsule without an explicit design change. | +| **Vertical alignment** | Keep **`path_height_offset`** (or equivalent) at **0** unless the capsule foot position and floor snap misalign; the pick already supplies floor **`Y`**. | +| **Arrival** | Align **`target_desired_distance`** (and related agent tolerances) with existing player **`ARRIVE_EPS`** (~**0.35** m) so stop distance matches prior steering feel unless QA says otherwise. | + +## Resolved — navigation sync / frame order (was risk) + +| Topic | Locked choice | +|--------|----------------| +| **Where motion runs** | Path queries (`get_next_path_position`, velocity toward next point) and **`move_and_slide`** run only in **`Player._physics_process`**, not in `_process` or UI thread. | +| **When `target_position` is set** | `main.gd` (or signal handler) may set the agent target in response to **`target_chosen`** / authority callbacks; the **first** consumable segment appears after the navigation map updates. | +| **First frame after new target** | If **`NavigationAgent3D.is_navigation_finished()`** or a zero-length first step appears on the **same** frame as `target_position` assignment, **defer** one tick: `await get_tree().physics_frame` **once** after setting target **or** simply rely on **next** `_physics_process` (preferred — avoid await in hot path). **Do not** poll path from `_input` without deferral. | +| **Map readiness** | After adding **`NavigationRegion3D`**, ensure the region is **baked** and enabled before F5; startup **`sync_from_server`** snap does not need a path. | + +## Files to add (expected) + +| Path | Purpose | +|------|---------| +| None required by name | Prefer extending existing `player.gd` / scene unless a dedicated `player_navigation.gd` reduces `main.gd` bloat. | + +## Files to modify + +| Path | Rationale | +|------|-----------| +| `client/scenes/main.tscn` | `NavigationRegion3D`, bake source meshes, agent node setup. | +| `client/scripts/player.gd` | Path-follow via `NavigationAgent3D`; clarify header (NS-14 → NS-23 evolution). | +| `client/scripts/main.gd` | Boot snap vs post-move nav goal wiring. | +| `client/scripts/position_authority_client.gd` | If needed: signal shape or phase so main can snap only on boot. | +| `client/README.md` | Manual check steps for path-follow + server. | +| `server/README.md` | Client nav vs authority note (AC). | +| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | Implementation snapshot when NS-23 ships. | + +## Tests + +| Action | What to cover | +|--------|----------------| +| **Manual (required)** | Server running; click-move toward targets (including stepped bumps); NS-19 reject targets — same rejection behavior as before; cold start — player matches server position. Optional: confirm multi-click can route around the prototype **`Obstacle`** when a single click bee-lines. | +| **Automated** | None required for this story unless an existing headless hook can assert nav baking (unlikely); prefer manual for Godot nav. | + +## Open questions / risks + +**Prototype tradeoff** is explicit under **Locked — prototype movement tradeoff** (bumps vs single-click obstacle routing). Agent/bake alignment and physics-frame path consumption remain locked in **Resolved — agent shape vs capsule** and **Resolved — navigation sync / frame order**. Reopen if scene scale or avatar collider changes materially. + +## PR / review + +Cross-check [E1.M1](../decomposition/modules/E1_M1_InputAndMovementRuntime.md), [client README](../../client/README.md), and [server README](../../server/README.md). Confirm NS-19 props in `main.tscn` still behave after nav region changes. diff --git a/docs/reviews/2026-04-05-NS-23.md b/docs/reviews/2026-04-05-NS-23.md new file mode 100644 index 0000000..de4d4f1 --- /dev/null +++ b/docs/reviews/2026-04-05-NS-23.md @@ -0,0 +1,55 @@ +# Code review: NS-23 client path-follow (`NS-23-client-path-follow`) + +**Date:** 2026-04-05 +**Scope:** Branch `NS-23-client-path-follow` vs `origin/main` (full NS-23 slice: client nav, wiring, docs, small server remark). +**Base:** `origin/main` + +**Follow-up:** Design and docs were aligned after the initial review; blocking items and suggestions below are **done** unless marked otherwise. + +## Verdict + +**Approve** — original **Request changes** (plan/README vs obstacle wording) and review **suggestions** 1–3 are **resolved** in follow-up commits. Optional **nit** (rename `DESCEND_GOAL_Y_MARGIN`) remains open. + +## Summary + +The branch adds **`NavigationRegion3D`** / **`NavigationAgent3D`**, sync nav bake in **`main.gd`**, boot snap vs post-move nav goal via **`PositionAuthorityClient`**’s second signal argument, and horizontal path-follow in **`player.gd`**. **`ground_pick.gd`** gains stepped walkable picking for steep surfaces. Server change is documentation-only on **`MoveCommandValidation`** (vertical step remark). + +**`player.gd`** intentionally skips nav waypoints when **`_auth_walk_goal.y < global_position.y - DESCEND_GOAL_Y_MARGIN`**, so for typical picks (floor **Y** vs mid-capsule origin) the client often **bee-lines in xz** toward the goal. That matches the **locked prototype tradeoff** (smooth bumps; multi-click around obstacles when needed). + +**Idle optimization:** skipping **`move_and_slide()`** when there is no walk goal, velocity is zero, and **`is_on_floor()`** is a reasonable jitter mitigation; residual vibration is tracked in **[NS-24](https://neon-sprawl.atlassian.net/browse/NS-24)** (Tech Debt). + +## Documentation checked + +| Document | Result (at closure) | +|----------|---------------------| +| `docs/plans/NS-23-implementation-plan.md` | **Matches** after **Locked — prototype movement tradeoff**, updated AC, tests, and **NS-24** link. | +| `docs/decomposition/modules/E1_M1_InputAndMovementRuntime.md` | **Matches** — snapshot notes single-click obstacle detours not guaranteed. | +| `docs/decomposition/modules/client_server_authority.md` | **Matches** — server owns **`PositionState`**; client presentation path valid. | +| `docs/decomposition/modules/module_dependency_register.md` / `documentation_and_implementation_alignment.md` | **N/A** deep diff. | +| `client/README.md` | **Matches** — tradeoff, manual steps, **Known issue** + **NS-24** link. | +| `server/README.md` | **Matches** — client nav presentation; detours not guaranteed on one click. | + +## Blocking issues + +**All resolved.** + +1. ~~**Plan and README vs behavior:**~~ **Done.** Plan and **`client/README.md`** now state the **locked tradeoff** (descend bypass → xz bee-line; navmesh detours not guaranteed on one click; multi-click OK). AC checklist and manual QA updated accordingly. + +## Suggestions + +**All done.** + +1. ~~**`DESCEND_GOAL_Y_MARGIN`:**~~ **Done.** Comment added in `client/scripts/player.gd` (surface pick Y vs mid-capsule; bumps vs obstacles). +2. ~~**`_skip_move_when_idle_on_floor`:**~~ **Done.** Doc comment above the helper (`is_on_floor()` / last `move_and_slide()`; static floors). +3. ~~**NS-24:**~~ **Done.** Out of NS-23 scope in plan; **[NS-24](https://neon-sprawl.atlassian.net/browse/NS-24)** linked from **`NS-23-implementation-plan.md`** and **`client/README.md`**. + +## Nits + +- **Nit (optional):** Consider renaming **`DESCEND_GOAL_Y_MARGIN`** to something that signals “auth pick Y vs body origin” to reduce future “why do we always bee-line?” confusion. **Not done** — purely optional. + +## Verification + +- `cd server/NeonSprawl.Server && dotnet test` (no logic change expected, sanity). +- Godot **F5** with server: boot snap; click floor; bump flow; gray **Obstacle** may need **multiple clicks** per docs. +- Confirm NS-19 reject targets still show rejection UX (`reasonCode` / label). +- `gdlint` / `gdformat` on touched GDScript if CI does not already run on the branch. diff --git a/server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs b/server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs index 0dc1c86..ef75be9 100644 --- a/server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs +++ b/server/NeonSprawl.Server/Game/PositionState/MoveCommandValidation.cs @@ -4,6 +4,8 @@ namespace NeonSprawl.Server.Game.PositionState; /// /// Precedence when multiple rules fail: , /// then , then . +/// Vertical uses abs(ΔY), so descending (target Y lower than current) is allowed when the +/// drop is within — same as climbing. /// public static class MoveCommandValidation { diff --git a/server/README.md b/server/README.md index d62b873..d254523 100644 --- a/server/README.md +++ b/server/README.md @@ -63,6 +63,8 @@ Unknown player ids return **404**. Full zone sync / replication is still out of **`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. +**Client navigation (NS-23):** The Godot prototype uses a **baked navigation mesh** only for **presentation**; the client may follow waypoints when active, but **automatic obstacle detours on a single click are not guaranteed** (tradeoff for smooth movement on stepped geometry). The server does **not** simulate navmesh; it validates **straight-line** step limits (NS-19) from the **last authoritative position** to the **requested target**. Cheating or divergent paths are out of scope for this slice. + **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`) |