mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
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.
This commit is contained in:
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user