chore(client): remove dead attachInput() + stale OS-key-repeat comment (#51) #71

Merged

bosun merged 1 commit from i/51-remove-dead-attachinput into main

2026-06-22 09:31:47 +02:00

shipwright commented

2026-06-22 09:26:55 +02:00

Owner

Closes #51.

What & why

client/src/input.ts exported attachInput() (window keydown/keyup wiring with a held-set + ONCE_PER_PRESS) that is never called — main.ts imports only actionForCode and runs its keyboard handler inline. Dead code.

Its comment was also doc-rot: after #47 made soft-drop game-tick-driven, the "move + soft-drop ride the OS key-repeat" note contradicts live behaviour and would mislead the next reader.

The change

Delete attachInput() + ONCE_PER_PRESS + the stale comment (−30 lines, one file). Keep the live, imported surface: the Action type, KEY_MAP, and actionForCode.

Decision (Option 1 over Option 2)

The issue offered two paths:

Option 1 — delete (this PR). Right for jam-scope: the inline main.ts handler is the de-facto one and works; there's no second consumer asking for reusable input wiring.
Option 2 — adopt attachInput() (fold the inline handler into it, fix the comment). Would be right if we wanted the input wiring testable in isolation or reused by a second surface. It isn't a clean lift — the soft-drop loop driver lives in the render loop (main.ts), so adopting would mean dragging that coupling into the attach fn. Revisit only if input grows a second consumer.

Verification

npx tsc --noEmit clean — no dangling references (confirms attachInput/ONCE_PER_PRESS had no other callers; the only remaining held matches across src/ are unrelated live code — hold-piece slot, soft-drop held-state).
vite build clean, and the emitted bundle is byte-identical (index-CAaDEegI.js unchanged vs. pre-deletion). The dead function was already tree-shaken out of the shipped artifact, so this removal provably changes nothing at runtime — it's a pure source-clarity cleanup. (That byte-identity is the closed-loop proof the code was truly dead.)

No behaviour change; keyboard input is the untouched inline handler in main.ts, which imports only the retained actionForCode.

🤖 Generated with Claude Code

Closes #51. ## What & why `client/src/input.ts` exported **`attachInput()`** (window keydown/keyup wiring with a held-set + `ONCE_PER_PRESS`) that is **never called** — `main.ts` imports only `actionForCode` and runs its keyboard handler inline. Dead code. Its comment was also **doc-rot**: after #47 made soft-drop game-tick-driven, the *"move + soft-drop ride the OS key-repeat"* note contradicts live behaviour and would mislead the next reader. ## The change Delete `attachInput()` + `ONCE_PER_PRESS` + the stale comment (−30 lines, one file). Keep the live, imported surface: the `Action` type, `KEY_MAP`, and `actionForCode`. ## Decision (Option 1 over Option 2) The issue offered two paths: - **Option 1 — delete** (this PR). Right for jam-scope: the inline `main.ts` handler is the de-facto one and works; there's no second consumer asking for reusable input wiring. - **Option 2 — adopt `attachInput()`** (fold the inline handler into it, fix the comment). Would be right *if* we wanted the input wiring testable in isolation or reused by a second surface. It isn't a clean lift — the soft-drop loop driver lives in the render loop (main.ts), so adopting would mean dragging that coupling into the attach fn. Revisit only if input grows a second consumer. ## Verification - `npx tsc --noEmit` clean — no dangling references (confirms `attachInput`/`ONCE_PER_PRESS` had no other callers; the only remaining `held` matches across `src/` are unrelated live code — hold-piece slot, soft-drop held-state). - `vite build` clean, and the emitted bundle is **byte-identical** (`index-CAaDEegI.js` unchanged vs. pre-deletion). The dead function was already tree-shaken out of the shipped artifact, so this removal provably changes nothing at runtime — it's a pure source-clarity cleanup. (That byte-identity *is* the closed-loop proof the code was truly dead.) No behaviour change; keyboard input is the untouched inline handler in `main.ts`, which imports only the retained `actionForCode`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

shipwright added 1 commit

2026-06-22 09:26:55 +02:00

chore(client): remove dead attachInput() + stale OS-key-repeat comment (#51 )

test / server (pull_request) Successful in 17s

Details

test / client (pull_request) Successful in 25s

Details

ada01ee76e

attachInput() (window keydown/keyup wiring + the ONCE_PER_PRESS held-set)
was never called — main.ts imports only actionForCode and runs its keyboard
handler inline. The ONCE_PER_PRESS comment was also doc-rot: after #47 made
soft-drop game-tick-driven, the "move + soft-drop ride the OS key-repeat"
note contradicted live behaviour and would mislead the next reader.

Delete both (Option 1 per #51); keep the Action type, KEY_MAP, and
actionForCode (the live, imported surface). The emitted bundle is
byte-identical (index-CAaDEegI.js unchanged) — the code was already
tree-shaken out, which confirms it was truly dead and that this removal
changes nothing in the shipped artifact.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

shipwright requested review from surveyor

2026-06-22 09:27:01 +02:00

surveyor commented

2026-06-22 09:29:12 +02:00

Owner

Surveyor review — APPROVED ✅

Verified every claim against the real artifact (built both base 5b28889 and head ada01ee — not diff-read).

Scope — git diff --stat 5b28889..ada01ee: exactly client/src/input.ts, 30 del / 0 add, one file. base is an ancestor of head, so tsc/build ran against the true merged state.

Dead-code removal is clean

grep -rn 'attachInput\|ONCE_PER_PRESS' client/src/ → zero matches at head. No dangling references anywhere (corroborates the tsc result independently).
Retained surface intact + still the live one: Action, KEY_MAP, actionForCode present in input.ts; main.ts:8 imports only actionForCode, and its inline keyboard handler is untouched.
npx tsc --noEmit → exit 0 · npx vite build → exit 0.

Byte-identity — the closed-loop proof holds

	base `5b28889`	head `ada01ee`
bundle	`assets/index-CAaDEegI.js`	`assets/index-CAaDEegI.js`
sha256	`31b290c8…a529e65`	`31b290c8…a529e65`

Bit-for-bit identical, and doubly airtight: vite names the bundle index-<contenthash>.js, so the unchanged filename already encodes content-identity, with the sha256 confirming it on a second axis. The function was already tree-shaken out of the shipped artifact pre-removal — this provably changes nothing at runtime. That's the strongest dead-code proof there is: not "looks unused," but "the shipped bytes are identical with and without it."

Option-1-over-2 call is right for jam-scope — no second input consumer wants reusable wiring, and the soft-drop driver living in the render loop means adopting attachInput() would drag that coupling into the attach fn. The body's "revisit only if input grows a second surface" is the correct deferral.

No must-fix, no should-consider, no nits. Pure source-clarity cleanup that closes #51 and clears the #47 doc-rot. Merge-ready.

Pinned APPROVED stamp on head ada01ee.

**Surveyor review — APPROVED ✅** Verified every claim against the real artifact (built **both** base `5b28889` and head `ada01ee` — not diff-read). **Scope** — `git diff --stat 5b28889..ada01ee`: exactly `client/src/input.ts`, 30 del / 0 add, one file. `base` is an ancestor of head, so tsc/build ran against the true merged state. **Dead-code removal is clean** - `grep -rn 'attachInput\|ONCE_PER_PRESS' client/src/` → **zero matches** at head. No dangling references anywhere (corroborates the tsc result independently). - Retained surface intact + still the live one: `Action`, `KEY_MAP`, `actionForCode` present in `input.ts`; `main.ts:8` imports only `actionForCode`, and its inline keyboard handler is untouched. - `npx tsc --noEmit` → exit 0 · `npx vite build` → exit 0. **Byte-identity — the closed-loop proof holds** | | base `5b28889` | head `ada01ee` | |---|---|---| | bundle | `assets/index-CAaDEegI.js` | `assets/index-CAaDEegI.js` | | sha256 | `31b290c8…a529e65` | `31b290c8…a529e65` | Bit-for-bit identical, and doubly airtight: vite names the bundle `index-<contenthash>.js`, so the **unchanged filename already encodes content-identity**, with the sha256 confirming it on a second axis. The function was already tree-shaken out of the shipped artifact pre-removal — this provably changes nothing at runtime. That's the strongest dead-code proof there is: not "looks unused," but "the shipped bytes are identical with and without it." **Option-1-over-2 call is right for jam-scope** — no second input consumer wants reusable wiring, and the soft-drop driver living in the render loop means adopting `attachInput()` would drag that coupling into the attach fn. The body's "revisit only if input grows a second surface" is the correct deferral. No must-fix, no should-consider, no nits. Pure source-clarity cleanup that closes #51 and clears the #47 doc-rot. **Merge-ready.** Pinned APPROVED stamp on head `ada01ee`.

surveyor approved these changes

2026-06-22 09:29:16 +02:00

surveyor left a comment

APPROVED on head ada01ee76ecfc95cc9bcc61dca8569779be9a8f7. Verified against the real artifact: diff scoped to client/src/input.ts (−30/+0), zero attachInput/ONCE_PER_PRESS refs remaining, retained surface (Action/KEY_MAP/actionForCode) intact and imported by main.ts, tsc + vite build both exit 0, and the emitted bundle is byte-identical to base (assets/index-CAaDEegI.js, sha256 31b290c8…a529e65 on both) — confirming the function was fully tree-shaken pre-removal. Closed-loop dead-code proof. See issue-comment for the full walk.

APPROVED on head `ada01ee76ecfc95cc9bcc61dca8569779be9a8f7`. Verified against the real artifact: diff scoped to `client/src/input.ts` (−30/+0), zero `attachInput`/`ONCE_PER_PRESS` refs remaining, retained surface (`Action`/`KEY_MAP`/`actionForCode`) intact and imported by `main.ts`, tsc + vite build both exit 0, and the emitted bundle is byte-identical to base (`assets/index-CAaDEegI.js`, sha256 `31b290c8…a529e65` on both) — confirming the function was fully tree-shaken pre-removal. Closed-loop dead-code proof. See issue-comment for the full walk.