Conversation
BT-291. The migration half of that ticket landed under BT-389, so this adds only the new demos, covering the parts of the random surface the migration never reached. - random-basics: shuffle vs shuffleInPlace, weighted, gaussian vs flat float, sign, and direction4 vs direction8, as five switchable scenes - seeded-worlds: two worlds side by side with seeds read back from BT.random.seedValue; copying a seed across makes the halves identical, plus a clone() vs fork() stream comparison - coordinate-patterns: an endless world from hash1i/hash2i/hash3i that stores nothing; the layer slider shows terrain ignoring the third coordinate while decorations follow it - noise: ValueNoise, PerlinNoise, and SimplexNoise at matched settings, with octaves switching noise2D for fbm2D, and a drift toggle driving the 3D variants Block sizes in the noise demo stop at 4px. 2px means 19,200 drawRectFill calls per frame and drops the demo to about 1 FPS, while 4px holds 60 FPS in every reachable state, drift and maximum octaves included. Registers all four in DEMO_ORDER and README, and corrects the README demo count, which was already stale at 40. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Vaclav Vancura <commit@vancura.dev>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe pull request adds four interactive randomness demos: random operations, seeded worlds, coordinate patterns, and noise landscapes. It registers the demos in navigation and documents them in the README. ChangesRandomness demos
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant NoiseDemo
participant NoiseGenerators
participant CachedField
participant Renderer
NoiseDemo->>NoiseGenerators: Sample configured Value, Perlin, or Simplex noise
NoiseGenerators->>CachedField: Rebuild cached field
CachedField->>Renderer: Provide palette-cell values
Renderer->>NoiseDemo: Render terrain or grayscale blocks
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/noise.js (1)
60-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the buffer size from
BLOCK_SIZES.
SMALLEST_BLOCKrepeats the smallest entry ofBLOCK_SIZES. If a smaller block size is added later,MAX_CELLSstays too small andthis.cells[...]writes past the end of theUint8Array, which JavaScript discards silently. Compute the smallest block from the array.Proposed refactor
-// The smallest block we ever draw decides how big the sample buffer has to be. -const SMALLEST_BLOCK = 4; -const MAX_CELLS = (DISPLAY_W / SMALLEST_BLOCK) * (DISPLAY_H / SMALLEST_BLOCK); +// The smallest block we ever draw decides how big the sample buffer has to be. Reading it +// straight out of BLOCK_SIZES means the buffer keeps fitting if a size is added later. +// Math.min(...BLOCK_SIZES) spreads the list out into separate arguments for Math.min. +const SMALLEST_BLOCK = Math.min(...BLOCK_SIZES); +const MAX_CELLS = Math.ceil(DISPLAY_W / SMALLEST_BLOCK) * Math.ceil(DISPLAY_H / SMALLEST_BLOCK);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/noise.js` around lines 60 - 65, Update the `SMALLEST_BLOCK` constant in `src/noise.js` to derive its value from `BLOCK_SIZES` rather than duplicating the current minimum. Keep `MAX_CELLS` based on this derived minimum so it automatically accommodates any smaller block size added to `BLOCK_SIZES`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/coordinate-patterns.js`:
- Around line 266-293: Update the tile-rendering loop in the grid drawing method
so the final row’s rectangle height is clipped to the remaining space within
GRID_H, rather than always using TILE. Apply the same clipped height to the tile
and decoration rendering as appropriate, while preserving full TILE height for
all earlier rows and keeping the frame drawn last.
In `@src/noise.js`:
- Around line 262-264: In the sample method’s animation comment, replace the
British spelling “travelled” with the American spelling “traveled”; leave the
surrounding behavior and code unchanged.
- Around line 385-387: Update the drift checkbox handling in the render/UI flow
to detect when the returned value differs from the previous this.animate value,
and set needsRebuild for that change so cached 2D/3D noise is regenerated.
Remove the stale comment claiming render() adjusts blockIndex when drift is
enabled; leave unrelated checkbox behavior unchanged.
In `@src/random-basics.js`:
- Around line 481-486: Update the clampInt upper bounds in the bug position
update to use area.x + area.width - 1 and area.y + area.height - 1, keeping the
lower bounds unchanged so the bug remains within the final pixel of its Rect2i
area.
In `@src/seeded-worlds.js`:
- Around line 360-364: Update rollSeed() to use a dedicated private random
generator for seed selection instead of the shared BT.random stream reseeded by
generateWorld(). Initialize or reuse that generator independently, while
preserving the inclusive SEED_MIN-to-SEED_MAX range.
---
Nitpick comments:
In `@src/noise.js`:
- Around line 60-65: Update the `SMALLEST_BLOCK` constant in `src/noise.js` to
derive its value from `BLOCK_SIZES` rather than duplicating the current minimum.
Keep `MAX_CELLS` based on this derived minimum so it automatically accommodates
any smaller block size added to `BLOCK_SIZES`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a80b8ce8-4fbc-408f-80bd-5d1394819d63
📒 Files selected for processing (6)
README.mdplugins/demo-order.jssrc/coordinate-patterns.jssrc/noise.jssrc/random-basics.jssrc/seeded-worlds.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
blit386/blit386(auto-detected)blit386/create-blit386(auto-detected)
Review follow-ups on the four new random demos. coordinate-patterns clipped its tile rows to the grid window. The grid is a cutout in the middle of the screen, not the screen itself, so the first and last rows spilled over the frame and covered the caption above and the panels below. Verified across all 64 scroll offsets that every drawn row and decoration now stays inside the window. seeded-worlds picks new seeds from a private generator. rollSeed() drew from BT.random, which generateWorld() had just reseeded, making each new seed a fixed consequence of the previous one: "Copy left seed" followed by "New right" handed back the identical seed every time, so the button looked broken. Confirmed against the engine's Random - four identical seeds before, four distinct after. noise rebuilds the field when drift is switched off, not just on. Drifting samples the 3D generators and standing still samples the 2D ones, so without this the picture kept showing the last 3D frame while the panel said otherwise. Also drops a stale comment about render() adjusting the block size, which stopped being true when the 2px option was removed, derives SMALLEST_BLOCK from BLOCK_SIZES so a finer size cannot outgrow the buffer, and corrects "travelled" to "traveled". random-basics keeps its bugs inside their squares. The clamp used area.x + area.width, one past the last pixel a Rect2i covers. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Vaclav Vancura <commit@vancura.dev>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/random-basics.js`:
- Around line 481-486: Update stepBug() so bug.pos is clamped to bounds inset by
the half-size of the 5x5 marker drawn by renderBug(), keeping the complete
marker inside the Rect2i while preserving the existing three-pixel movement and
area-boundary behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ac05a25-3d1c-4b91-be92-81e956263e32
📒 Files selected for processing (4)
src/coordinate-patterns.jssrc/noise.jssrc/random-basics.jssrc/seeded-worlds.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
blit386/blit386(auto-detected)blit386/create-blit386(auto-detected)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/seeded-worlds.js
- src/noise.js
- src/coordinate-patterns.js
Each bug is drawn as a 5x5 square centered on its position, but the clamp bounded that center to the wander area itself. At any edge the marker straddled the frame by two pixels: a bug at x = 24 in a square starting at x = 24 drew from x = 22. The bounds now come in by BUG_REACH at each end, and both the clamp and the drawing read the same two constants so they cannot drift apart. Step size and the wander areas are unchanged. Checked by driving 3,200 positions into every edge and corner of both squares: the whole marker stays inside the frame every time. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Vaclav Vancura <commit@vancura.dev>
Implements BT-291.
The migration half of BT-291 already landed under BT-389, so this PR adds only the new demos. That branch also
determined the scope: it reached for
int,float,pick,next,intInclusive,bool,angle,insideRect,and
pointInRange, leaving everything else in the random surface with no demo coverage. These four cover exactlythat gap, one per chapter of the engine's
guide-random.md.Demos
random-basicsshuffle/shuffleInPlace,weighted,gaussian,sign,direction4/direction8seeded-worldsBT.randomSeed,seedValue,clone(),fork()coordinate-patternshash1i,hash2i,hash3inoiseValueNoise,PerlinNoise,SimplexNoise,noise2D/3D,fbm2D/3DEach demo is one file, uses the shared UI kit for all on-screen chrome, works on touch as well as keyboard, and
carries the beginner comment style this repo requires.
Notable decisions
fork()beforeclone().fork()draws one number from the parent to seed the child, so cloning first wouldhave shown a clone that did not match its original - the opposite of the lesson.
seeded-worldsforks first.Noise block sizes stop at 4px. 2px blocks mean 19,200
drawRectFillcalls per frame and drop the demo to about1 FPS. This was measured, not assumed: it is the call count, not the noise sampling and not
Rect2iallocation,both of which I ruled out by measurement. 4px holds 60 FPS in every reachable state.
coordinate-patternslayer slider. Terrain comes fromhash2iso it ignores the layer, while decorations comefrom
hash3iso they follow it. Moving the slider shows the 2D/3D difference directly.Test plan
This repo has no automated tests by design, so everything below was checked by hand in the dev server.
pnpm run preflightpasses (format, lint, spellcheck, knip, docs:links, demo registry, build)random-basics: all five scenes render; weighted tally reached 73/23/9/0 against the requested 70/20/9/1seeded-worlds: "Copy left seed" makes both halves pixel-identical;clone()matches the original whilefork()diverges and reportsseedValueas unknowncoordinate-patterns: jumped 4,000 tiles away and back, compared screenshots withcmp- byte-identical,while the far location genuinely differs
noise: measured 60+ FPS in every reachable configuration (both block sizes, all three flavors, drift on,maximum octaves)
Notes
pnpm run devis broken from any git worktree becausevite.config.js:105hardcodes../blit386/dist/blit386.js. Not fixed here to keep this branch to BT-291's scope; worth its own ticket.🤖 Generated with Claude Code
Summary by CodeRabbit