diff --git a/Game/tests/networked_match_test_hooks.gd b/Game/tests/networked_match_test_hooks.gd index 71b87c65..debf062a 100644 --- a/Game/tests/networked_match_test_hooks.gd +++ b/Game/tests/networked_match_test_hooks.gd @@ -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) ]) diff --git a/multiplayer-todo.md b/multiplayer-todo.md index 3894f841..a0c7bf71 100644 --- a/multiplayer-todo.md +++ b/multiplayer-todo.md @@ -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=` to reach `FULL_TIME` in a short run, `--replay-log=`, `--fill-bots`/`--no-fill-bots`, `--max-spectators=`. 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.