feat(multiplayer): Phase 1 transport, connection, and lobby

Lands tasks 1.0-1.8 of multiplayer-todo.md: the pure-function test runner,
net_codec (wire format quantizers/pack-unpack), NetworkManager (ENet
transport, manual polling, min-RTT clock sync), MatchNet (handshake,
protocol/tick-rate gating, roster with team+ready state), lobby.tscn (team
columns, switch team, ready toggle), server_boot.tscn (headless dedicated
server with structured logging and an overrun watchdog), and main_menu.gd's
Host/Join-by-IP UI (connecting overlay, cancel, bounded failure path).

Followed by an adversarial review (Opus subagent) that found and fixed two
real bugs - an unvalidated player_name broadcast that let one client's
oversized name head-of-line-block the reliable channel for everyone, and a
server-side roster leak across a host/re-host cycle - plus three gaps in
the test suite itself where a claim of "verified" wasn't actually backed
by what the test checked. All five two-process smoke tests plus the
pure-function suite are green with the strengthened assertions in place.

multiplayer-todo.md

@@ -799,9 +799,9 @@ Every task lands on `master` independently, is verifiable in single-player today
 | 0.24 `[P]` | **DONE.** Both guarded with `if DisplayServer.get_name() == "headless": return` — `arena.gd:_ready()` skips the whole Environment block, `video_settings.gd:_ready()` skips `apply_aa()` | `scripts/arena.gd`, `scripts/video_settings.gd` | `--headless` allocates no Environment and no AA state |
 | 0.25 `[P]` | **DONE.** `_process` still calls `to_local()` every frame (needed for the comparison itself) but skips `set_shader_parameter()` — the actual GPU-facing cost — below a 0.05 m movement threshold | `scripts/arena_boundary.gd` | Field shader behaves identically; the expensive call is skipped on most frames |
 | **0.26** `[D:0.15b]` | **Bake the arena GI and retire SDFGI** (§5.7). `arena.gd`/`goal.gd` have no `_process`, no animation — the arena is fully static, and SDFGI is paying continuously to solve a dynamic-world problem this project does not have. Add UV2 to the arena shell, bake `LightmapGI` (or `VoxelGI` if bounce onto ships matters), disable `sdfgi_enabled` and re-evaluate `ssil_enabled` | `scenes/arena_base.tscn`, `scenes/arena_0*.tscn`, `scripts/arena_boundary.gd` | **Largest frame-time reduction of any task here, with equal or better image quality**; High preset keeps its look; bake is reproducible from a documented step |
-| **0.27** `[P]` | **Override expensive rendering defaults** (§5.7): `positional_shadow/atlas_size` 4096 → 2048, `directional_shadow/size` 4096 → 2048, `soft_shadow_filter_quality`. `project.godot [rendering]` currently has three keys and everything else is at engine default | `project.godot` | Measurable frame-time reduction; no visible shadow-quality regression at 1080p |
+| **0.27** `[P]` | **DONE.** `lights_and_shadows/positional_shadow/atlas_size` and `directional_shadow/size` set to 2048 (from the 4096 engine default), `soft_shadow_filter_quality=2` | `project.godot` | Measurable frame-time reduction; no visible shadow-quality regression at 1080p |
 | **0.28** `[D:0.15b]` | **CLOSED, not implemented — the problem it targets doesn't exist.** Was: prototype `physics/3d/run_on_separate_thread` (§5.7) to attack frame-time variance from the physics tick sharing the render thread — **the riskiest item in this phase**, since it changes when `_integrate_forces` runs relative to script code, and both `ship.gd:346-357` and the RL training path depend on that. §5.5.2's real-hardware measurement (RTX 3090) found a 1.85 ms p50 / 2.98 ms p99 baseline with every graphics effect enabled — both comfortably under even a 240 Hz frame budget, with no meaningful p99-over-p50 variance to explain away. Taking on this task's real risk (reordering `_integrate_forces` relative to script code, with the RL training path depending on today's ordering) for a variance problem that isn't measurably present is a bad trade. Reopen only if a lower-end-hardware pass (§5.5.2's "still open" item) finds real physics-tick-driven variance that 0.26 and the preset ladder don't already cover | — | *(closed without a code change; see §5.5.2 for the evidence)* |
-| **0.29** `[P]` | **Early-out in `ArenaBoundary.get_surface_pull`**: a bounds check against `wall_range`/`ceiling_range` before the `to_local()` and five `_falloff` calls, which currently run for every dynamic body every tick even mid-arena where every term is zero | `scripts/arena_boundary.gd` | Identical flight feel and identical RL observations; measurable tick-time reduction with 7 bodies |
+| **0.29** `[P]` | **DONE.** Bounds check against `wall_range`/`ceiling_range` at the top of `get_surface_pull`, returning `Vector3.ZERO` before `to_local()` and the five `_falloff` calls whenever every term would be zero mid-arena | `scripts/arena_boundary.gd` | Identical flight feel and identical RL observations; measurable tick-time reduction with 7 bodies |
 
 > **These tasks exist because of the high-refresh-rate mandate, and their order matters.** **0.15b blocked everything else, and did invalidate the a priori fps list** — but not in the direction first assumed (see §5.5.1 vs §5.5.2): on the Mac the game looked GPU-bound and undifferentiated; on real reference hardware (RTX 3090, §5.5.2) it runs at 540 fps p50 with everything on, nowhere near bound by anything. 0.17/0.17b/0.19 (done) are still the right frame-rate levers — SDFGI/SSIL genuinely dominate the optional-effects cost, exactly as originally assumed, just at a much smaller absolute scale than feared on this hardware tier. 0.16 and 0.20–0.25 are the per-frame hygiene that makes a high frame rate worth having. 0.18 buys nothing today — it is what keeps a future 120 Hz simulation a config change plus a retrain rather than a protocol rewrite. 0.28 closed without a code change (§5.5.2) — the frame-time variance it targeted isn't measurably present on reference hardware.
 >
@@ -817,15 +817,16 @@ Every task lands on `master` independently, is verifiable in single-player today
 
 | # | Task | Acceptance |
 |---|---|---|
-| 1.0 | **`tests/test_runner.tscn`** — minimal pure-function assertion runner exiting 0/1 | `godot --headless --path Game res://tests/test_runner.tscn` runs and exits 0 |
-| 1.1 `[D:1.0]` `[D:0.18]` | `net_codec.gd`: protocol constants, enums, channel ids, quantisers, pack/unpack for both hot packets. **Pure functions**, written against 1.0. Every timing constant derives from `TICK_HZ` — ring sizes, seq windows, `INTERP_DELAY`, timeouts (§5.4) | Round-trip, bounds, and quaternion-error tests pass; setting `TICK_HZ = 120` recomputes every derived constant with no other edit |
-| 1.2 `[D:1.1]` | `NetworkManager` autoload: `host`/`join`/`shutdown`, signals, **`server_relay = false`** | Two peers connect and disconnect cleanly |
-| 1.3 `[D:1.2]` | Manual multiplayer polling: `get_tree().set_multiplayer_poll(false)`; client flushes at the end of `_physics_process` after the input send **and polls for receive at the top of both `_process` and `_physics_process`, unthrottled** (§5.4c); server polls at tick start (drain) and tick end (flush). Defer any scene-tree mutation in `peer_connected`/`peer_disconnected`, which now fire mid-frame | Measured RTT drops by ~8–17 ms per leg versus default polling; **median age of the newest applied snapshot at render time drops by ≈ half a frame interval** versus polling once per physics tick |
-| 1.4 `[D:1.2]` | `MatchNet` autoload skeleton: `hello`/`welcome`/`player_joined`/`player_left`, strict `protocol_version` **and `physics_ticks_per_second`** gating, roster model | A mismatched client is rejected with a readable reason |
-| 1.5 `[D:1.4]` | `lobby.tscn` + `lobby.gd`: roster list, team swap, ready toggle, disconnect | Two clients see each other and both ready states |
-| 1.6 `[D:1.4]` `[P]` | `server_boot.tscn` + `server_boot.gd`: CLI parsing from `OS.get_cmdline_user_args()`, `Engine.max_fps = 60`, `Engine.max_physics_steps_per_frame` overrun watchdog, host, structured log lines | Headless server starts, logs, accepts connections, and idles at <5% of a core |
-| 1.7 `[D:1.5]` `[P]` | `main_menu.gd`: Host / Join-by-IP with a **connecting overlay, cancel, and a failure path** | Connect, cancel, and connection-refused all reach a sane UI state |
-| 1.8 `[D:1.2]` `[P]` | Clock: ping/pong, min-RTT-filtered offset, tick estimate (folded into `network_manager.gd`), plus a debug overlay showing RTT and offset | Offset converges within 2 s and stays within ±1 tick on a clean link |
+| 1.0 | **DONE, two real bugs found and fixed after adversarial review.** `tests/test_runner.tscn` + `test_runner.gd`: discovers every `*.gd` under `tests/cases/`, instances it, calls every `test_*()` method via `get_method_list()`, aggregates failures, `get_tree().quit(1 if failed else 0)`. `tests/test_case.gd` is the assertion base (`assert_true`/`assert_eq`/`assert_almost_eq`); case scripts use `extends "res://tests/test_case.gd"` (path-based) and the runner uses `preload()`, not a bare `class_name` reference — the global script-class cache isn't guaranteed populated on a fresh headless run (same class of issue as task 0.18's `SimConstants`). `tests/cases/test_smoke.gd` proves discovery/dispatch/aggregation and is the first real case file. **An Opus subagent's adversarial review found**: (1) GDScript has no exceptions, so a test that hit a runtime error before its first `assert_*` call left `failures` empty — exactly like every assertion passing — and was silently counted as a PASS. Fixed: `TestCase` now tracks `assertions_made`, incremented by every `assert_*`; the runner treats zero assertions as a failure in its own right ("made no assertions"). (2) A case file with a parse/compile error hung the whole runner forever — `load()` on a broken script does **not** return null here, it returns a non-null but uninstantiable `GDScript` resource, so a plain null check doesn't catch it; calling `.new()` on it threw an error severe enough to abort `_ready()` before ever reaching `quit()`. Fixed with `Script.can_instantiate()` as the real guard | `godot --headless --path Game res://tests/test_runner.tscn` runs and exits 0; verified exit 1 with a deliberately-failing assertion, then removed. Re-verified both fixes with scratch case files (not committed): a test that null-derefs before asserting now correctly fails with "made no assertions" (exit 1, not a false pass); an uncompilable case file now fails loudly and promptly (exit 1, not a 124-timeout hang) while the *other* valid case files in the same run still execute normally |
+| 1.1 `[D:1.0]` `[D:0.18]` | **DONE.** `scripts/net_codec.gd`: protocol constants, `PacketType` enum, channel ids, i16/i8/thrust-z-bin quantisers, `pack_input`/`unpack_input`, `pack_snapshot_body_segment`/`pack_snapshot_client_header`/`pack_snapshot`/`unpack_snapshot`. New `scripts/net_body_state.gd` is the plain per-body data holder the snapshot functions read/write (not Ship/Ball themselves, so the codec stays callable with no scene tree). `NetCodec.TICK_HZ` derives from `SimConstants.TICK_HZ` via `preload()` (same cache-timing reason as 0.18); ring sizes / seq windows / `INTERP_DELAY` / timeouts don't exist as constants yet — they land with the tasks that consume them (3.1+), so "derives from `TICK_HZ`" is satisfied for what exists today | `scripts/net_codec.gd`, `scripts/net_body_state.gd`, `tests/cases/test_net_codec.gd` | 14 tests pass (`godot --headless --path Game res://tests/test_runner.tscn`, exit 0): input round-trip (1 and 4-entry, redundancy clamp), snapshot round-trip across 7 bodies incl. quaternion sign-fold and ship→ball angular-velocity rescale, thrust-z bin edges, type/version nibble round-trip. Byte counts asserted against §2.3/§2.4's numbers directly: 40 B input (max redundancy), 169 B snapshot (7 bodies) |
+| 1.2 `[D:1.1]` | **DONE, strengthened after adversarial review.** `scripts/network_manager.gd` autoload (`NetworkManager` in `project.godot [autoload]`): `host(port, max_clients)`/`join(address, port)`/`shutdown()`, `client_connected`/`client_disconnected`/`connected_to_server`/`connection_failed`/`disconnected_from_server` signals forwarded from `multiplayer`'s own, `server_relay = false` set the moment a peer exists, `is_server`/`is_client` state. Gained a `shutting_down()` signal, emitted at the top of every `shutdown()` regardless of role or reason — see task 1.4's row for why | An Opus subagent's adversarial review (independently verified by the primary session before applying fixes) found the original `tests/net_smoke.gd` only proved each process exits cleanly on its own initiative, never that the OTHER peer actually observes the disconnect. Rewrote it: the host now waits for **both** `client_connected` and `client_disconnected` before passing; the client explicitly calls `shutdown()` mid-test (not just on process exit) and gives it a beat before quitting, same reasoning as §9 gotcha 26 for connects — a clean disconnect notice still needs a few `poll()` cycles to reach the wire, or the other side falls back to its ~5s peer timeout (gotcha 11) instead of a prompt one. Re-verified passing with both directions actually observed |
+| 1.3 `[D:1.2]` | **DONE for what exists today.** `NetworkManager._ready()` calls `get_tree().set_multiplayer_poll_enabled(false)` (Godot 4.7's actual method name — the doc's `set_multiplayer_poll(false)` was shorthand) and exposes `NetworkManager.poll()` as the one entry point every caller uses instead. Verified against `tests/net_smoke.gd`, updated to poll from both `_process` and `_physics_process` every frame — connect/disconnect still works cleanly under manual-only polling (§9 gotcha 26 still applies: give a beat after a connect signal before shutdown). **The per-call-site placement this task specifies (client: end-of-physics-tick flush after input send, top-of-frame receive; server: tick-start drain, tick-end flush) has no real per-tick caller yet** — there is no input/snapshot traffic until tasks 1.4+/Phase 2 exist to send any, so there's nothing to place a flush *after*. That placement, and the RTT/staleness measurement below, land with the input pipeline, not as a separate task | `godot --headless` two-process test still connects/disconnects cleanly with automatic polling off (verified). RTT/staleness improvement **not yet measured** — deferred until Phase 2/3's real per-tick traffic exists to measure against, same honesty as task 1.1's "constants that don't fully exist yet" |
+| 1.4 `[D:1.2]` | **DONE.** `scripts/match_net.gd` autoload (`MatchNet`): `_hello`/`_welcome`/`_player_joined`/`_player_left`/`_rejected` RPCs, `protocol_version` (`NetCodec.PROTOCOL_VERSION`) and `physics_ticks_per_second` (`SimConstants.TICK_HZ`) checked on the server before a peer is added to `roster`; on mismatch, server sends `_rejected` with a readable string then `disconnect_peer()`s after a 0.3s beat (§9 gotcha 26 applies here too — a bare RPC then immediate disconnect would drop the rejection message). `roster: Dictionary[int, PlayerInfo]` never contains peer 1 (§1.1 decision 2). A new peer is told about the existing roster via targeted RPCs before the broadcast that tells everyone (including itself) about the new peer, so no client ever observes an unexplained peer_id | Verified with a real two/three-process test (`tests/match_net_smoke.gd`/`.tscn`): matched client → both sides see `player_joined`/`welcomed`; deliberately wrong protocol version → client receives `rejected("protocol version mismatch: server=1 client=100")` and is disconnected. Caught and fixed one real bug in the process: the server's own `roster` update in `_hello()` didn't locally emit `player_joined` (the broadcast RPC is `call_remote`, never loops back to the sender) |
+| — | **Two more real bugs found by an Opus subagent's adversarial review, both confirmed independently and fixed.** (1) `_hello`'s `player_name` was completely unvalidated and broadcast verbatim to every peer — a demonstrated DoS: a multi-MB name relayed to all peers head-of-line-blocked the reliable control channel hard enough that a concurrently-joining client's own `_welcome` never arrived. Fixed with a hard `MAX_INPUT_LENGTH = 256` reject (any legitimate client only ever sends `local_player_name`, which the UI already keeps short — anything past this is a bug or an attacker, not a name to politely truncate) followed by `_sanitize_player_name()`: strips control/formatting characters, clamps to `MAX_PLAYER_NAME_LENGTH = 24`, falls back to `"Player"` if empty. (2) `MatchNet.roster` was never cleared when a HOST stopped hosting — only the client-side disconnect path cleared it, so Host → Lobby → Leave → Host again left a phantom player in `roster` permanently, mis-balancing teams and getting broadcast to every future joiner. Fixed via `NetworkManager`'s new `shutting_down()` signal (task 1.2), which `MatchNet` now clears `roster` on unconditionally, regardless of role or reason | `_sanitize_player_name` is `static` (pure function of its argument) with 5 dedicated unit tests in `tests/cases/test_match_net.gd`, plus a live rejection test (`match_net_smoke.gd --role=client-longname`, a 500 KB name, confirmed rejected before ever reaching a broadcast). New regression test `match_net_smoke.gd --role=host_recycle`: host, client joins (`roster.size()==1`), host leaves and re-hosts, confirms `roster.is_empty()` before any new connection — reproduced the bug pre-fix, confirmed fixed post-fix |
+| 1.5 `[D:1.4]` | **DONE, strengthened after adversarial review.** `scenes/lobby.tscn` + `scripts/lobby.gd`: roster split into two team columns (dynamically rebuilt `Label` rows on `MatchNet.player_joined`/`player_left`/`player_state_changed`/`welcomed`), Switch Team + Ready `CheckButton` (server process gets a read-only view — never a roster member, §1.1 decision 2), Leave. `MatchNet` grew `team`/`ready` fields on `PlayerInfo`, a balanced-team auto-assign on join (`_pick_balanced_team`), and `request_set_team`/`request_set_ready` + their server-authoritative RPCs, broadcasting `_state_changed` the same way `_player_joined` already did | Verified with a real two-process test (`tests/lobby_smoke.gd`/`.tscn`) that loads `lobby.tscn` via `change_scene_to_file` exactly as `main_menu.gd`'s Host/Join flow (task 1.7) does, then presses the real `%SwitchTeamButton`/`%ReadyButton` nodes via a persistent test-only helper (`tests/lobby_test_hooks.gd`, not a project autoload — parented under `get_tree().root` so it survives the scene swap, never referenced by production code). **An Opus subagent's adversarial review found the original test's host role never actually loaded `lobby.tscn` at all** — it only hosted and waited, so `lobby.gd`'s `is_server` branch (the read-only view a self-hosting player reaches via `main_menu.gd`'s own Host button — a real, production-reachable path, not a hypothetical) had never run under this task's own suite. Fixed: the host role now loads `lobby.tscn` too and a new `run_host_test()` in the shared test helper verifies `%ControlsRow` is hidden, the roster row renders, and the status text is correct, holding the connection open long enough (`MIN_HOST_LIFETIME_SECONDS`) for the client's own longer flow to finish against it. Confirmed: roster renders correctly server- **and** client-side (now genuinely, not just asserted), team switch moves the row to the other column, ready toggle updates the checkbox and the label's ✓ marker, row count matches roster size on both peers |
+| 1.6 `[D:1.4]` `[P]` | **DONE.** `scenes/server_boot.tscn` + `scripts/server_boot.gd`: `--port=`/`--max-clients=`/`--log-level=` from `OS.get_cmdline_user_args()`, `Engine.max_fps = 60`, structured `[elapsed] LEVEL event key=value…` log lines for `server_started`/`peer_connected`/`player_joined`/`player_left`/`peer_disconnected`, and a physics-overrun watchdog comparing `Engine.get_physics_frames()` deltas frame-to-frame. Does not spawn a match yet — that's Phase 2's `networked_match.gd` — this is just the process shell: listen, log, idle cheaply. **Two real bugs caught and fixed while verifying, both in the watchdog**: (1) the very first `_process()` after boot compared against a pre-`_ready()` baseline and logged a spurious one-time `steps=5`; skip the first measurement. (2) the initial `steps > 1` threshold fired continuously (every 30–100ms) on a perfectly idle, healthy server — because §9 gotcha 6 means frames legitimately alternate between 0 and 2 physics ticks under `physics_jitter_fix = 0.0`, not a flat 1/frame; that's quantisation, not backlog. Raised the threshold to `steps > 2` (3+ ticks = the accumulator actually failing to drain), which produced zero false positives over a 4.8s idle run | Verified with real headless runs: idle CPU measured via `ps -o %cpu` at 0.0% (bar is <5%); a real client connect/disconnect via `tests/net_smoke.gd --port=` produces exactly the expected 4-line log sequence with no spurious warnings |
+| 1.7 `[D:1.5]` `[P]` | **DONE.** `main_menu.tscn` gained a Multiplayer section (Host button; Join row with an IP `LineEdit`, default `127.0.0.1`; inline error label) and a full-screen `ConnectingOverlay` (status label + Cancel). `main_menu.gd`: `_on_host_pressed` calls `NetworkManager.host()` then goes straight to `lobby.tscn` (synchronous — no overlay needed); `_start_join` calls `NetworkManager.join()`, shows the overlay, and starts an app-level `CONNECT_TIMEOUT_SECONDS = 6.0` timer; `_on_connected_to_server`/`_on_connection_failed`/Cancel/timeout each resolve to the overlay hiding and either `lobby.tscn` or a visible error, gated by a token counter so a late/stray signal after the attempt was already resolved is a no-op | Verified with real multi-process runs of `scenes/main_menu.tscn` itself (not a wrapper — driven by a temporary-autoload test helper, `tests/main_menu_test_hooks.gd`, pressing the real `HostButton`/`JoinButton`/`ConnectingCancelButton`) across all four paths: Host → `lobby.tscn`; Join → connects → `lobby.tscn`; Join with nothing listening → times out → error shown, stays on menu; Join → Cancel → overlay hidden, stays on menu, `is_client` false. **Two real bugs found and fixed in the process, both pre-existing from earlier Phase 1 tasks, not new to 1.7**: (1) `NetworkManager`'s clock ping (task 1.8) gated only on `is_client`, which turns true the instant `join()` is called — a slow or refused connect attempt spammed "Trying to call an RPC via a multiplayer peer which is not connected" every frame; fixed by also requiring `_peer.get_connection_status() == CONNECTION_CONNECTED`. (2) ENet's own `connection_failed` proved **unbounded in practice** — verified empirically against a genuinely refused loopback connection, it hadn't fired even 14s in — which would have left a player staring at "Connecting…" indefinitely; task 1.7's own `CONNECT_TIMEOUT_SECONDS` is what actually satisfies "connection-refused reaches a sane UI state", not the built-in signal alone |
+| 1.8 `[D:1.2]` `[P]` | **DONE, strengthened after adversarial review.** Folded into `network_manager.gd`: client pings the server once a second (`_ping`/`_pong` RPCs, reliable, channel 0); `clock_offset_ms` is the min-RTT sample in a rolling 5s window (`_clock_samples`, pruned by wall time); `get_server_time_estimate_ms()` is the public API later phases (`INTERP_DELAY`, `tick_offset` seeding) will actually call; `clock_updated(rtt_ms, offset_ms)` signal for observers. New `scripts/net_debug_overlay.gd` autoload (F4, `toggle_net_overlay` input action) mirrors `perf_overlay.gd`'s headless-guarded pattern, shows RTT + offset client-side or peer count server-side | Verified with a real two-process test (`tests/clock_smoke.gd`/`.tscn`) on localhost: first sample at t=0.95s, offset converged to 1534.50ms by t=2.0s (well inside the 2s bar), and stayed within 1.5ms of that value through t=3.96s — comfortably under the ±1 tick (16.67ms) bar. **An Opus subagent's adversarial review correctly pointed out this self-consistency check couldn't have caught a *systematically*-wrong-but-stable offset** (e.g. a missing `/2` on RTT, or a sign flip — it would converge just as cleanly). Fixed by adding an independent ground-truth cross-check: both host and client compute `Time.get_unix_time_from_system()*1000.0 - Time.get_ticks_msec()` (each process's own offset from the shared OS wall clock — the *same* real clock on both, since they're on the same machine), exchanged via a shared temp file written by the host, purely for test orchestration and touching no production code. The true required offset is just the difference of those two numbers; re-run measured the converged offset against it and found **0.99ms of error**, comfortably inside a deliberately loose 250ms tolerance (OS wall-clock read resolution and sampling-instant skew, not NetworkManager's own precision, is what sets the tolerance floor here). Note the converged offset *value* itself is large and arbitrary (~1.5s) because `Time.get_ticks_msec()` counts from each process's own start, not a shared epoch — expected, and exactly what `clock_offset_ms` exists to absorb |
 
 > `main_menu.gd` gains its **first async flow**. Every existing handler is `GameSettings.x = y; change_scene_to_file(...)` — there is no loading screen, no error state, and no back-navigation state machine to extend. Budget for that.
 
@@ -999,6 +1000,12 @@ No own-ship prediction yet: the client renders everything, including its own shi
 22. **`Engine.max_physics_steps_per_frame = 8` is a client problem too**, not just a server one (gotcha 9). A client hitching to 20 fps runs 3 ticks per frame, and each of those frames also runs the per-frame camera rig and remote-visual sampling. Set it to 4 client-side (task 0.22). On a multi-tick frame the send path must transmit **every** tick's action (gotcha 6) — §4.3's `_physics_process` sampling does this naturally, but nothing else guarantees it.
 23. **`hint_screen_texture` forces a full-screen backbuffer copy on every frame the node is drawn**, regardless of what the shader then does with it. Branching inside the shader saves taps, not the copy. Hide the node when the effect is at rest.
 24. **`physics_jitter_fix` matters less the higher the frame rate.** Its purpose is smoothing when frame rate ≈ tick rate; at 240 fps against 60 Hz physics most frames run zero ticks and the accumulator is never near an edge. Gotcha 6's reasoning for setting it to `0.0` still holds, but do not expect a visible difference on a high-refresh machine — test that change at 60 fps.
+25. **`MultiplayerAPI.multiplayer_peer`'s default value is an `OfflineMultiplayerPeer` sentinel, not `null`.** Resetting it with `multiplayer_peer = null` (rather than a fresh `OfflineMultiplayerPeer.new()`) leaves the API in a state distinct from its own default and is a known source of "the server never sees `peer_connected`, `get_peers()` stays empty" bugs (godotengine/godot#81540) — confirmed the hard way while building task 1.2's `NetworkManager.shutdown()`. Always reset to a real `OfflineMultiplayerPeer`.
+26. **Don't tear down a peer the instant its own connect signal fires.** `connected_to_server` (client-side) fires once the client's *local* view of the handshake completes, but the final ACK the server needs to consider *its* side complete may not have hit the wire yet — closing the peer or quitting the process in the same callback can drop it, and the other side then never sees `peer_connected`/`connected_to_server` at all, even though your own side looked successful. This isn't a corner case: it reproduced on **every** attempt until fixed, is easy to misdiagnose as a server-side bug (the server-side symptom — `get_peers()` staying empty — is identical to gotcha 25's), and cost significant debugging time before the actual cause (client-side premature teardown) was found. Give at least one frame — in practice `tests/net_smoke.gd` uses 0.3 s — between a fresh connect signal and calling `shutdown()`/`quit()`. Directly relevant to task 5.6's disconnect/reconnect controller swap and any CLI test client that connects, asserts, and exits quickly.
+27. **`change_scene_to_file()` must be called on (or from a descendant of) the actual `get_tree().current_scene`, and never synchronously from `_ready()`.** Both failure modes were hit building task 1.5's `lobby.tscn`/`tests/lobby_smoke.gd`: (a) a test harness that instantiated `lobby.tscn` as a plain child of a driver node — rather than loading it as the real current scene, the way `main_menu.gd`'s Host/Join flow will — caused `lobby.gd`'s own (entirely correct, standard-pattern) `change_scene_to_file(ScenePaths.MAIN_MENU)` disconnect handler to hang the process completely on a real disconnect, with near-zero CPU (blocked, not spinning) and no error output; the fix was to load the scene the way production actually will, not to change the production code. (b) calling `change_scene_to_file()` (or `add_child()` on `get_tree().root`) synchronously from inside `_ready()` throws "Parent node is busy … Consider using `.call_deferred()`", because the tree is still mid-traversal adding the very node whose `_ready()` is running; `main_menu.gd`'s real button-press handlers won't hit this (they run outside any `_ready()`), but anything that needs to trigger a scene change during its own initialization must `.call_deferred()` it.
+28. **`ENetMultiplayerPeer`'s `connection_failed` signal is not bounded to anything a UI should make a player wait for.** Verified empirically (task 1.7): against a genuinely refused loopback connection (nothing listening on the target port), `connection_failed` had still not fired 14 seconds in. Don't rely on it alone to end a "Connecting…" state — run your own app-level timeout (`main_menu.gd`'s `CONNECT_TIMEOUT_SECONDS = 6.0`) that shuts the peer down and shows an error regardless of whether ENet ever gets around to reporting failure itself.
+29. **A `MultiplayerPeer`'s "am I a client" flag (however you track it — `NetworkManager.is_client` here) turns true the instant `join()`/`create_client()` is called, not once the connection actually completes.** Anything gated on that flag alone (task 1.8's clock ping, in `network_manager.gd`'s `_process`) will try to `rpc_id()` on a peer that's still `CONNECTING` — or has already failed — during a slow or refused connect attempt, and Godot logs "Trying to call an RPC via a multiplayer peer which is not connected" every single frame until it resolves. Gate on the peer's actual `get_connection_status() == MultiplayerPeer.CONNECTION_CONNECTED`, not just the higher-level intent flag.
+30. **`load()` on a `.gd` file with a parse/compile error does not return `null`.** Found via adversarial review of `tests/test_runner.gd`: it returns a non-null but uninstantiable `GDScript` resource, so `if script == null` silently fails to catch the failure — and the natural next line, `script.new()`, throws "Invalid call: Nonexistent function 'new'", severe enough to abort the *entire calling function* (not just that statement) without ever reaching whatever cleanup/exit code follows. In a loop over multiple files with no per-iteration error boundary, this reads as a hang: the loop that would have moved to the next file, and the code that would have called `quit()`, both never run. The real guard is `Script.can_instantiate()`.
 
 ---