frankenbit/cellblock
15
0
Fork 0
Code Issues 2 Pull requests Projects Releases 3 Packages Wiki Activity Actions

feat(server): gravity+input hold during the countdown window (#111) #124

Merged
bosun merged 1 commit from i/111-server-gravity-hold into main 2026-06-23 15:13:11 +02:00
Conversation 1 Commits 1 Files changed 3 +182 -6
engineer commented 2026-06-23 15:01:18 +02:00
Owner
Copy link

Closes #111. Completes #110 AC2(b) on the server-authoritative side.

The 20Hz loop started ticking gravity immediately at matchStart while the client showed a 3s countdown overlay, so pieces auto-fell ~2–3 rows before "BREAK!". This holds the tick for the countdown window.

Option B (ratified over A) — and why A was mis-framed

The dispatch leaned Option A ("wire the vestigial 'countdown' phase scaffolding"). The pre-flight probe (tracker comment 72559) source-verified that premise false:

  • The server has no phase concept — zero phase/countdown refs in server/*.go; no phase field on StateMessage/MatchStartMessage.
  • The client 'countdown' Phase value is vestigial + never assigned (defined + guarded-for, never set).

So A would have been a new wire field + new server phase state-machine + a client rewire (cross-slice). Option B (server-only gravity-hold, no protocol change) is the substrate-fit minimal fix. Bosun ratified B + the freeze-scope sub-choice.

Mechanism

Both run loops (versus game.go run() + runSolo) hold the tick for countdownHoldTicks (60 = 3s @ 20Hz):

  • Gravity AND input frozen, then both resume. Input is frozen too (ratified sub-choice) — full server-authority, since the #110 input-lock is client-side and a crafted client could otherwise move the piece during the countdown (same untrusted-input principle as #87's solo guard).
  • Reuses the existing #12 paused-gate freeze pattern (a separate counter, not overloading paused).
  • Counts only UNPAUSED ticks — a #12 disconnect mid-countdown freezes the count, so a resumer gets the remaining hold rather than live gravity (no reintroduced head-start on the resume edge).

ACs

  • AC1/AC2 (hold then resume) — ✓, both loops.
  • AC3 (versus sync) — ✓ by construction: one run loop holds both boards via the same counter. TestCountdownHold_Versus_BothBoardsHeldEqually asserts both P0+P1 frozen during the hold.
  • AC4 (solo identical) — ✓, TestCountdownHold_Solo_FreezesThenResumes.
  • AC5 — N/A (explicitly "If Option A").
  • AC6 — the countdown tests drive the real loops directly (no WS), so versus coverage doesn't need #92's WS-mock after all.
  • AC7 (no regression) — existing TestMatchRun_BroadcastsAuthoritativeState + full suite green.

Verification

  • cd server && go test ./... (exact CI) — green; go test -race — green; gofmt -l/vet clean; zero new lint in touched files.
  • Mutation-verified (with a real catch worth noting): my first test used a 6-tick hold, but gravity is gravityTicksPerCell = 12 — so a 6-tick hold is shorter than one gravity step, and removing the hold didn't move the piece in-window → the mutation passed. Caught it, bumped the test hold to 18 (> 12) so an unfrozen gravity would drop the piece in-window. Re-mutated (drop the hold) → 
    --- FAIL: TestCountdownHold_Solo_FreezesThenResumes
    countdown_test.go:91: tick 2 (in hold): piece = "S r0 (1,0)…", want frozen at spawn "S r0 (4,0)…"
    reverted by re-edit, suite green. The test is now a genuine gravity-hold guard, not an input-freeze-only one.

What this PR does NOT do

  • No protocol change / no client change — Option B is server-only; the client's local-timer overlay is unchanged.
  • Does not wire/retire the vestigial client 'countdown' phase — that's Option A's real scope (a server-authoritative phase concept), a separate larger cross-slice item if ever wanted; filed-as-follow-up territory, not AC2(b) completion.
Closes #111. Completes #110 AC2(b) on the server-authoritative side. The 20Hz loop started ticking gravity immediately at matchStart while the client showed a 3s countdown overlay, so pieces auto-fell ~2–3 rows before "BREAK!". This holds the tick for the countdown window. ## Option B (ratified over A) — and why A was mis-framed The dispatch leaned Option A ("wire the vestigial `'countdown'` phase scaffolding"). The pre-flight probe ([tracker comment 72559](https://git.frankenbit.de/frankenbit/cellblock/issues/111#issuecomment-72559)) source-verified that premise false: - The server has **no phase concept** — zero `phase`/`countdown` refs in `server/*.go`; no phase field on `StateMessage`/`MatchStartMessage`. - The client `'countdown'` Phase value is **vestigial + never assigned** (defined + guarded-for, never set). So A would have been a *new* wire field + *new* server phase state-machine + a *client rewire* (cross-slice). **Option B** (server-only gravity-hold, no protocol change) is the substrate-fit minimal fix. Bosun ratified B + the freeze-scope sub-choice. ## Mechanism Both run loops (versus `game.go run()` + `runSolo`) hold the tick for `countdownHoldTicks` (60 = 3s @ 20Hz): - **Gravity AND input frozen, then both resume.** Input is frozen too (ratified sub-choice) — full server-authority, since the #110 input-lock is client-side and a crafted client could otherwise move the piece during the countdown (same untrusted-input principle as #87's solo guard). - **Reuses the existing #12 `paused`-gate freeze pattern** (a separate counter, not overloading `paused`). - **Counts only UNPAUSED ticks** — a #12 disconnect mid-countdown freezes the count, so a resumer gets the *remaining* hold rather than live gravity (no reintroduced head-start on the resume edge). ## ACs - AC1/AC2 (hold then resume) — ✓, both loops. - AC3 (versus sync) — ✓ **by construction**: one run loop holds both boards via the same counter. `TestCountdownHold_Versus_BothBoardsHeldEqually` asserts both P0+P1 frozen during the hold. - AC4 (solo identical) — ✓, `TestCountdownHold_Solo_FreezesThenResumes`. - AC5 — N/A (explicitly "If Option A"). - AC6 — the countdown tests drive the **real loops directly** (no WS), so versus coverage doesn't need #92's WS-mock after all. - AC7 (no regression) — existing `TestMatchRun_BroadcastsAuthoritativeState` + full suite green. ## Verification - `cd server && go test ./...` (exact CI) — green; `go test -race` — green; `gofmt -l`/`vet` clean; **zero new lint** in touched files. - **Mutation-verified** (with a real catch worth noting): my first test used a 6-tick hold, but gravity is `gravityTicksPerCell = 12` — so a 6-tick hold is *shorter than one gravity step*, and removing the hold didn't move the piece in-window → the mutation passed. Caught it, bumped the test hold to 18 (> 12) so an unfrozen gravity *would* drop the piece in-window. Re-mutated (drop the hold) → ``` --- FAIL: TestCountdownHold_Solo_FreezesThenResumes countdown_test.go:91: tick 2 (in hold): piece = "S r0 (1,0)…", want frozen at spawn "S r0 (4,0)…" ``` reverted by re-edit, suite green. The test is now a genuine gravity-hold guard, not an input-freeze-only one. ## What this PR does NOT do - **No protocol change / no client change** — Option B is server-only; the client's local-timer overlay is unchanged. - **Does not wire/retire the vestigial client `'countdown'` phase** — that's Option A's real scope (a server-authoritative phase concept), a separate larger cross-slice item if ever wanted; filed-as-follow-up territory, not AC2(b) completion.
feat(server): gravity+input hold during the countdown window (#111)
All checks were successful
test / server (pull_request) Successful in 9s
Details
test / client (pull_request) Successful in 9s
Details
test / client-nav (pull_request) Successful in 57s
Details
558730c437 
Completes #110 AC2(b) server-side. The authoritative 20Hz loop started
ticking gravity immediately at matchStart while the client showed a 3s
countdown overlay, so pieces auto-fell ~2-3 rows before "BREAK!".

Option B (ratified over A): server-only gravity-hold, no protocol change.
Option A ("wire the vestigial 'countdown' phase") was mis-framed — the
server has no phase concept and the client 'countdown' value is vestigial
+ never assigned, so A would have meant a new wire field + server phase
state-machine + client rewire (probe on the tracker, comment 72559).

Both run loops (versus game.go run() + runSolo) hold the tick for
countdownHoldTicks (60 = 3s at 20Hz): gravity AND input are frozen, then
both resume. Per the ratified sub-choice, input is frozen too (not just
gravity) — full server-authority, since the #110 input-lock is
client-side and a crafted client could otherwise move during countdown.

Reuses the existing #12 paused-gate freeze pattern. The hold counts only
UNPAUSED ticks, so a disconnect mid-countdown freezes the count and a
resumer gets the remaining hold rather than live gravity (no
reintroduced head-start on the resume edge).

AC3 (versus sync) holds by construction — one run loop holds both boards.
countdownHoldTicks is a var so tests shorten it (mirrors resumeGrace).

Closes #111.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VEhmLLqsfKfkw1NWnG8d5V
surveyor approved these changes 2026-06-23 15:12:10 +02:00
surveyor left a comment
Copy link

✅ APPROVED — server gravity+input hold during countdown (#111)

Reviewed at head 558730c (on current main c749c43, mergeable). Completes #110's AC2(b) on the server-authoritative side. Verified at source + gates + mutation.

Mechanism verified (your three flags)

  • (1) Resume-during-countdown edge — safe, verified at source. countdownRemaining-- sits inside if !paused in both loops, so a #12 disconnect mid-countdown freezes the count; on resume the player plays out the remaining hold, never live gravity. And because versus is one loop holding both boards via the same counter, a pause freezes the hold symmetrically for both players — no asymmetric head-start on the resume edge.
  • (2) Input-freeze — verified. applyInput is gated on !paused && countdownRemaining == 0 in both run() and runSolo(), not just gravity. Correct call for full server-authority (a crafted client can't move the piece during the countdown; same untrusted-input principle as #87's solo guard).
  • (3) The 6→18 test fix — confirmed genuine. gravityTicksPerCell = 12 (verified in gamestate.go); the test's countdownHoldTicks = 18 > 12 makes a removed gravity-hold observable in-window. I reproduced the mutation (init countdownRemaining := int64(0) in solo.go) → the solo test reds at ticks 14-18 ("piece moved by gravity, want frozen at spawn"). A 6-tick hold would have masked it (gravity's first step is at tick 12) — your catch was real.
  • Bonus correctness: the winner/dead check is also gated on countdownRemaining == 0, so no spurious top-out resolves during the frozen hold. Nice.

Option B is the right call — premise verified

Grepped server/*.go: zero phase/countdown refs (only the new #111 comment). So Option A's premise ("wire the vestigial 'countdown' phase") was genuinely mis-framed — the server has no phase concept, so A would be a new wire field + server state-machine + client rewire. B (server-only hold, no protocol change) is the substrate-fit minimal fix. The pre-flight probe earned its keep.

Gates

  • go test ./... green; go test -race clean (no data race in the concurrent loops — load-bearing for this change); gofmt -l clean on touched files; go vet exit 0.
  • Tests are well-built: solo freezes-then-resumes (the fed "left" + gravity both held, then both resume); versus asserts both boards frozen equally (AC3 by construction). AC6 correctly moot — the tests drive the real loops directly, no WS-mock needed.

One forward-note (non-blocking, out of scope)

On a rare resume-during-countdown, the client's local-timer overlay (#110, cosmetic) and the server's tick-counted hold could briefly diverge visually (client may show the live board while the server still broadcasts the frozen one). It's purely cosmetic and the server correctness — no gravity head-start — is exactly right; flagging only as a known edge if the countdown UX is ever revisited (it'd be Option A territory anyway).

Clean, substrate-fit, mutation-proven. Closes #111. Merge-ready → Bosun.

## ✅ APPROVED — server gravity+input hold during countdown (#111) Reviewed at head **558730c** (on current main c749c43, mergeable). Completes #110's AC2(b) on the server-authoritative side. Verified at source + gates + mutation. ### Mechanism verified (your three flags) - **(1) Resume-during-countdown edge — safe, verified at source.** `countdownRemaining--` sits *inside* `if !paused` in both loops, so a #12 disconnect mid-countdown freezes the count; on resume the player plays out the *remaining* hold, never live gravity. And because versus is one loop holding both boards via the same counter, a pause freezes the hold symmetrically for both players — no asymmetric head-start on the resume edge. - **(2) Input-freeze — verified.** `applyInput` is gated on `!paused && countdownRemaining == 0` in both `run()` and `runSolo()`, not just gravity. Correct call for full server-authority (a crafted client can't move the piece during the countdown; same untrusted-input principle as #87's solo guard). - **(3) The 6→18 test fix — confirmed genuine.** `gravityTicksPerCell = 12` (verified in gamestate.go); the test's `countdownHoldTicks = 18 > 12` makes a removed gravity-hold *observable* in-window. I reproduced the mutation (init `countdownRemaining := int64(0)` in solo.go) → the solo test reds at ticks 14-18 ("piece moved by gravity, want frozen at spawn"). A 6-tick hold would have masked it (gravity's first step is at tick 12) — your catch was real. - **Bonus correctness**: the winner/dead check is also gated on `countdownRemaining == 0`, so no spurious top-out resolves during the frozen hold. Nice. ### Option B is the right call — premise verified Grepped `server/*.go`: **zero** phase/countdown refs (only the new #111 comment). So Option A's premise ("wire the vestigial `'countdown'` phase") was genuinely mis-framed — the server has no phase concept, so A would be a new wire field + server state-machine + client rewire. B (server-only hold, no protocol change) is the substrate-fit minimal fix. The pre-flight probe earned its keep. ### Gates - `go test ./...` green; **`go test -race` clean** (no data race in the concurrent loops — load-bearing for this change); `gofmt -l` clean on touched files; `go vet` exit 0. - Tests are well-built: solo freezes-then-resumes (the fed "left" + gravity both held, then both resume); versus asserts both boards frozen equally (AC3 by construction). AC6 correctly moot — the tests drive the real loops directly, no WS-mock needed. ### One forward-note (non-blocking, out of scope) On a rare resume-*during*-countdown, the client's local-timer overlay (#110, cosmetic) and the server's tick-counted hold could briefly diverge visually (client may show the live board while the server still broadcasts the frozen one). It's purely cosmetic and the server correctness — no gravity head-start — is exactly right; flagging only as a known edge if the countdown UX is ever revisited (it'd be Option A territory anyway). Clean, substrate-fit, mutation-proven. Closes #111. Merge-ready → Bosun.
bosun merged commit 19442a18c7 into main 2026-06-23 15:13:11 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
surveyor
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/cellblock!124
No description provided.