docs: record the assertion-first debugging habit, and refresh CLAUDE.md

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.
This commit is contained in:
Josh Creek
2026-09-05 23:10:06 +01:00
parent 4ea72be581
commit 52cc478b38
+90 -2
View File
@@ -6,7 +6,7 @@ Important rule: never create co-authored commits. Never mention Claude in commit
## Project overview
Cosmic Clash is an open-source, physics-based "vehicle soccer" game (a spiritual successor to Rocket League) built in Godot 4.7, using space ships instead of cars. The game and the dedicated server are GDScript/Godot only — the "C# backend" an early README described was never started, and the dedicated server is an export of this same Godot project. There is one component outside the Godot project: a **Go matchmaking control plane** in `server/` for casual/ranked queues, ranked ratings and Agones-based server allocation. It is a 1.0 launch blocker — see `docs/MATCHMAKING.md` for the design, `multiplayer-next.md` §0 and §7 for what remains (the allocation-to-connect pipeline is now wired end to end; what is left is external — a Steamworks App ID, custom GodotSteam builds, and a live Agones cluster), and `docs/TECH_STACK.md` for why the control plane is Go rather than C#, Rust or C++. README.md's "MVP is local-only against bots" section is historical: server-authoritative online multiplayer, a headless dedicated server, Docker/CI verification, and an optional Steam transport are all implemented (Phases 16). See `multiplayer-next.md` for what actually remains.
Cosmic Clash is an open-source, physics-based "vehicle soccer" game (a spiritual successor to Rocket League) built in Godot 4.7, using space ships instead of cars. The game and the dedicated server are GDScript/Godot only — the "C# backend" an early README described was never started, and the dedicated server is an export of this same Godot project. There is one component outside the Godot project: a **Go matchmaking control plane** in `server/` for casual/ranked queues, ranked ratings and Agones-based server allocation. It is a 1.0 launch blocker — see `docs/MATCHMAKING.md` for the design, `multiplayer-next.md` §0 and §7 for what remains (the allocation-to-connect pipeline is now wired end to end; what is left is external — a Steamworks App ID, custom GodotSteam builds, a registry to publish images to, and a live Agones cluster; `TODO.md` orders them), and `docs/TECH_STACK.md` for why the control plane is Go rather than C#, Rust or C++. README.md's "MVP is local-only against bots" section is historical: server-authoritative online multiplayer, a headless dedicated server, Docker/CI verification, and an optional Steam transport are all implemented (Phases 16). See `multiplayer-next.md` for what actually remains.
Because the gameplay concept (vehicle soccer) can't be copyrighted but specific expression can, all code/art/assets must be original — this is why the project uses Godot instead of Unreal/Unity and ships instead of cars. Keep this in mind when writing code or pulling in assets: don't port or closely mirror Rocket League's actual implementation.
@@ -22,7 +22,9 @@ The prose docs carry far more design rationale than the code comments, and sever
- `FLIGHT_MANUAL.md` — the player-facing flight model.
- `docs/MATCHMAKING.md` — casual/ranked queue design, and the locked constraints (Go/PostgreSQL/Redis/Agones) the `server/` module implements. Partially implemented; a 1.0 launch blocker, and the reason a backend service outside the Godot project exists at all.
- `docs/TECH_STACK.md` — what the project is built with and why, including the Go-vs-C#/Rust/C++ rationale for the matchmaking control plane.
- `TODO.md` — deferred non-multiplayer work (audio is the big one: there is none at all).
- `TODO.md` — deferred non-multiplayer work, **and** the ordered human-actionable backlog: which GitHub issue to do first, what each one unblocks, and which items are waiting on nobody. Start there when asking "what next". Audio is no longer absent — a procedural `AudioManager` covers UI, countdown, impact, goal and engine/turbo cues; what remains is authored assets.
- `docs/THREAT-MODEL.md`, `docs/SUPPLY-CHAIN.md`, `docs/OBSERVABILITY.md`, `docs/MATCHMAKING-SLOs.md`, `docs/ADR-001-matchmaking-platform.md` — the control plane's security, release, telemetry and SLO contracts. `server/security/*.py` asserts several of them against the checked-in manifests, so changing a manifest often means changing one of these.
- `docs/REVIEW-2026-09-feat-multiplayer.md` — a point-in-time adversarial review of this branch. Every finding in it is fixed; it is kept for the reasoning, not as a status report, and its header says so.
## Godot MCP server
@@ -149,6 +151,46 @@ To run one by hand, and for every config flag, see `SERVER.md`. `--smoke-force-g
Note what is *not* in CI: `make verify-multiplayer-local` (the combined local gate, which also runs the Python manifest/contract suites) and the `integration`-tagged Go tests, which need a real PostgreSQL/Redis and live in `scripts/run_*_integration.sh`. Run those by hand before landing server changes.
### When a gate fails, suspect the assertion first
The most expensive failures in this repo have not been broken behaviour. They
have been **assertions that cannot distinguish the two states they implicitly
claim to**, each reporting its own ambiguity as a confident verdict about the
system under test. Five in one session, several costing multiple CI round trips:
| Assertion | What it actually conflated |
|---|---|
| `kubectl wait --for=jsonpath='{.status.ready}'` on an Agones Fleet | field does not exist vs. condition unmet — it could never pass |
| `compose ps --status running \| grep -qx game-server` | not started *yet* vs. exited |
| `remote_residual_position_p99 < 0.3` | real regression vs. host scheduling noise |
| a validator reading `status.gameServer` | Agones' real response vs. an invented one, with unit tests asserting the invention |
| `docker image inspect` guarding a build | image is current vs. image merely exists, so a rerun verified stale code |
Before theorising about the code, ask: **can this check tell "broken" from
"not ready yet", "absent" from "unset", or "regressed" from "slow"?** If not,
that is the bug, whatever else is also true.
Two habits follow from it, and both repeatedly beat reading code:
- **Make the script say what it saw before diagnosing why.** Most gates here are
`curl -fsS` and bare `[[ ]]` under `set -e`, which abort silently — several CI
runs produced nothing but `make: *** Error 1`. Report the failing line and
command, print the value that failed its comparison, and dump the surrounding
state *before* any cleanup trap destroys it. Every root cause found in that
session came from doing this; essentially every confident guess made without
it was wrong.
- **Verify the diagnostics fire.** Two separate dumps were added and neither ran:
one behind a `kubectl cluster-info` guard that misjudged reachability, one
because a bare `trap ... ERR` does not fire inside functions or subshells
without `set -E`. A diagnostic that has never been seen working is not
evidence.
And when a test and the code agree but reality disagrees, suspect they were
written together. A validator and its fixtures both encoded a response shape
Agones never sends; nothing caught it because the gate had never run far enough
to see a real one.
### Other
- The `mcp/godot-mcp` submodule is a separate Node/TypeScript project with its own `npm install` / `npm run build` (see above) — it is tooling, not part of the game itself.
@@ -199,6 +241,52 @@ Server process: `scenes/server_boot.tscn` (`server_boot.gd`) is the shell — st
Known-insecure, and the reason public hosting is gated: **slot reclaim is keyed by display name**, so anyone who knows a disconnected player's name can take their reserved slot. Verified Steam identity (Phase 7) is the fix. Don't expose a server to strangers before then.
### Matchmaking control plane (`server/`, Go)
The only component outside the Godot project, and roughly a third of the
codebase. Layered so policy is testable without a database and persistence
without a network:
- `domain/` (~3.2k lines) — **pure policy, no I/O**: matcher formation and
rating tolerance, Glicko ratings and tiers, proposal/queue/match state
machines, casual lineup and backfill selection, probe validation, join
authorisations. Most behaviour worth asserting lives here and needs no
fixture. `ranked.go`'s arena list is checked against `arena_registry.gd` (see
Arena registry above).
- `store/` (~5.3k) — PostgreSQL boundaries. Every mutation goes through
`RunSerializable`; contention is expected rather than exceptional, so the
retry budget and jittered backoff there are load-bearing, not decoration.
- `api/` (~2.5k) — HTTP surface and the outbox dispatchers. `Service` is a
struct of optional providers, each nil-guarded into a 503, which is why a
binary can look healthy while a whole feature is unreachable — check what
`cmd/*/main.go` actually assigns before concluding a feature is broken.
- `allocator/`, `supervisor/`, `agones/` — allocation, the Go process that
wraps the exported Godot server in an Agones pod, and the Agones client.
- `matcher/`, `workload/`, `observability/`, `steam/`, `testkit/` — the matcher
worker loop, workload-token signing, metrics, the Steam Web API adapter, and
deterministic offline fakes.
`cmd/` holds seven binaries: `control-plane`, `matcher`, `allocator`,
`maintenance`, `game-server-supervisor`, `migrate`, and `testkit-api`.
**`testkit-api` is test-only** — it injects a fake Steam login that accepts any
ticket, and must never be deployed in place of `control-plane`.
Three things that are easy to get wrong:
- **Integration tests are behind `//go:build integration`** and need a real
PostgreSQL/Redis, so `go test ./...` silently skips them. Run them through
`scripts/run_*_integration.sh`, which start their own disposable containers.
`go vet -tags integration ./...` is worth running too, or those files rot
uncompiled.
- **Config is start-time.** Tier bands, the join-signing key set, Steam
credentials and the probe providers are all read once in `main()`. Changing
them is a rolling restart, not a hot reload — deliberate, and consistent with
how everything else in these binaries is supplied.
- **The wire contract is versioned.** `contracts/v1/openapi.json` and
`state-transitions.json` are asserted by `contracts/v1/test_contracts.py`;
changing a status code or operation ID without updating them breaks generated
clients silently.
### Steam transport
`net_transport.gd` (`NetTransport`) is a deliberately narrow boundary: a transport only *creates a peer*; `NetworkManager` keeps ownership of polling, RPC policy and lifecycle. `enet_transport.gd` and `steam_transport.gd` implement it. `NetworkManager.host()/join()` default to `"enet"`; passing `"steam"` **never falls back** — a missing custom build or failed init returns an error naming the missing prerequisite (`steam_bootstrap.gd` produces those messages). Discovery and server advertisement are intentionally unimplemented until a project-owned App ID exists; the local default is Valve's Spacewar App ID 480, which must never be used to advertise servers or ship.