Stop the map throwing away the ground you just looked at

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"
         ],

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<void> {
   }
 }
 
-/** `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 }[] = []

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,162 @@ export async function runWebSmoke(): Promise<void> {
         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')
+        }
+      }
+
+      // ---- 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<string, unknown> = {
+          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<string, unknown>
+        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
+        // 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)

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,25 @@ 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<string, HTMLCanvasElement | null>,
-  keep: { cx: number; cz: number }[]
+  marks: Map<string, StructureMark[]>,
+  waiting: Map<string, number>,
+  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)
+    // 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)
+  }
 }
 
 export function LiveMap({ serverId }: { serverId: string }): JSX.Element {
@@ -270,11 +290,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<string, HTMLImageElement | false>())
   const tiles = useRef(new Map<string, HTMLCanvasElement | null>())
   const tilesPending = useRef(false)
+  /** Chunk -> when it is worth asking for again, while its region is read. */
+  const waiting = useRef(new Map<string, number>())
   const [tick2, setTick2] = useState(0)
 
   const chunkBox = useCallback((): { x0: number; x1: number; z0: number; z1: number } | null => {
@@ -293,7 +316,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 +347,36 @@ 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()
+    let soonest = Infinity
     const want = visibleChunks()
-      .filter((c: { cx: number; cz: number }) => !tiles.current.has(c.cx + ',' + c.cz))
-      .slice(0, 64)
-    if (!want.length) return
-    trimTiles(tiles.current, visibleChunks())
+      .filter((c: { cx: number; cz: number }) => {
+        const k = c.cx + ',' + c.cz
+        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) {
+      // 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)
@@ -337,14 +385,27 @@ 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
           const t = r.tiles[k]
           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 +423,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 +433,7 @@ export function LiveMap({ serverId }: { serverId: string }): JSX.Element {
     if (!marks) return
     tiles.current.clear()
     markStore.current.clear()
+    waiting.current.clear()
   }, [marks])
 
   useEffect(() => {

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.' },
   {