From c4dbe9633510ba11c9db53869a3dd0f21e718515 Mon Sep 17 00:00:00 2001 From: CaYatur Date: Wed, 5 Aug 2026 21:11:20 +0300 Subject: [PATCH 1/2] Stop the map throwing away the ground you just looked at MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three faults behind "it loads piece by piece, and going somewhere else makes the first place load all over again". **It deleted what it had.** The desktop trimmed with `trimTiles(tiles, visibleChunks())` — keep the current viewport, drop everything else — so panning one screen deleted the screen you came from and panning back re-fetched all of it. The web kept an 8-chunk margin, which only moved the edge. Replaced by `tilesToDrop` in @shared/livemap: drop nothing until the cache is over its limit, then drop FARTHEST FROM THE VIEW first, so what survives is a ring of recently visited ground. The limit was also wrong in a way that guaranteed churn: 2048 tiles against a viewport that requests up to 4096, so a single view could evict itself and re-fetch on the next draw. It is now 8192, and the invariant that it must exceed the viewport cap is asserted rather than remembered. **It asked in bands.** 64 chunks a request, one in flight, is 64 sequential round trips for a full viewport. Now 512 — eight rounds. Both HTTP surfaces already gzip tile responses, so that is ~82 KB on the wire against ~10 KB. **It re-asked for what it was already waiting for.** The viewport is walked in order, so the chunks whose region was still being parsed were always at the front of the next request: the map spun on one band while the rest stayed blank. A chunk that comes back neither drawn nor listed empty is now left alone for 400 ms, and the request moves on to the rest of the view. `MAX_TILES_PER_REQUEST` is one constant in @shared/livemap, imported by the main process and interpolated into MAP_JS. It was two literals and a constant, and they have to agree: a client asking for more than the server reads gets a response that mentions neither the extra chunks nor a reason, and both handlers treated that silence as "empty" via `!pending` and would have blanked them permanently. That inference is gone — `empty` is the server's actual answer, and every requested chunk comes back drawn, listed empty, or still pending. Verification, all proved failable first (MSMS_SMOKE_WEB, by exit code): - restoring the viewport-only rule fails with "the cache was not trimmed to its limit: 2500" - inverting the distance order, which still trims to exactly the limit, fails with "a tile in view was dropped: 0,0" - desyncing MAP_JS back to 64 fails with "the web map asks for 64 but the server reads 512" The retention fixture is 10000 tiles, past the 8192 limit, because the body does not run below it — and it asserts the pan property directly: after panning one screen, at least 2000 of the previous screen's 2500 tiles are still held. Gates green: MSMS_SMOKE, _WORLDS, _MODUPDATE, _WEB. Closes #159 --- docs/openapi.json | 2 +- src/main/core/worldTiles.ts | 11 ++- src/main/smoke.ts | 88 +++++++++++++++++++++++- src/renderer/src/components/LiveMap.tsx | 68 ++++++++++++++---- src/shared/apiSurface.ts | 2 +- src/shared/livemap.ts | 91 +++++++++++++++++++++++++ src/shared/mapUi.ts | 57 +++++++++++++--- 7 files changed, 292 insertions(+), 27 deletions(-) diff --git a/docs/openapi.json b/docs/openapi.json index 4970ebb..4328b2f 100644 --- a/docs/openapi.json +++ b/docs/openapi.json @@ -2801,7 +2801,7 @@ "get": { "operationId": "getServersIdMapTiles", "summary": "Rendered surface colours for the requested chunks.", - "description": "Scope `view` on the server.\n\nAsk with `?c=cx,cz;cx,cz` (max 64) and `?dim=`. Answers only with regions already parsed and queues the rest — `pending` says how many are still coming, so a caller polls rather than blocking. A request never parses a region itself.", + "description": "Scope `view` on the server.\n\nAsk with `?c=cx,cz;cx,cz` (max 512) and `?dim=`. Answers only with regions already parsed and queues the rest — `pending` says how many are still coming, so a caller polls rather than blocking. A request never parses a region itself.", "tags": [ "players" ], diff --git a/src/main/core/worldTiles.ts b/src/main/core/worldTiles.ts index 8fc7453..f5a4409 100644 --- a/src/main/core/worldTiles.ts +++ b/src/main/core/worldTiles.ts @@ -26,6 +26,7 @@ import { createHash } from 'node:crypto' import { inflateSync, gunzipSync, gzipSync } from 'node:zlib' import { join } from 'node:path' import { cacheDir } from '../paths' +import { MAX_TILES_PER_REQUEST } from '@shared/livemap' import { decodeRegionTiles, encodeRegionTiles, normalizeMapPerf } from '@shared/tileCache' import type { MapPerfConfig } from '@shared/tileCache' import * as nbt from 'prismarine-nbt' @@ -946,8 +947,14 @@ async function drain(): Promise { } } -/** `cx,cz;cx,cz…`, capped so one call cannot ask for a whole world. */ -export const MAX_TILES_PER_REQUEST = 64 +/** + * `cx,cz;cx,cz…`, capped so one call cannot ask for a whole world. + * + * Re-exported rather than declared: the clients have to cap against the SAME + * number, and when they did not, everything past the server's limit came back + * unmentioned and was marked permanently empty (#159). + */ +export { MAX_TILES_PER_REQUEST } export function parseWantedTiles(raw: string | null | undefined): { cx: number; cz: number }[] { const out: { cx: number; cz: number }[] = [] diff --git a/src/main/smoke.ts b/src/main/smoke.ts index 1d93515..bf0b187 100644 --- a/src/main/smoke.ts +++ b/src/main/smoke.ts @@ -132,7 +132,7 @@ import * as areasMod from '@shared/chunkAreas' import * as areasMod2 from './core/chunkAreas' import * as tilesMod from './core/worldTiles' import * as tex from '@shared/textures' -import { MAP_CSS, MAP_HTML } from '@shared/mapUi' +import { MAP_CSS, MAP_HTML, MAP_JS } from '@shared/mapUi' import { getMapPageHtml } from './web/mapPageHtml' import * as pngMod from './core/png' import { deflateSync } from 'node:zlib' @@ -155,8 +155,12 @@ import { screenToWorld, worldToScreen, zoomAt, + tilesToDrop, MAX_SCALE, + MAX_TILES_PER_REQUEST, + MAX_VIEWPORT_CHUNKS, MIN_SCALE, + TILE_KEEP_LIMIT, PUBLIC_MAP_DEFAULTS } from '@shared/livemap' import { normalizeMapPage, mapPagePublic, MAP_PAGE_DEFAULTS } from '@shared/mapPage' @@ -9012,6 +9016,88 @@ export async function runWebSmoke(): Promise { if (PUBLIC_MAP_DEFAULTS.heads) return fail('avatar heads are on by default') } + // ---- what a client keeps when it pans (#159) ---- + { + // The invariant the whole policy rests on. Below this a single viewport + // exceeds the cache, so the map evicts tiles it just fetched and + // re-fetches them on the next draw — an endless loop, and the reason + // the old limit of 2048 was wrong against a 4096-chunk viewport. + if (TILE_KEEP_LIMIT <= MAX_VIEWPORT_CHUNKS) { + return fail('the tile cache is smaller than one viewport: ' + TILE_KEEP_LIMIT) + } + + // Under the limit nothing is given up, however far the view has moved. + const few = ['0,0', '1,0', '900,900'] + if (tilesToDrop(few, { x0: 0, x1: 1, z0: 0, z1: 1 }).length) { + return fail('tiles were dropped while the cache was under its limit') + } + + // A cache PAST the limit, which is the only state the body runs in — a + // fixture of a few dozen would step straight over it and pass with the + // whole policy deleted. + const held: string[] = [] + for (let z = 0; z < 100; z++) for (let x = 0; x < 100; x++) held.push(x + ',' + z) + if (held.length <= TILE_KEEP_LIMIT) return fail('the retention fixture never crosses the limit') + + // The view sits in the top-left corner; the far corner is what should go. + const box = { x0: 0, x1: 49, z0: 0, z1: 49 } + const dropped = tilesToDrop(held, box) + if (!dropped.length) return fail('nothing was dropped by an over-full cache') + if (held.length - dropped.length !== TILE_KEEP_LIMIT) { + return fail('the cache was not trimmed to its limit: ' + (held.length - dropped.length)) + } + const gone = new Set(dropped) + // Nothing on screen may be given up while anything off screen is held — + // that is precisely the eviction that made a map re-fetch itself. + for (let z = box.z0; z <= box.z1; z++) { + for (let x = box.x0; x <= box.x1; x++) { + if (gone.has(x + ',' + z)) return fail('a tile in view was dropped: ' + x + ',' + z) + } + } + if (!gone.has('99,99')) return fail('the farthest tile survived while nearer ones went') + + // The reported bug, as a property: pan one screen right, and the screen + // you came from is still held. The old rule kept the viewport and + // nothing else, so panning back re-fetched all of it. + const panned = new Set(tilesToDrop(held, { x0: 50, x1: 99, z0: 0, z1: 49 })) + let survivors = 0 + for (let z = 0; z <= 49; z++) for (let x = 0; x <= 49; x++) { + if (!panned.has(x + ',' + z)) survivors++ + } + if (survivors < 2000) { + return fail('panning one screen threw away the previous one: only ' + survivors + ' left') + } + + // Deterministic: two runs on the same input must agree, or a redraw + // between them silently changes what is held. + if (tilesToDrop(held, box).join('|') !== dropped.join('|')) { + return fail('the retention policy is not deterministic') + } + } + + // ---- every client caps at the number the server reads (#159) ---- + { + // A client that asks for more than the server looks at gets a response + // that mentions neither the extra chunks nor a reason, and the handlers + // used to read that silence as "empty" and blank them for good. The web + // map is a STRING, so the only way to check its copy is to read it. + const sliced = [...MAP_JS.matchAll(/want\.slice\(0,(\d+)\)/g)].map((m) => Number(m[1])) + if (sliced.length !== 1) { + return fail('expected exactly one tile-request cap in MAP_JS, found ' + sliced.length) + } + if (sliced[0] !== MAX_TILES_PER_REQUEST) { + return fail('the web map asks for ' + sliced[0] + ' but the server reads ' + MAX_TILES_PER_REQUEST) + } + // And the server really does read that many, rather than stopping at an + // older constant somewhere in the parser. + const asked: string[] = [] + for (let i = 0; i < MAX_TILES_PER_REQUEST + 50; i++) asked.push(i + ',0') + const parsed = tilesMod.parseWantedTiles(asked.join(';')) + if (parsed.length !== MAX_TILES_PER_REQUEST) { + return fail('the server parsed ' + parsed.length + ' of ' + MAX_TILES_PER_REQUEST + ' asked for') + } + } + // ---- the endpoints ---- // The map feed is view-gated and honest about the bridge being absent. r = await get('/api/servers/' + id + '/map', ft) diff --git a/src/renderer/src/components/LiveMap.tsx b/src/renderer/src/components/LiveMap.tsx index f2fd674..646e563 100644 --- a/src/renderer/src/components/LiveMap.tsx +++ b/src/renderer/src/components/LiveMap.tsx @@ -4,8 +4,19 @@ import { Map as MapIcon, Flame, Gauge, Shapes, Plus, Trash2, Check, X } from 'lu import { useStore } from '../store' import { normalizeMapPerf } from '@shared/tileCache' import type { MapPerfConfig } from '@shared/tileCache' -import { fitView, heatmap, mapBounds, panBy, screenToWorld, worldToScreen, zoomAt } from '@shared/livemap' -import type { LivePlayer, MapView, Viewport } from '@shared/livemap' +import { + fitView, + heatmap, + mapBounds, + panBy, + screenToWorld, + tilesToDrop, + worldToScreen, + zoomAt, + MAX_TILES_PER_REQUEST, + MAX_VIEWPORT_CHUNKS +} from '@shared/livemap' +import type { ChunkBox, LivePlayer, MapView, Viewport } from '@shared/livemap' import { avatarUrl } from '@shared/profile' import type { StructureMark } from '@shared/regionFormat' import { iconFor, ICON_BOX } from '@shared/mapIcons' @@ -118,16 +129,21 @@ function headFor( * Panning a big world would otherwise hold every chunk ever looked at. * * Each tile is small, but "small times unbounded" is still unbounded, and this - * runs for as long as the app is open. Dropping the ones no longer near the - * view costs a re-fetch that is already cached in the main process. + * runs for as long as the app is open. WHICH ones to give up is + * `tilesToDrop` — shared with the web map, and farthest-from-the-view first, + * because the old rule here kept the current viewport and deleted everything + * else the moment the cache passed its limit (#159). */ function trimTiles( tiles: Map, - keep: { cx: number; cz: number }[] + marks: Map, + box: ChunkBox | null ): void { - if (tiles.size <= 2048) return - const wanted = new Set(keep.map((c) => c.cx + ',' + c.cz)) - for (const k of tiles.keys()) if (!wanted.has(k)) tiles.delete(k) + if (!box) return + for (const k of tilesToDrop(tiles.keys(), box)) { + tiles.delete(k) + marks.delete(k) + } } export function LiveMap({ serverId }: { serverId: string }): JSX.Element { @@ -270,11 +286,14 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { setCleared(await window.msms.clearMapCache()) tiles.current.clear() markStore.current.clear() + waiting.current.clear() setTick2((n) => n + 1) } const headCache = useRef(new Map()) const tiles = useRef(new Map()) const tilesPending = useRef(false) + /** Chunk -> when it is worth asking for again, while its region is read. */ + const waiting = useRef(new Map()) const [tick2, setTick2] = useState(0) const chunkBox = useCallback((): { x0: number; x1: number; z0: number; z1: number } | null => { @@ -293,7 +312,7 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { const visibleChunks = useCallback((): { cx: number; cz: number }[] => { const b = chunkBox() if (!b) return [] - if ((b.x1 - b.x0 + 1) * (b.z1 - b.z0 + 1) > 4096) return [] + if ((b.x1 - b.x0 + 1) * (b.z1 - b.z0 + 1) > MAX_VIEWPORT_CHUNKS) return [] const out: { cx: number; cz: number }[] = [] for (let z = b.z0; z <= b.z1; z++) for (let x = b.x0; x <= b.x1; x++) out.push({ cx: x, cz: z }) return out @@ -324,11 +343,19 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { // Off, the map draws what it holds and asks for nothing more until the // operator presses to load (#136). if (!perf.loadOnPan && !loadNow) return + // A chunk whose region is still being parsed is not asked for again + // straight away. Without this the next request rebuilds the same list — + // the viewport is walked in order, so the unresolved chunks are always at + // the front — and the map spins on one band while the rest stays blank. + const now = Date.now() const want = visibleChunks() - .filter((c: { cx: number; cz: number }) => !tiles.current.has(c.cx + ',' + c.cz)) - .slice(0, 64) + .filter((c: { cx: number; cz: number }) => { + const k = c.cx + ',' + c.cz + return !tiles.current.has(k) && (waiting.current.get(k) ?? 0) <= now + }) + .slice(0, MAX_TILES_PER_REQUEST) if (!want.length) return - trimTiles(tiles.current, visibleChunks()) + trimTiles(tiles.current, markStore.current, chunkBox()) tilesPending.current = true window.msms .mapTiles(serverId, dim, want, marks) @@ -337,6 +364,11 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { // The empty list is "read, and nothing there" — as opposed to "not read // yet". Marking null only when the whole response had nothing pending // meant a genuinely empty chunk was re-requested on every draw (#136). + // + // `!r.pending` is NOT a second way to know that: it says nothing about + // chunks the server never looked at, and once a request can carry more + // than the server reads that inference blanks them permanently (#159). + // Every requested chunk comes back in `tiles`, in `empty`, or pending. const known = new Set(r.empty ?? []) for (const w of want) { const k = w.cx + ',' + w.cz @@ -344,7 +376,15 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { if (t) { tiles.current.set(k, bakeTile(t)) if (t.m) markStore.current.set(k, t.m) - } else if (known.has(k) || !r.pending) tiles.current.set(k, null) + waiting.current.delete(k) + } else if (known.has(k)) { + tiles.current.set(k, null) + waiting.current.delete(k) + } else { + // Still being read. Come back to it, but let the rest of the view + // be asked for first. + waiting.current.set(k, Date.now() + 400) + } } setTick2((n) => n + 1) // Ask again while anything is still coming, rather than waiting for the @@ -362,6 +402,7 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { useEffect(() => { tiles.current.clear() markStore.current.clear() + waiting.current.clear() setView(null) fitFor.current = '' }, [serverId, dim]) @@ -371,6 +412,7 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { if (!marks) return tiles.current.clear() markStore.current.clear() + waiting.current.clear() }, [marks]) useEffect(() => { diff --git a/src/shared/apiSurface.ts b/src/shared/apiSurface.ts index ca91f6c..7662fd8 100644 --- a/src/shared/apiSurface.ts +++ b/src/shared/apiSurface.ts @@ -200,7 +200,7 @@ export const API_ROUTES: ApiRoute[] = [ { method: 'GET', path: '/servers/{id}/map/perf', gate: 'settings', group: 'players', summary: 'What the live map is allowed to cost on this server.', params: [serverId] }, { method: 'POST', path: '/servers/{id}/map/perf', gate: 'settings', group: 'players', summary: 'Change it. Values are clamped on the way in.', params: [serverId], body: { cache: 'Keep parsed tiles on disk (default on).', memoryRegions: 'Regions held in memory, 2-64.', parseGapMs: 'Minimum gap between region parses, 0-5000.', cacheLimitMB: 'On-disk ceiling, oldest evicted first.' } }, { method: 'DELETE', path: '/servers/{id}/map/cache', gate: 'settings', group: 'players', summary: 'Drop this server\'s cached map tiles.', params: [serverId], notes: 'Only this server\'s: the cache filename carries the owner so one server\'s clear cannot take another\'s with it.' }, - { method: 'GET', path: '/servers/{id}/map/tiles', gate: 'view', group: 'players', summary: 'Rendered surface colours for the requested chunks.', params: [serverId], notes: 'Ask with `?c=cx,cz;cx,cz` (max 64) and `?dim=`. Answers only with regions already parsed and queues the rest — `pending` says how many are still coming, so a caller polls rather than blocking. A request never parses a region itself.' }, + { method: 'GET', path: '/servers/{id}/map/tiles', gate: 'view', group: 'players', summary: 'Rendered surface colours for the requested chunks.', params: [serverId], notes: 'Ask with `?c=cx,cz;cx,cz` (max 512) and `?dim=`. Answers only with regions already parsed and queues the rest — `pending` says how many are still coming, so a caller polls rather than blocking. A request never parses a region itself.' }, // ---- chunk areas ---- { method: 'GET', path: '/servers/{id}/areas', gate: 'view', group: 'areas', summary: 'Named chunk areas, including hidden ones.', params: [serverId], notes: 'The public map serves its own copy without the hidden areas or the timestamps.' }, { diff --git a/src/shared/livemap.ts b/src/shared/livemap.ts index b94f5ef..9fb91e7 100644 --- a/src/shared/livemap.ts +++ b/src/shared/livemap.ts @@ -357,3 +357,94 @@ export function heatmap(points: { x: number; z: number }[], cell = 16): HeatCell // Busiest first: a consumer that truncates should keep the hot spots. return [...buckets.values()].sort((a, b) => b.count - a.count || a.x - b.x || a.z - b.z) } + +// ---- what a client asks for, and what it keeps (#159) ---- + +/** + * Chunks one request may ask for. + * + * 64 was the original, and at 64 a full viewport of 4096 chunks took **64 + * sequential round trips** — the map filled in visible bands and an operator + * watching it called that "loading piece by piece". The cap was there so one + * call could not ask for a whole world; it is still there, eight times wider. + * + * Both HTTP surfaces gzip their tile responses (`sendTileJson`), so 512 chunks + * is about 82 KB on the wire against 10 KB for 64. Over IPC there is no wire. + * + * THIS CONSTANT MUST BE THE ONLY ONE. A client that sends more than the server + * reads gets a response that says nothing about the excess, and the handler + * below would then mark every unexamined chunk as permanently empty. + */ +export const MAX_TILES_PER_REQUEST = 512 + +/** + * Chunks a viewport will ask for at all. + * + * Zoomed out far enough a viewport covers tens of thousands, and at that scale + * a chunk is a fraction of a pixel — asking is pointless as well as expensive. + * The client draws whatever it still HOLDS beyond this, which is why the + * retention policy below matters more than it looks. + */ +export const MAX_VIEWPORT_CHUNKS = 4096 + +/** + * Tiles a client keeps baked before it starts evicting. + * + * Strictly greater than `MAX_VIEWPORT_CHUNKS`, and that is the whole point: at + * the old limit of 2048 a single viewport could exceed the cache, so the map + * evicted tiles it had just fetched and re-fetched them on the next draw. Any + * value below the viewport cap turns this into a loop. + */ +export const TILE_KEEP_LIMIT = 8192 + +/** A viewport in chunk coordinates, inclusive at both ends. */ +export interface ChunkBox { + x0: number + x1: number + z0: number + z1: number +} + +/** Chunks from the box edge — 0 for anything inside it. */ +function boxDistance(box: ChunkBox, cx: number, cz: number): number { + const dx = cx < box.x0 ? box.x0 - cx : cx > box.x1 ? cx - box.x1 : 0 + const dz = cz < box.z0 ? box.z0 - cz : cz > box.z1 ? cz - box.z1 : 0 + return Math.max(dx, dz) +} + +/** + * Which held tiles to drop, farthest from the view first. + * + * The old rule kept the CURRENT VIEWPORT AND NOTHING ELSE: pan one screen and + * every tile you came from was deleted, so panning back re-fetched all of it. + * That is the "load somewhere else and then wait for the first place to load + * again" an operator reported, and on the desktop there was not even a margin — + * `keep` was exactly the visible chunks. + * + * Distance ordering replaces the margin. Everything on screen is at distance 0, + * its surroundings are 1, 2, 3…, so the cache naturally holds a ring of + * recently visited ground and gives up the far edges of where you have been. + * Nothing is dropped at all until the limit is passed. + * + * Pure and total: the callers hold canvases and DOM objects, and the decision + * about what to throw away should be testable without either. + */ +export function tilesToDrop(held: Iterable, box: ChunkBox, limit = TILE_KEEP_LIMIT): string[] { + const keys = [...held] + const over = keys.length - Math.max(0, limit) + if (over <= 0) return [] + const ranked = keys.map((k) => { + const comma = k.indexOf(',') + const cx = Number(k.slice(0, comma)) + const cz = Number(k.slice(comma + 1)) + // A key that does not parse is not a chunk anyone can draw, so it goes + // first rather than sorting unpredictably on NaN. + const d = comma < 0 || !Number.isFinite(cx) || !Number.isFinite(cz) + ? Infinity + : boxDistance(box, cx, cz) + return { k, d } + }) + // Farthest first, then by key so two runs on the same input agree. + ranked.sort((a, b) => b.d - a.d || (a.k < b.k ? -1 : a.k > b.k ? 1 : 0)) + return ranked.slice(0, over).map((r) => r.k) +} diff --git a/src/shared/mapUi.ts b/src/shared/mapUi.ts index dbb1981..6ac68a7 100644 --- a/src/shared/mapUi.ts +++ b/src/shared/mapUi.ts @@ -9,7 +9,13 @@ * frames while doing nothing interesting. * * The host page provides `api(path)` and `mapServerId()`. + * + * The three tuning numbers are interpolated from `@shared/livemap` rather than + * written here: this is a string, so a copy in it cannot be typechecked against + * the server's cap, and a client asking for more than the server reads gets a + * response that says nothing about the excess (#159). */ +import { MAX_TILES_PER_REQUEST, MAX_VIEWPORT_CHUNKS, TILE_KEEP_LIMIT } from './livemap' export const MAP_CSS = ` .mp-wrap{display:flex;flex-direction:column;gap:10px} @@ -251,7 +257,7 @@ function mapRefresh(){ the overworld — so switching dimension without dropping them draws one world's terrain under another world's players. */ var dimChanged=MAP.dim!==d.dimension; - if(dimChanged){MAP_TILES={};MAP_MARKS={}} + if(dimChanged){MAP_TILES={};MAP_MARKS={};MAP_TILE_WAIT={}} MAP.data=d;MAP.dim=d.dimension; /* The public feed is asked for one dimension at a time, so a switch needs a new request; a pinned area from the last world would otherwise stay drawn. @@ -536,6 +542,8 @@ function mapToggleAreas(){MAP.areasOn=!MAP.areasOn; for every visible chunk is the difference between a map that pans and one that stutters; a chunk only changes when the server rewrites its region. */ var MAP_TILES={},MAP_TILE_PENDING=false; +/* Chunk -> when it is worth asking for again, while its region is read. */ +var MAP_TILE_WAIT={}; function mapTileKey(cx,cz){return cx+','+cz} /* The viewport in chunk coordinates. */ function mapChunkBox(){ @@ -550,7 +558,7 @@ function mapChunkBox(){ */ function mapVisibleChunks(){ var b=mapChunkBox();if(!b)return []; - if((b.x1-b.x0+1)*(b.z1-b.z0+1)>4096)return []; + if((b.x1-b.x0+1)*(b.z1-b.z0+1)>${MAX_VIEWPORT_CHUNKS})return []; var out=[]; for(var z=b.z0;z<=b.z1;z++)for(var x=b.x0;x<=b.x1;x++)out.push({cx:x,cz:z}); return out} @@ -591,10 +599,17 @@ function mapFetchTiles(force){ if((x1-x0+1)*(z1-z0+1)<=4096){ chunks=[]; for(var rz=z0;rz<=z1;rz++)for(var rx=x0;rx<=x1;rx++)chunks.push({cx:rx,cz:rz})}} - var want=chunks.filter(function(c){return MAP_TILES[mapTileKey(c.cx,c.cz)]===undefined}); + /* A chunk whose region is still being read is not asked for again straight + away. The viewport is walked in order, so without this the next request + rebuilds the same list and the map spins on one band while the rest of the + view stays blank (#159). */ + var now=Date.now(); + var want=chunks.filter(function(c){ + var k=mapTileKey(c.cx,c.cz); + return MAP_TILES[k]===undefined&&!(MAP_TILE_WAIT[k]>now)}); if(!want.length)return; mapTrimTiles(); - want=want.slice(0,64); + want=want.slice(0,${MAX_TILES_PER_REQUEST}); MAP_TILE_PENDING=true; mapGet(mapTilesUrl(MAP.dim,want.map(function(c){return c.cx+','+c.cz}).join(';'),MAP.marksOn)).then(function(d){ MAP_TILE_PENDING=false; @@ -604,13 +619,18 @@ function mapFetchTiles(force){ nothing pending was the bug: on a busy viewport something is always pending, so genuinely empty chunks were never marked and were re-requested on every single draw, forever (#136). */ + /* A zero "pending" is NOT a second way to know a chunk is empty: it says + nothing about chunks the server never looked at, and once a request can + carry more than the server reads that inference blanks them permanently + (#159). Every requested chunk comes back drawn, listed empty, or pending. */ var known={}; for(var e=0;e<(d.empty||[]).length;e++)known[d.empty[e]]=1; for(var i=0;ib.x1+8||czb.z1+8){delete MAP_TILES[keys[i]];delete MAP_MARKS[keys[i]]}}} + var dx=cxb.x1?cx-b.x1:0; + var dz=czb.z1?cz-b.z1:0; + ranked.push({k:keys[i],d:isFinite(cx)&&isFinite(cz)?Math.max(dx,dz):Infinity})} + ranked.sort(function(a,b2){return b2.d-a.d||(a.kb2.k?1:0)}); + for(var j=0;j Date: Wed, 5 Aug 2026 21:18:04 +0300 Subject: [PATCH 2/2] Review: wake the map up when every visible chunk is backing off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backoff added in the parent commit can empty the request list completely — every chunk on screen is usually in the same few regions, so they all back off together. The retry only fires after a RESPONSE, and in that state there is no response coming, so the effect returned having scheduled nothing and the view stopped filling until the operator moved it. On the desktop that is indefinite; on the web it recovered on the 2-second refresh. Either way it looks exactly like the bug this branch is fixing. Both clients now schedule their own return, at the soonest backoff still outstanding. Driven for real rather than reasoned about: MAP_JS is run in a vm with a recording setTimeout and a host whose every response says "still reading". The first pass asks, the second finds everything backing off, and the test asserts a wake-up was scheduled — and that the second pass did NOT re-ask, which is the behaviour the backoff exists for. It also asserts something actually ended up backing off, so it cannot pass by never reaching the case. Removing the wake-up fails it with "a fully-backed-off view scheduled no wake-up; the map would stall". Also: trimTiles dropped tiles and marks but not the backoff entry, so that map kept one entry per chunk ever looked at for the life of the window. Gates green: MSMS_SMOKE, _WORLDS, _MODUPDATE, _WEB, _ANALYSIS, _AUDIT. --- src/main/smoke.ts | 74 +++++++++++++++++++++++++ src/renderer/src/components/LiveMap.tsx | 27 ++++++++- src/shared/mapUi.ts | 16 +++++- 3 files changed, 111 insertions(+), 6 deletions(-) diff --git a/src/main/smoke.ts b/src/main/smoke.ts index bf0b187..c874ba1 100644 --- a/src/main/smoke.ts +++ b/src/main/smoke.ts @@ -9075,6 +9075,80 @@ export async function runWebSmoke(): Promise { } } + // ---- a view whose chunks are all still parsing must wake itself ---- + // + // The backoff that stops the map re-asking for chunks it is already + // waiting on can empty the request list entirely — every visible chunk is + // in the same few regions, so they all back off together. The retry only + // fires after a RESPONSE, and there is no response coming, so without a + // wake-up the view stops filling and looks exactly like the bug this all + // came from. Driven for real: MAP_JS is run with a recording timer. + { + const timers: number[] = [] + let served = 0 + const ctx: Record = { + setTimeout: (_fn: unknown, ms: number) => { + timers.push(ms) + return timers.length + }, + clearTimeout: () => {}, + setInterval: () => 0, + clearInterval: () => {}, + console, + Math, + Date, + Infinity, + isFinite, + Object, + Path2D: class {}, + document: { getElementById: () => null, createElement: () => null }, + // The host contract. Every response says "nothing yet, still reading", + // which is the state the wake-up exists for. + mapGet: () => { + served++ + return Promise.resolve({ tiles: {}, empty: [], pending: 40 }) + }, + mapPost: () => Promise.resolve(null), + mapServerId: () => 's', + mapFeedUrl: () => '/feed', + mapTilesUrl: () => '/tiles', + mapAreasUrlFor: () => '/areas', + mapAvatarUrl: () => '', + mapIconFor: () => ({ path: '', colour: '#fff' }), + mapIconSvg: () => '', + MAP_ICONS: {}, + STRUCTURE_ICONS: {} + } + ctx.window = ctx + runInNewContext(MAP_JS + '\n;globalThis.__map=this;', ctx) + + const M = ctx.MAP as Record + M.view = { cx: 0, cz: 0, scale: 1 } + M.vp = { width: 96, height: 96 } + M.world = true + M.loadOnPan = true + const fetchTiles = ctx.mapFetchTiles as (force?: boolean) => void + + // First pass: asks, and every chunk comes back still pending. + fetchTiles(true) + await new Promise((r) => setTimeout(r, 20)) + if (served !== 1) return fail('the map did not ask for tiles at all') + const waiting = Object.keys(ctx.MAP_TILE_WAIT as object).length + if (!waiting) return fail('a pending response left nothing backing off; the test proves nothing') + + // Second pass, while they are all still backing off: nothing to ask + // for, so it must have scheduled its own return instead of stopping. + const before = timers.length + fetchTiles(true) + await new Promise((r) => setTimeout(r, 20)) + if (served !== 1) return fail('the map re-asked for chunks it was already waiting on') + if (timers.length <= before) { + return fail('a fully-backed-off view scheduled no wake-up; the map would stall') + } + const delay = timers[timers.length - 1] + if (!(delay >= 50 && delay <= 400)) return fail('the wake-up delay is wrong: ' + delay) + } + // ---- every client caps at the number the server reads (#159) ---- { // A client that asks for more than the server looks at gets a response diff --git a/src/renderer/src/components/LiveMap.tsx b/src/renderer/src/components/LiveMap.tsx index 646e563..76dc1e4 100644 --- a/src/renderer/src/components/LiveMap.tsx +++ b/src/renderer/src/components/LiveMap.tsx @@ -137,12 +137,16 @@ function headFor( function trimTiles( tiles: Map, marks: Map, + waiting: Map, box: ChunkBox | null ): void { if (!box) return for (const k of tilesToDrop(tiles.keys(), box)) { tiles.delete(k) marks.delete(k) + // The backoff too, or it outlives every tile it was about and the map + // accumulates one entry per chunk ever looked at. + waiting.delete(k) } } @@ -348,14 +352,31 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element { // the viewport is walked in order, so the unresolved chunks are always at // the front — and the map spins on one band while the rest stays blank. const now = Date.now() + let soonest = Infinity const want = visibleChunks() .filter((c: { cx: number; cz: number }) => { const k = c.cx + ',' + c.cz - return !tiles.current.has(k) && (waiting.current.get(k) ?? 0) <= now + if (tiles.current.has(k)) return false + const until = waiting.current.get(k) ?? 0 + if (until > now) { + soonest = Math.min(soonest, until) + return false + } + return true }) .slice(0, MAX_TILES_PER_REQUEST) - if (!want.length) return - trimTiles(tiles.current, markStore.current, chunkBox()) + if (!want.length) { + // Everything left on screen is waiting on a region parse, so there is + // nothing to ask for THIS instant — but the retry below only fires after + // a response, and there is no response coming. Without a wake-up here the + // view stops filling until the operator moves it. + if (soonest !== Infinity) { + const timer = window.setTimeout(() => setTick2((n) => n + 1), Math.max(50, soonest - now)) + return () => window.clearTimeout(timer) + } + return + } + trimTiles(tiles.current, markStore.current, waiting.current, chunkBox()) tilesPending.current = true window.msms .mapTiles(serverId, dim, want, marks) diff --git a/src/shared/mapUi.ts b/src/shared/mapUi.ts index 6ac68a7..009fad5 100644 --- a/src/shared/mapUi.ts +++ b/src/shared/mapUi.ts @@ -603,11 +603,21 @@ function mapFetchTiles(force){ away. The viewport is walked in order, so without this the next request rebuilds the same list and the map spins on one band while the rest of the view stays blank (#159). */ - var now=Date.now(); + var now=Date.now(),soonest=Infinity; var want=chunks.filter(function(c){ var k=mapTileKey(c.cx,c.cz); - return MAP_TILES[k]===undefined&&!(MAP_TILE_WAIT[k]>now)}); - if(!want.length)return; + if(MAP_TILES[k]!==undefined)return false; + if(MAP_TILE_WAIT[k]>now){soonest=Math.min(soonest,MAP_TILE_WAIT[k]);return false} + return true}); + if(!want.length){ + /* Everything left on screen is waiting on a region parse, so there is + nothing to ask for this instant — and the retry below only fires after a + response, which is not coming. Without a wake-up here the view stops + filling until the visitor moves it. */ + if(soonest!==Infinity){ + clearTimeout(MAP_TILE_SOON); + MAP_TILE_SOON=setTimeout(function(){mapFetchTiles(true)},Math.max(50,soonest-now))} + return} mapTrimTiles(); want=want.slice(0,${MAX_TILES_PER_REQUEST}); MAP_TILE_PENDING=true;