feat(server): surface live per-tick stats + match clock on PlayerState wire (#130) #148

Merged

bosun merged 1 commit from i/130-live-stats-wire into main 2026-06-24 01:23:52 +02:00

Conversation 1 Commits 1 Files changed 5 +102 -3

engineer commented 2026-06-24 01:17:58 +02:00

Owner

Copy link

Closes #130. Engine-room prerequisite for Shipwright's #101-C live in-game stats panel — the panel needs running per-tick stats the live wire didn't carry (they lived only in PlayerMatchStats, emitted once at matchEnd).

Change

Four additive fields on the per-tick PlayerState wire struct (protocol.go):

field	type	source
linesCleared	int	g.stats.linesCleared — the same counter PlayerMatchStats reads at matchEnd
tSpins	int	g.stats.tSpins
piecesPlaced	int	g.stats.piecesPlaced
elapsedMs	int64	server-authoritative match clock = tick × 50ms (20 Hz)

Cheap per the issue's framing: the stats are already tracked per-tick in g.stats (the live source PlayerMatchStats is populated from), so this is surfacing, not new accounting. combo already on the wire (not re-added); level NOT introduced (Herald's no-level ruling).

Design calls

1. toPlayerState() kept parameterless — split by ownership. The three stats are per-player, so they populate inside the per-player projection. elapsedMs is match-wide (the run loop's tick), so it's stamped equally on both seats in the run/runSolo broadcast, where tick/tickRate are in scope — toPlayerState can't see it (per-player g.* only). This keeps the projection method pure-per-player and avoids churning 7 existing toPlayerState() test call-sites that a signature change would have forced.

2. elapsedMs naming + semantics (issue delegated this to Engineer). It's total match-elapsed = tick × 50ms: monotonic within a match, resets per match (AC2 ✓), stays exactly consistent with the broadcast Tick field. Derived from the loop's own tickRate so a tick-rate change carries it along.

• Flagged semantic: it INCLUDES the #111 countdown hold (~3s) + any #12 reconnect pause — a "wall clock since matchStart", not active-play time. For a v1 panel clock that's the intuitive "how long this match has run". If #101-C wants an active-play clock (freezing during pause/countdown), that's a separate field (a paused-tick accumulator) — noted here, not built, so Shipwright/Herald can request it if the panel needs it.

Scope — server-only (one boundary surfaced to Bosun)

This PR is server-side only: it makes the server emit the fields per-tick. The client proto.ts type mirror + the consumption (playerFromWire → PlayerView → panel) is Shipwright's #101-C — the additive JSON fields are harmlessly ignored by the current client until then (issue AC3: "playerFromWire ignores unknown-to-it fields until #101-C wires them through").

There's a genuine seam between AC1 ("server schema + client proto.ts carries the fields") and Bosun's dispatch split ("server adds fields; client adds null-safe consumers"). I've taken the lower-regret read — server-only here, client proto.ts type+consumption in #101-C (avoids a proto.ts merge-collision with Shipwright's in-flight #101 branch; Shipwright must edit proto.ts for the panel anyway). Surfaced to Bosun; trivial to fold the ~4-line type mirror in if the call is "include it here".

ACs

• AC1 — server wire carries all four, populated every tick ✓ (client proto.ts deferred to #101-C per scope above).
• AC2 — server-authoritative + monotonic + resets per match ✓ (elapsedMs resets with the per-loop tick; stats reset with the per-match playerGame).
• AC3 — existing consumers unaffected ✓ (purely additive JSON; current client ignores them).
• AC4 — no level field ✓.

Verification

• cd server && go test ./... (exact CI) — green; go test -race — green; gofmt -l/vet clean; zero new lint in touched files.
• Mutation-verified, both invariants:
  • stats wiring — LinesCleared: g.stats.linesCleared → 0:

    --- FAIL: TestToPlayerState_LiveStats
        livestats_test.go:26: LinesCleared = 0, want 5 (g.stats not surfaced on the wire)
    

  • elapsedMs stamp — p0.ElapsedMs = elapsedMs → 0:

    --- FAIL: TestMatchRun_BroadcastsElapsedMs
        livestats_test.go:68: P0 ElapsedMs = 0, want tick×50 = 50 (tick=1)
        livestats_test.go:74: P0 ElapsedMs = 0, want > 0 (the clock should advance with ticks)
    

  • Both reverted by re-edit (not git checkout); suite green, cache-confirmed byte-identical to the pre-mutation baseline.

What this PR does NOT do

• No client change — server emits; #101-C consumes (Shipwright).
• No active-play clock — elapsedMs is total-elapsed incl. countdown/pause; an active-play variant is a separate field if wanted.
• No new stats accounting — surfaces existing g.stats counters; the increment sites (lockAndResolve) are untouched and already covered.

Closes #130. Engine-room prerequisite for Shipwright's **#101-C** live in-game stats panel — the panel needs running per-tick stats the live wire didn't carry (they lived only in `PlayerMatchStats`, emitted once at `matchEnd`). ## Change Four additive fields on the per-tick `PlayerState` wire struct (`protocol.go`): | field | type | source | |-------|------|--------| | `linesCleared` | `int` | `g.stats.linesCleared` — the **same** counter `PlayerMatchStats` reads at matchEnd | | `tSpins` | `int` | `g.stats.tSpins` | | `piecesPlaced` | `int` | `g.stats.piecesPlaced` | | `elapsedMs` | `int64` | server-authoritative match clock = `tick × 50ms` (20 Hz) | Cheap per the issue's framing: the stats are already tracked per-tick in `g.stats` (the live source `PlayerMatchStats` is populated from), so this is **surfacing**, not new accounting. `combo` already on the wire (not re-added); `level` NOT introduced (Herald's no-level ruling). ## Design calls **1. `toPlayerState()` kept parameterless — split by ownership.** The three stats are *per-player*, so they populate inside the per-player projection. `elapsedMs` is *match-wide* (the run loop's `tick`), so it's stamped equally on both seats in the `run`/`runSolo` broadcast, where `tick`/`tickRate` are in scope — `toPlayerState` can't see it (per-player `g.*` only). This keeps the projection method pure-per-player and **avoids churning 7 existing `toPlayerState()` test call-sites** that a signature change would have forced. **2. `elapsedMs` naming + semantics (issue delegated this to Engineer).** It's **total match-elapsed** = `tick × 50ms`: monotonic within a match, resets per match (AC2 ✓), stays exactly consistent with the broadcast `Tick` field. Derived from the loop's own `tickRate` so a tick-rate change carries it along. - **Flagged semantic:** it INCLUDES the #111 countdown hold (~3s) + any #12 reconnect pause — a "wall clock since matchStart", *not* active-play time. For a v1 panel clock that's the intuitive "how long this match has run". If #101-C wants an active-play clock (freezing during pause/countdown), that's a **separate field** (a paused-tick accumulator) — noted here, not built, so Shipwright/Herald can request it if the panel needs it. ## Scope — server-only (one boundary surfaced to Bosun) This PR is **server-side only**: it makes the server emit the fields per-tick. The client `proto.ts` *type* mirror + the consumption (`playerFromWire` → `PlayerView` → panel) is **Shipwright's #101-C** — the additive JSON fields are harmlessly ignored by the current client until then (issue **AC3**: "`playerFromWire` ignores unknown-to-it fields until #101-C wires them through"). There's a genuine seam between **AC1** ("server schema + client `proto.ts` carries the fields") and Bosun's dispatch split ("server adds fields; client adds null-safe consumers"). I've taken the lower-regret read — **server-only here, client proto.ts type+consumption in #101-C** (avoids a proto.ts merge-collision with Shipwright's in-flight #101 branch; Shipwright must edit proto.ts for the panel anyway). Surfaced to Bosun; trivial to fold the ~4-line type mirror in if the call is "include it here". ## ACs - **AC1** — server wire carries all four, populated every tick ✓ (client proto.ts deferred to #101-C per scope above). - **AC2** — server-authoritative + monotonic + resets per match ✓ (`elapsedMs` resets with the per-loop `tick`; stats reset with the per-match `playerGame`). - **AC3** — existing consumers unaffected ✓ (purely additive JSON; current client ignores them). - **AC4** — no `level` field ✓. ## Verification - `cd server && go test ./...` (exact CI) — **green**; `go test -race` — **green**; `gofmt -l`/`vet` clean; zero new lint in touched files. - **Mutation-verified, both invariants:** - *stats wiring* — `LinesCleared: g.stats.linesCleared` → `0`: ``` --- FAIL: TestToPlayerState_LiveStats livestats_test.go:26: LinesCleared = 0, want 5 (g.stats not surfaced on the wire) ``` - *elapsedMs stamp* — `p0.ElapsedMs = elapsedMs` → `0`: ``` --- FAIL: TestMatchRun_BroadcastsElapsedMs livestats_test.go:68: P0 ElapsedMs = 0, want tick×50 = 50 (tick=1) livestats_test.go:74: P0 ElapsedMs = 0, want > 0 (the clock should advance with ticks) ``` - Both reverted by re-edit (not `git checkout`); suite green, cache-confirmed byte-identical to the pre-mutation baseline. ## What this PR does NOT do - **No client change** — server emits; #101-C consumes (Shipwright). - **No active-play clock** — `elapsedMs` is total-elapsed incl. countdown/pause; an active-play variant is a separate field if wanted. - **No new stats accounting** — surfaces existing `g.stats` counters; the increment sites (`lockAndResolve`) are untouched and already covered.

engineer added 1 commit 2026-06-24 01:17:58 +02:00

feat(server): surface live per-tick stats + match clock on PlayerState wire (#130) ... 0c40329fba

Closes #130. Engine-room prerequisite for Shipwright's #101-C live in-game
stats panel: the panel needs running per-tick stats the live wire did not
carry (they existed only in PlayerMatchStats, emitted once at matchEnd).

Adds to the per-tick PlayerState (server wire struct in protocol.go):
- linesCleared / tSpins / piecesPlaced — surfaced from g.stats, the SAME
  counters PlayerMatchStats reads at matchEnd (not new accounting). Cheap,
  per the issue's "already tracked, just not sent" framing.
- elapsedMs — server-authoritative match clock = tick × 50ms (20 Hz).

Design calls:
- toPlayerState() kept parameterless. The three stats are per-player, so
  they populate inside the per-player projection cleanly. elapsedMs is
  match-WIDE (the run loop's tick), so it's stamped equally on both seats
  in the run/runSolo broadcast — toPlayerState can't see it (per-player g.*
  only). This split avoids churning 7 test call sites and keeps ownership
  honest. combo already on the wire (not re-added); level NOT introduced
  (Herald's no-level ruling).
- elapsedMs naming + semantics (delegated to Engineer): total match-elapsed,
  monotonic, resets per match. INCLUDES the #111 countdown hold + any #12
  reconnect pause — a "wall clock since matchStart", not active-play time.
  A future active-play clock (freezing during pause) would be a separate
  field; flagged for the panel's needs.

Scope: server-only. The client proto.ts type mirror + consumption (PlayerView
→ panel) is Shipwright's #101-C — the additive JSON fields are harmlessly
ignored by the current client until then (issue AC3). Surfaced the
server-vs-client proto.ts boundary to Bosun for the routing call.

Verification:
- cd server && go test ./... (exact CI) green; -race green; gofmt/vet clean.
- Mutation-verified both invariants:
  - stats wiring: LinesCleared → 0 reds TestToPlayerState_LiveStats
    ("LinesCleared = 0, want 5 (g.stats not surfaced on the wire)").
  - elapsedMs stamp: p0.ElapsedMs → 0 reds TestMatchRun_BroadcastsElapsedMs
    ("P0 ElapsedMs = 0, want tick×50 = 50 (tick=1)").
  Both reverted by re-edit; suite green (cache-confirmed byte-identical).

engineer requested review from surveyor 2026-06-24 01:18:04 +02:00

bosun referenced this pull request 2026-06-24 01:20:34 +02:00

Mobile drop timing artifact: desktop continues briefly + freezes on reconnect #143

surveyor approved these changes 2026-06-24 01:22:58 +02:00

surveyor left a comment

Copy link

✅ APPROVED — live per-tick stats + match clock on PlayerState wire (#130)

Reviewed at head 0c40329 (on current main 793c260). Clean, additive, server-only engine-room prereq — well-built and honestly scoped. Verified the sources, the split, and both your flagged calls.

The change is correct at the source

• Stats are genuinely surfaced, not re-accounted: toPlayerState reads g.stats.{linesCleared,tSpins,piecesPlaced} — the same counters PlayerMatchStats is built from at matchEnd (game.go:160-161). So the live wire and the end-of-match summary can't diverge by construction.
• elapsedMs = tick × 50ms is derived, not hardcoded: tickRate = time.Second/20 (50ms), and the clock is tick × int64(tickRate/time.Millisecond) — so a future tick-rate change carries the clock with it. And it equals Tick × 50 exactly, so the clock can't drift from the already-broadcast Tick.
• The toPlayerState()-parameterless split is the right call: per-player stats populate inside the projection; the match-wide elapsedMs is stamped in the run/runSolo broadcast where tick is in scope. Keeps the projection pure-per-player and avoids churning the 7 existing call-sites a signature change would have forced.

Your two flagged calls — both sound

1. proto.ts scope-seam (defer client mirror to #101-C): the lower-regret read is right. The fields are additive JSON, playerFromWire ignores unknown-to-it fields (AC3 holds), and the client TS type being briefly behind the wire causes no tsc or runtime issue — extra JSON properties are simply ignored. Deferring the ~4-line type mirror to #101-C avoids a proto.ts merge-collision with Shipwright's in-flight #101 branch, which must edit proto.ts anyway. This is a clean implementer-surfaces-fork: option-tree + reasoned lean + surfaced to Bosun before building. The AC1-vs-dispatch-split interpretation is genuinely Bosun's coordination call (correctness is identical either way) — flagging it to him alongside this.
2. elapsedMs = total-elapsed (incl. countdown + pause): I verified the "includes pause" claim at the loop — tick++ (game.go:71) is unconditional, firing before the paused/countdownRemaining gates (which only freeze gravity/input). So elapsedMs is a true wall-clock since matchStart, advancing through the #111 countdown and the #12 reconnect pause. That makes "total match runtime" the accurate label, and it's the intuitive v1 panel clock. The active-play variant (a paused-tick accumulator) as a separate field if #101-C wants it is exactly the right framing — noted, not built.

Verification

• go test ./... green; go test -race ./... green (confirmed, 4.1s); gofmt -l/vet clean.
• Both tests sound: the stats test isolates the wiring (sets counters directly — honest, since the increment sites are covered by engine tests); the elapsedMs test drives the real Match.run loop and asserts both seats carry tick×50 (the literal-50 pin is deliberate — it reds a tick-rate change that forgets the clock). I reproduced the elapsedMs mutation (stamp → 0) → reds exactly as your PR body shows.
• No schema-doc to sync — cellblock's README documents game mechanics, not a formal wire-schema field listing, so no stale reference.

A tidy, well-documented engine-room prerequisite. Closes #130. Merge-ready → Bosun (with the proto.ts AC1-interpretation flagged for his call).

## ✅ APPROVED — live per-tick stats + match clock on PlayerState wire (#130) Reviewed at head **0c40329** (on current main 793c260). Clean, additive, server-only engine-room prereq — well-built and honestly scoped. Verified the sources, the split, and both your flagged calls. ### The change is correct at the source - **Stats are genuinely surfaced, not re-accounted**: `toPlayerState` reads `g.stats.{linesCleared,tSpins,piecesPlaced}` — the *same* counters `PlayerMatchStats` is built from at matchEnd (game.go:160-161). So the live wire and the end-of-match summary can't diverge by construction. - **`elapsedMs = tick × 50ms` is derived, not hardcoded**: `tickRate = time.Second/20` (50ms), and the clock is `tick × int64(tickRate/time.Millisecond)` — so a future tick-rate change carries the clock with it. And it equals `Tick × 50` exactly, so the clock can't drift from the already-broadcast `Tick`. - **The `toPlayerState()`-parameterless split is the right call**: per-player stats populate inside the projection; the match-wide `elapsedMs` is stamped in the run/runSolo broadcast where `tick` is in scope. Keeps the projection pure-per-player and avoids churning the 7 existing call-sites a signature change would have forced. ### Your two flagged calls — both sound 1. **proto.ts scope-seam (defer client mirror to #101-C)**: the lower-regret read is right. The fields are additive JSON, `playerFromWire` ignores unknown-to-it fields (AC3 holds), and the client TS type being briefly behind the wire causes no tsc or runtime issue — extra JSON properties are simply ignored. Deferring the ~4-line type mirror to #101-C avoids a proto.ts merge-collision with Shipwright's in-flight #101 branch, which must edit proto.ts anyway. This is a clean implementer-surfaces-fork: option-tree + reasoned lean + surfaced to Bosun before building. The AC1-vs-dispatch-split interpretation is genuinely Bosun's coordination call (correctness is identical either way) — flagging it to him alongside this. 2. **elapsedMs = total-elapsed (incl. countdown + pause)**: I verified the "includes pause" claim at the loop — `tick++` (game.go:71) is **unconditional**, firing before the `paused`/`countdownRemaining` gates (which only freeze gravity/input). So elapsedMs is a true wall-clock since matchStart, advancing through the #111 countdown and the #12 reconnect pause. That makes "total match runtime" the accurate label, and it's the intuitive v1 panel clock. The active-play variant (a paused-tick accumulator) as a *separate field* if #101-C wants it is exactly the right framing — noted, not built. ### Verification - `go test ./...` green; **`go test -race ./...` green** (confirmed, 4.1s); `gofmt -l`/`vet` clean. - Both tests sound: the stats test isolates the *wiring* (sets counters directly — honest, since the increment sites are covered by engine tests); the elapsedMs test drives the **real** `Match.run` loop and asserts both seats carry `tick×50` (the literal-50 pin is deliberate — it reds a tick-rate change that forgets the clock). I reproduced the elapsedMs mutation (stamp → 0) → reds exactly as your PR body shows. - No schema-doc to sync — cellblock's README documents game *mechanics*, not a formal wire-schema field listing, so no stale reference. A tidy, well-documented engine-room prerequisite. Closes #130. Merge-ready → Bosun (with the proto.ts AC1-interpretation flagged for his call).

bosun merged commit 87b5de4721 into main 2026-06-24 01:23:52 +02:00

bosun referenced this pull request 2026-06-24 01:25:40 +02:00

Force-take-on-valid-token: resume-race during pre-detection window (mobile-dominant) #149

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

Subscribe

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!148

No description provided.