benjivers/val Public Public: anyone can read this; the crew can change it.

The client should not re-derive the server's movement speed #28

Merged opened this in answer to

shipped production is serving it deploy b42aa0d

opened this

The snapshot now carries me.speed and the client's four-condition moveSpeed call is gone: speedOf in src/match.ts is exported, takes the state so the phases whose tick returns before it moves anybody (end, over) and the dead are inside the rule, and src/arena.ts sends its answer. static/val.js passes that number straight into stepLocal and imports only BODY_R from the engine — the bundle no longer contains moveSpeed at all. The client needs no reason, only the number; it still reads me.planting for the plant bar and prompt, and still knows the buy-phase pen, but that is a rule about where you may stand, not how fast.

Tests: 300 pass. Three that would fail without this — src/arena.test.ts walks the seat through haste, slow, an operator, planting, defusing, death and the two stopped phases and checks the sent number (and that slow, haste and defusing are off the wire); src/client.test.ts drives the real bundle with two snapshots a re-deriving client reads backwards (speed 0 with every visible reason to be running, speed 250 while planting > 0) and checks the drawn arrow — reverting val.js to the old derivation makes it fail with 82.5 units of lead where 0 is right; src/feel.test.ts now carries speed on its packets like the real snapshot and adds a hasted walk.

Feel, measured over three minutes of the jittery wire with the seat cycling through haste, slow, an operator and a plant (throwaway A/B harness, not committed): per-frame step spread over the 100 legs walked free of walls was 1.74% before (client-derived) and 1.74% after, corrections 0.67/min before and after, worst gap 30.6 vs 30.9 units. One deviation from the issue: Math.round is not a no-op — a whole number draws a hasted walk (337.5) half a unit a second fast and the cap clips it back every frame, which measured 1.97% spread. Sending it to a tenth, the rounding every other number in this snapshot gets, matches the exact rule to two decimals. Bytes: "speed":250, is 12 and "speed":337.5, is 14; dropping the three now-unread fields gives back ~36, so a seat's snapshot got about 23 bytes a tick smaller (measured mean 3473 → 3450), against ~3.4 KiB total.

First thirty seconds of the preview: press V for the top-down view and hold W — the arrow walks smoothly and ` ` shows jumps/min flat. Then walk onto a site with the spike and hold F: the body stops dead the instant the plant starts and does not creep, and when the round is decided everybody freezes under the banner — both of those are now the server's zero arriving, not the browser guessing. Deliberately left out: me.speed is passed through without a client-side guard, matching how val.js` treats every other snapshot field.

🤖 Generated with Claude Code

Closes #26

commented

Verified and good — this is going in. It needs a rebase onto main first, and the conflict is not yours.

pike's #25 landed a minute ago (zone positions are now gated on line of sight, same leak class as the shots one). It touches snapshotShape too, so the two of you collide on exactly one line: the ./match.ts import, where you add speedOf and pike adds PLAYER_R. Both are wanted. There may be a second small collision where your me block and its canSeeZone helper sit near each other.

Please rebase onto the current main and push. Nothing about your change needs rethinking — keep speed at a tenth, and keep the three fields you deleted deleted.

Two things I checked, so you know what not to break on the way through: moveSpeed is absent from the built bundle and not just from val.js, and both mutations bite — sending a constant 250 fails one test, dropping speed from the wire fails three including the real-bundle one. I would like all of that still true afterwards.

One note for the record: you were right and my issue was wrong about Math.round being a no-op. A hasted walk at 337.5 clipped back by the speed cap every frame is a real 1.97% versus 1.74% step spread, and sending a tenth is the correct call. Good catch, and the kind of thing I want argued back at me rather than silently obeyed.

commented

The branch now contains current main. One deviation from the request: I merged rather than rebased, because a rebase rewrites history and force pushes are refused here — the resulting tree is byte-identical to the rebase I did first (git diff against the rebased commit is empty), it is just reached by a merge commit, so squash or rebase-merge it on the way in if you want the linear history.

The conflict was the one you predicted and nothing else: the ./match.ts import line, now PLAYER_R (pike's canSeeZone) and speedOf (this branch's me.speed) side by side. Their canSeeZone helper sits at the top of snapshotShape and my speed field sits in the me block, so git merged those cleanly and I left both exactly as their authors wrote them.

Re-verified on the merged tree, not just before it: bun test is 301 pass / 0 fail (pike's leak test included); moveSpeed does not appear in the built bundle served at /static/val.25c21fe2df.js (75,310 bytes, grep -c returns 0); a live socket into the booted app carries speed: 250 with no haste, slow or defusing. Both mutations still bite exactly as before — a constant 250 on the client fails one test (the real-bundle one), and dropping speed from the wire fails three: the real-bundle prediction test, the arena snapshot test, and the bundled client connects, paints the HUD and draws a frame. speed stays at a tenth and the three deleted fields stay deleted.

Nothing else changed since the review; the first thirty seconds of the preview are still V for the top-down view and hold W to see the arrow walk smoothly, then hold F on the site to see the body stop dead the instant the plant starts.

🤖 Generated with Claude Code

Sign in to join this conversation.

↑↓ navigate ↵ open esc close

Keyboard shortcuts