Give the map a finger, stop it freezing the app, and fix the mobile layout (#151) - #152
Merged
Conversation
The map had mouse handlers only. On a phone it could not be moved at all, and there was no zoom whatsoever, because a phone has no wheel — while the hint under it read "Drag to pan · wheel to zoom" on the one device that can do neither. One finger pans, two pinch. Both halves were needed and only one of them is JavaScript: without `touch-action:none` on the canvas the browser claims the gesture as a page scroll BEFORE the listener runs, and `preventDefault` has nothing left to prevent. That is the same trap as the passive wheel listener in #135, and it would have cost another cycle to rediscover. A tap is a click with a finger, told from a drag by how far it moved — the same distance check the mouse already used. It is also the only way to read an area's note on a phone, because there is no hover. The desktop map needed the same treatment, and for the same reason its wheel handler is bound by hand: React registers touch listeners as passive too. The area tooltip now appears AT THE POINTER rather than in the bottom-right corner. On a fullscreen map a corner is nowhere near the area it describes, and on a touch screen it landed under the thumb that had just opened it. Clamped to the canvas, because anchored near an edge it would otherwise hang off the side. The "nothing generated here" note sat under the map page's floating control bar. Fixed in `mapPageHtml.ts` and not in the shared CSS: on the panel and the public site the bar is in the column ABOVE the canvas and the note is already right, so changing the shared rule would move a note that is not broken on three surfaces. The offset is measured from the bar's real height, because the bar wraps and at phone width becomes one scrolling row — any constant is wrong at some width. And the public site at phone width: headings stack instead of fighting their notes for one row, side padding drops from 22px to 16px between 460 and 720 (the width a phone in landscape actually reports, which the existing rules skipped), and the map's bar becomes one scrolling row instead of three that pushed the map off the screen. Verified: 12/12 gates. The smoke's DOM stub now RECORDS which events a page binds rather than swallowing them — it could not have caught a map with no touch handlers otherwise. Renaming `touchstart` fails it with "the panel map never binds touchstart; it cannot be panned by touch". The app stutter while the map loads is not in here. It needs measuring first, and it must not hold three working fixes behind it.
Loading the map made the whole interface stutter. Measuring said why, and said it was not the renderer: a region file is 1024 chunks, each one a decompress plus an NBT parse, and the loop ran to completion in one synchronous block. This thread answers every IPC call and reads the server console, so for the length of that block the app is frozen. Measured on a real 1024-chunk region: 132 ms of work in ONE block. Not a worker thread. The loop is already 1024 independent steps, so the cheap fix is to let the event loop run every 32 of them — same work, interruptible. A worker would have meant a second bundle entry, a new way to get packaging wrong inside the asar, and a fallback path to maintain, to solve a problem a `setImmediate` already solves. Same region after: 132 ms total, longest uninterrupted block 14 ms — under a frame. The number that matters is the block, not the total; the total is work that has to happen either way. `parseSlot` is shared by both the synchronous and the sliced parse, so the two cannot disagree about what one chunk becomes — a duplicated loop was the obvious risk and there is no second loop to drift. The synchronous `loadRegion` stays for callers that cannot await. Only the queue parses in slices, and only the queue ever parses at all. Verified: 12/12 gates, and this is now measured rather than asserted — the smoke builds a real region file of 1024 deflated NBT chunks, parses it through the queue's own path and fails if any single block exceeds 60 ms. Setting the slice back to 1024 fails it with "a parse slice blocked for 74 ms; the interface will stutter".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #151.
The map had no touch support at all
Mouse handlers only, so on a phone it could not be moved and there was no zoom whatsoever — while the hint under it read "Drag to pan · wheel to zoom" on the one device that can do neither.
One finger pans, two pinch. Both halves were needed and only one is JavaScript: without
touch-action:noneon the canvas the browser claims the gesture as a page scroll before the listener runs, andpreventDefaulthas nothing left to prevent. Same trap as the passive wheel listener in #135.A tap is a click with a finger, told from a drag by the distance check the mouse already used — and it is the only way to read an area's note on a phone, because there is no hover. The desktop map needed the same treatment, and for the same reason its wheel handler is hand-bound: React registers touch listeners as passive too.
The app froze while the map loaded
Measured, not guessed, and it was not the renderer. A region file is 1024 chunks — a decompress plus an NBT parse each — and the loop ran to completion in one synchronous block on the thread that answers every IPC call and reads the server console.
Not a worker thread. The loop is already 1024 independent steps, so letting the event loop run every 32 of them is the same work made interruptible — where a worker would have meant a second bundle entry, a new way to get packaging wrong inside the asar, and a fallback path to maintain.
parseSlotis shared by the synchronous and sliced paths, so there is no second loop to drift.Two things in the wrong place
mapPageHtml.ts, not the shared CSS — on the other three surfaces the bar is in the column above the canvas and the note is already right. The offset is measured from the bar's real height, because the bar wraps and at phone width becomes one scrolling row.Public site on a phone
Headings stack instead of fighting their notes for a row; side padding drops from 22px to 16px between 460 and 720 — the width a phone in landscape reports, which the existing rules skipped entirely; and the map bar becomes one scrolling row instead of three that pushed the map off the screen.
Verification
12/12 gates. The smoke's DOM stub now records which events a page binds rather than swallowing them — it could not have caught a map with no touch handlers otherwise.
touchstartrenamedthe panel map never binds touchstart; it cannot be panned by toucha parse slice blocked for 74 ms; the interface will stutter