pike/val Public Public: anyone can read this; nobody can change it.
forked from benjivers/val
workbenchthis machine
Say the property instead of guessing a number, and drop the race I added
The preview caught my fix being wrong, which is the second time today the
runner has been right and my laptop wrong.
I had stood the seat in a known place so the view would be fixed, then bound
the fill count to half the columns. Two things were wrong with that. The
wait after moving the camera was `await until(() => true)`, which waits for
nothing — so whether the new position had reached the client before the
frame was counted was a race, the exact bug class this commit exists to
remove. And the bound was still a guess: the preview drew 525 fills of 900
columns, a perfectly correct frame of a different view, and failed.
So the camera placement is gone, and with it the race. What is left is the
sentence the original comment already contained — a frame is not one fill a
screen column — written as the assertion:
fills > 0 it drew a world
fills < worldColumns() run-batched, not per column
with the column count read off the client's own offscreen buffer rather than
hardcoded, because the frame budget moves the world between 960 and 448
columns and no constant can tell "a run a wall" at one rung from "a fill a
column" at another. Every view satisfies these; only the regression does not.
Checked it still bites: forcing the renderer to emit one run per column fails
the test. 25 full-suite runs, 0 failures. 304 tests.
The counts that made this worth doing, for whoever meets it next: 154-236
fills running alone, 31 on the run that failed, 525 in the preview. All
correct frames. The old floor of 40 and ceiling of 900 were luck.
1 file
changed +20
−35
src/client.test.ts +20 −35
| @@ -9,8 +9,7 @@ | ||
| 9 | 9 | */ |
| 10 | 10 | |
| 11 | 11 | import { expect, test } from "bun:test"; |
| 12 | import { TICK } from "./arena.ts"; | |
| 13 | import { HEIGHT, WIDTH } from "./map.ts"; | |
| 12 | ||
| 14 | 13 | import { mkdtempSync } from "node:fs"; |
| 15 | 14 | import { tmpdir } from "node:os"; |
| 16 | 15 | import { join } from "node:path"; |
| @@ -264,33 +263,25 @@ test("the bundled client connects, paints the HUD and draws a frame", async () = | ||
| 264 | 263 | |
| 265 | 264 | // A frame of the first-person view draws the world into its own buffer and |
| 266 | 265 | // blits it, with the minimap's tiles blitted too. It is a picture, but it is |
| 267 | // not one fill a screen column, nor one a map tile: the map has 570 tiles | |
| 268 | // and the buffer up to 960 columns, and the frame used to pay for both. | |
| 269 | // The minimap's tiles are the blit; the walls are a run a fill. | |
| 266 | // NOT one fill a screen column, nor one a map tile: the map has 570 tiles and | |
| 267 | // the buffer up to 960 columns, and the frame used to pay for both. The | |
| 268 | // minimap's tiles are the blit; the walls are a run a fill. | |
| 270 | 269 | // |
| 271 | // What is asserted is that run-batching, and NOT a particular count. This | |
| 272 | // used to demand more than 40 fills, which failed about one full-suite run | |
| 273 | // in twenty-five while passing forty runs out of forty on its own. The loop | |
| 274 | // above advances the shared match until somebody dies — anywhere between | |
| 275 | // five and two hundred seconds of play — so the view being measured was | |
| 276 | // wherever that happened to leave the seat: 154 to 236 fills when run | |
| 277 | // alone, and 31 on the run that failed. A floor on a number nobody chose, | |
| 278 | // over a view nobody fixed, is not a test. | |
| 270 | // That sentence is the whole assertion, and it is now written as one. It | |
| 271 | // used to demand more than 40 fills and fewer than 900, which failed about | |
| 272 | // one full-suite run in twenty-five: the loop above advances the SHARED | |
| 273 | // match until somebody dies — five to two hundred seconds of play — so the | |
| 274 | // view being counted was wherever that left the seat. Alone it draws 154 to | |
| 275 | // 236 fills; the run that failed drew 31, and a preview drew 525. Every one | |
| 276 | // of those is a correct frame of a different view. A bound on the count of | |
| 277 | // an unfixed view can only ever be luck. | |
| 279 | 278 | // |
| 280 | // So the seat is stood somewhere known first. The walls in shot are then | |
| 281 | // the same every run; the few fills that still move are the bodies standing | |
| 282 | // among them, which is why the bound below is a fraction rather than an | |
| 283 | // equality. | |
| 284 | const arenaNow = await import("./arena.ts"); | |
| 285 | const camera = arenaNow.match().players.find((p) => !p.bot)!; | |
| 286 | camera.x = WIDTH / 2; | |
| 287 | camera.y = HEIGHT / 2; | |
| 288 | camera.angle = 0; | |
| 289 | camera.alive = true; | |
| 290 | camera.blind = 0; | |
| 291 | arenaNow.advance(TICK * 2); // a snapshot carrying that position reaches the client | |
| 292 | await until(() => true); | |
| 293 | ||
| 279 | // So the bounds are the two things true of EVERY view: it drew something, | |
| 280 | // and it cost fewer fills than the frame had columns. The column count is | |
| 281 | // read off the client's own offscreen buffer rather than hardcoded, because | |
| 282 | // the frame budget moves the world between 960 and 448 columns and a | |
| 283 | // constant cannot tell "a run a wall" at one rung from "a fill a column" at | |
| 284 | // another. | |
| 294 | 285 | const tally = () => ({ |
| 295 | 286 | fills: drawn.filter((op) => op.startsWith("fillRect")).length, |
| 296 | 287 | blits: drawn.filter((op) => op.startsWith("drawImage")).length, |
| @@ -304,14 +295,8 @@ test("the bundled client connects, paints the HUD and draws a frame", async () = | ||
| 304 | 295 | const again = tally(); |
| 305 | 296 | |
| 306 | 297 | expect(fp.blits).toBeGreaterThanOrEqual(2); // the world, and the map's tiles |
| 307 | // It drew a world, not an empty buffer. | |
| 308 | expect(fp.fills).toBeGreaterThan(0); | |
| 309 | // Run-batched, said against the columns the frame ACTUALLY used. The budget | |
| 310 | // moves the world between 960 and 448 columns depending on what frames cost, | |
| 311 | // so no constant can separate "a run a wall" from "a fill a column": at the | |
| 312 | // coarsest rung a per-column frame is 448, which sits under any constant | |
| 313 | // loose enough to survive the finest one. | |
| 314 | expect(fp.fills).toBeLessThan(worldColumns() / 2); | |
| 298 | expect(fp.fills).toBeGreaterThan(0); // it drew a world, not an empty buffer | |
| 299 | expect(fp.fills).toBeLessThan(worldColumns()); // run-batched, not per column | |
| 315 | 300 | // The same state drawn twice costs the same. A frame whose cost wanders |
| 316 | 301 | // while nothing moves is a renderer bug, and it is the shape of thing the |
| 317 | 302 | // old bound was mistaking for flakiness. |