Files
CosmicClash/Game/scripts/input_jitter_buffer.gd
T
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

179 lines
8.3 KiB
GDScript

class_name InputJitterBuffer
extends RefCounted
# Per-player server-side input state (multiplayer-todo.md §3, task 3.2).
# Deliberately a standalone RefCounted with no scene/RPC dependency — same
# reason net_codec.gd and net_interpolator.gd are pure classes — so task
# 3.5's unit tests can drive it with scripted arrival traces with no live
# match. NetworkedMatch owns one instance per connected slot and is the only
# thing that talks to the network layer; this class only knows about
# sequence numbers and ShipActions.
#
# Ring is fixed-size and slot-tagged (§3.1 step 5's "a client can never make
# the server allocate"): ingest() writes seq % RING_SIZE regardless of how
# large or malicious seq is, and consume() only ever trusts a slot whose
# stored seq exactly matches the one it expects — a stale or wrapped-around
# entry is indistinguishable from an empty one. Range/rate validation of seq
# against the current server tick is the CALLER's job (task 3.4), not this
# class's, since only the caller knows the current server tick.
const RING_SIZE := 32
# 500ms at 60Hz (multiplayer-todo.md §3.2's own numbers) — a duration, not a
# tick-rate-derived constant, so left as a literal rather than pulling in
# SimConstants for one number.
const STARVE_ZERO_TICKS := 30
var last_applied_seq := -1 # -1: consume() has never been called yet
var last_action := ShipAction.new()
var starved_ticks := 0
var stalled := false
var _ring_action: Array = []
var _ring_seq: PackedInt32Array = PackedInt32Array()
# True once ingest() has ever been called for real. Consumption is a no-op
# (no starvation counted, no advancement) until then — see ingest()'s own
# comment for why an un-seeded buffer would otherwise never converge with
# what the client is actually sending.
var _seeded := false
# Highest seq ever seen by ingest(), regardless of whether it's still in the
# ring — consume()'s only way to tell "the data is gone because the ring
# overflowed" apart from "the data just hasn't arrived yet". See consume()'s
# own comment for why this exists: an adversarial review found that without
# it, a backlog bigger than RING_SIZE (a host stall, or persistent client/
# server clock drift) permanently zeroed a connected player's input for the
# rest of the match.
#
# Deliberately public (no underscore), same as last_applied_seq: the
# networked_match.gd caller's seq-range guard (§3.1 step 4) must bound
# against THIS, not against last_applied_seq. A second adversarial review
# found that bounding against last_applied_seq caps every accepted seq at
# last_applied_seq + RING_SIZE, which in turn caps this field at the same
# ceiling — making the resync condition below (which needs this field to
# reach expected + RING_SIZE) arithmetically unreachable on the only call
# path that exists in production. The two fixes looked independent but
# shared a variable and silently cancelled each other out. highest_ingested
# tracks the client's own send epoch instead, which the guard can safely
# let run ahead of a lagging consumer.
var highest_ingested_seq := -1
func _init() -> void:
_ring_action.resize(RING_SIZE)
_ring_seq.resize(RING_SIZE)
for i in RING_SIZE:
_ring_seq[i] = -1
# newest_seq/actions match NetCodec.unpack_input's own "seq"/"actions"
# fields directly: actions[i] is the action for sequence (newest_seq - i),
# newest-first. Already-consumed or stale entries are silently discarded
# (§3.1 step 5) — this is what makes redundant re-delivery of an already-
# applied tick harmless.
func ingest(newest_seq: int, actions: Array) -> void:
if not _seeded:
# The server starts calling consume() every tick the instant this
# slot exists — well before this player's first packet has had time
# to arrive (connection handshake, arena/ship spawn, first
# _physics_process tick on the client all take real time first). An
# un-seeded last_applied_seq of -1 would have consume() "expecting"
# sequence 0, 1, 2, ... via pure starvation the whole time, racing
# arbitrarily far ahead of whatever the client's own from-1
# numbering has actually reached by the time real packets show up —
# and since both sides only ever advance monotonically with no
# resync mechanism, that gap would never close. Seed to align
# "expected" with reality the moment real data first exists.
last_applied_seq = newest_seq - actions.size()
_seeded = true
if newest_seq > highest_ingested_seq:
highest_ingested_seq = newest_seq
for i in actions.size():
var seq: int = newest_seq - i
if seq <= last_applied_seq:
continue
var idx := seq % RING_SIZE
_ring_seq[idx] = seq
_ring_action[idx] = actions[i]
# Contiguous run of not-yet-applied entries starting right after
# last_applied_seq — reported as input_buffer_depth in every snapshot
# (§3.3) and consumed client-side by the input_lead control loop (task 3.3).
func depth() -> int:
if not _seeded or last_applied_seq < 0:
return 0
var d := 0
var seq := last_applied_seq + 1
while d < RING_SIZE and _ring_seq[seq % RING_SIZE] == seq:
d += 1
seq += 1
return d
# Called once per server physics tick, before the step (§3.2). A no-op
# (returns the zero-initialized last_action, no starvation counted) until
# this player's first real packet has ever arrived — see ingest()'s comment.
func consume() -> ShipAction:
if not _seeded:
return last_action
var expected := last_applied_seq + 1
var idx := expected % RING_SIZE
# Ring-overflow resync. A fixed-size ring can only ever hold RING_SIZE
# ticks of not-yet-consumed data at once — if the caller has fallen
# further behind the newest data actually arriving than that (a host
# stall, or persistent client/server clock drift), every tick between
# "expected" and "highest_ingested_seq - RING_SIZE" has already been
# irrecoverably overwritten by more recent arrivals landing on the same
# ring slots. Waiting for it tick-by-tick would starve — and, past
# STARVE_ZERO_TICKS, zero this player's ship — for the ENTIRE gap even
# though fresh, real input already exists in the ring right now. An
# adversarial review found and reproduced this exact failure (a ~0.7s
# host freeze permanently zeroed a connected player's input for the
# rest of the match, with no self-recovery). Skip the unrecoverable
# span and resync directly to what the ring can still actually provide.
if highest_ingested_seq - expected >= RING_SIZE:
last_applied_seq = highest_ingested_seq - RING_SIZE
expected = last_applied_seq + 1
idx = expected % RING_SIZE
if _ring_seq[idx] == expected:
last_action = _ring_action[idx]
starved_ticks = 0
stalled = false
last_applied_seq = expected
return last_action
# Repeat-last, not zero: inputs are heavily autocorrelated at 60Hz,
# and the client already predicted with the real input either way,
# so repeating minimises expected divergence (§3.2). Only zero after
# a sustained stall, so a disconnecting player's ship doesn't fly
# into a wall at full throttle forever.
starved_ticks += 1
if starved_ticks > STARVE_ZERO_TICKS:
last_action = ShipAction.new()
stalled = true
# Only GIVE UP on `expected` when strictly newer data has actually
# arrived, which proves it was lost or reordered rather than merely late.
#
# Advancing unconditionally (what this did originally) is catastrophic
# rather than merely lossy, because ingest() discards anything
# `seq <= last_applied_seq`. One starve on a sequence the client has not
# even sent yet leaves the server permanently one ahead of arrivals:
# both sides then advance one per tick, the gap never closes, and every
# honest packet is discarded on arrival for the rest of the match. An
# adversarial review reproduced exactly that on a clean LAN — the client's
# own input_lead RELEASE (delta == 0, which deliberately issues no new
# sequence for one tick) is sufficient to trigger it, so it fired roughly
# every 6.5s of ordinary play, blacking out input for 30 ticks until the
# lead controller's debounce allowed a +3 attack to jump the client clear.
#
# Holding cannot deadlock: if the client genuinely goes silent,
# highest_ingested_seq stops moving, starved_ticks still climbs, and the
# STARVE_ZERO_TICKS zeroing plus `stalled` above still fire on schedule.
# If it falls far behind instead, the ring-overflow resync above still
# jumps the cursor forward. Both escape paths are unchanged.
if highest_ingested_seq > expected:
last_applied_seq = expected
return last_action