fix(tests): measure cumulative travel, not displacement, in the ENet host check

run_ci_host_check asserted input reached the server by comparing each bot
ship's position against one recorded before the check forced a goal. But a
goal's kickoff teleports every ship back to spawn (_begin_kickoff ->
reset_ships), so that comparison measured only the distance covered since the
last reset — a window whose length depends on when the sample lands relative
to the kickoff rather than on whether input was flowing at all.

It failed on master with peers at 0.51m and 0.23m against a 0.5m threshold:
one passed by a centimetre, the other failed, with both connected, neither
stalled, and every other assertion in the run green. The commit it failed on
touches only training JSON, and the push two minutes earlier passed on
identical game code.

Accumulate per-tick path length in _await_recording_score instead, discarding
any single-frame step over 2.0m as a teleport — Ship.max_speed (35 m/s) is
hard-clamped each tick in _integrate_forces, so 60Hz caps legitimate travel at
~0.58m. Same 0.5m threshold now reads 28-75m across runs, and it is strictly
stronger than before: it asserts input kept arriving for the whole wait rather
than that the ship merely ended up somewhere else.
This commit is contained in:
Josh Creek
2026-08-24 08:40:06 +01:00
parent dffc2812e1
commit 46fe696a58
+47 -12
View File
@@ -535,12 +535,39 @@ func run_client_check(settle_seconds: float, drive_seconds: float, exercise_ball
# each distinct value. Lets the comparison below check a client's recorded
# score against a state the server genuinely passed through, rather than
# against whatever it happens to hold seconds later.
func _await_recording_score(match_scene, seconds: float, history: Array[String]) -> void:
#
# `path_lengths`, when pre-seeded with peer_id -> 0.0, also accumulates how far
# each slot's ship actually travelled while this wait runs. Any caller that
# forces a goal must measure movement this way rather than by start-to-end
# displacement: a goal's kickoff teleports every ship back to spawn
# (_begin_kickoff -> reset_ships in networked_match.gd, which calls the teleport
# its own comment describes), so displacement from a position sampled before the
# goal measures only the distance covered since the last reset — a window whose
# length depends on when the sample lands relative to the kickoff rather than on
# whether input was flowing at all. Summing per-tick steps is reset-proof and
# strictly stronger: it asserts input kept arriving across the whole wait, not
# merely that the ship finished somewhere other than where it started.
func _await_recording_score(match_scene, seconds: float, history: Array[String], path_lengths: Dictionary = {}) -> void:
# Ship.max_speed (35 m/s) is hard-clamped every tick in Ship._integrate_forces,
# so at 60Hz no ship can legitimately cover more than ~0.58m between physics
# frames. A step past this is the kickoff teleport, not travel, and must not
# count toward the total.
const MAX_TICK_TRAVEL := 2.0
var deadline := Time.get_ticks_msec() + int(seconds * 1000.0)
var previous_positions: Dictionary = {}
while Time.get_ticks_msec() < deadline:
var current := JSON.stringify(match_scene.score)
if history[history.size() - 1] != current:
history.append(current)
for slot in match_scene._slots:
if not path_lengths.has(slot.peer_id) or not is_instance_valid(slot.ship):
continue
var position: Vector3 = slot.ship.global_position
if previous_positions.has(slot.peer_id):
var step: float = (previous_positions[slot.peer_id] as Vector3).distance_to(position)
if step < MAX_TICK_TRAVEL:
path_lengths[slot.peer_id] += step
previous_positions[slot.peer_id] = position
await get_tree().physics_frame
@@ -1227,14 +1254,16 @@ func run_ci_host_check(run_seconds: float) -> void:
# depend on client input ever reaching the server at all — it kept
# reporting PASS with the input pipeline completely dead (verified by
# injecting the ring-overflow bug this session's critical fix
# addresses, mid-run). Record each ship's starting position now, before
# anything moves, so real server-side movement over the run can be
# checked directly — the same signal run_client_check already uses for
# a human client, applied here per-bot instead of just for "my own ship".
var start_positions: Dictionary = {}
# addresses, mid-run). Accumulate each ship's real server-side travel over
# the run so the input pipeline is checked directly — the same signal
# run_client_check already uses for a human client, applied here per-bot
# instead of just for "my own ship". Seeding a peer_id here is what opts it
# into the per-tick accumulation _await_recording_score performs; see that
# function for why this cannot be a start-to-end displacement.
var path_lengths: Dictionary = {}
for slot in match_scene._slots:
if is_instance_valid(slot.ship):
start_positions[slot.peer_id] = slot.ship.global_position
path_lengths[slot.peer_id] = 0.0
var goals: Array = match_scene.arena.get_goals() if match_scene.arena else []
if is_instance_valid(match_scene.ball) and not goals.is_empty():
@@ -1262,7 +1291,7 @@ func run_ci_host_check(run_seconds: float) -> void:
# accumulate meaningful motion. The bots remain connected for an extra
# three seconds after their active run, leaving a generous live margin.
var movement_check_delay := maxf(1.0, run_seconds - 0.5)
await _await_recording_score(match_scene, movement_check_delay, score_history)
await _await_recording_score(match_scene, movement_check_delay, score_history, path_lengths)
var connected_peers := multiplayer.get_peers()
var input_reached_server := true
for slot in match_scene._slots:
@@ -1270,14 +1299,20 @@ func run_ci_host_check(run_seconds: float) -> void:
if not still_connected:
input_reached_server = false
print("SMOKE FAIL: peer %d already disconnected at movement-sample time (connected_peers=%s) — margin too tight" % [slot.peer_id, str(connected_peers)])
if not is_instance_valid(slot.ship) or not start_positions.has(slot.peer_id):
if not is_instance_valid(slot.ship) or not path_lengths.has(slot.peer_id):
input_reached_server = false
print("SMOKE FAIL: peer %d has no valid ship to check movement on" % slot.peer_id)
continue
var moved: float = start_positions[slot.peer_id].distance_to(slot.ship.global_position)
# Cumulative travel, so a live pipeline clears this by an enormous margin
# (seconds of play at up to 35 m/s is tens of metres) while a dead one
# reads ~0. Kept deliberately loose rather than tightened to match: the
# check exists to catch a totally dead input path, and the printed value
# is the evidence for raising it later if that is ever worth doing.
const MIN_SERVER_TRAVEL := 0.5
var travelled: float = path_lengths[slot.peer_id]
var stalled: bool = slot.jitter_buffer.stalled
print("SMOKE INFO: peer %d moved %.2fm server-side (connected=%s), stalled=%s" % [slot.peer_id, moved, str(still_connected), str(stalled)])
if moved <= 0.5 or stalled:
print("SMOKE INFO: peer %d travelled %.2fm server-side (connected=%s), stalled=%s" % [slot.peer_id, travelled, str(still_connected), str(stalled)])
if travelled <= MIN_SERVER_TRAVEL or stalled:
input_reached_server = false
# Extra buffer beyond run_seconds: clients run for their own run_seconds