|
| 1 | +# Development conventions |
| 2 | + |
| 3 | +Working notes for this repository: the commands, the architecture, and the |
| 4 | +handful of invariants that are not obvious from reading a single file. If you |
| 5 | +are changing physics or rendering, read the **Invariants** section first — every |
| 6 | +entry in it corresponds to a bug that actually shipped. |
| 7 | + |
| 8 | +## Commands |
| 9 | + |
| 10 | +| Command | What it does | |
| 11 | +|---|---| |
| 12 | +| `npm run dev` | Vite dev server on :3000, opens a browser. **Does not typecheck.** | |
| 13 | +| `npm run typecheck` | `tsc --noEmit` over `src`, `tests` and `vite.config.ts` | |
| 14 | +| `npm test` | Vitest, headless, no DOM — the whole simulation core | |
| 15 | +| `npm run test:watch` | the same in watch mode | |
| 16 | +| `npm run build` | `tsc && vite build` into `dist/` | |
| 17 | +| `npm run preview` | serve the built `dist/` | |
| 18 | +| `npm run smoketest` | build first, then drive `dist/` in headless Chromium | |
| 19 | +| `npm run screenshots` | the smoke test again, writing `screenshots/*.png` | |
| 20 | +| `npm run verify:install` | would CI's npm accept `package-lock.json`? | |
| 21 | + |
| 22 | +`npm run dev` uses esbuild, which strips types without checking them. **A green |
| 23 | +dev server proves nothing about whether the project builds** — this is exactly |
| 24 | +how the repo reached its first public commit with eleven `tsc` errors and no |
| 25 | +working `npm run build`. Run `npm run typecheck` before believing anything. |
| 26 | + |
| 27 | +## Architecture |
| 28 | + |
| 29 | +Deliberately layered so that everything except `Renderer` and `main` is testable |
| 30 | +without a browser: |
| 31 | + |
| 32 | +``` |
| 33 | +main.ts p5 sketch: input, UI wiring, the frame loop |
| 34 | + ├── Camera zoom/pan, screen<->world, and the visible world rectangle |
| 35 | + ├── PhysicsEngine particle list, pairwise forces, integration |
| 36 | + │ ├── Particle state, F=ma, the force law, trail |
| 37 | + │ └── VectorField field sampling (uniform | adaptive) + OccupancyGrid |
| 38 | + └── Renderer all drawing; the only file that talks to p5's canvas API |
| 39 | + └── Vector2D immutable 2D vector maths, used everywhere |
| 40 | +``` |
| 41 | + |
| 42 | +**Only `main.ts`, `Camera.ts` and `Renderer.ts` import p5.** `PhysicsEngine`, |
| 43 | +`Particle`, `VectorField` and `Vector2D` are plain TypeScript, which is why 73 |
| 44 | +tests run under Node in about a second with no DOM and no canvas. Keep it that |
| 45 | +way: if a physics change seems to need p5, the abstraction is in the wrong place. |
| 46 | + |
| 47 | +`Camera` imports only the `ViewBounds` *type* from `VectorField`, so the |
| 48 | +dependency is erased at compile time. |
| 49 | + |
| 50 | +## Invariants |
| 51 | + |
| 52 | +### Forces are cleared at the start of a step, never at the end |
| 53 | + |
| 54 | +`Particle.netForce` is what the renderer draws as the orange force arrow, and it |
| 55 | +reads it *after* `PhysicsEngine.step()` returns. `Particle.update()` must |
| 56 | +therefore leave `netForce` alone; clearing belongs in `resetForces()`, called at |
| 57 | +the top of `computeForces()`. |
| 58 | + |
| 59 | +This was wrong originally — `update()` zeroed `netForce` on its way out, so the |
| 60 | +renderer's `netForce.magnitude() > 0` gate never once passed and the arrow |
| 61 | +advertised in the README and the on-page legend had never drawn a pixel. |
| 62 | +Pinned by `tests/PhysicsEngine.test.ts` → *"leaves netForce readable after a |
| 63 | +step"*. |
| 64 | + |
| 65 | +The same ordering is what velocity Verlet will need when it lands (roadmap M1), |
| 66 | +since it caches accelerations across steps. |
| 67 | + |
| 68 | +### p5's colour mode is RGB, except inside the vector-field pass |
| 69 | + |
| 70 | +`Renderer` assumes p5's default **RGB** mode. The only exception is |
| 71 | +`drawVectorField()`, which pushes, switches to `HSB(360, 100, 100, 100)` because |
| 72 | +hue encodes force strength, and pops. |
| 73 | + |
| 74 | +Originally `setup()` set HSB **globally** while most of `Renderer` was written |
| 75 | +for RGB. Nothing errored; the colours were simply wrong and stayed wrong: |
| 76 | + |
| 77 | +| call | intent | what actually rendered | |
| 78 | +|---|---|---| |
| 79 | +| `background(10, 15, 30)` | dark navy | `rgb(77, 67, 65)`, a brown | |
| 80 | +| `fill(150, 200, 255)` | pale blue body | bright green | |
| 81 | +| `stroke(255, 200, 0)` on the drag preview | amber | **black** — HSB brightness 0 | |
| 82 | + |
| 83 | +Colour constants now live at the top of `Renderer.ts` and match the hex values |
| 84 | +in the legend in `index.html`. If you change one, change both. |
| 85 | + |
| 86 | +`push()`/`pop()` save and restore colour mode, so a scoped switch is safe — but |
| 87 | +a bare `colorMode()` call leaks to every later draw call in the frame. |
| 88 | + |
| 89 | +### The vector field is built for the camera's view, not for the canvas |
| 90 | + |
| 91 | +`VectorField.update()` takes `ViewBounds` from `Camera.getViewBounds()`. It used |
| 92 | +to sample a fixed box the size of the canvas centred on the world origin, so |
| 93 | +panning away from the origin showed empty space no matter what was there. |
| 94 | + |
| 95 | +Sample lattices are anchored to **world** coordinates (`Math.floor(min / grid) * |
| 96 | +grid`), not to the viewport or to the particle. Anchoring them to a moving |
| 97 | +reference makes every arrow crawl across the screen as the camera or the body |
| 98 | +moves. Pinned by *"anchors its lattice to the world, not to the particle"*. |
| 99 | + |
| 100 | +### Sample count is capped, and uniform mode coarsens rather than truncates |
| 101 | + |
| 102 | +`MAX_SAMPLES` is 12,000. Visible world area grows as 1/zoom², so at the minimum |
| 103 | +zoom of 0.1 an uncapped uniform lattice asks for ~113,000 arrows and freezes the |
| 104 | +tab. Uniform mode increases its spacing to fit the budget; adaptive mode stops |
| 105 | +adding. Do not remove the cap without replacing it. |
| 106 | + |
| 107 | +### `OccupancyGrid` must answer exactly what the linear scan answered |
| 108 | + |
| 109 | +Adaptive mode rejects a candidate sample if an accepted one is already within |
| 110 | +`gridSize / 2` on **both** axes. That was a scan over every accepted sample — |
| 111 | +quadratic, and the dominant frame cost. `OccupancyGrid` is a spatial hash that |
| 112 | +narrows the search without changing the predicate. |
| 113 | + |
| 114 | +Its cell size must be **at least the largest `half` ever queried** (the outermost |
| 115 | +zone's `1.2 × baseGridSize`, halved) or lookups will miss points in adjacent |
| 116 | +cells and silently emit duplicates. `tests/OccupancyGrid.test.ts` compares it |
| 117 | +against the naive implementation over thousands of queries, including a |
| 118 | +clustered distribution, because uniform random points rarely collide and would |
| 119 | +let a broken grid pass. |
| 120 | + |
| 121 | +### UI state is read from the DOM at startup, never duplicated in TypeScript |
| 122 | + |
| 123 | +`syncStateFromControls()` pushes every control's markup value into the |
| 124 | +simulation during `setup()`. The sliders in `index.html` are the single source |
| 125 | +of truth for starting values. |
| 126 | + |
| 127 | +Before this existed the two drifted: the mass slider read 200 while new bodies |
| 128 | +were created with mass 50, and the range slider read 150 while the field used |
| 129 | +300. If you add a control, add it to `syncStateFromControls()` as well as to |
| 130 | +`setupUI()`. |
| 131 | + |
| 132 | +### Native DOM listeners, not p5's `select().input()` |
| 133 | + |
| 134 | +`@types/p5` declares `input()` and `changed()` on the **p5 instance**, not on |
| 135 | +`p5.Element`, so the wrapper form does not typecheck. Use |
| 136 | +`document.getElementById` and `addEventListener` — which is what the typed `el()` |
| 137 | +helper in `main.ts` is for. |
| 138 | + |
| 139 | +### `Vector2D` is immutable |
| 140 | + |
| 141 | +Every operation returns a new vector. `a.add(b)` does not modify `a`. Convenient, |
| 142 | +and it allocates: the field sampler creates a few thousand short-lived vectors |
| 143 | +per frame. That is currently well within budget, and is the first thing to |
| 144 | +revisit if the frame time regresses. |
| 145 | + |
| 146 | +## Testing |
| 147 | + |
| 148 | +`tests/` mirrors `src/`. Vitest runs in the `node` environment — no jsdom, no |
| 149 | +canvas. `Camera` is tested with a five-property stub in place of p5. |
| 150 | + |
| 151 | +What belongs where: |
| 152 | + |
| 153 | +- **Unit tests** — anything expressible without a browser: the force law, |
| 154 | + integration, field sampling, camera maths, the occupancy grid. |
| 155 | +- **`tools/smoketest.mjs`** — anything that only exists once pixels are on a |
| 156 | + canvas. It serves the real `dist/` over HTTP, drives the app with real mouse |
| 157 | + and wheel events, and **judges colour by sampling the canvas backing store**, |
| 158 | + never by eye. Every check in it maps to a defect that shipped. |
| 159 | + |
| 160 | +Both run in CI, on Linux and Windows for the unit tests. |
| 161 | + |
| 162 | +## Before pushing |
| 163 | + |
| 164 | +1. `npm run typecheck` |
| 165 | +2. `npm test` |
| 166 | +3. `npm run build` — must be warning-free; the chunk-size limit is set so that |
| 167 | + real growth in the app bundle surfaces |
| 168 | +4. `npm run smoketest` |
| 169 | +5. `npm run verify:install` if `package.json` or the lockfile changed |
| 170 | + |
| 171 | +Step 5 is not paranoia. CI's npm is decided by the Node version in `.nvmrc` |
| 172 | +(Node 22 bundles npm 10), which is routinely a major behind a developer's global |
| 173 | +npm, and npm 11 will happily reinstall from a lockfile that npm 10 rejects. See |
| 174 | +the header comment in `tools/verify-install.mjs`. |
| 175 | + |
| 176 | +## Dependency notes |
| 177 | + |
| 178 | +`vite` is pinned to `^7` deliberately. `vitest` 4 requires vite `^6 || ^7 || ^8`; |
| 179 | +against an older pin, npm resolves the conflict by nesting a second copy of vite |
| 180 | +and writing an incomplete lockfile that CI's npm then refuses. Keep the two in |
| 181 | +step. The current tree has **0** npm advisories — check it stays that way. |
| 182 | + |
| 183 | +p5 is LGPL-2.1 and is deliberately emitted as its own chunk. See |
| 184 | +[`NOTICE.md`](NOTICE.md) before changing `build.rollupOptions`. |
0 commit comments