Client-side position sync + prediction/reconciliation
Real client-side networking for the player controller, verified with a genuine two-process integration test (an actual dedicated server process + a real ENet-connected client), per docs/architecture/networking.md §5. scripts/network_client.gd: NetworkClient -- predicts movement locally on input (same movement constants as scripts/player_spawner.gd), sends input to the server via RPC, and reconciles against the server's authoritative snapshots when they diverge beyond a threshold. scripts/network_receiver_proxy.gd + network_player_proxy.gd: the RPC plumbing that makes this actually route correctly, which took real debugging to get right: - Godot's rpc_id() resolves the receiving node by matching the sending node's NodePath between peers. scripts/server.gd is the SceneTree root node "Server" when the dedicated server runs, and spawns each player as "Player_<peer_id>" under itself -- so the client needs its own node tree at those same paths purely for RPC routing. - Godot also enforces an RPC "node checksum": both peers must declare the *same set* of @rpc methods at a matching path, or every RPC at that path fails, not just a genuinely missing one. Every @rpc method on scripts/server.gd's root and scripts/player_spawner.gd had to be mirrored here (mostly as no-op stubs), not just the ones this class actually calls. Known, explicitly-scoped simplification (stated in the code, not hidden): scripts/server.gd's snapshot doesn't currently echo back a per-peer "last input seq processed", so reconciliation corrects against the most recent prediction rather than true tick-exact rollback-and-replay (networking.md §5.2 step 4). A real follow-up once the server echoes that ack, not faked here. Verified: real two-process test (godot --headless --path . scenes/ server.tscn as an actual persistent process, a second process running scripts/smoke_test_network_client.gd against it over real ENet) -- 4/4 passing: connects, predicts locally before any server round trip, sends real input, and a deliberately-desynced position (moved 9999 units away) gets corrected by the very next real snapshot. Full existing client suite (10 suites) + 61 backend pytest still passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M6rUEynmv2QmweocAQYARN
This commit is contained in:
@@ -53,7 +53,8 @@ these yet):
|
|||||||
|
|
||||||
## Godot client — `scripts/smoke_test_*.gd`
|
## Godot client — `scripts/smoke_test_*.gd`
|
||||||
|
|
||||||
**10 smoke tests merged into `main`.** Each needs a live backend —
|
**11 smoke tests merged into `main`.** Ten need a live backend, one
|
||||||
|
(`smoke_test_network_client.gd`) needs a live dedicated server instead —
|
||||||
`AGP_API_BASE_URL` env var, falling back to the
|
`AGP_API_BASE_URL` env var, falling back to the
|
||||||
`application/agp/api_base_url` project setting (default
|
`application/agp/api_base_url` project setting (default
|
||||||
`http://127.0.0.1:8000`) if unset.
|
`http://127.0.0.1:8000`) if unset.
|
||||||
@@ -70,6 +71,7 @@ these yet):
|
|||||||
| `smoke_test_test_level.gd` | Loads `scenes/worlds/test_level.tscn` directly (the level is player-agnostic and self-contained per `docs/architecture/pc2_test_level.md` §4) and confirms it has real content: 5 props, a `DirectionalLight3D` ("Sun"), and a `WorldEnvironment` — not an empty void. |
|
| `smoke_test_test_level.gd` | Loads `scenes/worlds/test_level.tscn` directly (the level is player-agnostic and self-contained per `docs/architecture/pc2_test_level.md` §4) and confirms it has real content: 5 props, a `DirectionalLight3D` ("Sun"), and a `WorldEnvironment` — not an empty void. |
|
||||||
| `smoke_test_player.gd` | Loads `scenes/game.tscn` and confirms the player exists as a real `CharacterBody3D` with a `CollisionShape3D`+shape, a `CapsuleMesh`, and spawns at the level's `PlayerSpawn` marker position. Also asserts `JUMP_VELOCITY` is positive and a `Camera3D` exists and is `current` — both were real bugs found 2026-08-20/21 (jump velocity was negative, pushing the player down instead of up; no camera existed at all, so the game was unplayable even though the character could technically move). Structural checks only — real movement/jump *direction* correctness was verified separately via a throwaway headless physics simulation during development, not by an automated test that's part of this suite. |
|
| `smoke_test_player.gd` | Loads `scenes/game.tscn` and confirms the player exists as a real `CharacterBody3D` with a `CollisionShape3D`+shape, a `CapsuleMesh`, and spawns at the level's `PlayerSpawn` marker position. Also asserts `JUMP_VELOCITY` is positive and a `Camera3D` exists and is `current` — both were real bugs found 2026-08-20/21 (jump velocity was negative, pushing the player down instead of up; no camera existed at all, so the game was unplayable even though the character could technically move). Structural checks only — real movement/jump *direction* correctness was verified separately via a throwaway headless physics simulation during development, not by an automated test that's part of this suite. |
|
||||||
| `smoke_test_enter_world.gd` | The full PC2 path end to end: register → seed a real world → load `world_list.tscn` → click the real "Enter World" button on a real card (found by walking the card's children, not by index, so it doesn't silently pass if the card layout changes) → confirms `scenes/game.tscn` actually becomes `current_scene` with the player spawned at the level's `PlayerSpawn` marker → sends a real `KEY_ESCAPE` input event (not a direct method call) and confirms it returns to `world_list.tscn`. This is the test that proves "browse → enter → walk around → leave" actually works as one continuous flow, not just that each piece works in isolation. |
|
| `smoke_test_enter_world.gd` | The full PC2 path end to end: register → seed a real world → load `world_list.tscn` → click the real "Enter World" button on a real card (found by walking the card's children, not by index, so it doesn't silently pass if the card layout changes) → confirms `scenes/game.tscn` actually becomes `current_scene` with the player spawned at the level's `PlayerSpawn` marker → sends a real `KEY_ESCAPE` input event (not a direct method call) and confirms it returns to `world_list.tscn`. This is the test that proves "browse → enter → walk around → leave" actually works as one continuous flow, not just that each piece works in isolation. |
|
||||||
|
| `smoke_test_network_client.gd` | **Real two-process integration test** for `scripts/network_client.gd` (client-side prediction + reconciliation, docs/architecture/networking.md §5) — needs an actual dedicated server running first: `godot --headless --path . scenes/server.tscn &`, *not* the usual HTTP backend. Connects a real `NetworkClient` over ENet, confirms local input prediction moves the player immediately (no waiting on a round trip), confirms real snapshots arrive and get processed, then deliberately desyncs the predicted position by 9999 units and confirms the very next snapshot corrects it back — the actual proof reconciliation logic works, not just that it never had to fire. Known simplification, stated in the script's own docstring: the server's snapshot doesn't currently echo back a per-peer "last input seq processed," so reconciliation corrects against the *most recent* prediction rather than true tick-exact rollback-and-replay (docs/architecture/networking.md §5.2 step 4) — a real, scoped follow-up once the server echoes that ack, not something faked here. |
|
||||||
|
|
||||||
Run (once, to build the class cache): `godot --headless --path . --import`,
|
Run (once, to build the class cache): `godot --headless --path . --import`,
|
||||||
then for each smoke test: `godot --headless --path . --script
|
then for each smoke test: `godot --headless --path . --script
|
||||||
|
|||||||
@@ -196,7 +196,6 @@ func _handle_snapshot(snapshot_data: Dictionary) -> void:
|
|||||||
# RPC payloads can arrive as String or int depending on the transport
|
# RPC payloads can arrive as String or int depending on the transport
|
||||||
# encoding, so check both rather than assuming.
|
# encoding, so check both rather than assuming.
|
||||||
var my_state: Variant = players.get(my_id, players.get(str(my_id), null))
|
var my_state: Variant = players.get(my_id, players.get(str(my_id), null))
|
||||||
print("DEBUG _handle_snapshot my_id=", my_id, " my_state=", my_state, " buffer_size=", _prediction_buffer.size(), " net.position=", position)
|
|
||||||
if my_state == null or not (my_state is Dictionary):
|
if my_state == null or not (my_state is Dictionary):
|
||||||
return
|
return
|
||||||
|
|
||||||
|
|||||||
@@ -20,7 +20,6 @@ var owner_client: Node = null
|
|||||||
## config, a mismatch here would silently drop the call.
|
## config, a mismatch here would silently drop the call.
|
||||||
@rpc("any_peer", "call_remote", "reliable")
|
@rpc("any_peer", "call_remote", "reliable")
|
||||||
func _receive_snapshot(snapshot_data: Dictionary) -> void:
|
func _receive_snapshot(snapshot_data: Dictionary) -> void:
|
||||||
print("DEBUG proxy received snapshot: ", snapshot_data)
|
|
||||||
if owner_client and is_instance_valid(owner_client):
|
if owner_client and is_instance_valid(owner_client):
|
||||||
owner_client._handle_snapshot(snapshot_data)
|
owner_client._handle_snapshot(snapshot_data)
|
||||||
|
|
||||||
|
|||||||
@@ -68,9 +68,17 @@ func _run() -> void:
|
|||||||
# not just that it never had to fire.
|
# not just that it never had to fire.
|
||||||
net.position = Vector3(9999, 9999, 9999)
|
net.position = Vector3(9999, 9999, 9999)
|
||||||
var desynced_pos := net.position
|
var desynced_pos := net.position
|
||||||
var reconciled_this_round := false
|
# GDScript closures capture local value-type vars (bool/float) by value,
|
||||||
var correction_seen: float = -1.0
|
# not by reference -- assigning to them *inside* the lambda doesn't
|
||||||
var reconcile_handler := func(err): reconciled_this_round = true; correction_seen = err
|
# mutate the outer scope's copy. A single-element Array is the standard
|
||||||
|
# workaround (Arrays are reference types, so mutating its contents from
|
||||||
|
# inside the closure is actually visible outside it). Confirmed this was
|
||||||
|
# the real bug here, not the reconciliation logic itself: debug logging
|
||||||
|
# showed net.position correctly snapping back from (9999,9999,9999) to
|
||||||
|
# ~(0,0,0) within a single snapshot, while a bool-based version of this
|
||||||
|
# same check never observed it.
|
||||||
|
var reconcile_state := [false, -1.0] # [reconciled_this_round, correction_seen]
|
||||||
|
var reconcile_handler := func(err): reconcile_state[0] = true; reconcile_state[1] = err
|
||||||
net.reconciled.connect(reconcile_handler)
|
net.reconciled.connect(reconcile_handler)
|
||||||
for i in 60:
|
for i in 60:
|
||||||
# Keep predicting (with no movement input) so the prediction buffer's
|
# Keep predicting (with no movement input) so the prediction buffer's
|
||||||
@@ -79,13 +87,13 @@ func _run() -> void:
|
|||||||
# entries, and _handle_snapshot has nothing real to compare against.
|
# entries, and _handle_snapshot has nothing real to compare against.
|
||||||
net.predict_and_send(1.0 / 30.0, 0.0, 0.0, false)
|
net.predict_and_send(1.0 / 30.0, 0.0, 0.0, false)
|
||||||
await process_frame
|
await process_frame
|
||||||
if reconciled_this_round:
|
if reconcile_state[0]:
|
||||||
break
|
break
|
||||||
net.reconciled.disconnect(reconcile_handler)
|
net.reconciled.disconnect(reconcile_handler)
|
||||||
_check("a deliberately desynced position gets corrected by the next snapshot",
|
_check("a deliberately desynced position gets corrected by the next snapshot",
|
||||||
reconciled_this_round and net.position.distance_to(desynced_pos) > 100.0)
|
reconcile_state[0] and net.position.distance_to(desynced_pos) > 100.0)
|
||||||
if reconciled_this_round:
|
if reconcile_state[0]:
|
||||||
print(" correction distance was ~", correction_seen)
|
print(" correction distance was ~", reconcile_state[1])
|
||||||
|
|
||||||
net.disconnect_from_server()
|
net.disconnect_from_server()
|
||||||
_finish()
|
_finish()
|
||||||
|
|||||||
Reference in New Issue
Block a user