mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-11 00:14:00 +00:00
cb06300685de2ebcd2e8cc87b12597937ecbe0e9
21 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Josh Creek
|
23c1c3d231 | ci: add ENet integration coverage | ||
|
Josh Creek
|
9e4609d5b3 | docs(multiplayer): add concise next-work checklist | ||
|
Josh Creek
|
6bafdc7794 | feat(multiplayer): add Steam transport foundation | ||
|
Josh Creek
|
624d1c6b78 |
docs(multiplayer): add a single index of outstanding work, and record the identity defect
The document had no one place that answered "what is left". Outstanding items were spread across two phase-gate lines, two phase tables, §11, and prose buried in the phase notes - and the name-keyed slot-reservation hijack, which an adversarial review demonstrated with a real three-process run, was not written down anywhere at all. It existed only in a conversation. New §0 indexes everything not done, in four groups: verification a machine cannot do (the Phase 4 playtest, the Phase 5 3v3 gate), known defects left unfixed with their severity, the one open architectural question, and the two unstarted phases. Each row points at the detailed write-up rather than duplicating it, and the phase gates now point back. §11 gains the identity defect in full: reservations match on slot.player_name and nothing else, with no uniqueness constraint on names anywhere, so a peer joining during the 30s window with a departed player's name is handed their slot, ship and team. A bespoke token would be half of task 7.4 thrown away, so it stays deferred - with the consequence stated plainly rather than implied, and listed as a precondition of Phase 6's internet-facing gate. Also refreshes the stale status paragraph and task 5.10 for the replay log's reject recording, write-failure handling, close(), and dump tool. |
||
|
Josh Creek
|
ff725e1ffa |
feat(multiplayer): §6.3 late joiners take a vacated slot at the next kickoff
"Spectate now, take the slot at the next kickoff" was a print statement. The server logged it and never acted; on the client, _is_spectator was assigned once in _on_match_config_received and never revisited - and that handler returns early whenever _slots is non-empty, so no rebroadcast could promote an in-match spectator. The reconnect path only worked because a returning player is a fresh process. Server: late joiners are queued in arrival order and the queue is drained from _begin_kickoff, before the reset transforms are read, so a promoted player's ship is placed by that same kickoff and the controller swap lands on an already-frozen body. A slot is available only once its player has gone AND their 30s reservation has lapsed - §6.4 outranks §6.3, since taking a reserved slot would quietly break the reconnect promise. _abort_if_abandoned now counts a waiting spectator as somebody present, or the one person queued for the slot that just opened is dumped to the lobby at the moment they were about to get it. Client: new broadcast slot_assigned (reliable, channel 0). Broadcast because every client holds its own slot list and one naming the wrong peer keeps flying somebody else's ship as a remote body; reliable because no per-snapshot field would re-converge a client that missed it. The promoted client undoes what made the body remote - fresh interpolator, physics interpolation back on, offsets cleared - and deliberately does not unfreeze, clearing _local_prediction_ready so the next snapshot teleports it to a real authoritative pose first. The controller-attach block moved to _take_local_ownership rather than being copied. New --role=host-latejoin/--role=client-latejoin and --slot-reservation-seconds=. Verified 4/4 both sides: queued, NOT promoted merely because the reservation lapsed, takes the slot at the kickoff, same ship instance, and both peers independently measure ~45.7m under its input. Control with a 90s reservation: kickoff fires, nothing is promoted, the slot still reads the departed player's name. |
||
|
Josh Creek
|
5714829c13 |
test(multiplayer): grade §6.4's reconnect from the returning player's side
The disconnect scenario only ever asserted the server's bookkeeping, and the client's half was failing every run. run_disconnect_host_check ticked 60 physics frames past the reclaim and then shut the server down, so the reconnecting client - whose wiring check waits a 2.0s settle before it looks at anything - had its peer torn out from under it and reported "current_scene is not NetworkedMatch after 2.0s". The host printed PASS throughout, and the host was the side anyone read. The hold is now a real window (8s), and the host also asserts that the reconnected player's input reaches the server and moves the ship the server owns - every other assertion there is slot bookkeeping that would hold identically for a client whose input pipeline came back dead. Both position and connection state are sampled while the peer is still connected: the client leaves on its own schedule, and an end-of-hold sample reported still_connected=false for a good run. New --role=client-reconnect asserts the returning player is not a spectator, owns a slot with its own peer_id, has a real ship, rejoined a live match with the clock already known (§6.2 step 2's bootstrap), and can still drive. That set is chosen because a stale _last_match_config once made a reconnecting player a spectator, and that bug was visible in this scenario's own logs while it reported PASS. Verified 3/3 both sides. Control: rejoining while the slot is still occupied fails on is_player=false - and since the first control run reported it as the generic "lost its ship mid-drive", the spectator case is now diagnosed before the drive rather than after. |
||
|
Josh Creek
|
866efa0d9b |
fix(multiplayer): server no longer rate-limits a backlog it caused itself
Closes task 5.10's three recording gaps, and the gap-closing found a real input-loss bug. Replay log: a failed write now ends the log permanently instead of desyncing every later record's framing; close() is called from _exit_tree with a summary, since the RefCounted destructor closes it implicitly but never says whether the log is complete; rejected packets are recorded with their reason in the kind byte (framing unchanged, FORMAT_VERSION 2 so "no rejects" differs from "this build never recorded them"). Recording is capped at 8 per peer per window - uncapped, the diagnostic is a remote disk-fill amplifier, since the attacker picks the packet rate. Uncapped totals live on MatchSim and survive the peer's disconnect. The bug: a 2s host stall has the client sending at 60Hz throughout, and ENet delivers that whole backlog in the first window after resume - 70 of an honest client's packets rejected as "rate limit exceeded". Redundancy does not cover it, because the dropped packets are contiguous: 0 of 70 rescued, and 82 of 923 sequences (8.88%, ~1.4s of input) never reached the server, against 0.00% with no stall. Every prediction gate passed. Fixed by granting each already-tracked peer a capped, two-window packet grace when the server detects its own wall-clock stall. Rate-limit rejects 70 -> 0, sequences missing 8.88% -> 0.00%, seq-guard rejects 9 -> 0. Controls on the unfixed build lost 4.34/7.52/7.86%. All three abuse roles still disconnect and no flood induced a stall, so the grace cannot be farmed. Also corrects an earlier wrong conclusion: the reviewer's free-flight p95 0.688 is real and reproduces on two processes with 0.0% snapshot loss. The plain --role=client drive fails the 0.5 free-flight bound in 3 of 8 runs because that drive is mostly a contact test - the harness comment already said so - leaving a cohort as small as 12 samples. Near-surface error is genuinely several times open-air error, so the calibrated bound now belongs to --exercise-free-flight alone and the plain role asserts the always-well-sampled all-cohort percentiles at 1.2/2.0, printing the free-flight numbers as reported-not-asserted. 6/6 plain runs pass where 3/7 failed; tightening to 0.3 still fails. tools/replay_dump.gd reads a log back: counts by kind, plus how much of the input sequence stream reached the server once redundancy is counted. |
||
|
Josh Creek
|
e51dc765a2 |
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. |
||
|
Josh Creek
|
a5cbc977b5 |
feat(multiplayer): Phase 5 tasks 5.6-5.10 - disconnects, spectators, replay log
Completes Phase 5's implementation. Every task is verified at 1v1; the 3v3 phase gate itself has not been run and remains outstanding. 5.6/5.7 disconnects: a ship is never despawned. The slot keeps it and swaps the controller (--fill-bots gives it a bot, the default leaves it inert per §1.4), sets `stalled` immediately so the nameplate greys out rather than waiting ~500ms for the abandoned jitter buffer to starve, and reserves the slot for 30s keyed by player name so a reconnect gets the same ship back. 5.7 was a real bug, found by the test rather than by review: SlotInfo.controller was declared RLShipController, but the takeover swaps in an AIShipController or the base controller - the narrower 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 run. The per-tick action write is now also gated on `is RLShipController`, since a disconnected slot's bot drives itself and overwriting it from a starving buffer would pin it to the departed player's last input. §6.4's two rules conflict: reserve for 30s, but abort when the last human leaves. Applied naively the abort wins instantly in a 1v1 and the reservation can never be redeemed, making reconnect unreachable exactly when it matters. Abort now waits for no connections AND no outstanding reservations. 5.8 spectators: a slotless peer spawns no ship and receives the same snapshot broadcast. HUDController.spectator_mode keeps the clock, score and goal celebration and hides only the ship instrument cluster - it previously push_error'd and bailed, leaving a spectator with a dead HUD. Camera cycles ships in slot order then the ball. --max-spectators caps it, counted from the live peer list so a dropped spectator cannot leak a unit of the cap. 5.9 escape respawn: new GameMode._on_bodies_respawned() virtual; NetworkedMatch bumps reset_gen through Phase 2's deferred path so the bump and the respawned pose land in the same broadcast. Single-player modes are unaffected - the base is a no-op. 5.10 replay log: scripts/replay_log.gd, --replay-log=<path>, storing the wire bytes verbatim in both directions rather than re-serialising - a re-encode would launder away precisely the malformed payload being chased. A live 6s match recorded 1115 records (557 inputs / 558 snapshots) and a stored snapshot decodes back to server_tick=100 match_state=WARMUP bodies=2. Note for future work: --check-only --script is the only thing that catches a parse error in networked_match.gd, because the unit runner never loads it. Two separate breakages passed the full unit suite while breaking every two-process run. A new class_name also needs --import before it resolves. Test surface: --role=host-disconnect (three-process 5.6/5.7 scenario), --match-length=<s>, --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, which 5.3 broke by adding KICKOFF_YAW_JITTER; thrusting while turning took it from 2/3 to 5/5. Regression: 87 unit tests; free-flight LAN p99 0.094m with 0 hard snaps; transition gate 0.00%; ball contact 5/5; lifecycle goal cycle and full match to RESULTS/LOBBY; disconnect+reconnect; two-bot CI. |
||
|
Josh Creek
|
3d6906b981 |
feat(multiplayer): Phase 5 tasks 5.2-5.5 - clock, kickoff, goals, full time
Implements the rest of the §6.2 lifecycle on top of 5.1's state machine.
5.3 kickoff: the server resets every body and broadcasts the RESULTING
transforms, never a seed - §1's locked decision, because shared-seed
determinism needs both sides to consume the RNG stream in identical
order forever and the first randf() added to the reset path desyncs
silently. Countdown is derived from server_tick on both peers, and a
kickoff that lands after its own resume tick applies immediately and
skips the countdown rather than scheduling into the past.
5.4 goals: goal_scored(scoring_team, score, goal_tick, resume_tick).
Score is authoritative at sensor time, before any presentation. The
reset moved OUT of the sensor path and into the kickoff at resume_tick,
which is what stops the server resetting while clients are still
mid-celebration. Engine.time_scale is never touched.
5.2 clock: tick-derived, no Timer and no _process polling. The goal
pause shifts the absolute end_tick by (resume_tick - goal_tick) rather
than pausing anything, so no float drift accumulates across goals.
5.5 full time: clock expiry -> FULL_TIME -> sudden death on a draw or
RESULTS, golden goal in overtime, then LOBBY on both peers - clients
return to the lobby, not the main menu. get_tree().paused is never used.
Four bugs found and fixed while building this, each by a failing run
rather than by inspection:
- Tick order was load-bearing: _update_kickoff_countdown() clears the
same _kickoff_resume_tick that _update_match_state() reads to leave
WARMUP, so running the countdown first wiped the transition condition
and the match sat frozen in WARMUP forever.
- _apply_match_state resets _state_deadline_tick on every transition, so
a GOAL_PAUSE deadline assigned before _set_match_state was wiped and
the match never resumed. Deadlines are now owned by _apply_match_state.
- Freezing "all bodies" is wrong on a client. Remote ships and the ball
are permanently FREEZE_MODE_KINEMATIC and transform-driven; freezing
them all unfroze the remote ones on the way back out, so they fell
under gravity while the interpolator fought them - 210 hard snaps and
an infinite p99. A client now freezes only the one body it simulates.
- A frozen body never runs _integrate_forces, so the queued kickoff
teleport was stranded by an immediate set_deferred("freeze", true).
Freeze now happens on a strictly later tick, the same pattern Phase 2
used for _pending_reset_gen_bump_tick.
Prediction and reconciliation are suspended while the match is not live:
during a countdown or goal pause the local ship is frozen on both peers,
and running delta transport over those frozen states produced a p95
position error of 2.4e10 m. Input keeps flowing so the server's jitter
buffer does not starve into `stalled`.
Also fixed: a kickoff can arrive before match_config, and body order is
slot order - applying it early placed the BALL at positions[0], on top
of the first ship, which the ball-cam reported as "target vector can't
be zero" 95 times. It is now held until the roster exists.
Test changes: the ball-contact scenario steered by a hand-tuned fixed
heading, which 5.3 broke because kickoff applies KICKOFF_YAW_JITTER - it
flew past the ball in 3/3 runs. It now closes the loop on the actual
bearing using real input actions. Assertions that read a frozen ship
(freeze, thrust) are gated on the match being live, and the hooks now
survive the scene teardown at RESULTS instead of hanging on freed
objects for the full timeout.
Regression: 81 unit tests; free-flight LAN p99 0.143m and 80±20ms, both
0 hard snaps; transition gate 0.00%; ball contact 3/3; two-bot CI.
|
||
|
Josh Creek
|
9f28c02488 |
feat(multiplayer): Phase 5 task 5.1 - match lifecycle state machine
Adds the §6.1 state machine, its broadcast, and the client side that follows it. Physics, freezing and input are deliberately NOT gated on state yet - 5.3 and 5.4 own freeze/unfreeze at kickoff and goal, and doing it here would change the conditions every Phase 4 prediction gate was measured under. scripts/match_state.gd holds the enum and transition table as pure data with no scene or RPC dependency, so the table is checked exhaustively rather than by example: every state reachable, every state has an exit, no self-transitions, abort-to-LOBBY from anywhere per §6.4, illegal shortcuts rejected, unknown values refused rather than coerced. The enum values are the wire format - match_state has been a u8 in the snapshot header since §2.4 - so a test pins them; only append, never renumber. The server validates every transition and push_errors an illegal one rather than following it. Clients deliberately do NOT enforce the table: authoritative state must be accepted, and a late joiner legitimately jumps straight to PLAYING. Two channels carry the state. state_change (reliable, channel 0) is prompt and carries an absolute at_tick, never a duration. The snapshot's match_state byte is the catch-up path for a client not yet sent a transition - a late joiner, or the window between scene load and the first RPC. The byte needs a tick guard, and this was found the hard way. Snapshots are unreliable_ordered on channel 2 and ordering holds only within a channel, so a state_change for tick N routinely arrives before an in-flight snapshot from tick N-2. Without the guard the client applies the new state then gets dragged back by the older byte, oscillating on every transition - observed directly as LOADING -> WARMUP -> LOBBY -> PLAYING -> LOBBY while running a deliberately-broken-byte control. Only a byte at least as new as match_state_since_tick is accepted. WARMUP_TICKS/GOAL_PAUSE_TICKS are honest placeholders so 5.1 drives real transitions to verify against; 5.3 and 5.4 replace them. The server also leaves LOADING immediately rather than waiting for scene_ready, which does not exist yet. New smoke flag --exercise-match-state, passed to both roles: the host forces a goal to drive a GOAL_PAUSE cycle, the client records the sequence and asserts every consecutive pair is legal, that ticks are monotonic, and that the wire byte agrees with its own state. Observed LOADING -> WARMUP -> PLAYING -> GOAL_PAUSE -> WARMUP with tick deltas matching the configured durations exactly. Verified against a control: hardcoding the snapshot byte back to 0 fails both the byte assertion and the transition-legality assertion. The byte is asserted separately from the RPC precisely because everything else in the check is RPC-driven and would pass with a dead byte - the same gap that hid the Phase 4 label bug (gotcha 47). Regression: 81 unit tests; 60s free-flight LAN (p99 0.148m, 0 hard snaps, marker 0/3364); transition gate 0.00%; ball contact; two-bot CI. |
||
|
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. |
||
|
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. |
||
|
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. |
||
|
Josh Creek
|
10040f7339 |
docs(multiplayer): close out Phase 3 in multiplayer-todo.md
Documents all seven Phase 3 tasks (3.1-3.7) with DONE status and verification evidence, updates the top-level status summary, and records the phase gate as met - re-verified today under the gate's own exact condition (--net-sim-latency 80 --net-sim-loss 0.05) on both the human smoke test and the two-bot CI driver, not just the looser conditions used during individual task development. Adds one new gotcha (#38): GDScript lambdas capture enclosing locals by value, not by reference, which silently broke two separate Phase 3 test scripts' own disconnect-detection assertions this session (the production disconnect logic was correct both times; only the test's own flag-capture pattern was wrong). Also records a deliberate scope decision for task 3.4: server-side input_lead enforcement from arrival times was scoped down to observability rather than built as active enforcement, since the concrete security requirements (rate limiting, malformed-packet counting, seq-range rejection, disconnect policy) already close the load-bearing gaps and the doc's own text calls the remaining edge "small" - flagged to revisit once Phase 4's prediction work exists to judge against. |
||
|
Josh Creek
|
14698d4ccb |
fix(multiplayer): adversarial review fixes for Phase 2
An Opus subagent's adversarial review of Phase 2 found real bugs the smoke tests couldn't catch, since constant-velocity dead reckoning still moves a ship far enough to pass a "moved > 1.0" check: - The interpolator never actually interpolated. NetInterpolator.to_tick() assumes physics_frame * TICK_MS == Time.get_ticks_msec() on the server, which is off by a steady ~45-55ms in practice (real startup work before the first physics step, widened by any dropped tick). Every sample_at() call took the extrapolation branch, 100% of the time, defeating the interpolation buffer entirely. Fixed with a shared, min-filtered rolling bias estimate in networked_match.gd, applied before every to_tick() call. - Goals caused a ~27m visual slide: _reset_gen was bumped before the queued teleport actually landed, so the client's buffer-clear kept exactly the stale in-goal sample and lerped a slide to the next, real one. Fixed by tracking the tick the goal was detected on and only bumping the generation once strictly later ticks confirm the teleport has landed - a naive "next _physics_process" boolean flag doesn't work, since a goal Area's body_entered fires before that same tick's _physics_process runs, not on the next one. - _local_input_sampler (a Node, never added to the tree) was never freed - this was the unexplained "3 resources still in use at exit" warning on every Phase 2 test run. - Ball angular velocity decoded 8x too small (rescale_avel was never called); get_server_time_estimate_ms() was used before the clock had synced; net_sim.gd's delayed-send timer stopped ticking while the tree was paused and didn't check connection status before firing; _broadcast_snapshot's ball index could silently break if a ship were ever despawned; declared-but-unemitted HUD lifecycle signals showed a permanently frozen timer widget. Also confirmed, empirically, several things the review checked and found fine: a hostile client sending malformed input cannot crash the server, skipping GameMode's super() drops nothing load-bearing, deterministic slot assignment is correct with 2 real simultaneous clients, and RPC authority enforcement genuinely rejects a forging client. All fixes verified with real two-process runs (including forcing an actual goal and reading the server's own broadcast stream) and temporary instrumentation, removed once each fix was confirmed. Full Phase 1 + Phase 2 regression suite, including the net-sim-latency milestone gate, re-run clean after every fix. |
||
|
Josh Creek
|
7b150ef72e |
feat(multiplayer): task 2.8 net_sim.gd, close out Phase 2
New NetSim autoload: seeded, CLI-driven (--net-sim-latency/-jitter/-loss/-dup) latency/jitter/loss/duplicate decorator, a true no-op passthrough unless a flag is set. Wraps MatchSim.send_input/send_snapshot per the design doc's scope, plus NetworkManager's ping/pong so the already-tested RTT/clock measurement becomes the acceptance signal for "raises observed RTT" without waiting on Phase 3's per-peer snapshot echo. Two real bugs found while building and verifying this against Phase 2's own milestone gate (a real match under --net-sim-latency 80 --net-sim-jitter 20, not just LAN): a timestamp captured inside a delayed RPC closure silently ate that side's own added delay out of the round-trip measurement instead of adding to it; and a delayed send whose target disconnected (or whose own process had already shut down) during the hold threw RPC errors, since the existing get_peers() filtering only checked validity at schedule time. Fixed by capturing timestamps before handing off to NetSim, and by having NetSim re-validate the target at fire time. Phase 2's milestone gate now passes for real: a full 1v1 under simulated 80ms latency / 20ms jitter still shows clean server-authoritative movement and zero RPC errors. Full Phase 1 + Phase 2 regression suite re-verified clean with NetSim present but inactive. |
||
|
Josh Creek
|
39a41c016c |
feat(multiplayer): Phase 2 server-authoritative simulation, dumb client
Implements tasks 2.1-2.7: NetworkedMatch spawns a deterministic slot layout from the lobby roster, the server drives each connected peer's ship via RLShipController fed by decoded client input and broadcasts 60Hz snapshots, and the client renders everything (including its own ship) from a per-body NetInterpolator with no local prediction yet. Dual-time remote entities split collider updates (present-time, for correct contacts) from $Visual updates (interp-delayed, for smoothness). Camera/HUD wiring and remote engine-flame VFX fell out of the existing Ship API for free once snapshots were flowing. Three real bugs found and fixed while getting a two-process test green: an RPC method named _input collided with Node's built-in _input virtual and broke the whole MatchSim autoload from loading; networked_match.gd never called NetworkManager.poll(), so nothing sent via RPC in this scene reached the wire despite Phase 1's manual polling being wired up everywhere else; and a match_config request/response fallback (added to close a startup race) could double-deliver once polling was fixed, requiring an idempotency guard. Verified with tests/networked_match_smoke: a real headless two-process host+client run shows the client rendering 31m of server-authoritative movement from a held forward-thrust input, with thrust_z=1.0 confirmed on the interpolated snapshot mid-drive and camera/HUD both wired. Full Phase 1 regression suite re-run clean alongside it. Task 2.8 (net_sim.gd latency/jitter/loss decorator) is not yet done; Phase 2's own gate needs it before it's fully met. |
||
|
Josh Creek
|
4533da34e0 |
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. |
||
|
Josh Creek
|
e83bb4fa0c |
fix(multiplayer): revert stray match.tscn team_size, record 0.15b real-hardware results
match.tscn had picked up team_size=3 from an earlier diagnostic dry run, which would have made every normal Match spawn 3v3 instead of 1v1 - reverted to the scene's intended default. multiplayer-todo.md: task 0.15b's real blocker turned out to be measuring on a Mac (Apple Silicon's tile-based GPU architecture gave a misleading, undifferentiated cost profile). Re-ran the same 6-ship-match profiling harness on reference hardware (RTX 3090) via a real GPU-bound X session - results in §5.5.2 show the game comfortably clears 500+fps with every effect on, and SDFGI/SSIL dominate the (now tiny) effects budget as originally expected. This closes 0.28 (physics threading) as unnecessary - there's no frame-time variance problem on reference hardware to fix - and reframes 0.26 (bake GI) as a real but smaller win than assumed, worth revisiting on lower-end hardware. Also corrected two stale/inaccurate task rows (0.13, 0.17) found while reconciling the doc against what actually landed. |
||
|
Josh Creek
|
04691aaa48 |
chore(multiplayer): Phase 0 refactors + graphics/perf settings groundwork
Lands the non-networked Phase 0 tasks from multiplayer-todo.md (ship/camera/ arena refactors, sim constants, background FPS handling) plus a first pass at exposing graphics/performance settings (presets, resolution scaling, vsync, FPS cap, perf overlay) and a GPU profiling harness for the real-hardware follow-up in task 0.15b. |