From 8fa53b778ca5804f7398098c641f378cc4632cb7 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 14:13:36 +0100 Subject: [PATCH] fix(multiplayer): stop an infinite queue-ticket resync loop at revision 0 Found via a new real end-to-end integration test (next commit), not by inspection: a client that just called begin_queue() and receives the server's first confirmation at the same revision (0) always treated it as a conflict and requested a resync -- forever, since the resync response is itself a same-revision confirmation hitting the exact same false mismatch. A real Godot client against a real running server would loop on GET /v1/queue/{id} without ever settling into QUEUED. Root cause: apply_ticket_update()'s incoming_revision == revision branch never adopts fields on acceptance, but _ticket_differs() compared expires_at_unix -- a field begin_queue() has no way to set in advance, since it doesn't know the server-assigned expiry yet. Every first same-revision confirmation therefore looked like a conflict unconditionally, not just occasionally. Fix: exclude expires_at_unix from the conflict check (a differing expiry at the same revision is expected, not a sign of corruption -- real conflicts are still caught via state/playlist), and adopt it on acceptance so the field doesn't just become permanently stale instead. The existing "same-revision conflict requests recovery" unit test still passes unchanged: its fixture differs on `state`, not expires_at_unix, so it was never actually exercising this bug. --- Game/scripts/matchmaking_state.gd | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/Game/scripts/matchmaking_state.gd b/Game/scripts/matchmaking_state.gd index 919ab656..a750b821 100644 --- a/Game/scripts/matchmaking_state.gd +++ b/Game/scripts/matchmaking_state.gd @@ -54,6 +54,13 @@ func apply_ticket_update(update: Dictionary) -> bool: if incoming_revision == revision: if _ticket_differs(update): return _request_resync(self.ticket_id) + # expires_at_unix is deliberately not part of _ticket_differs' conflict + # check (see its own comment) but is still adopted here: begin_queue() + # has no way to know the server-assigned expiry in advance, so the + # very first same-revision confirmation is the only place a freshly + # queued ticket's expiry is ever set at all. + if update.has("expires_at_unix"): + expires_at_unix = int(update["expires_at_unix"]) return true if incoming_revision > revision + 1: return _request_resync(self.ticket_id) @@ -185,7 +192,17 @@ func snapshot() -> Dictionary: func _ticket_differs(update: Dictionary) -> bool: - return String(update["state"]) != phase or (update.has("playlist") and String(update["playlist"]) != playlist) or (update.has("expires_at_unix") and int(update["expires_at_unix"]) != expires_at_unix) + # expires_at_unix is excluded on purpose: begin_queue()'s optimistic local + # state has no way to know the server-assigned expiry before the first + # real response arrives, so comparing it here made the very first + # same-revision confirmation after every begin_queue() look like a + # conflict, unconditionally -- found by an actual client hitting a real + # server: apply_ticket_update() kept requesting a resync, whose own + # response hit exactly the same false mismatch, forever, which + # control_plane_smoke.gd (a live end-to-end test, not a mock) surfaced as + # a request that legitimately never terminates. It's still kept current + # via the direct assignment below, just not treated as a conflict signal. + return String(update["state"]) != phase or (update.has("playlist") and String(update["playlist"]) != playlist) func _request_resync(resource_id: String) -> bool: