mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
test(multiplayer): report transport health on prediction-quality failures
A percentile alone cannot tell "the predictor regressed" from "the client never received the data". The client gate now prints snapshot_loss / snapshot_age / rtt on every run, and on a quality failure with >20% loss says explicitly that the run was transport-starved. It deliberately does not convert the failure into a pass: a client that cannot receive snapshots is still a failed run, just a differently-diagnosed one. Both directions of the new branch verified non-vacuously (forced true so it fires and formats; restored so it stays quiet on a healthy run while the INFO line still prints). Records the investigation behind it in multiplayer-todo.md: the reviewer's 3-process p95 0.688 did not reproduce. An idle third process costs nothing (p99 0.094), a spectator costs a small but real amount (p99 0.094-0.146), and snapshot loss held at 0.0% even under 2x CPU oversubscription - all an order of magnitude inside the 0.5/2.0 gates. Also notes that a previously working class_name can silently drop out of the .godot class cache, which surfaces as a bogus parse error with nothing in git status to explain it.
This commit is contained in:
@@ -290,6 +290,18 @@ func run_client_check(settle_seconds: float, drive_seconds: float, exercise_ball
|
||||
# server consumes, so both the same-sequence raw residual and the exposed
|
||||
# render discontinuity are meaningful free-flight gates. Hard corrections
|
||||
# remain separately gated by cohort.
|
||||
# Transport health, printed alongside the quality numbers and asserted
|
||||
# separately below. Without this a p95 failure is undiagnosable: "the
|
||||
# predictor got worse" and "the client never received the data" look
|
||||
# identical in a percentile. An adversarial review hit exactly that — a
|
||||
# 3-process run failed at p95 0.688 with roughly a third of snapshots
|
||||
# missing, and it could not be told apart from a real regression.
|
||||
var snapshot_loss_pct := float(net_stats.get("snapshot_loss_pct", 0.0))
|
||||
var snapshot_age_ms := float(net_stats.get("snapshot_age_ms", 0.0))
|
||||
print("SMOKE INFO: transport snapshot_loss=%.1f%% snapshot_age=%.1fms rtt=%.1fms" % [
|
||||
snapshot_loss_pct, snapshot_age_ms, NetworkManager.rtt_ms
|
||||
])
|
||||
|
||||
var raw_quality_p95: float = float(prediction_stats.get("free_flight_position_error_p95", INF))
|
||||
var raw_quality_p99: float = float(prediction_stats.get("free_flight_position_error_p99", INF))
|
||||
var raw_rotation_p95: float = float(prediction_stats.get("free_flight_rotation_error_p95", INF))
|
||||
@@ -438,6 +450,12 @@ func run_client_check(settle_seconds: float, drive_seconds: float, exercise_ball
|
||||
])
|
||||
|
||||
var success := verification_movement > 1.0 and local_prediction_ok and prediction_quality_ok and ball_contact_ok and match_state_ok
|
||||
# A run starved of snapshots has not measured prediction quality at all, so
|
||||
# say so explicitly instead of blaming the predictor. Deliberately does NOT
|
||||
# convert the failure into a pass — a client that cannot receive snapshots
|
||||
# is still a failed run, just a differently-diagnosed one.
|
||||
if not prediction_quality_ok and snapshot_loss_pct > 20.0:
|
||||
print("SMOKE FAIL: transport-starved, not a prediction regression (snapshot_loss=%.1f%%) — check host CPU contention before suspecting the predictor" % snapshot_loss_pct)
|
||||
print("SMOKE %s: client locally predicted %.2fm horizontal, local_prediction_ok=%s prediction_quality_ok=%s" % [
|
||||
"PASS" if success else "FAIL", moved_horizontal, str(local_prediction_ok), str(prediction_quality_ok)
|
||||
])
|
||||
|
||||
+3
-1
@@ -1012,7 +1012,9 @@ Verified against a control: hardcoding the snapshot byte back to `0` fails both
|
||||
|
||||
**Task 5.7's bug was real and the test found it.** `SlotInfo.controller` was declared `RLShipController`, but §6.4's takeover swaps in an `AIShipController` or the base controller — a narrower declared type makes that assignment fail its type check, leaving the field pointing at the controller `set_controller()` just `queue_free()`d. It surfaced as `controller_valid=false` on the first disconnect run. The per-tick `slot.controller.action` write is now also gated on `is RLShipController`: a disconnected slot's bot drives itself, and overwriting its action from a permanently-starving buffer would pin it to the departed player's last input.
|
||||
|
||||
**`--check-only --script` is the only thing that catches a parse error in `networked_match.gd`.** The unit runner never loads it, so `bot_model_path` being undefined (and later `ReplayLog` being unregistered) both passed 81/87 unit tests while breaking every two-process run. Validate touched scripts directly. A newly added `class_name` also needs `godot --headless --path Game --import` before anything can resolve it.
|
||||
**`--check-only --script` is the only thing that catches a parse error in `networked_match.gd`.** The unit runner never loads it, so `bot_model_path` being undefined (and later `ReplayLog` being unregistered) both passed 81/87 unit tests while breaking every two-process run. Validate touched scripts directly. A newly added `class_name` also needs `godot --headless --path Game --import` before anything can resolve it — and the same `--import` is the fix when a *previously working* `class_name` stops resolving, which happens on its own: `.godot/global_script_class_cache.cfg` silently lost `MatchState` between sessions, and every two-process run then died with `Cannot infer the type of "live" variable` at the `MatchState.is_live()` call, with nothing in `git status` to explain it. Read that error as "the class cache is stale", not "the code is wrong".
|
||||
|
||||
**The three-process prediction-quality caveat could not be reproduced, and the durable fix was diagnosis, not a code change.** The second adversarial review reported a 3-process run failing the free-flight gate at p95 0.688 (bar 0.5) with roughly a third of snapshots missing. Separating the two candidate causes — a third process merely competing for CPU, versus a spectator that the server must actually serve — showed a **spectator costs a small but real amount and an idle third process costs nothing**: two-process p95/p99 0.084/0.098, idle third process 0.084/0.094, spectator 0.084–0.098 / 0.094–0.146 across four runs. All of that is an order of magnitude inside the 0.5/2.0 gates. Snapshot loss stayed at **0.0% even under deliberate 2x CPU oversubscription** (20 spinners on 10 cores), where the only thing that moved was `snapshot_age` (14ms → 32.3ms) and the run still passed. 0.688 never recurred. What *was* worth keeping is that a percentile alone cannot distinguish "the predictor regressed" from "the client never received the data", so the client gate now prints `snapshot_loss` / `snapshot_age` / `rtt` on every run and, on a quality failure with >20% loss, says explicitly that the run was transport-starved — **without converting the failure into a pass**, because a client that cannot receive snapshots is still a failed run. Both directions of that branch were verified non-vacuously (forced true → it fires and formats; restored → it stays quiet on a healthy run while the INFO line still prints).
|
||||
|
||||
**New/changed test surface:** `--exercise-match-state` (both roles; host forces a goal, client validates the whole observed sequence and the wire byte), `--role=host-disconnect` for the 5.6/5.7 three-process scenario, `--match-length=<s>` to reach `FULL_TIME` in a short run, `--replay-log=<path>`, `--fill-bots`/`--no-fill-bots`, `--max-spectators=<n>`. The ball-contact scenario now **steers at the ball with closed-loop real input** instead of a hand-tuned fixed-heading burst, which 5.3 broke by adding `KICKOFF_YAW_JITTER` (0 contacts in 3/3 runs); it thrusts while turning rather than hovering to aim, which took it from 2/3 to 5/5.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user