4 Commits

Author	SHA1	Message	Date
Josh Creek	75f485667b	feat(multiplayer): Phase 4 prediction correctness + two input-death fixes ... Closes Phase 4's outstanding action-sequence-correctness invariant, then fixes two server-side bugs an adversarial review of that work uncovered. Server simulation, bot observations, collision resources and tick rate are unchanged: the server_physics_parity trace is byte-for-byte identical to HEAD across 360 ticks including both ships' full observation vectors. 4.11 - prediction history filed under the ISSUING sequence _send_local_input filed each post-step predicted state under the timeline's estimate of the sequence the server would consume this tick, trailing issuance by input_lead. The body had integrated the intent issued under _input_seq, so predicted[S] held "state after the intent from now" while the server's authority for S is "state after action(S)". They agree only while the stick is still. Filing under _input_seq costs nothing: which action the ship uses is decided in LocalNetShipController.get_action() and is untouched. Every prior Phase 4 gate held its input steady, and a steady input cannot falsify a sequence label - the 60s runs honestly reported marker=0/3784. New --exercise-input-transitions role toggles thrust every 6 ticks; it is the only gate that can catch a label regression. Verified non-vacuous: the old label fails it at 50%. 4.12 - issued-but-unsimulated sequences, and the release path An attack (delta > 1) issues and sends several sequences for one local physics step. Those gap sequences had no recorded prediction, so a server ack of one reported missing_not_recorded - indistinguishable from ring loss, costing a teleport and resync suppression several times a minute. They are now recorded stateless via record_unsimulated() and answered with a new "skip" decision mode. Free-flight hard snaps: 25/8/4 -> 0/0/0. A release (delta == 0) re-recorded at the unchanged _input_seq, filing the current intent under a sequence that went out carrying a different action; LocalInputTimeline deliberately refuses to mutate an issued sequence, so the ring contradicted the wire. Recording is now skipped on release ticks. 4.13 - two Phase 3 bugs silently killing player input (a) InputJitterBuffer.consume() advanced last_applied_seq on every tick including a starve. Since ingest() discards seq <= last_applied_seq, one starve on a sequence the client had not sent yet stranded the stream one ahead of arrivals permanently - both sides advancing in lockstep, every honest packet discarded on arrival. The client's own input_lead release is enough to trigger it, so input died for ~30 ticks roughly every 6.5s on a clean LAN. Now only gives up on a sequence once strictly newer data proves it lost. Silent-client stall and ring-overflow resync are unchanged. (b) The seq-range guard bounded incoming seq against highest_ingested_seq, which only advances inside ingest(), which that guard gates. After a ~2s host hitch every packet was rejected forever with no diagnostic (600+ consecutive rejections reproduced via SIGSTOP). Third iteration of this guard; each previous version bounded against a value only the accepted path could advance. Adds an escape after 10 consecutive rejections, which grants an attacker nothing the rate limiter does not already bound. (c) The transitions gate reported PASS at 3.76% while input was completely dead, because suppression stops _record_metrics - a worse outage yields fewer samples and a LOWER rate. Now scales the required sample count with run length and asserts the wire's server_stalled bit. Reverting both fixes makes it fail at samples 292/600, server_stalled=true, input_lead=12. Fixing (a) also explained a residual the review had already traced: 151 of 151 action-marker mismatches were the server repeating a stale action on a starve, not a prediction defect. Marker is now 0.00% in all three conditions (was 1.7-2.5%), and free-flight p99 improved to 0.141/0.168/0.154m from 0.170/0.176/0.184m. Two pre-existing test defects fixed alongside: the ball gate asserted RTT-masking on a link with no RTT (flaked 2 in 5; now asserted only at rtt >= 20ms, 5/5 under latency), and the two-bot CI compared scores across a 3-5s window (now polls the scores the server actually held; note score_changed is emitted only on the client path). QA: 72 unit tests; 60s free-flight at LAN/80+-20ms/5% loss; transition gate in all three; 2.0s and 3.5s host-freeze recovery; ball contact x5; two-bot CI x3; all three abuse roles; net/match_net/clock/lobby smokes. Phase 4 sign-off still pending a human playtest at ~100ms RTT - the milestone asks how it feels, which no gate here answers.	2026-08-21 09:17:19 +01:00
Josh Creek	cf73074e27	fix(multiplayer): resolve composition regression from second adversarial review ... A second adversarial review of the previous fix commit found two of its nine fixes silently defeated each other: the seq-range guard (fix for a MEDIUM epoch-mismatch finding) capped the exact variable the ring-overflow resync (fix for the original CRITICAL finding) depends on, making the resync unreachable in production and recreating permanent input death at a lower failure threshold, reachable via ordinary server tick loss alone. - CRITICAL: rebind the seq-range guard to InputJitterBuffer's own highest_ingested_seq (now public) instead of the consumer-side last_applied_seq, so it tracks the client's send epoch rather than a value that can lag arbitrarily far behind during a stall. - HIGH: InputLeadController's release logic still ANDed the old `lead > LEAD_MIN` gate onto the new depth-driven condition, so a backlog the controller never caused still couldn't drain. Split into two independent decisions: the seq-duplicate action follows real depth alone; lead's own bookkeeping separately never drops below its floor. - MEDIUM: widen the CI driver's movement/stalled sampling margin (run_seconds - 2.0, was - 0.5) and assert the peer is still in multiplayer.get_peers() at sample time, since the old margin let the check pass on residual starvation grace after a bot had already disconnected. - LOW: measure horizontal-only displacement in the human smoke test's movement check — the old 3D-distance bar was beatable by pure gravity settling with fully dead input. - LOW: fix a real "clean stderr" violation (match_net.gd broadcasting a departure notice to a peer whose ENet channels are already torn down, including a second peer disconnecting in the same poll batch) by deferring the notification to the next idle frame. - Wire the server's per-slot stalled bit into the client debug overlay for real — a prior commit message claimed this already reached the overlay when only the CI gate actually read it. Re-verified end-to-end against the real production RPC path (not just unit tests in isolation, which is how the composition bug got past the first round): a 2-bot CI match with a 1.5s host SIGSTOP freeze injected mid-run, well past the 0.6s threshold the review reproduced the bug at, now recovers cleanly on repeated runs with zero stderr noise.	2026-08-20 18:26:12 +01:00
Josh Creek	2325313ad2	fix(multiplayer): adversarial review fixes for Phase 3 ... An Opus subagent's adversarial review of Phase 3 found a critical, silent, permanent bug plus eight smaller real issues, all empirically verified with real two- and three-process runs: CRITICAL: InputJitterBuffer's 32-entry ring permanently bricked a player's input once the un-consumed backlog exceeded the ring's capacity - a fresh arrival would land in the exact slot consume() was still waiting on, and since both counters only ever advance, the gap never closed. Reproduced with a real SIGSTOP/SIGCONT host freeze: client movement dropped from ~26m to 0.00m at ~0.7s, worse under real loss (a lossy link lowered the fatal threshold to ~400ms), and reachable via ordinary clock drift with no external trigger at all. Fixed by tracking the highest seq ever ingested and having consume() jump directly to what the ring can still provide once the gap exceeds capacity, instead of starving through an unrecoverable span. Re-verified with a 3s freeze (well past the original threshold): full recovery. HIGH: InputLeadController's release logic was gated on its own past attacks (lead > LEAD_MIN) rather than the real server-reported depth, so a backlog it didn't itself cause was never drained. Fixed to gate on actual depth vs target. MEDIUM-HIGH: the rate limiter's "N consecutive over-budget seconds" streak hard-reset to 0 on any clean window, letting a duty-cycled flood (burst, one clean window, repeat) sustain ~33x budget indefinitely with zero warnings. Replaced with a leaky-bucket accumulator immune to the same evasion by construction. MEDIUM: the seq > server_tick + 20 guard compared two unrelated clock epochs (server process uptime vs. client's own from-zero seq numbering), so it never actually protected anything on a long-running server and could silently drop an honest client's input forever. Bound against the buffer's own last_applied_seq instead. MEDIUM: InputJitterBuffer.stalled was computed but never reached the wire - the one signal that would have made the ring-overflow bug visible anywhere. Now wired through _ship_to_net_body_state. MEDIUM: task 3.6's CI driver's assertions didn't depend on client input reaching the server at all, so it kept passing with the ring-overflow bug actively triggered. Added real ship-movement and non-stalled checks, sampled while bots are still connected (an initial attempt sampled after their own legitimate disconnect, which starves identically to the bug). LOW-MEDIUM: a lead change silently mislabelled _input_history's older entries, since the wire format has no per-entry seq field. Fixed by handling each delta case (ordinary/release/attack) on its own terms. LOW: bandwidth and snapshot-loss overlay metrics froze at their last value during a total outage instead of decaying - exactly when they matter most. Both now report honest post-outage values. LOW: a guard comment on NetworkManager._ping misdescribed the actual disconnect_peer() arguments in use. Corrected. New permanent regression tests: test_ring_overflow_resyncs_to_fresh_data _instead_of_starving_forever, test_release_drains_a_backlog_it_never_ caused_itself, and client-abuse-flood-dutycycle (reproduces the exact duty-cycle evasion). Full regression suite, including the net-sim-latency milestone gate, all abuse roles, and the CI driver, re-run clean after every fix.	2026-08-20 15:28:44 +01:00
Josh Creek	86a597f0f5	feat(multiplayer): Phase 3 tasks 3.1/3.2/3.5 - input redundancy + server jitter buffer ... Client now sends the last 4 ticks' actions per packet (newest-first, already-supported by net_codec's wire format from Phase 1) instead of a single action with no redundancy. Server gains a real per-slot ring buffer (new InputJitterBuffer class, scripts/input_jitter_buffer.gd) that consumes exactly one sequence number per physics tick: repeats the last action on a starve, zeroes only after a sustained 500ms stall, and reports real input_buffer_depth/last_input_seq/echo_client_send_ms in every snapshot instead of the hardcoded zeros Phase 2 shipped with. InputJitterBuffer is a standalone, scene-free RefCounted (same pattern as net_codec.gd/net_interpolator.gd) specifically so it's unit-testable against scripted arrival traces (tests/cases/test_input_jitter_buffer.gd): sequential consumption, redundancy surviving a 3-packet burst loss (3.1's own acceptance criterion), starvation repeat-then-zero timing, stale/ reordered packet handling, buffered-depth reporting, and ring-wraparound slot-tagging safety. One real bug found wiring this into a live match: the server's ring buffer started counting its own "expected sequence" from 0 the instant a player's slot was created - well before that player's first real packet could possibly have arrived (connection handshake, arena/ship spawn all take real time first). Since both sides only ever advance monotonically with no resync mechanism, that gap between the server's arbitrary local counter and the client's actual from-1 sequence numbers never closed, so the ship simply never received the client's input (0m movement in a two-process test). Fixed by seeding the buffer's expected-sequence counter from the client's own numbering on first real ingest, rather than assuming a shared from-zero baseline. Verified with real two-process runs: clean baseline movement restored, zero starvation observed under 25% random simulated input loss (well above what redundancy-4 needs to fully absorb), and correct starve-then- stall behaviour confirmed under 100% loss as a sanity check that the mechanism isn't a silent no-op. Full regression suite, including the net-sim-latency milestone gate, re-run clean.	2026-08-20 13:07:33 +01:00