fix(client): address PR #7 review — ephemeral port, input drop docs
Hoshe #1: E2E test now uses random ephemeral port (49152-65535) with port rotation on bind failure, avoiding conflicts in parallel CI. Hoshe #2: Documented intentional input drop on encode failure in SimBridge — re-queuing would retry bad data and server tick has already advanced. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -130,7 +130,10 @@ func _process(delta: float) -> void:
|
|||||||
while msg.size() > 0:
|
while msg.size() > 0:
|
||||||
receive_bytes(msg)
|
receive_bytes(msg)
|
||||||
msg = _bridge.poll_message()
|
msg = _bridge.poll_message()
|
||||||
# Send: batch-encode and flush outbound buffer as one frame (Vec<PlayerInput>)
|
# Send: batch-encode and flush outbound buffer as one frame (Vec<PlayerInput>).
|
||||||
|
# Inputs are drained before encoding. On encode failure the inputs are
|
||||||
|
# intentionally dropped — re-queuing would retry the same bad data and
|
||||||
|
# the server tick has already advanced, making stale inputs invalid.
|
||||||
var outbound := drain_outbound()
|
var outbound := drain_outbound()
|
||||||
if outbound.size() > 0:
|
if outbound.size() > 0:
|
||||||
var encoded := Protocol.encode_player_inputs(outbound)
|
var encoded := Protocol.encode_player_inputs(outbound)
|
||||||
@@ -139,7 +142,7 @@ func _process(delta: float) -> void:
|
|||||||
if err != OK:
|
if err != OK:
|
||||||
push_error("SimBridge: failed to send message: %s" % error_string(err))
|
push_error("SimBridge: failed to send message: %s" % error_string(err))
|
||||||
else:
|
else:
|
||||||
push_error("SimBridge: failed to batch-encode %d inputs" % outbound.size())
|
push_error("SimBridge: failed to batch-encode %d inputs (dropped)" % outbound.size())
|
||||||
StreamPeerTCP.STATUS_CONNECTING:
|
StreamPeerTCP.STATUS_CONNECTING:
|
||||||
pass # Should not happen in CONNECTED state
|
pass # Should not happen in CONNECTED state
|
||||||
StreamPeerTCP.STATUS_ERROR:
|
StreamPeerTCP.STATUS_ERROR:
|
||||||
|
|||||||
@@ -5,22 +5,45 @@
|
|||||||
class_name TestE2EConnection
|
class_name TestE2EConnection
|
||||||
extends GdUnitTestSuite
|
extends GdUnitTestSuite
|
||||||
|
|
||||||
const TEST_PORT: int = 19876
|
|
||||||
const CONNECT_TIMEOUT: float = 3.0
|
const CONNECT_TIMEOUT: float = 3.0
|
||||||
const RESPONSE_TIMEOUT: float = 5.0
|
const RESPONSE_TIMEOUT: float = 5.0
|
||||||
|
const MAX_PORT_ATTEMPTS: int = 5
|
||||||
|
|
||||||
var _server_pid: int = -1
|
var _server_pid: int = -1
|
||||||
var _bridge: LocalBridge = null
|
var _bridge: LocalBridge = null
|
||||||
|
var _test_port: int = 0
|
||||||
|
|
||||||
|
|
||||||
func _server_binary_path() -> String:
|
func _server_binary_path() -> String:
|
||||||
# Resolve path relative to the Godot project root
|
|
||||||
var project_dir := ProjectSettings.globalize_path("res://")
|
var project_dir := ProjectSettings.globalize_path("res://")
|
||||||
return project_dir.path_join("../server/target/debug/settled-reach-server")
|
return project_dir.path_join("../server/target/debug/settled-reach-server")
|
||||||
|
|
||||||
|
|
||||||
|
## Pick a random high port to avoid conflicts in parallel CI runs.
|
||||||
|
## Range 49152-65535 is the dynamic/ephemeral port range (IANA).
|
||||||
|
static func _random_test_port() -> int:
|
||||||
|
return 49152 + (randi() % (65535 - 49152 + 1))
|
||||||
|
|
||||||
|
|
||||||
|
## Spawn server with port rotation — if the port is in use, the server exits
|
||||||
|
## immediately (bind failure). Detect this and retry with a new random port.
|
||||||
|
func _spawn_server(server_path: String) -> bool:
|
||||||
|
for attempt in range(MAX_PORT_ATTEMPTS):
|
||||||
|
_test_port = _random_test_port()
|
||||||
|
var addr := "127.0.0.1:%d" % _test_port
|
||||||
|
_server_pid = OS.create_process(server_path, [addr])
|
||||||
|
if _server_pid <= 0:
|
||||||
|
continue
|
||||||
|
# Give server time to bind or fail
|
||||||
|
await get_tree().create_timer(0.15).timeout
|
||||||
|
if OS.is_process_running(_server_pid):
|
||||||
|
return true
|
||||||
|
# Server exited — port likely in use, try another
|
||||||
|
_server_pid = -1
|
||||||
|
return false
|
||||||
|
|
||||||
|
|
||||||
func after_test() -> void:
|
func after_test() -> void:
|
||||||
# Clean up regardless of test outcome
|
|
||||||
if _bridge != null:
|
if _bridge != null:
|
||||||
_bridge.disconnect_from_server()
|
_bridge.disconnect_from_server()
|
||||||
_bridge = null
|
_bridge = null
|
||||||
@@ -37,24 +60,22 @@ func test_send_input_receive_snapshot() -> void:
|
|||||||
push_warning("E2E test skipped: server binary not found at %s" % server_path)
|
push_warning("E2E test skipped: server binary not found at %s" % server_path)
|
||||||
return
|
return
|
||||||
|
|
||||||
# Spawn server
|
# Spawn server with port rotation (retries if port is in use)
|
||||||
var addr := "127.0.0.1:%d" % TEST_PORT
|
var spawned := await _spawn_server(server_path)
|
||||||
_server_pid = OS.create_process(server_path, [addr])
|
assert_bool(spawned).is_true()
|
||||||
assert_that(_server_pid).is_greater(0)
|
|
||||||
|
|
||||||
# Connect with retries (server needs time to bind port)
|
# Connect with retries (server needs time to accept)
|
||||||
_bridge = LocalBridge.new()
|
_bridge = LocalBridge.new()
|
||||||
var connected := false
|
var connected := false
|
||||||
var elapsed := 0.0
|
var elapsed := 0.0
|
||||||
while elapsed < CONNECT_TIMEOUT:
|
while elapsed < CONNECT_TIMEOUT:
|
||||||
if _bridge.get_status() == StreamPeerTCP.STATUS_NONE:
|
if _bridge.get_status() == StreamPeerTCP.STATUS_NONE:
|
||||||
_bridge.connect_to_server("127.0.0.1", TEST_PORT)
|
_bridge.connect_to_server("127.0.0.1", _test_port)
|
||||||
_bridge.poll()
|
_bridge.poll()
|
||||||
if _bridge.get_status() == StreamPeerTCP.STATUS_CONNECTED:
|
if _bridge.get_status() == StreamPeerTCP.STATUS_CONNECTED:
|
||||||
connected = true
|
connected = true
|
||||||
break
|
break
|
||||||
if _bridge.get_status() == StreamPeerTCP.STATUS_ERROR:
|
if _bridge.get_status() == StreamPeerTCP.STATUS_ERROR:
|
||||||
# Reset and retry
|
|
||||||
_bridge.disconnect_from_server()
|
_bridge.disconnect_from_server()
|
||||||
_bridge.reset()
|
_bridge.reset()
|
||||||
await get_tree().create_timer(0.1).timeout
|
await get_tree().create_timer(0.1).timeout
|
||||||
@@ -91,7 +112,7 @@ func test_send_input_receive_snapshot() -> void:
|
|||||||
assert_that(snapshot.entities.size()).is_equal(1)
|
assert_that(snapshot.entities.size()).is_equal(1)
|
||||||
|
|
||||||
# Player started at (16, 16, 0), moved north (y-1) to (16, 15, 0)
|
# Player started at (16, 16, 0), moved north (y-1) to (16, 15, 0)
|
||||||
# Render coords: tile center offset → (16.5, 15.5, 0)
|
# Render coords: tile center offset -> (16.5, 15.5, 0)
|
||||||
var player: Dictionary = snapshot.entities[0]
|
var player: Dictionary = snapshot.entities[0]
|
||||||
assert_float(player.x).is_equal_approx(16.5, 0.001)
|
assert_float(player.x).is_equal_approx(16.5, 0.001)
|
||||||
assert_float(player.y).is_equal_approx(15.5, 0.001)
|
assert_float(player.y).is_equal_approx(15.5, 0.001)
|
||||||
|
|||||||
Reference in New Issue
Block a user