diff --git a/.github/workflows/server-unit-tests.yml b/.github/workflows/server-unit-tests.yml new file mode 100644 index 00000000..c03e5853 --- /dev/null +++ b/.github/workflows/server-unit-tests.yml @@ -0,0 +1,47 @@ +# The Go control plane is ~24k lines, and until this workflow existed the only +# Go tests CI ever ran were the two load tests in multiplayer-load.yml. Nothing +# else — domain policy, the wire/store boundaries, the allocator, the Steam +# adapter — gated a change. The Godot unit suite is covered (verify-phase6 runs +# test_runner.tscn as its first step); this closes the equivalent gap on the +# Go side. +# +# Deliberately Docker-free and cluster-free so it stays fast enough to gate +# every push. Tests that need a real PostgreSQL or Redis are behind the +# `integration` build tag and stay with their own scripts; `go vet` is still +# run over that tag so those files cannot rot uncompiled. +name: Server Unit Tests + +on: + push: + pull_request: + +permissions: + contents: read + +jobs: + go-tests: + runs-on: ubuntu-latest + defaults: + run: + working-directory: server + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: server/go.mod + cache-dependency-path: server/go.sum + - name: Build + run: go build ./... + - name: Vet + run: go vet ./... + # Integration-tagged files are excluded from the default build, so + # without this a signature change could leave them broken until someone + # ran the integration scripts by hand. + - name: Vet integration-tagged tests + run: go vet -tags integration ./... + - name: Test + run: go test ./... + # The control plane is concurrent by design: outbox dispatchers, the + # event hub, the matcher worker and the allocator all run in parallel. + - name: Test with race detector + run: go test -race ./... diff --git a/CLAUDE.md b/CLAUDE.md index 5b4b4050..47241c58 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -132,7 +132,22 @@ To run one by hand, and for every config flag, see `SERVER.md`. `--smoke-force-g ### CI -`.github/workflows/` has exactly two jobs, both running the Make targets above: `dedicated-server-smoke.yml` (`make verify-phase6`) and `enet-integration.yml` (`make verify-enet-integration` inside the `enet-test` image). There is no unit-test-only workflow — `verify-phase6` runs `test_runner.tscn` as its first step. +`.github/workflows/` has eight jobs, all but one running a Make target: + +| Workflow | Runs | Needs | +|---|---|---| +| `server-unit-tests.yml` | `go build`/`vet`/`vet -tags integration`/`test`/`test -race` in `server/` | nothing (no Docker) | +| `dedicated-server-smoke.yml` | `make verify-phase6` | Docker, several GB | +| `enet-integration.yml` | `make verify-enet-integration` inside the `enet-test` image | Docker | +| `allocated-compose.yml` | `make verify-allocated-compose` | Docker Compose | +| `agones-integration.yml` | `make verify-kind-agones` | kind + Helm | +| `multiplayer-chaos.yml` | `make verify-chaos-recovery` | Docker | +| `multiplayer-load.yml` | `make verify-multiplayer-load` (two `-tags load` Go tests) | — | +| `supply-chain.yml` | `make verify-supply-chain` | — | + +**The Godot unit suite runs via `verify-phase6`**, which invokes `test_runner.tscn` as its first step — there is no separate Godot workflow. The Go unit suite has its own workflow because until it existed the only Go tests CI ran were `multiplayer-load`'s two load tests, so ~24k lines of control plane gated nothing. + +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. ### Other @@ -148,7 +163,7 @@ The structure was deliberately chosen so an RL-trained AI opponent and, later, m - **Scene flow**: `scenes/main_menu.tscn` (`main_menu.gd`, one handler per mode) → `free_play.tscn` (practice: no timer, R resets ball), `match.tscn` (150s timer, per-team score, kickoff resets), `spectate.tscn` (bot vs bot exhibition), `settings.tscn`, or — for online — `lobby.tscn` → `networked_match.tscn`. Esc returns to the menu. Canonical paths live in `scripts/scene_paths.gd`; use those constants rather than string literals. - **Controller seam (do not bypass)**: `Ship` (`scripts/ship.gd`, `RigidBody3D`) never reads `Input`. Each physics tick, `_integrate_forces` pulls one `ShipAction` (`scripts/ship_action.gd`: thrust `Vector3`, rotation `Vector3`, turbo `bool`, each axis -1..1) from its `ShipController` child (`scripts/ship_controller.gd`, base returns a zero action). `PlayerShipController` reads input actions; `AIShipController` runs an RL policy; `RLShipController` is driven by the training bridge; `LocalNetShipController` wraps another controller to record inputs into the network timeline. A ship with no controller is inert but simulated. The ShipAction shape *is* the RL action space and *is* what the wire format quantises — change it deliberately and everywhere at once. - **Arena vs game mode**: an arena (`scripts/arena.gd`, group `"arena"`) is a stateless stadium — a setting, an enclosing `Boundary` (instance of `objects/arena_boundary.tscn`), two `Goal` instances, `BallSpawn` and `SpawnsTeam0/1` Marker3Ds — queried via `get_ball_spawn()`/`get_ship_spawns(team)`/`get_goals()`. All arenas are a standard size: they instance the shared `arena_boundary.tscn`, and `scripts/arena_boundary.gd` (`ArenaBoundary`) holds the canonical play-volume constants (`INNER_HALF_X` 18, `INNER_HALF_Z` 27, `INNER_HEIGHT` 18, `GOAL_LINE_Z` = `INNER_HALF_Z`) that field-size logic must derive from instead of restating numbers. Game modes extend `GameMode` (`scripts/game_mode.gd`, group `"game"`): the mode's scene contains an Arena + HUD, and the mode spawns ball/ships/controllers/camera **in code** (`spawn_ship(team, index, controller)` etc.) so ship counts and controller mixes stay flexible. -- **Arena registry**: `scripts/arena_registry.gd` is the single source of truth for the arena list — three settings × floor/elevated goal variants. `"random": true` gates which arenas Match/Spectate/the dedicated server may pick; **elevated-goal variants are Free-Play-only** until a checkpoint trained on `training_elevated.tscn` is promoted, because the current bots cannot score on an elevated goal. `path_for_match(match_index, mode)` is deliberately pure arithmetic so "the server cycles arenas" is unit-testable. `arena_base.tscn` is the scenery-free physical layout the dedicated server loads (clients still render the variant `MatchSim` names). +- **Arena registry**: `scripts/arena_registry.gd` is the single source of truth for the arena list — three settings × floor/elevated goal variants. `"random": true` gates which arenas Match/Spectate/the dedicated server may pick; **elevated-goal variants are Free-Play-only** until a checkpoint trained on `training_elevated.tscn` is promoted, because the current bots cannot score on an elevated goal. `path_for_match(match_index, mode)` is deliberately pure arithmetic so "the server cycles arenas" is unit-testable. **The Go control plane keeps its own copy of the ranked-eligible subset** (`server/domain/ranked.go`'s `rankedArenas`), because ranked arena selection is a server-authoritative decision made before any Godot process exists. That copy is not free to drift: `server/domain/arena_registry_sync_test.go` parses this file and fails if the two disagree in either direction, or if a ranked path has no scene behind it. Editing the arena list therefore means updating `ranked.go` too — the test says so when it fails. `arena_base.tscn` is the scenery-free physical layout the dedicated server loads (clients still render the variant `MatchSim` names). - **Goals are dumb sensors**: `scripts/goal.gd` (`Area3D`, group `"goal"`, `@export team`) only emits `goal_scored(team)` when a body in group `"ball"` enters; `GameMode` debounces it (`_handle_goal_scored`) and modes decide consequences. Never put scoring/reset logic in the goal. - **Ship physics**: all movement is force/torque-based (`_integrate_forces`), not kinematic — inputs become world-space forces/torques relative to ship orientation, with manual drag and speed clamps per tick. Physics properties (mass, inertia, friction material) live in `objects/ship.tscn`, not in `_ready` overrides — keep the scene truthful; RL tuning and the client/server parity trace depend on it. - **Surface pull (wall/ceiling grav-plating)**: `ArenaBoundary.get_surface_pull()` is a wall+ceiling-only proximity force field (the floor stays plain default gravity) that `Ship` and `Ball` (`scripts/ball.gd`) each apply in their own `_integrate_forces` with independently-tuned strength/range, discovered via the `"arena_boundary"` group — enabling wall-rides and ceiling shots with no collision-shape changes. diff --git a/multiplayer-next.md b/multiplayer-next.md index 8056b743..f7babc1b 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -215,7 +215,7 @@ are done; everything below is what's left on the tasks still open. | 8.17 `[D:8.14,8.16]` | Proposal policy (response window, cooldowns, offender/innocent split) | Live PostgreSQL execution and allocation integration remain | | 8.18 `[D:8.5,8.14,8.17]` | Store layer (serializable retries, claim SQL, atomic promotion) | Allocation runtime integration remains | | 8.19 `[D:8.18]` | Casual lineup (2–6 humans, bot backfill) | Queue candidate selection, opt-in 10 s backfill proposals, reconnect/leave penalties, live integration remain | -| 8.20 `[D:8.18]` | Ranked admission (six unique verified humans) | `ArenaRegistry` integration and allocation wiring remain | +| 8.20 `[D:8.18]` | Ranked admission (six unique verified humans) | Done. Allocation wiring was already complete end to end (allocator sets the `cosmic-clash.io/arena-path` annotation → `supervisor.withAllocatedCompatibility` maps it to `--arena-path` → `server_boot.gd` → `ServerMatchLoop.allocated_arena_path`), with coverage at each hop. `ArenaRegistry` integration is now a cross-language guard rather than a shared list: `server/domain/ranked.go` must keep its own ranked-eligible subset (the choice is server-authoritative and made before any Godot process exists), so `arena_registry_sync_test.go` parses `arena_registry.gd` and fails if the two disagree in either direction, if rotation order diverges, or if a ranked path has no scene behind it. Verified against four drift scenarios including promoting an elevated variant, which the registry's own comment anticipates. Live ranked admission against a real cluster remains ([#17](https://github.com/jcreek/CosmicClash/issues/17)) | | 8.21 `[D:8.5,8.20]` | Rating core (Glicko-2, weights, transactional updates) | Live maintenance/DB execution remains | | 8.22 `[D:8.21]` | Ranked profile (provisional games, tiers) | Persisted tier policy, client UI, reconnect transport remain | | 8.23 `[D:8.21]` | Ranked season policy (compression, rollover) | Live maintenance/DB execution remains | diff --git a/server/domain/arena_registry_sync_test.go b/server/domain/arena_registry_sync_test.go new file mode 100644 index 00000000..14bdc86c --- /dev/null +++ b/server/domain/arena_registry_sync_test.go @@ -0,0 +1,165 @@ +package domain + +import ( + "os" + "path/filepath" + "regexp" + "testing" +) + +// arenaRegistryPath is the Godot-side single source of truth for the arena +// list (CLAUDE.md says so explicitly). rankedArenas in ranked.go is a +// hand-maintained mirror of its floor-goal entries, and nothing has ever +// 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's +// `random` flag to true once a checkpoint trained on that geometry is +// promoted. Ranked would keep excluding it indefinitely. +// - Adding an arena leaves ranked never selecting it. +// - Renaming or removing one leaves the allocator handing out a scene path +// that no longer exists, and an allocated ranked server fails to load its +// arena at match start -- after allocation, so it burns a real match. +const arenaRegistryPath = "../../Game/scripts/arena_registry.gd" + +// gameScenesDir resolves a res:// path to the checked-out scene file. +const gameScenesDir = "../../Game" + +var arenaEntryPattern = regexp.MustCompile(`\{"name":\s*"([^"]*)",\s*"path":\s*"([^"]*)",\s*"random":\s*(true|false)\}`) + +type registryArena struct { + Name string + Path string + Random bool +} + +func parseArenaRegistry(t *testing.T) []registryArena { + t.Helper() + source, err := os.ReadFile(arenaRegistryPath) + if err != nil { + t.Fatalf("read the Godot arena registry: %v", err) + } + matches := arenaEntryPattern.FindAllStringSubmatch(string(source), -1) + arenas := make([]registryArena, 0, len(matches)) + for _, match := range matches { + arenas = append(arenas, registryArena{Name: match[1], Path: match[2], Random: match[3] == "true"}) + } + + // Guard the guard. If the literal format changes and the pattern stops + // matching, every assertion below would pass vacuously against an empty + // list -- which is the exact failure mode this test exists to prevent. + if len(arenas) < 2 { + t.Fatalf("parsed %d arenas from %s; the entry format probably changed and this parser needs updating", len(arenas), arenaRegistryPath) + } + var eligible, ineligible int + for _, arena := range arenas { + if arena.Random { + eligible++ + } else { + ineligible++ + } + } + if eligible == 0 || ineligible == 0 { + t.Fatalf("parsed %d eligible and %d ineligible arenas; expected both kinds, so the `random` flag is probably not being read correctly", eligible, ineligible) + } + return arenas +} + +// TestRankedArenasMatchTheGodotRegistry is the cross-language contract. It is +// the arena equivalent of the golden join-authorisation token in +// Game/tests/cases/test_match_net.gd: one side owns the truth, and this fails +// loudly when the other stops agreeing. +func TestRankedArenasMatchTheGodotRegistry(t *testing.T) { + registry := parseArenaRegistry(t) + + expected := map[string]string{} + var expectedOrder []string + for _, arena := range registry { + if !arena.Random { + continue + } + expected[arena.Path] = arena.Name + expectedOrder = append(expectedOrder, arena.Path) + } + + actual := map[string]string{} + for id, arena := range rankedArenas { + actual[arena.Path] = id + } + + for path, name := range expected { + if _, present := actual[path]; !present { + t.Errorf("registry arena %q (%s) is ranked-eligible in Godot but missing from rankedArenas.\n"+ + "If a checkpoint trained on this geometry was promoted, add it to rankedArenas and rankedArenaOrder in ranked.go.", path, name) + } + } + for path, id := range actual { + if _, present := expected[path]; !present { + t.Errorf("rankedArenas contains %q (id %q), which is not a random:true entry in %s.\n"+ + "Ranked would allocate a scene the Godot registry no longer offers.", path, id, arenaRegistryPath) + } + } + + // Rotation order must follow the registry's declaration order, since + // RankedArenaForProposal indexes rankedArenaOrder and callers reason about + // "the arenas, in order" across both languages. + if len(rankedArenaOrder) != len(expectedOrder) { + t.Fatalf("rankedArenaOrder has %d entries, registry has %d eligible", len(rankedArenaOrder), len(expectedOrder)) + } + for index, id := range rankedArenaOrder { + arena, known := rankedArenas[id] + if !known { + t.Fatalf("rankedArenaOrder[%d] = %q, which is not a key of rankedArenas", index, id) + } + if arena.Path != expectedOrder[index] { + t.Errorf("rotation position %d is %q, registry declares %q there", index, arena.Path, expectedOrder[index]) + } + } +} + +// A ranked arena path is handed to an allocated server after allocation, so a +// path with no scene behind it fails at match start rather than at selection -- +// burning a real match and a real server. Cheap to catch here instead. +func TestRankedArenaPathsResolveToRealScenes(t *testing.T) { + for id, arena := range rankedArenas { + relative, ok := scenePathFromRes(arena.Path) + if !ok { + t.Errorf("ranked arena %q has path %q, which is not a res:// path", id, arena.Path) + continue + } + if _, err := os.Stat(filepath.Join(gameScenesDir, relative)); err != nil { + t.Errorf("ranked arena %q points at %q, which does not exist: %v", id, arena.Path, err) + } + } +} + +// Elevated-goal variants stay ranked-ineligible until a policy trained on that +// geometry is promoted; the current bots cannot score on one. Assert this +// against the registry's own flag rather than a second hardcoded list, so the +// exclusion tracks the registry instead of drifting alongside it. +func TestIneligibleRegistryArenasAreRejectedForRanked(t *testing.T) { + registry := parseArenaRegistry(t) + checked := 0 + for _, arena := range registry { + if arena.Random { + continue + } + checked++ + if IsRankedArenaPath(arena.Path) { + t.Errorf("%q (%s) is random:false in the Godot registry but accepted for ranked", arena.Path, arena.Name) + } + } + if checked == 0 { + t.Fatal("no ineligible arenas were checked") + } +} + +func scenePathFromRes(path string) (string, bool) { + const prefix = "res://" + if len(path) <= len(prefix) || path[:len(prefix)] != prefix { + return "", false + } + return path[len(prefix):], true +}