The main menu clipped its own title and bottom button in debug builds. The
project lays out in a hard-fixed 1920x1080 logical viewport
(window/stretch/mode="viewport"), and the only overflow strategy in the
scene was a CenterContainer, which centres its child rather than clipping
and scrolling. With DevSection visible the content measures 1133px against
1080, so roughly 53px spilled off both ends with no way to reach it — and
main_menu.gd grabs focus on a button that may itself be off-screen.
Worth recording because it is counter-intuitive: this is not
resolution-dependent. Because the viewport is fixed, a 4K display magnifies
the same clipped 1080p frame rather than giving the menu more room, so the
fix has to make the layout scroll, not scale.
Each menu is now MarginContainer > ScrollContainer > CenterContainer >
VBoxContainer. ScrollContainer sizes its child to max(own size, child
minimum), so an expanding CenterContainer keeps today's centred look when
the content is short and grows past the viewport when it is tall — which is
exactly when scrolling should start. follow_focus is on so keyboard and
controller navigation cannot strand focus off-screen. Lobby and matchmaking
share the same shape and get the same treatment before they hit the same
wall; settings gained its wrapper alongside the Controls tab.
Also stops the dev bot dropdowns widening the whole menu: they are filled
from res://bots filenames and expand horizontally, so a long checkpoint
name dragged the layout past its 420px minimum.
test_menu_layout asserts each screen's bottom-most control really sits
inside a ScrollContainer. That is a structural guard against the wrapper
being removed or a new section being added outside it — not proof that
nothing visually clips, which was checked by hand at 1000x600, 1280x720 and
1920x1080.
Playing with a gamepad did not work: all six move_* actions had no joypad
event at all, so a pad could yaw/pitch/roll/turbo but could not translate.
Nothing caught it because every action existed and the game booted fine —
no assertion checked that an action is reachable on *both* devices.
Controller layout, on the 6DOF convention (left stick aims, right stick
translates), using all six of the pad's analog axes for the ship's six
degrees of freedom:
left stick yaw + pitch right stick strafe + vertical
LB / RB roll RT / LT forward / back
L3 turbo R3 ball camera
Input is now read with Input.get_axis instead of is_action_pressed, so
triggers and sticks are proportional. Keyboard values are unchanged.
Three rotation bugs found by measuring a real Ship rather than reading the
code:
- apply_torque() is world-space and the torque was never rotated into the
hull's frame (unlike thrust, which uses -ship_basis.z). Roll input became
pitch after a 90 degree turn and inverted at 180, so the controls were
correct flying up-field and backwards flying back.
- ship.tscn's inertia is Vector3(7, 1, 7) but a flat torque was applied to
every axis, giving yaw 7x the angular acceleration of pitch and roll
(172 deg/s vs 52). Torque is now scaled per-axis by inertia, so
rotation_acceleration means rad/s^2 and all three axes match. Yaw is
unchanged.
- pitch_down pitched the nose UP: get_axis's arguments were reversed, so
the I/K keys and the stick each did the opposite of their label.
Menus were unusable on a pad for a separate reason: Godot 4.7 gives
ui_up/down/left/right joypad events by default but leaves ui_accept and
ui_cancel with none (verified against a pristine project), so a controller
could move the highlight and never press anything. A confirms and B goes
back. Gameplay exits on a new leave_gameplay action (Escape / Start) rather
than ui_cancel, so carrying B for menus cannot abandon a live match.
Bindings for both devices are rebindable in Settings -> Controls, persisted
to user://input.cfg — a separate file from settings.cfg because
VideoSettings.save() rewrites that file wholesale and would drop any
section it does not know about. project.godot stays the source of truth for
defaults; overrides are only ever a delta on top of a boot-time snapshot.
Verified: 268 unit tests, the ENet integration gate, and a 16-sample
before/after comparison of networked prediction residuals showing the
physics change does not regress them (median 0.083m -> 0.065m).
Note for follow-up: every policy in Game/bots/ was trained against the old
sluggish, world-axis rotation and will over-rotate until retrained.
TECH_STACK.md explained why each choice was made but never listed what is
actually pinned, so there was no single place to answer "what version of X do
we use". Adds a Version inventory section covering the shipped game, the Go
control plane's four direct dependencies, the datastore/platform versions and
the exactly-pinned training stack, plus a Verification toolchain subsection for
the Make/Docker/kind/Kustomize/Actions harness.
Also corrects two things the doc had outgrown: the allocation pipeline is now
wired end to end and gated in CI, so only the provider deployment remains; and
the Steam section covered only the GodotSteam client transport, omitting the
server-side Web API ticket verifier in server/steam.
Adds the lesson this branch paid for repeatedly: the expensive failures
were not broken behaviour but assertions that could not distinguish the
two states they implicitly claimed to, each reporting its own ambiguity
as a confident verdict about the system under test. Waiting on a Fleet
field that does not exist, treating not-yet-started as exited, a p99
that conflated regression with scheduler noise, a validator reading a
response shape Agones never sends, and a build guard that verified stale
code. Five in one session, several costing multiple CI round trips.
Two habits go with it, both of which beat reading code every time they
were tried: make the script report what it saw before theorising about
why, and verify the diagnostics actually fire -- two dumps were added
here and neither ran, one suppressed by a reachability guard and one by
an ERR trap that cannot fire inside functions without errtrace.
Also fixes two stale claims and one gap. Audio is no longer "none at
all"; a procedural AudioManager covers UI, countdown, impact, goal and
engine cues, and only authored assets remain. Five docs/ contracts that
server/security asserts against the manifests were unlisted. And the Go
control plane -- a third of the codebase and the current focus -- had no
structural description at all, so it now gets one: package layout, which
binary is test-only, and the three things easiest to get wrong
(integration tests hidden behind a build tag, start-time config, the
versioned wire contract).
TODO.md's entry now points at its ordered backlog rather than describing
it as deferred non-multiplayer work.
Every factual claim in the new section was checked against the tree.
Allocated Compose failed on a docs-only commit, so nothing functional
had changed. Its own diagnostics showed why: the game server logged a
clean `server_started` -- the exact string the readiness loop waits for
-- and the script reported "game server exited before becoming ready".
The guard asked whether the service was absent from
`compose ps --status running`, which is also true of a container that
has been created but has not started yet. On a slow runner the first
poll can land in that window, and the script concluded the server was
dead when it was still coming up. Ask whether it actually exited
instead.
Also set errtrace. This failure produced no "failed at line N" report
despite the ERR trap added in 432e5a11, because a bare `trap ... ERR`
does not fire inside functions or subshells without it -- the
instrumentation had a blind spot exactly where a readiness loop lives.
The other --status running check, after an explicit `compose stop`, is
correct and unchanged: stop is synchronous, so absence there really does
mean stopped.
Verified by two consecutive local runs.
The backlog listed what is left but not what to do first, and priority
labels do not answer that: #33 is P2 yet belongs before the P0 cluster,
because standing the cluster up first means migrating a running one
afterwards.
Add an explicit ordering to TODO.md in three parts -- a critical path
where each item unblocks the next, a Steam track that runs in parallel
and should start early because its lead time is Valve's, and the set
that is unblocked today and waiting on nobody. The playtests, training
runs and asset work need no cluster and could start now, which was not
obvious from a flat list sorted by priority.
#31 is called out as the highest-leverage item: two answers unblock the
whole of Phase 8, and the work behind them is an agent's.
Two open issues were in no list at all -- #23's design question and
#32's backfill work -- so TODO.md now covers every open issue.
Also record the dependency direction on GitHub rather than only here:
#17, #32 and #33 carried no blocked-by statement, so the graph was
invisible from the issue tracker. And note in §7 task 8.12 why the
workload namespace enforces privileged and where the split is tracked.
The issue read as broadly human-gated. Most of it is not: GHCR accepts
the built-in GITHUB_TOKEN with packages: write for the repository's own
namespace, so publishing needs no account, stored secret or spend
approval, and signing and tagging policy can land as a reviewable
default rather than waiting on a decision.
Two things genuinely block. Every manifest references
ghcr.io/cosmic-clash/*, and no such organisation exists -- the API
returns 404 and it is not among this account's orgs -- so nothing can be
pushed there. And this repository is private, so GHCR packages inherit
that, while no manifest declares imagePullSecrets; public packages work
as written, private ones need pull secrets threaded through every
workload.
Same optimistic-to-pessimistic drift the §7 audit found in fifteen other
entries: work described as blocked on a person when the person only owes
a decision.
The client-bot smoke asserted remote_residual_position_p99 < 0.3. Commit
52ee1810 both passed and failed on that assertion, in runs three seconds
apart on identical code: two clients in one run reported 0.324 and 0.187
with matching snapshot counts, scores and slot checks.
A p99 over a few hundred samples is its worst handful, so on a shared CI
host it measures how often the process was descheduled as much as how
well the interpolator tracks. Locally the same test reports p95 0.025m
and p99 0.081-0.149m; CI's p99 runs 2-4x higher on a healthy build,
which is the entire margin the 0.3 bar had.
Assert two bars instead of one. The tight numeric bar moves to p95,
which is stable run to run, and p99 is bounded by the product's own
REMOTE_VISUAL_MAX_OFFSET/REMOTE_VISUAL_MAX_ROTATION_DEGREES: past those
the visual smoother stops absorbing a correction in a single step, so
exceeding them is a real defect rather than a slow runner. Referencing
the constants also means the test follows the product if that tolerance
is ever retuned.
This is a genuine trade, not a free win: a regression that pushed p99
from 0.2 to 0.35 while leaving p95 healthy now passes where it once
failed. That band is exactly where the noise lives -- 0.324 was observed
on a healthy build -- so the old bar could not tell that regression from
a busy runner either, and paid for the ambiguity with false reds.
Both percentiles are printed with their bars so a future failure shows
which one moved.
Checked first whether goal-driven kickoff teleports were polluting the
metric; they are not. Both the ship and ball paths already skip
accumulation across a reset_gen change.
With the Fleet readiness wait corrected, the gate reached the allocation
check for the first time and failed with "allocation did not return a
GameServer" -- while the cluster dump shows the allocation plainly
succeeded: one GameServer Allocated, Fleet reporting ALLOCATED 1.
GameServerAllocationStatus is flat: state, gameServerName, address,
ports, nodeName. It does not embed the allocated GameServer. The
validator read status.gameServer.metadata.name and
status.gameServer.status.{address,ports}, a shape Agones never sends,
and its unit tests asserted that same invented shape -- so validator and
tests agreed with each other while both disagreed with Agones. Nothing
caught it because the gate had never once allocated anything.
Read the real fields, keeping every existing check: non-empty name,
address neither blank nor unspecified, exactly one named "game" port in
range.
Also print the response body when validation fails. work_dir is removed
by the EXIT trap, so a shape mismatch was otherwise invisible from CI --
which is how this survived. If the shape is still not what I expect, the
next run says so instead of costing another round trip.
The gate asserted `--for=jsonpath='{.status.ready}'=2`. An Agones Fleet's
status carries replicas, readyReplicas, reservedReplicas and
allocatedReplicas -- there is no `ready` -- so the wait could never match
however healthy the Fleet was.
It failed in the most misleading way available: as "the Fleet never
became ready", which sent three separate investigations after the game
server. Two of those found genuine bugs, but the gate would have stayed
red with both fixed.
The evidence is in the previous CI run's own dump, which the new
per-container diagnostics produced: both GameServers Ready and stable for
5m6s, and the Fleet reporting DESIRED 2 / CURRENT 2 / READY 2, while
kubectl wait timed out beside it. That same dump also confirms the health
fix in 0de97381 worked -- those GameServers had been churning every ~20s
before it.
Assert the corrected jsonpath in test_fleet_manifests.py and reject the
old one, alongside the build-by-default behaviour, so neither silently
regresses.
Every allocated GameServer reached Ready and was recycled by Agones ~20s
later. Health pings are the game process's job by design -- the
supervisor has no health implementation at all -- so a server that stops
pinging is exactly what Agones is built to reclaim.
start_health() armed a Timer on a node that might not be inside the
SceneTree. A Timer only ticks inside the tree, so the node reported
itself configured, sent nothing, and said nothing about it. It now
returns a bool, refuses loudly when unconfigured, and defers to _ready()
when called before parenting, so the SDK arms its own timer and no
caller has to get the ordering right. server_boot.gd defers the add like
every sibling does (§9 gotcha 27) and logs when AGONES_SDK_HTTP_PORT is
missing, which previously read identically to a healthy start.
Also bounded the in-flight latch: it is set across an await, so a request
that never completes would silence health permanently. Defence in depth
rather than an observed fault.
Tests target the contract rather than the mechanism: a test that parents
the SDK correctly and asserts pings passes with the bug present, because
the defect was in the wiring. The unit tests assert start_health()
cannot claim success out of tree, and were confirmed to fail against the
previous code. The smoke gains a counting sidecar and asserts a
*repeating* ping -- it reports "health pings in 3.0s = 1, want at least
2" when the loop is broken, which is the production symptom exactly. It
is also now actually run: nothing referenced it before.
Two diagnostic fixes, both of which changed conclusions during this work:
The kind gate only built the game-server image when the tag was absent,
so a local rerun silently verified whatever was built last. That is why
local runs and CI disagreed about the same commit. It now builds by
default, with KIND_REUSE_GAME_SERVER_IMAGE=1 as the opt-in fast path.
The failure dump logged only not-ready pods, and used --all-containers
with a shared tail. A GameServer recycled after reaching Ready leaves no
unready pod behind, and the Agones sidecar out-logs the game server, so
the relevant output was never captured. It now dumps every pod, per
container, current and previous, plus the GameServer and Fleet resources
-- Agones' own state machine is what rejects these.
Several independent causes, all of which had to be right before the
Fleet could reach Ready.
The supervisor pointed --sdk-base-url at 127.0.0.1:9357, which is the
Agones sidecar's gRPC port; its HTTP surface is 9358, and that is what
AGONES_SDK_HTTP_PORT carries and what agones_sdk.gd reads. An HTTP
client against the gRPC port could never have worked, in kind or in
production.
The supervisor also treated the sidecar's first incomplete /gameserver
response as fatal. The sidecar accepts requests before the controller
populates status.address and status.ports, so this produced a restart
loop precisely during normal Agones startup. It now polls until the
endpoint is assigned or ReadyTimeout elapses.
server_boot.gd started ServerControl and the Agones SDK only under
--allocated-mode, but the kind smoke deliberately strips that flag, so
nothing served the readiness probe and the GameServer could never become
Ready. Lifecycle now keys on AGONES_SDK_HTTP_PORT, which Agones injects
into every managed container, while allocation and roster semantics stay
tied to --allocated-mode. The SDK node is added to the tree
non-deferred, since start_health() creates a Timer immediately.
Fleet: Agones assigns its own SDK service account and masks that token
from the game container while keeping it for the injected sidecar, so
the manifest must not pin serviceAccountName or
automountServiceAccountToken. Godot stores user:// under HOME, so HOME
points at the writable runtime volume to keep the root filesystem
read-only, and fsGroup makes that volume writable for the non-root user.
Namespace: Agones' Dynamic port policy injects a hostPort, which both
the baseline and restricted Pod Security Standards forbid, so the
workload namespace enforces privileged while continuing to audit and
warn against restricted.
NetworkPolicy: the injected sidecar reaches the Kubernetes API over
HTTPS, and NetworkPolicy applies to the whole Pod rather than to the
container whose token was masked.
The kind runner creates the namespace before Helm so Agones can install
its per-namespace SDK RBAC, scopes gameservers.namespaces to it, forces
the allocator and ping Services to ClusterIP because LoadBalancer
ingress never becomes ready in plain kind, and labels the node so the
production Fleet's on-demand/zone constraints are exercised rather than
edited out of the rendered manifest.
The dump added in 9ab1bec8 never ran. A `kubectl cluster-info`
reachability guard suppressed it, so its first exercise produced exactly
the silence it was written to prevent. Every command inside is already
`|| true`, so the guard bought nothing and cost the whole dump; removed.
That run did establish something the CI logs cannot: after clearing
local Docker pressure, the Agones install completes cleanly (controller
and allocator both reach "condition met") and the gate instead fails
later, waiting for the Fleet's game-server pods to become Ready. CI
never reaches that point because the Agones install times out first.
So there are likely two failures stacked, and fixing the CI timeout will
probably expose the Fleet one. Recorded in AGONES-CI-INVESTIGATION.md
along with the reasons the Fleet failure warrants suspicion -- fleet.yaml
changed its join-signing key mount from raw bytes to a JSON map this
branch -- and the reasons it may be unrelated.
This gate fails with nothing but Helm's "context deadline exceeded" and
three Deployments reporting Available: 0/1, then the EXIT trap deletes
the cluster -- so there is no way to learn why the pods never became
ready. Both CI runs and a local run are equally uninformative.
Dump node capacity and conditions, pods and recent events for
agones-system and cosmic-clash, and describe plus current/previous logs
for every not-ready pod, on any failure and before deletion. Events
matter as much as pod status here: FailedScheduling, ImagePullBackOff
and probe failures are all invisible in a status column.
KIND_KEEP_ON_FAILURE=1 retains the cluster for interactive inspection.
Same approach that just found the allocated-Compose cause, where a
silent assertion had hidden a real 422-instead-of-409 API bug across
several CI runs.
Both idempotency paths returned a bare fmt.Errorf, and writeDomainError
maps anything it does not recognise to its 422 "invalid_request"
default. So reusing a key with a different payload answered 422 where
openapi.json declares 409 and state-transitions.json requires
"reject_conflict_without_state_change".
That is the difference between "your request was malformed" and "that
key is taken". A client acting on 422 would rewrite a request that was
never wrong, and the 409 branch of every generated client was
unreachable.
Wrap domain.ErrConflict on both the create and mutate paths, and add an
integration test covering identical replay and conflicting reuse.
Pre-existing: both bare errors are unchanged from 089c127c, which is why
verify-allocated-compose failed in CI before this branch's work as well.
Found only after adding the diagnostics in 432e5a11 and fc2f5c86 --
until then the assertion aborted silently and three CI runs reported
nothing but "make: *** Error 1".
The ERR trap added in 432e5a11 located the CI failure at the
`[[ "$conflict_status" == 409 ]]` assertion, but the request discarded
its body and the assertion printed nothing, so three failing runs never
revealed what the API actually returned.
Print the status and body on mismatch.
This suite fails on GitHub Actions while passing locally, and it failed
the same way at 089c127c -- the branch head before any of this branch's
recent work -- so it is pre-existing rather than newly broken.
Diagnosing it is currently impossible from CI alone. The script is
mostly `curl -fsS` and bare [[ ]] assertions under `set -e`, all of
which abort with no output, so the run log contains nothing but
"make: *** Error 1". Both failing runs are equally silent.
Add an ERR trap that reports the script line and the failing command,
and dump `compose ps` plus the service logs on any non-zero exit rather
than only when COMPOSE_KEEP_ON_FAILURE is set. The next CI run should
therefore say what actually broke instead of needing another round trip
to find out.
No behaviour change on success; the target still passes locally.
compose.allocated-smoke.yml and verify_allocated_compose.sh are already
independent of compose.phase6-smoke.yml -- the script says so explicitly
and reuses none of its ports -- so the allocated-mode flow no longer
inherits that fixture's hardcoded port, first-come slots or
--max-matches=2.
Fifteenth stale backlog entry found this session, and the first in
TODO.md rather than multiplayer-next.md §7. It was also the only
remaining item marked agent-actionable.
Making cmd/allocator refuse to start without join-signing material was
right -- an allocator that binds allocations it can never publish rosters
for strands every match silently -- but I updated the Kubernetes
manifests and the kind fixture without updating the Compose one. The
allocator container exited at startup, so no allocation was ever bound
and verify-allocated-compose failed with "allocator did not bind a
provider allocation".
Mount the same join-signing-keys.json fixture the game server already
uses and name the key it was written with. Caught by running the target
locally rather than by CI after a push.
A backfilled player's join authorisation is issued after their server
started, but the supervisor fetches the roster once before launching the
game child and the game process has no reload path, so backfill cannot
work end to end regardless of how good the selection rule is.
Decided: the control plane marks the roster changed, the supervisor
re-fetches and rewrites the roster file, then signals the game process to
reload. Chosen because it reuses the authenticated channel and roster
endpoint that already exist -- no inbound path into the game pod, no new
trust boundary -- and keeps the roster an allowlist the server was told
to expect rather than admitting anyone holding a valid signature.
Signature verification is untouched and already binds match, server, slot
and generation.
Recorded in docs/MATCHMAKING.md, which the repo treats as the design
source of truth, so the decision is not re-litigated from a task row.
Remaining implementation is tracked in #32 and summarised in §7 8.19.
First slice of task 8.19. docs/MATCHMAKING.md specifies the choice
precisely -- "the oldest ordinary casual ticket that meets the same
build, region <=100 ms and current anchor-tolerance rules for the
vacated human slot; ties use ticket ID" -- and that rule is needed
whatever is decided about delivering a late authorisation to a running
server, so it is worth landing on its own.
Kept a pure function over an already-fetched candidate set: the choice
is then reproducible and testable without a database, and claiming the
ticket stays a durable transaction as it is for ordinary proposals. Ties
break on ticket ID rather than scan order, so two replicas evaluating
the same queue cannot offer one slot to different players.
Region matching is stricter than ordinary formation: the server already
exists in one region, so a candidate must have RTT evidence for that
region specifically, not merely share some region with the others.
Eligibility is re-checked here as well as at the durable boundary, so an
ineligible mid-play or human-occupied slot never reaches selection at
all. Ranked is refused outright.
Corrected a comment I had written claiming the backfill window is
shorter than an ordinary proposal's; both are 10 seconds. What makes a
backfill offer separate is its payload and the absent decline penalty,
not its timing.
This does not yet make backfill work end to end -- see the roster
delivery question raised alongside this commit.
Two things that made the integration gate untrustworthy.
The retry budget was too small for expected contention.
TestPostgreSQLConcurrentIdenticalResultSubmission fires five identical
concurrent submissions and requires all five to succeed; it failed 4 runs
in 20. The error was retryable and retries did fire -- three attempts
simply was not enough. Contention here is normal rather than
exceptional: several game servers can submit results, and several
matchers can claim candidates, against the same rows at once. Raised to
five attempts, which is 0 failures in 40 runs.
Also jittered the backoff, but measured rather than assumed: my first
theory was a thundering herd, since the delay was exactly
RetryBackoff*(attempt+1) and every loser of a race woke at the same
instant. Isolating the two changes showed jitter alone moved 4/20 to
3/20, while the budget alone reached 0/20. The budget was the real
constraint. Jitter is kept because it costs nothing and its benefit
grows with the number of contending writers -- production is not capped
at five -- but the comment now says plainly that it is the smaller half,
so nobody inherits my wrong explanation.
Second, the integration scripts leaked one throwaway database volume per
run. --rm does reclaim anonymous volumes on a normal exit, but these
scripts force-remove the container from a trap, and `docker rm -f`
without -v keeps the volume. Sixty-four accumulated during this branch
until PostgreSQL stopped starting, surfacing only as the scripts' own
readiness timeout rather than as a disk error -- which is what the
"Docker storage exhausted locally" notes were really describing.
Measured at one volume per run before, zero after, across all five
scripts.
The audit established which rows understated what was built by locating
implementations and their call sites. That proves code exists, not that
it works, so each corrected claim is now tied to an executable test.
Seven of the nine were already covered and just needed naming: the
allocated ServerConfig fields, signed-authorisation admission, endpoint
wiring, casual lineup being reached through formation, three of the four
penalty kinds, signed roster metadata, the season countdown, and the
matcher deployments.
Two had no proof at all:
- INITIAL_CONNECT_NO_SHOW was the one penalty kind with no integration
coverage, so "all four penalty kinds are written durably" rested
entirely on reading the code.
- Cross-replica revocation is a behavioural property. An in-memory cache
in front of the session read would break it while leaving every call
site looking correct, so no amount of reading establishes it.
Writing the first one found my own error rather than a defect: casual
deliberately waits past InitialConnectWindow to CasualBotStartAfter
before deciding a no-show, giving a slow-loading player longer than the
ranked deadline. Reconciling at the earlier window only yields WAIT.
Both new tests were mutation-checked -- removing the penalty insert and
removing the revoked_at check each make them fail -- so they assert
something real rather than passing incidentally.
Three tasks in a row this branch turned out to be partly built already:
8.20's allocation wiring was complete end to end, 8.22's client UI was
built, and 7.4/8.7 listed durable ban storage and the Steam adapter as
outstanding after both had landed. That is a systematic problem, not
three coincidences, so this checks all 56 rows rather than fixing them
one at a time as they are picked up.
Nine more rows understated what exists:
- 8.6 every allocated-mode ServerConfig field is present, signed
authorisation admission is in MatchNet, endpoint wiring is in
AssignmentState
- 8.8 distributed revocation needs no cross-replica protocol: sessions
are durable and read on every authenticated request
- 8.19 casual lineup formation is built and wired, and all four penalty
kinds are written durably; only the backfill proposal path is
genuinely missing
- 8.30 signed roster metadata landed with 8.31
- 8.42 the season countdown is implemented
- 8.16/8.43 the matcher is deployed; what remains is soak, not integration
- 8.13/8.52 cross-referenced to #31 rather than described loosely
The drift runs one way -- rows keep listing work that has since landed --
which inflates the apparent backlog and invites rebuilding what exists.
8.19 is the clearest case: it reads as four missing pieces and is one.
The §7 preamble claimed every row lists only what is still open. It
doesn't, so it now says so and points at the audit note, with the
standing instruction to verify a row against the code before planning
against it.
Task 8.22. Tier bands lived in domain.DefaultTierPolicy(), compiled into
every API binary, so retuning one meant building and rolling a new image
-- least attractive exactly when it is most needed, as the rating
distribution settles after launch.
Bands now live in a tier_bands table, seeded by the migration with the
exact policy the binaries hardcode, so this changes durable state without
changing behaviour. Retuning is a rolling restart rather than a rebuild.
Three properties the loader deliberately holds:
- A malformed durable policy stops startup. Falling back on error would
silently mis-tier every player, which is worse than not starting.
- An empty table is supported and falls back to the compiled default, so
an operator can truncate back to known-good without a deploy, and a
fresh database works before the seed is reviewed.
- PROVISIONAL is rejected as a band. It is derived from ranked game
count, not rating, so a band claiming it would be unreachable at best
and would shadow a real tier at worst.
Bands stay backend-owned; clients still receive only the resulting label,
per docs/MATCHMAKING.md. UNIQUE(min_rating) rejects two bands sharing a
threshold, catching an ambiguous policy before NewTierPolicy does.
testkit-api loads it too, so the control-plane integration scripts
exercise the durable path rather than the compiled default.
Integration tests cover the seeded policy matching the compiled one,
retuning taking effect from the database alone, truncation falling back,
and each invalid-policy shape being rejected. Verified they fail against
a loader that ignores durable bands.
The other two parts of 8.22 needed no work: the client UI already renders
tier, provisional status, ranked games and the season countdown, and
reconnect transport is 8.42's, dependent on live backend events.
Task 8.20. `arena_registry.gd` is the documented single source of truth
for arenas, but `domain/ranked.go` keeps a hand-maintained mirror of its
floor-goal entries and nothing checked the two against each other --
ranked_test.go asserts the same three paths the production code
hardcodes, so both could drift together silently.
Drift is not hypothetical in either direction. The registry's own comment
anticipates flipping an elevated variant to random:true once a checkpoint
trained on that geometry is promoted, which ranked would then keep
excluding indefinitely. A rename or removal is worse: the allocator would
hand out a scene path that no longer exists, and the ranked server fails
to load its arena at match start -- after allocation, so it burns a real
match and a real server.
Keeping the two copies is deliberate rather than a wart: ranked arena
selection is server-authoritative and happens before any Godot process
exists. So this guards the relationship instead of removing it, the same
way the golden join-authorisation token guards the signing format. It
parses the registry and fails if the sets disagree either way, if
rotation order diverges from declaration order, or if a ranked path has
no scene behind it. The parser asserts it found both eligible and
ineligible entries, so a format change cannot make everything pass
vacuously. Verified against four drift scenarios.
The allocation-wiring half of 8.20 turned out to be already complete end
to end, with coverage at each hop; recorded in the task row rather than
rebuilt.
Add a Server Unit Tests workflow, because none of this would otherwise
run: the only Go tests CI executed were multiplayer-load's two load
tests, so ~24k lines of control plane gated nothing. Docker-free so it
can gate every push, and it vets the integration-tagged files too, since
those are excluded from the default build and could otherwise rot
uncompiled.
CLAUDE.md's CI section claimed two workflows and no unit-test job; there
were seven and now eight.
The Steam adapter took --steam-publisher-key/--steam-app-id and the
matching env vars, but no manifest supplied them, so a deployed control
plane would have kept sign-in returning 503 even once the App ID from
#15 arrived -- that issue would have unblocked nothing on landing.
Mount them from a new cosmic-clash-steam Secret, into the control-plane
Deployment alone: the publisher key is issued to us, never to a client,
and no other workload (least of all a game server) has any use for it. A
manifest test asserts both the wiring and that the Secret appears in no
other manifest; verified it fails in both directions.
Both keys are optional, so the Deployment still rolls out before the App
ID exists and sign-in simply stays 503.
Also correct task rows this branch made stale: 7.4 (durable ban storage
landed), 8.7 (adapter, bans and secret store landed), 8.39 (cross-replica
fan-out landed), and 8.5's migration range, which stopped at 0013.
Move the branch review into docs/ with a header marking it a point-in-time
artefact -- all thirteen findings are addressed, and its present tense
would otherwise read as current behaviour.
Record gotcha 52: the integration scripts use `docker run --rm`, which
reclaims the container but not its anonymous volume. Sixty-four of them,
~4 GB, accumulated during this session until PostgreSQL stopped starting
-- surfacing only as the script's own readiness timeout, not as a disk
error. That is the real cause behind the "Docker storage exhausted
locally" notes those rows carried.
Issue #31 covers the gap that no workflow builds or pushes the images
deploy/k8s/base references, and that every digest there is still an
all-zero placeholder which the supply-chain gate accepts because it runs
without --require-concrete. It blocks #17.
docs/MATCHMAKING.md is the stated source of truth for this design, so it
changes first: the probe challenge endpoint and why probing gates
matching rather than merely improving it, and the key-ID rotation
procedure that makes overlapping-key rotation concrete.
multiplayer-next.md §0's root blocker is closed rather than deleted --
what it was, why it blocked everything, and how it was resolved, since
the reasoning is what a future reader needs. Tasks 7.6, 8.15 and 8.31
updated to what actually remains, which in every case is now external
rather than unbuilt.
TODO.md #14 asked for a join-signing design decision; that decision is
recorded with its rationale. CLAUDE.md no longer says a real deployment
cannot complete a match end to end.
newAPIService never supplied SteamLogin, so POST /v1/session/steam
always returned 503 auth_unavailable in production. The only
implementation was cmd/testkit-api's fake, which derives an identity
from the ticket string itself and accepts anything -- so the passing
integration path was neither deployable nor secure. On the client side
the game started with an empty token and a loopback base URL, and no
production code called configure() or login_steam(); the menu entered
matchmaking directly, so every request failed ERR_UNAUTHORIZED before
reaching the network.
Add a real ISteamUserAuth/AuthenticateUserTicket adapter behind an
interface, so the production login path is testable with only the Valve
call stubbed. It rejects family-shared copies (the account playing does
not own the app) and, by default, VAC- or publisher-banned accounts, and
refuses malformed tickets locally rather than forwarding them.
Crucially it separates our faults from the player's: a Valve outage or a
revoked publisher key returns 503, not 401. Answering 401 would tell a
legitimate player their login failed and send them to fix an account
that is fine while the real fault went unnoticed. A banned identity now
returns 403 rather than a misleading 503.
Sign-in is configuration-gated on the publisher key and App ID: without
them the endpoint keeps returning 503, since silently accepting an
unverified ticket would be worse than refusing to authenticate. A
returning player keeps the player ID they already had, so ratings,
penalties and bans follow the account rather than the session.
Client side: acquire a web-API ticket through GodotSteam's async
signal -- requesting one returns a handle, not a ticket -- using the
existing dynamic-call pattern so stock Godot still parses the project.
The endpoint is configurable for release builds, and matchmaking
completes sign-in before it will queue.
Verified against real PostgreSQL; 232 Godot tests pass.
The Kubernetes base deployed a control-plane image the Dockerfile never
built -- cmd/control-plane was absent from the Go build stage and no
target existed -- while the Dockerfile built a matcher image no manifest
ever deployed. Applying the checked-in base therefore could not produce
the advertised topology: one required workload had no repository-defined
artifact, and nothing consumed queued tickets. Tickets could be created
but never became proposals.
Add the production control-plane build and image target, explicitly not
the testkit-api target, which injects a fake login accepting any ticket.
Add casual and ranked matcher Deployments as separate workloads: they
have different match sizes, and separating them keeps a ranked backlog
from delaying casual formation. One replica each -- CreateProposal's
SKIP LOCKED fences make more replicas safe, but they would halve the
candidate pool each worker sees per poll and worsen formation for no
throughput gain at this scale. Their PDB uses maxUnavailable, since
minAvailable against a single replica blocks node drains outright.
Also fix both blocked traffic directions. No ingress policy admitted
UDP/7777 to game-server pods, so an allocated server was unreachable
from the internet under the namespace-wide default deny. And
control-plane ingress admitted only edge-gateway pods, so roster fetch,
registration, connection receipts, shutdown and result submission from
game servers were dropped even inside the cluster, despite their egress
being permitted. The default deny stays.
Manifest tests now assert every required role is deployed, both
playlists are scheduled, every referenced image maps to a real
Dockerfile target, and both traffic directions are permitted. Each was
verified to fail against the defect it covers. The control-plane image
was built and run to confirm the target works.
domain.validCandidate hard-requires a non-empty PredictedRTT map, but
CreateQueueTicket persisted an empty one and the only endpoint that
could fill it returned 503 in every real binary, because Service.Probe
was assigned nowhere outside api tests. No client-created ticket could
ever be selected by the matcher. The Godot client had no probe method at
all, so even a wired backend was unreachable from the game.
Four distinct defects had to be fixed for this path to work:
Nothing issued the nonce ProbeProvider was meant to compare against, so
the contract could not be satisfied even in principle. Add
POST /v1/probes/{region}/challenge, backed by a durable single-use
challenge -- durable because any replica may serve the answer for a
challenge another replica issued. RTT is the interval between issuing
and receiving, so no client-reported latency reaches placement.
CreateQueueTicket marshalled a nil map to JSON `null`, a JSONB scalar
rather than an object, and jsonb_set rejects that with "cannot set path
in scalar". RecordProbe would have failed at runtime even once wired.
Persist an object, and normalise non-object values in the update for
rows already written.
A nil ProbeRecorder made the handler report success while persisting
nothing, which silently leaves the ticket unmatchable. That is a
misconfiguration, not a successful probe; it now returns 503.
A successful probe updated PostgreSQL only. The candidate inserted at
enqueue time carries an empty RTT map, and the Redis keyspace has its
TTL continually refreshed, so the stale entry need never repair itself.
Refresh that player's projection after the probe commits.
Client side: add the challenge/answer round trip and have the
matchmaking screen collect evidence before creating a ticket, since
queueing first produces a search that can never match. Probing every
region fully is not required -- placement uses whichever regions
answered -- but queueing with none is refused rather than silently
stalling.
New integration test drives the real enqueue and probe paths and then
asks the actual matcher predicate, rather than hand-building a candidate
the way the unit tests do -- which is exactly why they missed this.
Also make the integration schema reset drop the whole public schema: the
enumerated table list silently broke with each new migration.
The root blocker (issue #14). The worker bound the provider allocation
and stopped. Service.PublishRoster and store.SaveVerifiedAssignmentRoster
both existed, fully tested, with zero non-test callers, and the
production allocator configured neither a roster store nor a signing
key. Nothing ever wrote the assignments table.
The allocated supervisor fetches a non-empty roster before it launches
the game child, so every real allocation failed at that fetch: no match
could reach ASSIGNMENT_READY or accept a player. Existing tests seeded
assignments directly, which is exactly why the missing hand-off went
unnoticed.
The worker now builds one join authorisation per durable participant,
signs each with the active key, and publishes them. Participants are
read through the same query SaveVerifiedAssignmentRoster re-validates
against, so the allocator cannot construct a roster the persistence
boundary would reject. The manifest commits to a digest over the whole
roster, so a server cannot be handed a truncated roster whose surviving
entries are each individually valid.
Persist the provider endpoint on the allocation: it arrived on the
provider response and was never stored, so a worker crashing between
allocating and publishing had no endpoint to recover and would have
stranded the match permanently. Republishing is idempotent, so that
crash now simply retries.
cmd/allocator refuses to start without key material rather than running
an allocator that binds allocations and silently strands every match.
The k8s allocator Deployment mounts the same key set the Fleet does, and
both now take the JSON key map so a rotation can publish several.
New integration test drives the real worker through to the supervisor's
own roster read path without seeding the assignments table. Verified it
fails with "assignments = 0, want 2" when the publish step is removed.
Prerequisite for wiring the allocator to publish rosters. The signing
key is a shared HMAC secret mounted into both the allocator and the
allocated game server; without a key ID, rotating it would invalidate
every authorisation already issued for an in-flight match, because a
server holding only the new key cannot verify a token signed with the
old one.
Add KeyID to JoinAuthorisation and append it to the canonical claim
bytes, so it is covered by the signature and cannot be repointed at a
different key than the one that actually signed. Allocated servers now
hold a set of currently-valid keys and select by ID: a rotation
publishes the new key alongside the old, and the old is dropped once no
live match can still reference it.
The key file becomes a JSON map of key ID to base64 key. A file of raw
key bytes is still accepted as a single key under the empty ID, which is
what an unrotated deployment and the kind fixture use.
Game/scripts/match_net.gd builds the canonical bytes independently, so
it changes in lockstep; the cross-language golden token in
test_match_net.gd is regenerated from the Go implementation and now
carries a key ID. Added tests cover accepting either key mid-rotation,
rejecting a retired key ID, and rejecting a token whose key ID was
swapped to name a key the server does hold.
Go suite and 223 Godot tests pass.
Each ten-second queue heartbeat mints a fresh idempotency key and
permanently inserts a row. Published outbox rows and expired/revoked
sessions were never purged either -- the maintenance role performed
lifecycle reconciliation only. At 10,000 queued players heartbeats alone
add roughly 60,000 durable rows per minute, so table and index growth,
vacuum pressure, backup size and recovery time were all unbounded on a
service intended to scale horizontally.
Add retention windows chosen to exceed every retry and recovery horizon
that could still consult the row -- deleting an idempotency key early
would turn a client replay into a second real mutation, so this is a
correctness bound, not just a housekeeping one. Dead-lettered outbox
rows are kept longest, being the record of events never delivered.
Deletes run in bounded SKIP LOCKED batches so a purge never blocks live
traffic, never holds a long transaction, and concurrent maintenance
replicas do not contend. Indexes back each predicate so a pass cannot
degrade into a sequential scan of the table it is bounding. The
maintenance role reports rows purged, the backlog past its window
(deletion lag), and any dead-lettered events.
Also make the migration-rollback test derive its step counts instead of
hardcoding them: adding a migration silently shifted the fixed counts so
the failure surfaced as an unrelated "0006 rollback did not drop
matches.allocation_id".
The Deployment runs two replicas, but WebSocket subscribers live only in
each process's in-memory hub. Every replica races to read the same
global unpublished outbox rows, and publishing succeeded even when the
winning replica held no matching local subscriber -- that replica then
set the single global published_at. A client connected to the other
replica never received the event, and delivery degraded further with
each replica added. REST recovery eventually converged, but short-lived
proposal transitions could be observed late or not at all.
Publish committed events through PostgreSQL LISTEN/NOTIFY so the replica
that owns the subscriber's connection delivers it, regardless of which
replica drained the row. The listener holds its own pgx connection --
LISTEN is session state, so a pooled database/sql connection cannot
carry it -- and reconnects with backoff, since losing it would silently
downgrade that replica's subscribers to REST-only recovery.
The fan-out is optional: without EventFanout configured, behaviour is
unchanged local-hub publication, which stays correct for a single
replica and for tests. Only outbox-sourced events are routed through it;
the in-request-path publishes remain local, as those are a latency
optimisation for the caller's own connection.
Fan-out needs a wire shape of its own because ControlPlaneEvent hides
PlayerID from clients, and the recipient is exactly what a peer replica
needs to route on.
banned_until and ban_reason have been in the schema since 0001, but no
production query ever read them -- grepping the tree found no reference
outside the migration itself. The only ban check was an in-memory map on
domain.TicketVerifier used by domain tests. Once real Steam login is
wired, a banned identity would keep full access through every existing
session until expiry and could obtain new ones.
Make the ban part of the durable authentication transaction rather than
a policy each login adapter must remember to re-implement:
- Session issuance inserts only when the identity exists and has no
active ban, so a banned player cannot mint a session.
- Authentication joins the identity and rejects an active ban on every
request, so a ban takes effect immediately on every replica rather
than at session expiry.
- ApplyIdentityBan sets the ban and revokes that identity's sessions in
one serializable transaction, closing the window where the ban is
durable but another replica still accepts an issued session.
Bans are time-bounded and clearing one does not resurrect sessions the
ban revoked.
Tests cover enforcement across two independently constructed stores
standing in for two replicas, expiry/unban semantics, and -- separately,
because revocation would otherwise mask it -- that a ban applied without
revoking anything still blocks the next request.
Both playlists shared one hash and sorted set, causing two independent
failures.
Starvation: Snapshot performed an unbounded ZRANGEBYSCORE and HMGET,
decoded the whole queue, and the matcher then truncated to its candidate
limit *before* filtering by playlist. A large casual prefix could
therefore leave the ranked worker with zero candidates indefinitely even
while ranked tickets were queued further down the set.
Mutual erasure: each matcher captured only its own playlist as the
durable source, but Rebuild replaced the shared keys, so a casual repair
wiped ranked projections and vice versa.
Namespace the keys per playlist, push the limit into Redis (LIMIT 0 N)
so reads no longer scale with total queue depth, and scope Rebuild to
one namespace. Rebuild now rejects a candidate whose playlist does not
match the namespace, which would reintroduce the starvation. Upsert
derives the namespace from the candidate; Remove takes the playlist,
since a ticket ID alone no longer identifies its namespace.
Add tests for a 300-deep casual backlog not starving ranked, for neither
playlist's rebuild erasing the other, and for the limit being applied
without losing enqueue ordering.
The candidate projection selected only from queue_tickets and its scan
never set Candidate.Rating, so every PostgreSQL-sourced ranked candidate
arrived with Go's zero value. Rating tolerance, selection scoring and
team partitioning all read that field, so ranked matchmaking treated a
900-rated player as identical to a 2100-rated one. Unit tests missed it
because they construct candidates with ratings already populated.
Join the ratings table, defaulting to domain.GlickoInitialRating for a
player with no ratings row yet -- a genuinely new profile, matching the
column default.
Fix the same defect on the Redis path too, which is reached differently:
the projection is seeded from the candidate CreateQueueTicket builds,
not from the candidate query, and that candidate also left Rating unset.
Resolve the rating inside the enqueue transaction so both projections
agree on one authoritative value. The rating is never client-supplied.
Add a store-backed test with deliberately distant ratings (900 vs 2100)
plus an unrated player, asserting both projections and that the spread
survives. Verified it fails without the fix.
ApplyInitialConnectPlan wrote a payload of {match_id,state,action},
omitting event, revision, resource_id, occurred_at and player_ids --
every field deliverStateOutboxEvent requires. Delivery rejected the row,
dispatch returned on the first error so it was never acknowledged, and
because reads are ordered oldest-first it was retried ahead of every
later state_changed event on every 100ms poll. One initial-connect
transition therefore blocked lifecycle delivery for all matches, not
just its own.
Two independent fixes, since either alone leaves the system fragile:
Build envelopes through one validating helper (MarshalOutboxEnvelope)
and convert all five writers to it. A writer that omits a required
field now fails its own transaction instead of committing a row that
can only ever poison the queue. The helper takes revision as int64 so
the -1 "nothing matched" sentinel some CTEs return surfaces as an error
rather than wrapping to a huge uint64.
Make dispatch resilient regardless: a delivery failure is now counted
against that row and the batch continues, with the row dead-lettered
after MaxOutboxDeliveryAttempts so a poison event degrades to one lost
notification instead of a stalled queue. Ordering within an aggregate
is still honoured -- later events of a failed match are deferred, so no
client observes that match's newer state before its older state. An ack
failure still stops the batch, being a database rather than a payload
problem.
Initial-connect events now address every participant, not just the
connected ones: a no-show needs to learn their ticket was failed and a
penalty applied.
test_contracts.py required operation ID `recordPlayerConnected`, but
openapi.json names that endpoint `claimPlayerConnection` — the accurate
name, since POST /servers/{id}/connect claims a connection lease and
returns a generation. Align the test on the document and assert the set
difference, so a future mismatch names the missing operation instead of
reporting "False is not true".
test_observability_manifests.py copied only two of the four files the
checker reads, so it died on a missing kustomization.yaml before ever
reaching the mutated namespace. Copy the full fixture, split the
namespace and scrape-path mutations into separate cases so either
defect produces its own diagnostic, and add an unmutated-copy case so a
broken fixture can't make the mutation cases pass vacuously.
Neither suite was invoked by any Make target or workflow, which is why
both could sit red. Add them, plus test_threat_model.py, to
verify_multiplayer_local.sh.
File the outstanding work that needs a person — hardware, an external
account, a playtest, a design decision — as GitHub issues (#14-#29),
labelled needs:human plus a P0-blocker..P3-low priority, an area and a
phase. Link each one inline from the doc bullet it came from.
The split: issues carry status for human-gated work, these documents
keep the detail, and the numbered agent-actionable tasks in
multiplayer-next.md §7 deliberately get no issues. Mark the 8.48 Compose
fixture bullet as agent-actionable so its lack of an issue does not read
as an omission.
multiplayer-next.md was a 1662-line mix of standing architecture spec
and task-completion tracking, most of which was dense per-task DONE
evidence for finished Phases 0-6. Split it:
- MULTIPLAYER_SPEC.md (new): the locked architecture decisions, wire
format, server-side input handling, prediction/reconciliation,
latency/frame-rate budget, and match lifecycle state machine -
standing design reference, not task-tracked.
- multiplayer-next.md (trimmed 1662 -> ~370 lines): only outstanding
work remains - §0 status, §7 Phase 7/8 task tables condensed to
"what's left" per task, §8-11 reference material (refactoring notes,
gotchas, testing, flagged items). Phases 0-6 collapsed to a pointer
at git history instead of ~500 lines of DONE evidence.
Also:
- Repointed every `multiplayer-next.md §N` code comment (N 1-6) across
Game/scripts, Game/tools and Game/tests to MULTIPLAYER_SPEC.md, since
those sections moved. Task-number references (`task N.N`, §7-11)
correctly still point at multiplayer-next.md.
- Updated CLAUDE.md's doc index and docs/TECH_STACK.md's spec-section
citations to match.
- TODO.md: added a "what's left to actually finish multiplayer
(human-actionable)" checklist pulled from multiplayer-next.md §0 and
docs/MATCHMAKING.md - things that need a person (hardware, a design
decision, a Steam App ID, hands on a controller), not more agent code.
Add a Go-vs-C#/Rust/C++ rationale for the matchmaking control plane to
TECH_STACK.md, and point at it from MATCHMAKING.md and README.md.
Also correct CLAUDE.md and README.md, which still described the backend
as unstarted/not built even though server/ has ~13k lines of Go across
matcher, allocator, api, store, security, supervisor and agones.
Found while investigating why the connect-wiring fix (§8.41,
ControlPlaneClient._connect_when_assigned) would still not work
end to end in a real deployment: nothing in production ever
publishes a player's signed match assignment.
store.SaveAssignment/SaveAssignments/SaveVerifiedAssignmentRoster --
the only functions that ever write the assignments table -- are
called only from tests, never from allocator/worker.go, cmd/allocator,
or anywhere else in the real service. allocator.Service.PublishRoster
(wired to store.PostgresRosterStore) is likewise never called from
production code.
allocation_match_sql.go's AdvanceServerRegistration SQL requires an
assignments row for every match participant before allowing the
ASSIGNMENT_READY transition. With nothing ever creating those rows, a
real match cannot advance past PROCESS_READY -- no player can ever
receive a real assignment or connect, regardless of how correct the
client-side connect-wiring fix from earlier this session is.
TestRealSupervisorRegistersAllocatedServerThroughControlPlane -- the
test that was supposed to prove this end to end -- manually seeds
store.SaveAssignment in its own setup rather than exercising the real
production write path, which is why this was never caught.
Per the user's explicit direction, this is flagged rather than fixed:
closing it needs new security-relevant design (a join-signing key
shared between the allocator, which would sign, and the game server,
which fleet.yaml already mounts a verification key for via
--join-authorisations-key-file but which no control-plane binary has
a matching signing flag for; roster-digest computation; per-player
domain.JoinAuthorisation construction from match_participants/identities
via the already-built domain.SignJoinAuthorisationHMAC), not a simple
wiring fix -- a wrong design choice here is a join-authorization
forgery risk, not just a UX gap, so it isn't something to build
unprompted the way the smaller fixes earlier this session were.
Recorded in three places for visibility: a new root-blocker callout
in §0 (the outstanding-work index), the top Status line, and expanded
detail in §8.31's own row, which previously undersold this as merely
'production signer... remain'.
No code changes. Verified the doc edit didn't touch anything else:
go build/vet/test -race clean, full Godot suite 220/220 clean
(unchanged, as expected).
Closes §8.43's 'failed-reconnect UX' gap, found immediately after
wiring connect_to_assignment() itself: even with that fix in place, a
connection failure had nowhere to go. connect_to_assignment()'s
synchronous failures (assignment missing/expired, invalid endpoint,
NetworkManager.join() erroring immediately) only ever emitted
assignment_connection_failed -- a signal nothing in the client
listened to. state.phase would stay stuck at ASSIGNED, the UI would
keep showing "Your match server is ready" forever, with no way back
to a fresh search.
Worse, the likelier real-world failure mode had no handler at all:
NetworkManager.join() returns OK immediately once the attempt starts,
but the actual ENet handshake can still fail asynchronously afterward
(unreachable server, refused connection, ENet's own ~5s connect
timeout). This is exactly the gap main_menu.gd's own
_on_connection_failed exists to cover for the direct-join flow (see
its header comment) -- nothing covered the equivalent for a
matchmaking-driven connect.
ControlPlaneClient now connects both assignment_connection_failed and
NetworkManager.connection_failed (guarded to state.phase == CONNECTING,
so it never misattributes an unrelated direct-join failure to a
matchmaking search) to state.fail(...), so either failure mode now
surfaces as a failed search the player can actually retry from.
Verified: three new test_control_plane_client.gd tests cover the
synchronous failure path, the async NetworkManager.connection_failed
path (via a real end-to-end ASSIGNED -> CONNECTING flow), and that an
unrelated connection_failed outside CONNECTING is correctly ignored.
220/220 tests pass, stable across 3 repeated runs, no crash, no
engine-level error; full make verify-multiplayer-local and the
complete make verify-enet-integration suite (all five cases) both
clean; zero new crash reports throughout.
Also fixes a markdown table-integrity mistake introduced while
documenting this in the same edit pass: an earlier Edit call
accidentally duplicated a sentence and dropped the row's closing
'remains' clause in §8.43 -- caught and corrected before commit via
the usual pipe-count check.
Closes §8.43's 'version-mismatch-specific client messaging' gap. Per
the user's explicit go-ahead to design new server behavior for this
(rather than only wiring up something that already existed, the
pattern every other fix this session followed): before this, there
was no server-side protocol rejection at all. queue_create accepted
any protocol_version >= 1 unconditionally, so an outdated client could
only ever discover a mismatch by waiting in the queue forever
unmatched -- the matcher's own compatibility check requires every
formed player to share an identical protocol_version -- with no error
and no explanation given to the player.
Server: Service.MinProtocolVersion (opt-in, zero by default so every
existing caller keeps accepting protocol_version 1 unconditionally)
rejects a below-floor queue_create with 426 Upgrade Required /
client_outdated before the request ever reaches the candidate
provider. Wired via cmd/control-plane's new --min-protocol-version
flag (validated non-negative at startup).
Client: ControlPlaneClient recognises HTTPClient.RESPONSE_UPGRADE_REQUIRED
on queue_create specifically and sets a distinct 'Your client is out
of date -- please update to continue searching' message instead of
the server's raw generic error string, and clears _last_queue_create
so can_retry_queue_create() never offers 'Retry Search' for a failure
that retrying with the same build can never fix.
Verified: go build/vet/test -race clean across every server package.
TestQueueCreateEnforcesMinProtocolVersion covers below-floor rejection
(candidate provider never reached), exactly-at-floor acceptance, and
the error body naming client_outdated; TestQueueCreateMinProtocolVersionZeroIsDisabled
proves the opt-in default doesn't change behavior for every existing
caller. Godot: test_outdated_client_receives_a_distinct_message_and_no_retry_offer
proves the distinct message and suppressed retry offer. Full Godot
suite (217/217, 0 failed, no crash), full make verify-multiplayer-local
gate, zero new crash reports.
Josh Creek