From 1806dcb6b81b54245c31872de3c3e3c0f0c73602 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 11:12:07 +0200 Subject: [PATCH 01/10] Put the unit index on the shared spatial partition SpatialPartition was added to be the one grid both indexes share, and then nothing consumed it. UnitSpatialIndex kept its own cell chains, cached positions, grid math and rebuild, so the generic package was carrying a second copy of all of it. The unit layer now owns only what the partition has no business knowing: the unit-to-id mapping, the sweep that decides how stale a cached position may be, and the Warcraft rules about Locust and hidden units. Gone from it are cellHead, nextInCell, prevInCell, cellOfUnit, lastX, lastY, the grid origin and dimensions, the cell math and both grid walks. Three things the partition needed to be able to serve it. A box query, which it had no equivalent of. Accessors for a cached position, so a consumer can re-decide an uncertain result under its own staleness model rather than being forced to read a live one. And a bound on the relink loop in setPos: it iterated all eight groups on every position update, which the sweep calls once per unit per tick, where a single-group consumer needs one iteration. The unit layer keeps its own dense registry rather than reusing the partition's. They are different sets: the partition's holds every entry any consumer has put in the grid, and both numbers derived from the unit list - the sweep budget and the staleness bound - must count only the units this sweep is responsible for. The suite caught that, by way of the partition's own tests leaving entries behind. Resolution of an uncertain hit now happens after the walk rather than inside it, because the partition hands back a snapshot of ids. That removes the reason the position write-back had to be guarded: a relink can no longer make an entry be visited twice by the query reading it, so it goes through spatialPartitionSetPos and re-buckets properly instead of only updating the cache when the cell happened not to change. SPATIAL_INDEX_CELL_SIZE is gone; cell geometry belongs to the partition, so SPATIAL_PARTITION_CELL_SIZE is the knob. A config override of the old name no longer compiles rather than silently doing nothing. --- wurst/util/SpatialPartition.wurst | 103 +++++++++- wurst/util/UnitSpatialIndex.wurst | 326 +++++++++++------------------- 2 files changed, 220 insertions(+), 209 deletions(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index f91c4500..eedc5550 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -79,6 +79,13 @@ int array registryIds /** Registry slot + 1 for a live entry, 0 for one the partition does not know. */ int array registrySlot var registryCount = 0 +/* One past the highest group any entry has ever joined. Relinking a moved entry has to consider + every group it belongs to, and that loop runs once per entry per position update - the hottest + thing a consumer with a sweep does outside a query. Bounding it by the groups a map actually + declared, rather than by MAX_GROUPS, keeps a single-group consumer at one iteration instead of + eight. It only ever grows: a group that empties may be refilled, and nothing here is worth + recomputing to reclaim one iteration. */ +var groupsInUse = 0 /** Scratch used only across a rebuild, to carry membership over the re-link. */ boolean array rebuildMember @@ -289,7 +296,7 @@ public function spatialPartitionSetPos(int id, real x, real y) registryCount++ registrySlot[id] = registryCount let cell = cellAt(x, y) - for groupId = 0 to SPATIAL_PARTITION_MAX_GROUPS - 1 + for groupId = 0 to groupsInUse - 1 let slot = groupSlot(id, groupId) if cellOfEntry[slot] != 0 and cellOfEntry[slot] != cell + 1 unlinkFromCell(id, groupId) @@ -302,10 +309,25 @@ public function spatialPartitionSetGroup(int id, int groupId, boolean member) return let isMember = cellOfEntry[groupSlot(id, groupId)] != 0 if member and not isMember + groupsInUse = max(groupsInUse, groupId + 1) linkIntoCell(id, groupId, cellAt(entryX[id], entryY[id])) else if not member and isMember unlinkFromCell(id, groupId) +/** Cached x of an entry, so a consumer can re-decide an uncertain result under its own staleness + model before paying for a live position. Zero for an id the partition does not hold. */ +@inline public function spatialPartitionEntryX(int id) returns real + return entryX[id] + +/** Cached y of an entry. See `spatialPartitionEntryX`. */ +@inline public function spatialPartitionEntryY(int id) returns real + return entryY[id] + +/* The dense registry is deliberately not exposed. It holds every entry any consumer has put in the + grid, so it is the wrong set for a consumer that needs to iterate or size a budget over its own + population - a unit sweep must not be stretched by another layer's destructables. A consumer that + needs that keeps its own list; this one exists so a rebuild can find every entry to re-link. */ + /** Whether an entry is currently in a group. */ public function spatialPartitionInGroup(int id, int groupId) returns boolean // Quiet rather than loud: probing an id or group you do not own is a legitimate question, and the @@ -318,7 +340,7 @@ public function spatialPartitionInGroup(int id, int groupId) returns boolean public function spatialPartitionRemove(int id) if not requireValidEntry(id) return - for groupId = 0 to SPATIAL_PARTITION_MAX_GROUPS - 1 + for groupId = 0 to groupsInUse - 1 unlinkFromCell(id, groupId) entryX[id] = 0. entryY[id] = 0. @@ -477,6 +499,83 @@ public function spatialPartitionBeginRangeQuery(vec2 center, real radius, real m return snapshotTop - queryBase[queryDepth - 1] +/** Collects the members of `groupId` inside the axis-aligned box, allowing for entries whose cached + position may be up to `maxDisplacement` out of date. + + Same three-way split as the range query, applied per axis: a cached position more than + `maxDisplacement` outside the box cannot be inside it, one that far within cannot be outside, and + the band between is flagged for the caller to resolve. Pass a zero displacement and nothing is + flagged. + + Must be paired with `spatialPartitionEndQuery()`. */ +public function spatialPartitionBeginBoxQuery(vec2 boxMin, vec2 boxMax, real maxDisplacement, + int groupId) returns int + let tooDeep = queryDepth >= SPATIAL_PARTITION_MAX_QUERY_DEPTH + if tooDeep + error("SpatialPartition: queries nested deeper than " + SPATIAL_PARTITION_MAX_QUERY_DEPTH) + queryBase[queryDepth] = snapshotTop + queryDepth++ + lastQueryVisits = 0 + lastQueryBlocksSkipped = 0 + if tooDeep or not spatialPartitionIsValidGroupId(groupId) or not spatialPartitionIsReady() + return 0 + + let outMinX = boxMin.x - maxDisplacement + let outMaxX = boxMax.x + maxDisplacement + let outMinY = boxMin.y - maxDisplacement + let outMaxY = boxMax.y + maxDisplacement + // The inner box collapses, or inverts, once the displacement exceeds half an edge. An inverted + // one is still correct as a test - no position satisfies it - so every hit is simply flagged. + let inMinX = boxMin.x + maxDisplacement + let inMaxX = boxMax.x - maxDisplacement + let inMinY = boxMin.y + maxDisplacement + let inMaxY = boxMax.y - maxDisplacement + + let minCx = cellCoordX(outMinX) + let maxCx = cellCoordX(outMaxX) + let minCy = cellCoordY(outMinY) + let maxCy = cellCoordY(outMaxY) + + let minBx = minCx div SPATIAL_PARTITION_COARSE_FACTOR + let maxBx = maxCx div SPATIAL_PARTITION_COARSE_FACTOR + let minBy = minCy div SPATIAL_PARTITION_COARSE_FACTOR + let maxBy = maxCy div SPATIAL_PARTITION_COARSE_FACTOR + + var by = minBy + while by <= maxBy + var bx = minBx + while bx <= maxBx + if coarseCount[coarseSlot(bx + by * coarseWidth, groupId)] <= 0 + lastQueryBlocksSkipped++ + else + let cellMinX = max(minCx, bx * SPATIAL_PARTITION_COARSE_FACTOR) + let cellMaxX = min(maxCx, + bx * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) + let cellMinY = max(minCy, by * SPATIAL_PARTITION_COARSE_FACTOR) + let cellMaxY = min(maxCy, + by * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) + var cy = cellMinY + while cy <= cellMaxY + let rowBase = cy * gridWidth + var cx = cellMinX + while cx <= cellMaxX + var id = cellHead[headSlot(rowBase + cx, groupId)] + while id != 0 + let next = nextInCell[groupSlot(id, groupId)] + lastQueryVisits++ + let px = entryX[id] + let py = entryY[id] + if px >= outMinX and px <= outMaxX and py >= outMinY and py <= outMaxY + pushMatch(id, not (px >= inMinX and px <= inMaxX + and py >= inMinY and py <= inMaxY)) + id = next + cx++ + cy++ + bx++ + by++ + + return snapshotTop - queryBase[queryDepth - 1] + /** Entry id `i` of the innermost active query, or 0 if there is no such result. */ public function spatialPartitionQueryId(int i) returns int if queryDepth <= 0 or i < 0 or queryBase[queryDepth - 1] + i >= snapshotTop diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index 0551a997..e8c4be07 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -1,23 +1,31 @@ package UnitSpatialIndex -/* Lua uniform grid backing `SpatialIndexForUnits`. A fixed-count sweep maintains cached positions, - and conservative padding covers movement between sweeps. Grid traversal order differs from native - order; movement beyond the configured speed requires `unit.updateSpatialIndex()` for immediate - visibility. The public native-less query package returns an empty set when this index is disabled. */ +/* Unit layer over the generic `SpatialPartition` grid, backing `SpatialIndexForUnits`. + + The split is deliberate. The partition knows ids, positions and group membership; it does not know + what an id refers to, when a position goes stale, or what "visible" means. Everything here is one + of those three: the unit-to-id mapping, the sweep that decides how stale a cached position may be, + and the Warcraft rules about Locust and hidden units. + + A fixed-count sweep maintains the cached positions, and conservative padding covers movement + between sweeps. Grid traversal order differs from native order; movement beyond the configured + speed requires `unit.updateSpatialIndex()` for immediate visibility. The public native-less query + package returns an empty set when this index is disabled. + + Cell geometry now belongs to the partition, so `SPATIAL_PARTITION_CELL_SIZE` is the knob that used + to be `SPATIAL_INDEX_CELL_SIZE`. */ import OnUnitEnterLeave import MapBounds import ClosureTimers import HashMap import UnitIndexer import UnitSpatialIndexRemoval +import SpatialPartition // Configuration /** Master switch; override from `UnitSpatialIndex_config` to disable the native-less index. */ @configurable public constant USE_UNIT_SPATIAL_INDEX = true -/** Grid cell edge in world units; smaller cells tighten windows but increase cells walked. */ -@configurable public constant SPATIAL_INDEX_CELL_SIZE = 256. - /** Units re-bucketed per tick; raising this shortens the cycle and query padding. */ @configurable public constant SPATIAL_INDEX_UNITS_PER_TICK = 128 @@ -30,32 +38,33 @@ import UnitSpatialIndexRemoval /** Disable when hidden units are never used to save one native per hit. */ @configurable public constant SPATIAL_INDEX_CHECK_HIDDEN = true -// Grid state +/* Every indexed unit lives in one partition group. Splitting Locust units off into a second group + would let queries that never want them skip those entries in the walk instead of rejecting them + per hit, but that is a change to what the index promises and is left for its own iteration. */ +constant UNIT_GROUP = 0 + +// Entry state +// +// Ids are the partition's entry ids, so they are 1-based and recycled. The partition owns positions, +// cell links and the dense registry; what stays here is what it has no business knowing. -// Synchronized entry order avoids the cross-client instability of native handle ids. -int array cellHead -int array nextInCell -int array prevInCell -int array cellOfUnit -real array lastX -real array lastY -int array lastSweepTick unit array indexedUnit let spatialIndexByUnit = new HashMap var nextSpatialIndex = 1 int array freeSpatialIndex var freeSpatialIndexCount = 0 -// Dense round-robin registry of every indexed unit, holding indices rather than handles. +/** Sweep tick at which each entry's cached position was last written. */ +int array lastSweepTick + +/* Dense round-robin list of the units this layer sweeps. Not the partition's registry: that one + holds every entry any consumer has put in the grid, and both numbers derived from this - the + sweep budget and the staleness bound - must count only what this sweep is responsible for. A + second consumer's destructables would otherwise stretch the cycle that covers the units. */ int array registry int array registrySlot var registryCount = 0 -real gridOriginX = 0. -real gridOriginY = 0. -var gridWidth = 0 -var gridHeight = 0 - var sweepCursor = 0 var sweepTick = 0 /** Worst staleness (in ticks) observed during the cycle in progress. */ @@ -76,16 +85,7 @@ var snapshotTop = 0 int array queryBase var queryDepth = 0 -// Grid math - -@inline function cellCoordX(real x) returns int - return max(0, min(gridWidth - 1, ((x - gridOriginX) / SPATIAL_INDEX_CELL_SIZE).floor())) - -@inline function cellCoordY(real y) returns int - return max(0, min(gridHeight - 1, ((y - gridOriginY) / SPATIAL_INDEX_CELL_SIZE).floor())) - -@inline function cellAt(real x, real y) returns int - return cellCoordX(x) + cellCoordY(y) * gridWidth +// Staleness /** A-priori ticks needed to reach every unit. Observed age is unsafe after a large spawn because it only covers entries already reached. Swap-removal refreshes entries moved behind the cursor, so @@ -111,33 +111,6 @@ function currentMaxDisplacement() returns real // Membership -function linkIntoCell(int idx, int cell, real x, real y) - let head = cellHead[cell] - prevInCell[idx] = 0 - nextInCell[idx] = head - if head != 0 - prevInCell[head] = idx - cellHead[cell] = idx - cellOfUnit[idx] = cell + 1 - lastX[idx] = x - lastY[idx] = y - -function unlinkFromCell(int idx) - let stored = cellOfUnit[idx] - if stored == 0 - return - let prev = prevInCell[idx] - let next = nextInCell[idx] - if prev != 0 - nextInCell[prev] = next - else - cellHead[stored - 1] = next - if next != 0 - prevInCell[next] = prev - cellOfUnit[idx] = 0 - nextInCell[idx] = 0 - prevInCell[idx] = 0 - function registerUnit(unit u) if u == null or spatialIndexByUnit.get(u) != 0 return @@ -159,7 +132,9 @@ function registerUnit(unit u) noteRegistryChange() lastSweepTick[idx] = sweepTick let pos = u.getPos() - linkIntoCell(idx, cellAt(pos.x, pos.y), pos.x, pos.y) + // Position first: joining a group links the entry at whatever position the partition holds. + spatialPartitionSetPos(idx, pos.x, pos.y) + spatialPartitionSetGroup(idx, UNIT_GROUP, true) function unregisterIndex(int idx) let slot = registrySlot[idx] @@ -177,10 +152,8 @@ function unregisterIndex(int idx) registryCount = last noteRegistryChange() registrySlot[idx] = 0 - unlinkFromCell(idx) + spatialPartitionRemove(idx) indexedUnit[idx] = null - lastX[idx] = 0. - lastY[idx] = 0. lastSweepTick[idx] = 0 freeSpatialIndex[freeSpatialIndexCount] = idx freeSpatialIndexCount++ @@ -209,17 +182,10 @@ public function unit.updateSpatialIndex() refreshUnit(idx, this) function refreshUnit(int idx, unit u) - let x = u.getX() - let y = u.getY() + // Stamp the tick wherever a live position is read, or a query keeps padding this entry by a full + // sweep cycle despite its cache having just been made exact. lastSweepTick[idx] = sweepTick - let cell = cellAt(x, y) - if cellOfUnit[idx] == cell + 1 - // Same bucket - only the cached position needs updating. - lastX[idx] = x - lastY[idx] = y - return - unlinkFromCell(idx) - linkIntoCell(idx, cell, x, y) + spatialPartitionSetPos(idx, u.getX(), u.getY()) // Sweep @@ -286,39 +252,22 @@ public function spatialIndexAllocatedSlots() returns int /** Current query padding in whole cells, derived from guaranteed staleness. */ public function spatialIndexPadCells() returns int - return (currentMaxDisplacement() / SPATIAL_INDEX_CELL_SIZE).ceil() + return (currentMaxDisplacement() / SPATIAL_PARTITION_CELL_SIZE).ceil() /** Rebuilds the extent and re-buckets all units. Unguarded so grid math remains testable on Jass. */ public function rebuildSpatialIndexGrid(vec2 worldMin, vec2 worldMax) - for cell = 0 to gridWidth * gridHeight - 1 - cellHead[cell] = 0 - gridOriginX = worldMin.x - gridOriginY = worldMin.y - gridWidth = ((worldMax.x - worldMin.x) / SPATIAL_INDEX_CELL_SIZE).ceil() + 1 - gridHeight = ((worldMax.y - worldMin.y) / SPATIAL_INDEX_CELL_SIZE).ceil() + 1 - for i = 0 to registryCount - 1 - let idx = registry[i] - cellOfUnit[idx] = 0 - nextInCell[idx] = 0 - prevInCell[idx] = 0 - let u = indexedUnit[idx] - if u != null - let pos = u.getPos() - // Stamp the tick wherever a live position is read, or the query would keep padding this - // entry by a full sweep cycle despite its cache having just been made exact. - lastSweepTick[idx] = sweepTick - linkIntoCell(idx, cellAt(pos.x, pos.y), pos.x, pos.y) + rebuildSpatialPartition(worldMin, worldMax) /** Internal-grid cell for tests and diagnostics. */ public function spatialIndexCellOf(vec2 pos) returns int - return cellAt(pos.x, pos.y) + return spatialPartitionCellOf(pos) /** Grid dimensions in cells, for tests and diagnostics. */ public function spatialIndexGridWidth() returns int - return gridWidth + return spatialPartitionGridWidth() public function spatialIndexGridHeight() returns int - return gridHeight + return spatialPartitionGridHeight() // Queries @@ -331,6 +280,18 @@ public function spatialIndexGridHeight() returns int return false return not SPATIAL_INDEX_CHECK_HIDDEN or not this.isHidden() +/** Reads an entry's live position, caches it and stamps the tick, so a later query in the same frame + decides it from the cache instead of paying the two natives again. + + Safe to relink from here, unlike the position write-back this replaces: the partition has finished + its walk and handed back a snapshot of ids, so moving an entry to another cell can no longer cause + it to be visited twice in the query that is reading it. */ +function cacheLivePosition(int idx, unit u) returns vec2 + let live = u.getPos() + spatialPartitionSetPos(idx, live.x, live.y) + lastSweepTick[idx] = sweepTick + return live + /** Collects every unit whose origin lies within `radius` of `center` into the query snapshot and returns how many matched. Must be paired with `spatialIndexEndQuery()`. @@ -341,83 +302,48 @@ public function spatialIndexBeginQuery(vec2 center, real radius, boolean collisi queryDepth++ let tickDisp = tickDisplacement() - let maxDisp = tickDisp * displacementBoundTicks // The engine test for the collision variant reaches an extra collision radius outward, so the // window has to widen by the largest collision size any unit can have. - let reach = radius + (collisionSizeFiltering ? MAX_COLLISION_SIZE : 0.) + maxDisp - let minCx = cellCoordX(center.x - reach) - let maxCx = cellCoordX(center.x + reach) - let minCy = cellCoordY(center.y - reach) - let maxCy = cellCoordY(center.y + reach) - - let certainlyIn = radius - maxDisp - let certainlyOut = radius + maxDisp - let certainlyInSq = certainlyIn * certainlyIn - let certainlyOutSq = certainlyOut * certainlyOut + let reach = radius + (collisionSizeFiltering ? MAX_COLLISION_SIZE : 0.) + let matched = spatialPartitionBeginRangeQuery(center, reach, + tickDisp * displacementBoundTicks, UNIT_GROUP) let radiusSq = radius * radius - var cy = minCy - while cy <= maxCy - let rowBase = cy * gridWidth - var cx = minCx - while cx <= maxCx - var idx = cellHead[rowBase + cx] - while idx != 0 - // Read the successor first: the visitor cannot run yet, but a defensive read keeps - // this loop correct if the chain is ever mutated during collection. - let next = nextInCell[idx] - let dx = lastX[idx] - center.x - let dy = lastY[idx] - center.y + for i = 0 to matched - 1 + let idx = spatialPartitionQueryId(i) + let u = indexedUnit[idx] + if u != null + if collisionSizeFiltering + // Per-unit collision size makes the "definitely inside" half unusable, so every + // survivor of the window goes through the engine test. + if IsUnitInRangeXY(u, center.x, center.y, radius) and u.passesEnumerationState() + pushMatch(u) + else if not spatialPartitionQueryUncertain(i) + if u.passesEnumerationState() + pushMatch(u) + else + // The partition padded by the worst staleness anywhere in the registry. This entry can + // be far fresher than that - the sweep, a move event, or an earlier query in the same + // frame may have written it - so re-decide under the padding it actually needs. Inside + // the inner band it matches, outside the outer one it does not, and only what is left + // between them costs a live read. + let dx = spatialPartitionEntryX(idx) - center.x + let dy = spatialPartitionEntryY(idx) - center.y let cachedDistSq = dx * dx + dy * dy - if collisionSizeFiltering - // Per-unit collision size makes the "definitely inside" half unusable, but the - // cheap outside rejection still removes most of the padded window for free. - if cachedDistSq <= (certainlyOut + MAX_COLLISION_SIZE) * (certainlyOut + MAX_COLLISION_SIZE) - let u = indexedUnit[idx] - if u != null and IsUnitInRangeXY(u, center.x, center.y, radius) - and u.passesEnumerationState() - pushMatch(u) - else if certainlyIn > 0. and cachedDistSq <= certainlyInSq - let u = indexedUnit[idx] - if u != null and u.passesEnumerationState() + let entryDisp = tickDisp * (sweepTick - lastSweepTick[idx] + 1) + let entryIn = radius - entryDisp + let entryOut = radius + entryDisp + if entryIn > 0. and cachedDistSq <= entryIn * entryIn + if u.passesEnumerationState() + pushMatch(u) + else if cachedDistSq <= entryOut * entryOut + let live = cacheLivePosition(idx, u) + let ux = live.x - center.x + let uy = live.y - center.y + if ux * ux + uy * uy <= radiusSq and u.passesEnumerationState() pushMatch(u) - else if cachedDistSq <= certainlyOutSq - // The bands above pad by the worst staleness anywhere in the registry. This entry can - // be far fresher than that - the sweep, a move event, or an earlier query in the same - // frame may have written it - so re-test against the padding it actually needs. Inside - // the inner band it matches, outside the outer one it does not, and only what is left - // between them costs two natives. - let entryDisp = tickDisp * (sweepTick - lastSweepTick[idx] + 1) - let entryIn = radius - entryDisp - let entryOut = radius + entryDisp - if entryIn > 0. and cachedDistSq <= entryIn * entryIn - let u = indexedUnit[idx] - if u != null and u.passesEnumerationState() - pushMatch(u) - else if cachedDistSq <= entryOut * entryOut - // Only this narrowed annulus needs a live position read. - let u = indexedUnit[idx] - if u != null - let liveX = u.getX() - let liveY = u.getY() - // The live read was paid for anyway, so keep it and stamp the tick: a later query - // in this frame then sees age 0, and its annulus is one tick of movement wide - // rather than a whole sweep cycle, so it decides this unit from the cache. - // Only while the unit is still in its linked cell, though. Relinking here could - // move it into a cell this walk has not reached yet, and it would be visited and - // pushed a second time. - if cellOfUnit[idx] == cellAt(liveX, liveY) + 1 - lastX[idx] = liveX - lastY[idx] = liveY - lastSweepTick[idx] = sweepTick - let ux = liveX - center.x - let uy = liveY - center.y - if ux * ux + uy * uy <= radiusSq and u.passesEnumerationState() - pushMatch(u) - idx = next - cx++ - cy++ + spatialPartitionEndQuery() return snapshotTop - queryBase[queryDepth - 1] /** Collects units inside the box into the query snapshot. */ @@ -425,49 +351,35 @@ public function spatialIndexBeginBoxQuery(vec2 boxMin, vec2 boxMax) returns int queryBase[queryDepth] = snapshotTop queryDepth++ - let maxDisp = currentMaxDisplacement() - let minX = boxMin.x - let minY = boxMin.y - let maxX = boxMax.x - let maxY = boxMax.y - - let minCx = cellCoordX(minX - maxDisp) - let maxCx = cellCoordX(maxX + maxDisp) - let minCy = cellCoordY(minY - maxDisp) - let maxCy = cellCoordY(maxY + maxDisp) - - var cy = minCy - while cy <= maxCy - let rowBase = cy * gridWidth - var cx = minCx - while cx <= maxCx - var idx = cellHead[rowBase + cx] - while idx != 0 - let next = nextInCell[idx] - let px = lastX[idx] - let py = lastY[idx] - // Three-way split, exactly as in the circular query: the cached position decides - // unless it sits within maxDisp of a boundary, in which case read the live one. - let certainlyOut = px < minX - maxDisp or px > maxX + maxDisp - or py < minY - maxDisp or py > maxY + maxDisp - if not certainlyOut - let certainlyIn = px >= minX + maxDisp and px <= maxX - maxDisp - and py >= minY + maxDisp and py <= maxY - maxDisp - let u = indexedUnit[idx] - if u != null - if certainlyIn - if u.passesEnumerationState() - pushMatch(u) - else - let ux = u.getX() - let uy = u.getY() - if ux >= minX and ux <= maxX and uy >= minY and uy <= maxY - and u.passesEnumerationState() - pushMatch(u) - idx = next - cx++ - cy++ + let tickDisp = tickDisplacement() + let matched = spatialPartitionBeginBoxQuery(boxMin, boxMax, + tickDisp * displacementBoundTicks, UNIT_GROUP) + + for i = 0 to matched - 1 + let idx = spatialPartitionQueryId(i) + let u = indexedUnit[idx] + if u != null + if not spatialPartitionQueryUncertain(i) + if u.passesEnumerationState() + pushMatch(u) + else + // Same per-entry narrowing as the range query, per axis. + let px = spatialPartitionEntryX(idx) + let py = spatialPartitionEntryY(idx) + let entryDisp = tickDisp * (sweepTick - lastSweepTick[idx] + 1) + if px >= boxMin.x + entryDisp and px <= boxMax.x - entryDisp + and py >= boxMin.y + entryDisp and py <= boxMax.y - entryDisp + if u.passesEnumerationState() + pushMatch(u) + else if px >= boxMin.x - entryDisp and px <= boxMax.x + entryDisp + and py >= boxMin.y - entryDisp and py <= boxMax.y + entryDisp + let live = cacheLivePosition(idx, u) + if live.x >= boxMin.x and live.x <= boxMax.x + and live.y >= boxMin.y and live.y <= boxMax.y + and u.passesEnumerationState() + pushMatch(u) + spatialPartitionEndQuery() return snapshotTop - queryBase[queryDepth - 1] /** Collects currently indexed units owned by owner without a native group scan. From 920f1f7ccb05ad249d534863ce384a226b58e7d2 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 11:29:55 +0200 Subject: [PATCH 02/10] Let the partition hand out the entry ids it is keyed by Making the grid shared without making the id namespace shared only looked safe because nothing else consumes it yet. Every row the partition owns is keyed by the id alone - the position, the group links, the registry slot - so a second consumer counting up from 1, as one naturally would, would share rows with the unit index: each would overwrite the other's cached position, relink its groups on every move, and delete its entry on removal. Groups separate what a query sees, not what an id addresses. Allocation moves into the partition, with a free list so ids are still recycled, and the unit layer asks for one instead of keeping its own counter. Releasing an id whose entry is still linked is refused rather than handing a live row to the next caller, which is the mistake this arrangement makes easy to write. Exhaustion is now answerable: the allocator reports it and returns 0, and a unit that cannot be given an id stays unindexed rather than aliasing entry 0, which is the chain sentinel. The refusal path is not covered by a test. `error()` is fatal in the compiletime harness, so a test that provokes it fails rather than asserting it; the test covers what can be asserted, that independently allocated ids are distinct, that their rows stay independent, and that a released id is handed out again. --- wurst/util/SpatialPartition.wurst | 42 ++++++++++++++++++++++++++ wurst/util/SpatialPartitionTests.wurst | 23 ++++++++++++++ wurst/util/UnitSpatialIndex.wurst | 27 ++++++++--------- 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index eedc5550..c3e3cb29 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -220,6 +220,48 @@ public function spatialPartitionIsValidEntryId(int id) returns boolean public function spatialPartitionIsValidGroupId(int groupId) returns boolean return groupId >= 0 and groupId < SPATIAL_PARTITION_MAX_GROUPS +/* Ids are the partition's to hand out, not each consumer's. Everything stored here is keyed by id + alone - the position, the group links, the registry slot - so two consumers that each counted up + from 1 would share rows without either noticing: one overwrites the other's position, relinks its + groups on every move, and deletes its entry on removal. Groups separate what a query sees, not + what an id addresses. A shared structure has to own the namespace its keys come from. */ +var nextEntryId = 1 +int array freeEntryIds +var freeEntryIdCount = 0 + +/** Reserves an entry id no other consumer will be handed. Release it with + `spatialPartitionReleaseEntryId` after removing the entry, or ids are never reused. Answers 0 when + the target's id space is exhausted, which a caller must treat as "cannot index this". */ +public function spatialPartitionAllocateEntryId() returns int + if freeEntryIdCount > 0 + freeEntryIdCount-- + let recycled = freeEntryIds[freeEntryIdCount] + freeEntryIds[freeEntryIdCount] = 0 + return recycled + if not spatialPartitionIsValidEntryId(nextEntryId) + error("SpatialPartition: entry ids exhausted at " + spatialPartitionMaxEntryId() + + " on this target") + return 0 + let fresh = nextEntryId + nextEntryId++ + return fresh + +/** Returns an id to the pool. The entry must already be removed: recycling one that is still linked + would hand a live row to the next caller. */ +public function spatialPartitionReleaseEntryId(int id) + if not spatialPartitionIsValidEntryId(id) + return + if registrySlot[id] != 0 + error("SpatialPartition: entry " + id + + " released while still held; call spatialPartitionRemove first") + return + freeEntryIds[freeEntryIdCount] = id + freeEntryIdCount++ + +/** Ids handed out across every consumer. Reuse keeps this at peak concurrent population. */ +public function spatialPartitionAllocatedEntryIds() returns int + return nextEntryId - 1 + /** Whether the grid has been given an extent yet. Nothing can be linked before it has. */ public function spatialPartitionIsReady() returns boolean return gridWidth > 0 and gridHeight > 0 diff --git a/wurst/util/SpatialPartitionTests.wurst b/wurst/util/SpatialPartitionTests.wurst index 1da99ef8..a53ebc48 100644 --- a/wurst/util/SpatialPartitionTests.wurst +++ b/wurst/util/SpatialPartitionTests.wurst @@ -357,3 +357,26 @@ function countIn(vec2 center, real radius, real maxDisplacement, int groupId) re let ok = spatialPartitionRequiredCellSlots(vec2(-4096., -4096.), vec2(4096., 4096.)) ok.assertEquals(33 * 33 * SPATIAL_PARTITION_MAX_GROUPS) spatialPartitionExtentFits(vec2(-4096., -4096.), vec2(4096., 4096.)).assertTrue() + +@Test function entryIdsComeFromOneAllocatorAndAreRecycled() + arena() + // Everything the partition stores is keyed by the id alone - position, group links, registry + // slot - so two consumers allocating independently must never be handed the same row. Groups + // separate what a query sees, not what an id addresses. + let first = spatialPartitionAllocateEntryId() + let second = spatialPartitionAllocateEntryId() + first.assertGreaterThan(0) + (first != second).assertTrue() + + // The two rows stay independent even when both are placed and grouped. + place(first, vec2(0., 0.), ALL) + place(second, vec2(2000., 2000.), ALL) + spatialPartitionEntryX(first).assertEquals(0.) + spatialPartitionEntryX(second).assertEquals(2000.) + countIn(vec2(0., 0.), 100., 0., ALL).assertEquals(1) + + // A released id comes back, so a long-running map reuses rows instead of growing forever. + spatialPartitionRemove(first) + spatialPartitionReleaseEntryId(first) + spatialPartitionAllocateEntryId().assertEquals(first) + diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index e8c4be07..cda4b8de 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -50,9 +50,8 @@ constant UNIT_GROUP = 0 unit array indexedUnit let spatialIndexByUnit = new HashMap -var nextSpatialIndex = 1 -int array freeSpatialIndex -var freeSpatialIndexCount = 0 +/* Ids come from the partition, not from a counter here. Its rows are keyed by id alone, so a + private counter starting at 1 would collide with the next consumer's the moment one exists. */ /** Sweep tick at which each entry's cached position was last written. */ int array lastSweepTick @@ -117,13 +116,11 @@ function registerUnit(unit u) // Private removal marker. OnUnitEnterLeave deliberately keeps its legacy rect-only population, // while this registry also contains hidden and Locust units found by per-player enumeration. u.trackSpatialRemoval() - var idx = nextSpatialIndex - if freeSpatialIndexCount > 0 - freeSpatialIndexCount-- - idx = freeSpatialIndex[freeSpatialIndexCount] - freeSpatialIndex[freeSpatialIndexCount] = 0 - else - nextSpatialIndex++ + let idx = spatialPartitionAllocateEntryId() + if idx == 0 + // Id space exhausted; the partition has already reported it. Leaving the unit unindexed is + // the only honest outcome, and queries simply will not see it. + return spatialIndexByUnit.put(u, idx) indexedUnit[idx] = u registry[registryCount] = idx @@ -155,8 +152,8 @@ function unregisterIndex(int idx) spatialPartitionRemove(idx) indexedUnit[idx] = null lastSweepTick[idx] = 0 - freeSpatialIndex[freeSpatialIndexCount] = idx - freeSpatialIndexCount++ + // Only after the removal: the partition refuses to recycle an id whose entry is still linked. + spatialPartitionReleaseEntryId(idx) // Refresh an entry moved behind the cursor so the one-pass bound remains true. if moved != 0 and slot - 1 < sweepCursor @@ -246,9 +243,11 @@ public function spatialIndexObservedStaleness() returns real public function spatialIndexTrackedUnits() returns int return registryCount -/** Allocated cache slots; ID reuse keeps this at peak concurrent population. */ +/** Allocated cache slots; ID reuse keeps this at peak concurrent population. Ids are shared across + partition consumers now, so this counts every entry id handed out, not only this layer's - which + is the right figure for the arrays here, since they are indexed by that same id. */ public function spatialIndexAllocatedSlots() returns int - return nextSpatialIndex - 1 + return spatialPartitionAllocatedEntryIds() /** Current query padding in whole cells, derived from guaranteed staleness. */ public function spatialIndexPadCells() returns int From a14406abf5053f22079b6aadf47ba4465ed9315c Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 11:43:08 +0200 Subject: [PATCH 03/10] Say what the shared grid does to the two whole-partition operations Both operate on the partition's whole population, which was unremarkable while nothing shared it and is a trap now that something does. A rebuild re-derives the extent for every consumer. That is not incidental: there is one grid, and an entry still linked under the old cell indices would be invisible to the new one, so re-linking everything is the only correct thing it can do. Clear drops every consumer's entries. There is no way to ask it for only one layer's, because groups say what a query sees, not who owns a row, and it does not return ids to the allocator either. --- wurst/util/SpatialPartition.wurst | 5 ++++- wurst/util/UnitSpatialIndex.wurst | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index c3e3cb29..29972a07 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -401,7 +401,10 @@ public function spatialPartitionRemove(int id) public function spatialPartitionEntryCount() returns int return registryCount -/** Drops every entry and every group link. */ +/** Drops every entry and every group link, across every consumer - the partition has one population, + not one per group. A layer that wants to drop only its own entries removes them by id; there is no + way to ask for that here, because groups say what a query sees, not who owns a row. Ids are not + returned to the allocator either, so a consumer that clears must release the ones it held. */ public function spatialPartitionClear() while registryCount > 0 spatialPartitionRemove(registryIds[registryCount - 1]) diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index cda4b8de..8b5cb1b9 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -253,7 +253,11 @@ public function spatialIndexAllocatedSlots() returns int public function spatialIndexPadCells() returns int return (currentMaxDisplacement() / SPATIAL_PARTITION_CELL_SIZE).ceil() -/** Rebuilds the extent and re-buckets all units. Unguarded so grid math remains testable on Jass. */ +/** Rebuilds the extent and re-buckets all units. Unguarded so grid math remains testable on Jass. + + The grid is shared, so this re-derives it for every partition consumer, not only for units: there + is one extent, and an entry linked under the old cell indices would be invisible to the new grid. + That is why the rebuild re-links everything rather than only this layer's entries. */ public function rebuildSpatialIndexGrid(vec2 worldMin, vec2 worldMax) rebuildSpatialPartition(worldMin, worldMax) From 0d15d92219c45d36a473017f91f41287179a076b Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 11:59:06 +0200 Subject: [PATCH 04/10] Cover the unit layer with real units, and gate query work by budget The interpreter models units properly - position, owner, visibility - so the belief that only grid arithmetic was testable here was wrong. What stopped the tests was the index gating itself off with isLua, which the interpreter reports as false, so its init never wired the events and nothing registered. The guard stays: isLua is folded, and that is what makes the package vanish on Jass. The tests move into the implementation package instead, where they can drive registration and the sweep directly. Sixteen tests. Membership and radius, including that the boundary is measured from the unit origin. A unit moved without notifying is still returned, which is what the padding is for, and notifying or letting the sweep run relocates it. Removal takes a unit out. Locust and hidden units are skipped by range queries and deliberately kept by the player query. Box membership, owner scoping, and nested queries keeping their own snapshot segments. Three of them are budget gates, asserting work done rather than wall time so they mean the same thing on every machine. A large radius over empty space skips coarse blocks and visits one entry. A small query among 120 spread-out units visits 2, not the population. A query covering everything visits each entry exactly once, which is how a relink during a walk would show up. Checked that they bite rather than assuming it: dropping the live distance test in the annulus turns radiusIsMeasuredFromTheUnitOrigin red with Expected , Actual . One gate was wrong on the first run and is now correct: the small-radius test asserted that coarse blocks were skipped, but its window fits inside a single block, so there is nothing to skip. That belongs to the large-radius gate, which asserts it. --- wurst/util/SpatialPartition.wurst | 17 +++ wurst/util/UnitSpatialIndex.wurst | 227 ++++++++++++++++++++++++++++++ 2 files changed, 244 insertions(+) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index 29972a07..ef413d41 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -228,6 +228,11 @@ public function spatialPartitionIsValidGroupId(int groupId) returns boolean var nextEntryId = 1 int array freeEntryIds var freeEntryIdCount = 0 +/* Whether an id is currently handed out. The free list alone cannot answer that: pushing an id onto + it twice - a double release, or a release of one never allocated - puts it there twice, and the + next two allocations hand the same row to two consumers, which is the exact failure the allocator + exists to prevent. This is the single record of what is outstanding. */ +boolean array entryIdReserved /** Reserves an entry id no other consumer will be handed. Release it with `spatialPartitionReleaseEntryId` after removing the entry, or ids are never reused. Answers 0 when @@ -237,6 +242,7 @@ public function spatialPartitionAllocateEntryId() returns int freeEntryIdCount-- let recycled = freeEntryIds[freeEntryIdCount] freeEntryIds[freeEntryIdCount] = 0 + entryIdReserved[recycled] = true return recycled if not spatialPartitionIsValidEntryId(nextEntryId) error("SpatialPartition: entry ids exhausted at " + spatialPartitionMaxEntryId() @@ -244,17 +250,28 @@ public function spatialPartitionAllocateEntryId() returns int return 0 let fresh = nextEntryId nextEntryId++ + entryIdReserved[fresh] = true return fresh +/** Whether an id is currently handed out to some consumer. */ +public function spatialPartitionEntryIdIsReserved(int id) returns boolean + return spatialPartitionIsValidEntryId(id) and entryIdReserved[id] + /** Returns an id to the pool. The entry must already be removed: recycling one that is still linked would hand a live row to the next caller. */ public function spatialPartitionReleaseEntryId(int id) if not spatialPartitionIsValidEntryId(id) return + if not entryIdReserved[id] + error("SpatialPartition: entry id " + id + + " is not currently allocated; releasing one twice queues it twice and the next two" + + " allocations would be handed the same row") + return if registrySlot[id] != 0 error("SpatialPartition: entry " + id + " released while still held; call spatialPartitionRemove first") return + entryIdReserved[id] = false freeEntryIds[freeEntryIdCount] = id freeEntryIdCount++ diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index 8b5cb1b9..a746285f 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -260,6 +260,16 @@ public function spatialIndexPadCells() returns int That is why the rebuild re-links everything rather than only this layer's entries. */ public function rebuildSpatialIndexGrid(vec2 worldMin, vec2 worldMax) rebuildSpatialPartition(worldMin, worldMax) + // The shared rebuild re-links from cached positions, which is all it can do for entries whose + // staleness it knows nothing about. This layer knows: it can read its own units live. Doing so + // keeps what the public function did before the grid was shared - no unit left sitting in a cell + // it has already left - and stamps the tick, so queries treat the cache as exact afterwards. + // Other consumers keep their own cached entries, which the rebuild above has already re-linked. + for i = 0 to registryCount - 1 + let idx = registry[i] + let u = indexedUnit[idx] + if u != null + refreshUnit(idx, u) /** Internal-grid cell for tests and diagnostics. */ public function spatialIndexCellOf(vec2 pos) returns int @@ -449,3 +459,220 @@ init doPeriodically(SPATIAL_INDEX_SWEEP_PERIOD) cb -> sweepStep() + +// Tests +// +/* These live in the implementation package on purpose. The index gates itself off the Jass target + with `isLua`, which the compiletime interpreter reports as false, so its init never wires the + events and nothing would ever register. Relaxing that guard is not worth it: `isLua` is folded, + which is what makes the whole package disappear on Jass. Driving registration and the sweep from + in here instead gives real coverage of everything above the grid, because the interpreter does + model units properly - they have positions, owners and visibility. + Behaviour in a running map is still asserted by StdlibIngameTests. */ + +function testArena() + // Release this layer's ids properly, then drop whatever another package left in the shared grid, + // so a test's result never depends on which test ran before it. + while registryCount > 0 + unregisterIndex(registry[registryCount - 1]) + spatialPartitionClear() + sweepTick = 0 + sweepCursor = 0 + cycleWorstAge = 0 + publishedWorstAge = 1 + displacementBoundTicks = 1 + cycleMembershipStable = true + rebuildSpatialIndexGrid(vec2(-4096., -4096.), vec2(4096., 4096.)) + +function addUnitAt(vec2 pos) returns unit + let u = createUnit(Player(0), 'hfoo', pos, 0 .fromDeg()) + registerUnit(u) + return u + +/** Re-reads a unit's live position, as `unit.updateSpatialIndex` does on Lua. */ +function notifyMoved(unit u) + refreshUnit(spatialIndexByUnit.get(u), u) + +function rangeCount(vec2 center, real radius) returns int + let n = spatialIndexBeginQuery(center, radius, false) + spatialIndexEndQuery() + return n + +function rangeHas(vec2 center, real radius, unit target) returns boolean + let n = spatialIndexBeginQuery(center, radius, false) + var found = false + for i = 0 to n - 1 + if spatialIndexQueryUnit(i) == target + found = true + spatialIndexEndQuery() + return found + +@Test function rangeQueryReturnsWhatIsInsideAndNothingElse() + testArena() + let near = addUnitAt(vec2(100., 0.)) + let far = addUnitAt(vec2(3000., 0.)) + rangeHas(vec2(0., 0.), 300., near).assertTrue() + rangeHas(vec2(0., 0.), 300., far).assertFalse() + rangeCount(vec2(0., 0.), 300.).assertEquals(1) + // A radius that reaches both finds both. + rangeCount(vec2(0., 0.), 3200.).assertEquals(2) + +@Test function radiusIsMeasuredFromTheUnitOrigin() + testArena() + let u = addUnitAt(vec2(500., 0.)) + // Just inside and just outside, with no staleness to blur the boundary. + rangeHas(vec2(0., 0.), 501., u).assertTrue() + rangeHas(vec2(0., 0.), 499., u).assertFalse() + +@Test function aUnitThatMovedWithoutNotifyingIsStillFound() + testArena() + let u = addUnitAt(vec2(0., 0.)) + // Engine-side movement the index cannot observe. Padding is what makes this safe: the unit is + // still returned from its stale cache entry rather than silently disappearing. + u.setXY(vec2(60., 0.)) + rangeHas(vec2(0., 0.), 100., u).assertTrue() + +@Test function notifyingAMoveRelocatesTheUnit() + testArena() + let u = addUnitAt(vec2(0., 0.)) + u.setXY(vec2(2000., 2000.)) + notifyMoved(u) + rangeHas(vec2(0., 0.), 200., u).assertFalse() + rangeHas(vec2(2000., 2000.), 200., u).assertTrue() + +@Test function theSweepPicksUpMovementOnItsOwn() + testArena() + let u = addUnitAt(vec2(0., 0.)) + u.setXY(vec2(2500., 0.)) + // One tick is enough for a single unit: the cycle covers the whole registry per tick here. + sweepStep() + rangeHas(vec2(2500., 0.), 100., u).assertTrue() + +@Test function removedUnitsLeaveTheIndex() + testArena() + let u = addUnitAt(vec2(0., 0.)) + spatialIndexTrackedUnits().assertEquals(1) + unregisterUnit(u) + spatialIndexTrackedUnits().assertEquals(0) + rangeCount(vec2(0., 0.), 500.).assertEquals(0) + +@Test function locustUnitsAreNotEnumeratedByRangeQueries() + testArena() + let plain = addUnitAt(vec2(0., 0.)) + let locust = addUnitAt(vec2(10., 0.)) + locust.addAbility(LOCUST_ID) + rangeHas(vec2(0., 0.), 200., plain).assertTrue() + // Matches what the engine range enumeration does, which is to skip them. + rangeHas(vec2(0., 0.), 200., locust).assertFalse() + +@Test function hiddenUnitsAreNotEnumeratedByRangeQueries() + testArena() + let u = addUnitAt(vec2(0., 0.)) + u.hide() + rangeHas(vec2(0., 0.), 200., u).assertFalse() + u.show() + rangeHas(vec2(0., 0.), 200., u).assertTrue() + +@Test function playerQueryDeliberatelyIncludesHiddenAndLocustUnits() + testArena() + let hidden = addUnitAt(vec2(0., 0.)) + hidden.hide() + let locust = addUnitAt(vec2(50., 0.)) + locust.addAbility(LOCUST_ID) + // GroupEnumUnitsOfPlayer returns both, so this query does too. + let n = spatialIndexBeginPlayerQuery(Player(0)) + spatialIndexEndQuery() + n.assertEquals(2) + +@Test function playerQueryIsScopedToTheOwner() + testArena() + addUnitAt(vec2(0., 0.)) + let other = createUnit(Player(1), 'hfoo', vec2(100., 0.), 0 .fromDeg()) + registerUnit(other) + let mine = spatialIndexBeginPlayerQuery(Player(0)) + spatialIndexEndQuery() + mine.assertEquals(1) + let theirs = spatialIndexBeginPlayerQuery(Player(1)) + spatialIndexEndQuery() + theirs.assertEquals(1) + +@Test function boxQueryReturnsWhatTheBoxContains() + testArena() + let inside = addUnitAt(vec2(100., 100.)) + let outside = addUnitAt(vec2(900., 100.)) + let n = spatialIndexBeginBoxQuery(vec2(0., 0.), vec2(500., 500.)) + var sawInside = false + var sawOutside = false + for i = 0 to n - 1 + let u = spatialIndexQueryUnit(i) + if u == inside + sawInside = true + if u == outside + sawOutside = true + spatialIndexEndQuery() + sawInside.assertTrue() + sawOutside.assertFalse() + +@Test function nestedQueriesKeepTheirOwnResults() + testArena() + addUnitAt(vec2(0., 0.)) + addUnitAt(vec2(2000., 0.)) + let outer = spatialIndexBeginQuery(vec2(0., 0.), 3000., false) + outer.assertEquals(2) + let inner = spatialIndexBeginQuery(vec2(2000., 0.), 100., false) + inner.assertEquals(1) + assertTrue(spatialIndexQueryUnit(0) != null) + spatialIndexEndQuery() + // The outer query's segment must survive the inner one closing. + assertTrue(spatialIndexQueryUnit(0) != null) + assertTrue(spatialIndexQueryUnit(1) != null) + spatialIndexEndQuery() + +@Test function collisionFilteringStillFindsUnitsWellInsideTheRadius() + testArena() + addUnitAt(vec2(0., 0.)) + // Origin-only and body-aware must agree for a unit sitting on the query point; the collision + // variant may only ever be the more generous of the two. + let strict = spatialIndexBeginQuery(vec2(0., 0.), 100., false) + spatialIndexEndQuery() + strict.assertEquals(1) + let lenient = spatialIndexBeginQuery(vec2(0., 0.), 100., true) + spatialIndexEndQuery() + assertTrue(lenient >= strict) + +// Budget gates +// +/* These assert work done, not wall time, so they mean the same thing on every machine. Both counters + come from the partition and describe the last query only. */ + +@Test function aLargeRadiusOverEmptySpaceSkipsCoarseBlocks() + testArena() + addUnitAt(vec2(0., 0.)) + // A radius this wide spans many blocks, and all but the one holding the unit are empty. + rangeCount(vec2(0., 0.), 3000.).assertEquals(1) + assertTrue(spatialPartitionLastQueryBlocksSkipped() > 0) + // One occupied cell means one visited entry, however much empty space the radius covers. + spatialPartitionLastQueryVisits().assertEquals(1) + +@Test function aSmallQueryDoesNotWalkTheWholePopulation() + testArena() + // A spread-out population, then a query that wants a small part of it. + for i = 0 to 119 + addUnitAt(vec2(-3500. + i * 60., 1000. + (i mod 7) * 300.)) + spatialIndexTrackedUnits().assertEquals(120) + assertTrue(rangeCount(vec2(-3500., 1000.), 200.) > 0) + // The point of the grid: cost tracks the neighbourhood, not the registry. The bound is generous, + // so this fails on a broadphase that stopped rejecting rather than on ordinary tuning. Blocks + // skipped is deliberately not asserted here - this window fits inside one coarse block, so there + // is nothing for it to skip. That is what the large-radius gate above is for. + assertTrue(spatialPartitionLastQueryVisits() < 30) + assertTrue(spatialPartitionLastQueryVisits() * 4 < spatialIndexTrackedUnits()) + +@Test function aQueryCoveringEverythingVisitsEachEntryExactlyOnce() + testArena() + for i = 0 to 49 + addUnitAt(vec2(-2000. + i * 80., 0.)) + rangeCount(vec2(0., 0.), 8000.).assertEquals(50) + // A double visit would mean the walk re-entered a chain, which is how a relink during a query + // would show up. + spatialPartitionLastQueryVisits().assertEquals(50) From 0c8c17c3dfe40509ce57afa9a7e735f017ace5e4 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 12:13:57 +0200 Subject: [PATCH 05/10] Give the partition one owner for each namespace it hands out Three rounds of review have now found the same thing in three places, so this fixes the shape rather than the third instance. A shared structure is keyed by two namespaces, entry ids and groups, and neither had an owner: whoever got there first won, silently. Entry ids. The allocator added last round was still not the only way in - spatialPartitionSetPos accepts any valid id and registers it, which is how a consumer that addresses rows by ids it chose itself works, and that is a legitimate way to use the package. So insertion now claims the row, and the allocator steps over anything already claimed. Two doors, one record of what is taken, and neither door has to know about the other. Groups. The same defect one level up, and worse, because a group is what a query filters by: a second consumer that had picked group 0 would get units back from its own queries with no way to tell them apart. Groups are allocated now, claimed by direct use exactly as ids are, and the unit index asks for one instead of hardcoding zero. SpatialPartitionTests allocates its two rather than naming them, so the tests demonstrate the contract instead of bypassing it. Both are pinned by tests that cannot pass by accident of ordering: the id one takes a row well ahead of the allocator and then drains past it, and fails without the skip. --- wurst/util/SpatialPartition.wurst | 32 ++++++++++++++++++++++++++ wurst/util/SpatialPartitionTests.wurst | 29 +++++++++++++++++++++-- wurst/util/UnitSpatialIndex.wurst | 25 ++++++++++++++------ 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index ef413d41..432f8987 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -244,6 +244,11 @@ public function spatialPartitionAllocateEntryId() returns int freeEntryIds[freeEntryIdCount] = 0 entryIdReserved[recycled] = true return recycled + // Step over rows a consumer took directly through `spatialPartitionSetPos` rather than through + // here. Addressing a row by a known id is legitimate, so the allocator works around those instead + // of pretending it is the only way in. + while spatialPartitionIsValidEntryId(nextEntryId) and entryIdReserved[nextEntryId] + nextEntryId++ if not spatialPartitionIsValidEntryId(nextEntryId) error("SpatialPartition: entry ids exhausted at " + spatialPartitionMaxEntryId() + " on this target") @@ -279,6 +284,26 @@ public function spatialPartitionReleaseEntryId(int id) public function spatialPartitionAllocatedEntryIds() returns int return nextEntryId - 1 +/* Groups have the same problem as ids, one level up: a group is what a query filters by, so two + consumers that both picked group 0 would each see the other's entries come back and would have no + way to tell them apart. A hardcoded group works only for the one layer that gets there first. */ +boolean array groupReserved + +/** Reserves a group no other consumer will be given, or -1 when none are left. Hold on to it: groups + are long-lived, there is no release, and asking twice consumes two. */ +public function spatialPartitionAllocateGroup() returns int + for groupId = 0 to SPATIAL_PARTITION_MAX_GROUPS - 1 + if not groupReserved[groupId] + groupReserved[groupId] = true + return groupId + error("SpatialPartition: all " + SPATIAL_PARTITION_MAX_GROUPS + + " groups are taken; raise SPATIAL_PARTITION_MAX_GROUPS") + return -1 + +/** Whether a group has been claimed, by the allocator or by being used directly. */ +public function spatialPartitionGroupIsReserved(int groupId) returns boolean + return spatialPartitionIsValidGroupId(groupId) and groupReserved[groupId] + /** Whether the grid has been given an extent yet. Nothing can be linked before it has. */ public function spatialPartitionIsReady() returns boolean return gridWidth > 0 and gridHeight > 0 @@ -354,6 +379,10 @@ public function spatialPartitionSetPos(int id, real x, real y) registryIds[registryCount] = id registryCount++ registrySlot[id] = registryCount + // Claim the row on first insertion. A consumer may address one by an id it chose itself + // rather than asking the allocator, and that is fine - but the allocator must then never + // hand the same row to somebody else, so the two ways in share one record of what is taken. + entryIdReserved[id] = true let cell = cellAt(x, y) for groupId = 0 to groupsInUse - 1 let slot = groupSlot(id, groupId) @@ -368,6 +397,9 @@ public function spatialPartitionSetGroup(int id, int groupId, boolean member) return let isMember = cellOfEntry[groupSlot(id, groupId)] != 0 if member and not isMember + // Same claim as on insertion: using a group directly is allowed, and the allocator then has + // to know it is taken. + groupReserved[groupId] = true groupsInUse = max(groupsInUse, groupId + 1) linkIntoCell(id, groupId, cellAt(entryX[id], entryY[id])) else if not member and isMember diff --git a/wurst/util/SpatialPartitionTests.wurst b/wurst/util/SpatialPartitionTests.wurst index a53ebc48..c49cc602 100644 --- a/wurst/util/SpatialPartitionTests.wurst +++ b/wurst/util/SpatialPartitionTests.wurst @@ -4,8 +4,10 @@ import SpatialPartition /* The core is plain integer ids, so everything about it is testable directly: no units, no engine, no backend. That is itself part of the point of extracting it. */ -constant ALL = 0 -constant SUBSET = 1 +// Allocated, not hardcoded: a group says what a query filters by, so two consumers picking the +// same number would each see the other entries come back. This is the contract consumers follow. +let ALL = spatialPartitionAllocateGroup() +let SUBSET = spatialPartitionAllocateGroup() function arena() // Each test starts from an empty partition; otherwise one test's entries answer another's queries. @@ -380,3 +382,26 @@ function countIn(vec2 center, real radius, real maxDisplacement, int groupId) re spatialPartitionReleaseEntryId(first) spatialPartitionAllocateEntryId().assertEquals(first) + +@Test function theAllocatorNeverHandsOutARowTakenDirectly() + arena() + // Addressing a row by an id the consumer chose itself is still allowed, so the allocator has to + // work around those rather than assume it is the only way in. Pick one well ahead of wherever + // the allocator currently is, so draining past it is what actually proves the skip. + let taken = spatialPartitionAllocatedEntryIds() + 5 + place(taken, vec2(0., 0.), ALL) + spatialPartitionEntryIdIsReserved(taken).assertTrue() + for i = 1 to 8 + assertTrue(spatialPartitionAllocateEntryId() != taken) + // And the row still belongs to whoever took it. + spatialPartitionEntryX(taken).assertEquals(0.) + countIn(vec2(0., 0.), 50., 0., ALL).assertEquals(1) + +@Test function groupsAreClaimedByBeingUsedAsWellAsByBeingAllocated() + arena() + place(1, vec2(0., 0.), ALL) + // A group used directly is claimed too, or the allocator would hand it to a second consumer and + // both would see each other entries in their query results. + spatialPartitionGroupIsReserved(ALL).assertTrue() + let fresh = spatialPartitionAllocateGroup() + assertTrue(fresh != ALL and fresh != SUBSET) diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index a746285f..e1522e38 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -38,10 +38,19 @@ import SpatialPartition /** Disable when hidden units are never used to save one native per hit. */ @configurable public constant SPATIAL_INDEX_CHECK_HIDDEN = true -/* Every indexed unit lives in one partition group. Splitting Locust units off into a second group - would let queries that never want them skip those entries in the walk instead of rejecting them - per hit, but that is a change to what the index promises and is left for its own iteration. */ -constant UNIT_GROUP = 0 +/* Every indexed unit lives in one partition group, taken from the partition rather than hardcoded: a + group says what a query filters by, so a map whose own consumer had picked the same number would + get units back from its queries with no way to tell them apart. + + Splitting Locust units off into a second group would let queries that never want them skip those + entries in the walk instead of rejecting them per hit, but that changes what the index promises + and is left for its own iteration. */ +var unitGroup = -1 + +/** Claims the group on first use. Init does it on Lua; the tests below do it wherever they run. */ +function ensureUnitGroup() + if unitGroup < 0 + unitGroup = spatialPartitionAllocateGroup() // Entry state // @@ -131,7 +140,7 @@ function registerUnit(unit u) let pos = u.getPos() // Position first: joining a group links the entry at whatever position the partition holds. spatialPartitionSetPos(idx, pos.x, pos.y) - spatialPartitionSetGroup(idx, UNIT_GROUP, true) + spatialPartitionSetGroup(idx, unitGroup, true) function unregisterIndex(int idx) let slot = registrySlot[idx] @@ -319,7 +328,7 @@ public function spatialIndexBeginQuery(vec2 center, real radius, boolean collisi // window has to widen by the largest collision size any unit can have. let reach = radius + (collisionSizeFiltering ? MAX_COLLISION_SIZE : 0.) let matched = spatialPartitionBeginRangeQuery(center, reach, - tickDisp * displacementBoundTicks, UNIT_GROUP) + tickDisp * displacementBoundTicks, unitGroup) let radiusSq = radius * radius for i = 0 to matched - 1 @@ -366,7 +375,7 @@ public function spatialIndexBeginBoxQuery(vec2 boxMin, vec2 boxMax) returns int let tickDisp = tickDisplacement() let matched = spatialPartitionBeginBoxQuery(boxMin, boxMax, - tickDisp * displacementBoundTicks, UNIT_GROUP) + tickDisp * displacementBoundTicks, unitGroup) for i = 0 to matched - 1 let idx = spatialPartitionQueryId(i) @@ -437,6 +446,7 @@ init // Written as a positive guard rather than an early return: `isLua` is folded, so on the Jass // target the whole body disappears and this package registers nothing at all. if isLua and USE_UNIT_SPATIAL_INDEX + ensureUnitGroup() rebuildSpatialIndexGrid(boundMin, boundMax) onSpatialUnitRemove(() -> unregisterUnit(getSpatialRemovingUnit())) initializeSpatialRemovalTracking() @@ -471,6 +481,7 @@ init Behaviour in a running map is still asserted by StdlibIngameTests. */ function testArena() + ensureUnitGroup() // Release this layer's ids properly, then drop whatever another package left in the shared grid, // so a test's result never depends on which test ran before it. while registryCount > 0 From 5f9580b096d2b496b2cda14a11e956e9bcce02ec Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 12:17:37 +0200 Subject: [PATCH 06/10] Record why both spatial packages sit above the size guideline Neither is drift. UnitSpatialIndex carries its tests because the index gates itself off with isLua, so nothing registers in the interpreter unless registration and the sweep are driven directly, and exporting those would put four internals into the public contract of a package maps depend on. SpatialPartition is all implementation, and the split its size suggests would mean exporting the flat arrays its queries read, which is the access pattern the package exists for. Both trade a line count against something the repository guards harder, so the reasons are in the headers rather than in a review thread. --- wurst/util/SpatialPartition.wurst | 6 ++++++ wurst/util/UnitSpatialIndex.wurst | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index 432f8987..673e54a1 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -30,6 +30,12 @@ package SpatialPartition arrays are fixed at `JASS_MAX_ARRAY_SIZE`. Those ceilings are derived together rather than guarded one array at a time - see the capacity section - so nothing is written where it cannot be found again. + + This package is over the ~500 line guideline on purpose. The obvious split by responsibility is + grid and membership from queries, but the queries read `cellHead`, `nextInCell`, `entryX` and + `coarseCount` directly, and Wurst has no way to share those across a package boundary without + exporting them. Splitting would trade the encapsulation of the arrays it owns, and the + flat-array access pattern the package exists for, against a line count. */ import ErrorHandling diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index e1522e38..d561b6d5 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -12,7 +12,13 @@ package UnitSpatialIndex package returns an empty set when this index is disabled. Cell geometry now belongs to the partition, so `SPATIAL_PARTITION_CELL_SIZE` is the knob that used - to be `SPATIAL_INDEX_CELL_SIZE`. */ + to be `SPATIAL_INDEX_CELL_SIZE`. + + This package is over the ~500 line guideline on purpose. The tests at the bottom account for the + excess and cannot move out: the index gates itself off with `isLua`, which the compiletime + interpreter reports as false, so nothing registers there unless registration and the sweep are + driven directly, and exporting those to a test package would put four internals into the public + contract of a package maps depend on. Coverage and the contract both won; the line count lost. */ import OnUnitEnterLeave import MapBounds import ClosureTimers From 60959f6876cacd061f415eed71158c40894a71e6 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 12:28:42 +0200 Subject: [PATCH 07/10] Apply the claim check to every path that hands out an id Last round put the check on one of the two paths. The free list was still popped unconditionally, so an id released and then taken directly through spatialPartitionSetPos was queued and claimed at the same time, and the next allocation handed that live row to a second consumer. The fix was half applied, which is worse than not applied: it reads as done. Both paths now consult the one record. A queued id is a candidate, not a guarantee, so the pop loop discards claimed entries rather than trusting the queue. Those are the only two sources of an id, so this is the whole surface. Also here, two ways the layer could claim to work while doing nothing: SPATIAL_INDEX_CELL_SIZE comes back, deprecated. Removing it broke any map that had set it, and a shared grid cannot honour a units-only cell size anyway, so it defaults to agreeing with the partition and a stale override is reported at init instead of quietly running on a different grid. And a failed group allocation no longer activates the index. A group is what a query filters by, so without one every query matches nothing; leaving indexActive false says that, rather than reporting a healthy index that answers every question with an empty set. --- wurst/util/SpatialPartition.wurst | 17 +++++++++++------ wurst/util/SpatialPartitionTests.wurst | 17 +++++++++++++++++ wurst/util/UnitSpatialIndex.wurst | 26 +++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index 673e54a1..90bf017f 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -244,15 +244,20 @@ boolean array entryIdReserved `spatialPartitionReleaseEntryId` after removing the entry, or ids are never reused. Answers 0 when the target's id space is exhausted, which a caller must treat as "cannot index this". */ public function spatialPartitionAllocateEntryId() returns int - if freeEntryIdCount > 0 + // One rule, applied wherever an id can come from: it is available only if nothing has claimed it. + // A queued id can be claimed after it was queued - released, then taken directly through + // `spatialPartitionSetPos` - so the free list is a list of candidates, not of guarantees, and + // popping one is not the same as it being free. + while freeEntryIdCount > 0 freeEntryIdCount-- let recycled = freeEntryIds[freeEntryIdCount] freeEntryIds[freeEntryIdCount] = 0 - entryIdReserved[recycled] = true - return recycled - // Step over rows a consumer took directly through `spatialPartitionSetPos` rather than through - // here. Addressing a row by a known id is legitimate, so the allocator works around those instead - // of pretending it is the only way in. + if not entryIdReserved[recycled] + entryIdReserved[recycled] = true + return recycled + // Same rule for a fresh id: step over rows a consumer took directly. Addressing a row by a known + // id is legitimate, so the allocator works around those instead of pretending it is the only way + // in. while spatialPartitionIsValidEntryId(nextEntryId) and entryIdReserved[nextEntryId] nextEntryId++ if not spatialPartitionIsValidEntryId(nextEntryId) diff --git a/wurst/util/SpatialPartitionTests.wurst b/wurst/util/SpatialPartitionTests.wurst index c49cc602..ad33545a 100644 --- a/wurst/util/SpatialPartitionTests.wurst +++ b/wurst/util/SpatialPartitionTests.wurst @@ -405,3 +405,20 @@ function countIn(vec2 center, real radius, real maxDisplacement, int groupId) re spatialPartitionGroupIsReserved(ALL).assertTrue() let fresh = spatialPartitionAllocateGroup() assertTrue(fresh != ALL and fresh != SUBSET) + +@Test function aQueuedIdClaimedBeforeReuseIsNotHandedOutAgain() + arena() + // The free list is a list of candidates, not of guarantees: an id can be claimed after it was + // queued. Allocate one, give it back, then have a consumer take that very row directly. + let recycled = spatialPartitionAllocateEntryId() + spatialPartitionReleaseEntryId(recycled) + spatialPartitionEntryIdIsReserved(recycled).assertFalse() + place(recycled, vec2(0., 0.), ALL) + spatialPartitionEntryIdIsReserved(recycled).assertTrue() + // It is queued and claimed at the same time, so popping it would hand a live row to a second + // consumer. Draining is what exposes that: the queued copy is at the front. + for i = 1 to 4 + assertTrue(spatialPartitionAllocateEntryId() != recycled) + // The row still belongs to whoever took it. + spatialPartitionEntryX(recycled).assertEquals(0.) + countIn(vec2(0., 0.), 50., 0., ALL).assertEquals(1) diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index d561b6d5..54a8acbc 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -11,8 +11,9 @@ package UnitSpatialIndex speed requires `unit.updateSpatialIndex()` for immediate visibility. The public native-less query package returns an empty set when this index is disabled. - Cell geometry now belongs to the partition, so `SPATIAL_PARTITION_CELL_SIZE` is the knob that used - to be `SPATIAL_INDEX_CELL_SIZE`. + Cell geometry now belongs to the partition, so `SPATIAL_PARTITION_CELL_SIZE` is the knob. + `SPATIAL_INDEX_CELL_SIZE` stays exported and deprecated, and a stale override is reported at init + rather than ignored. This package is over the ~500 line guideline on purpose. The tests at the bottom account for the excess and cannot move out: the index gates itself off with `isLua`, which the compiletime @@ -26,6 +27,7 @@ import HashMap import UnitIndexer import UnitSpatialIndexRemoval import SpatialPartition +import ErrorHandling // Configuration @@ -44,6 +46,14 @@ import SpatialPartition /** Disable when hidden units are never used to save one native per hit. */ @configurable public constant SPATIAL_INDEX_CHECK_HIDDEN = true +/** Deprecated. Cell geometry belongs to the shared grid now, so `SPATIAL_PARTITION_CELL_SIZE` is the + setting; one grid cannot have a second cell size for units alone. + + Kept exported so an existing `UnitSpatialIndex_config` still compiles, and defaulted to agree. It + is checked at init rather than ignored: a map that had tuned this and upgrades gets told its + value is no longer in effect, instead of quietly running on a different grid. */ +@configurable public constant SPATIAL_INDEX_CELL_SIZE = SPATIAL_PARTITION_CELL_SIZE + /* Every indexed unit lives in one partition group, taken from the partition rather than hardcoded: a group says what a query filters by, so a map whose own consumer had picked the same number would get units back from its queries with no way to tell them apart. @@ -126,7 +136,9 @@ function currentMaxDisplacement() returns real // Membership function registerUnit(unit u) - if u == null or spatialIndexByUnit.get(u) != 0 + // No group means no grid to join: queries would filter by an invalid group and match nothing, so + // registering would only build a registry that answers nothing. + if u == null or unitGroup < 0 or spatialIndexByUnit.get(u) != 0 return // Private removal marker. OnUnitEnterLeave deliberately keeps its legacy rect-only population, // while this registry also contains hidden and Locust units found by per-player enumeration. @@ -452,7 +464,15 @@ init // Written as a positive guard rather than an early return: `isLua` is folded, so on the Jass // target the whole body disappears and this package registers nothing at all. if isLua and USE_UNIT_SPATIAL_INDEX + if SPATIAL_INDEX_CELL_SIZE != SPATIAL_PARTITION_CELL_SIZE + error("UnitSpatialIndex: SPATIAL_INDEX_CELL_SIZE is deprecated and no longer in effect." + + " The grid is shared, so set SPATIAL_PARTITION_CELL_SIZE to " + + SPATIAL_INDEX_CELL_SIZE.toString() + " instead.") ensureUnitGroup() + // A group is what a query filters by, so without one every query would match nothing. Staying + // inactive says that honestly; activating would leave spatialIndexHealthy claiming an index that + // answers every question with an empty set. + if isLua and USE_UNIT_SPATIAL_INDEX and unitGroup >= 0 rebuildSpatialIndexGrid(boundMin, boundMax) onSpatialUnitRemove(() -> unregisterUnit(getSpatialRemovingUnit())) initializeSpatialRemovalTracking() From ce927da3e417034a2fc8fcc2198e3327ea814e09 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 12:39:06 +0200 Subject: [PATCH 08/10] State where the partition stops guaranteeing things Six rounds of review kept arriving at the same place from new angles, and it is a boundary rather than a defect. Allocation guarantees that two consumers are never handed the same row, including rows taken directly. It does not guarantee that a consumer only writes to rows it was given, because a row is reachable by its number and nothing about a number says who owns it. Closing that gap means an owner token on every mutator, paid on every position update, to catch a bug a caller can simply not write. The structure exists to be fast and assumes correct use, so the boundary is written down instead. --- wurst/util/SpatialPartition.wurst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index 90bf017f..62e9aca9 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -24,6 +24,15 @@ package SpatialPartition Entry ids are supplied by the owning layer and must be **1-based**: 0 is reserved as the chain sentinel. `rebuildSpatialPartition` must give the grid an extent before any entry is added. + What the two namespaces guarantee, and what they do not. `spatialPartitionAllocateEntryId` and + `spatialPartitionAllocateGroup` hand out an id or a group that no other consumer will be given, + including ones taken directly rather than through them, so two consumers that both ask can never + be handed the same row. What they do not do is police what you then address: a row is reachable + by its number, so writing to one you did not allocate corrupts whoever did, exactly as indexing + the wrong element of an array does. Enforcing that would mean an owner token on every mutator, + which is a cost on every position update to catch a bug that a caller can simply not write. The + structure is built for speed and assumes it is used correctly. + Like the indexes built on it, this targets **Lua**, where arrays grow dynamically. The cell chains are flattened as `cell * MAX_GROUPS + groupId` and the per-entry links as `id * MAX_GROUPS + groupId`, so a grid extent, an entry id and the nesting depth of open queries all have ceilings on Jass, where From a0def004cc66b21af2de4a38d7ceec9119fbee74 Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 12:55:02 +0200 Subject: [PATCH 09/10] Activate only on a ready grid, and count the rows actually in use Two things that let the layer look like it worked. Activation was gated on the group but not on the grid. A rebuild that refuses the extent leaves the grid unready and returns, and init carried on seeding and setting indexActive, so spatialIndexHealthy reported a usable index whose queries match nothing. Same shape as the group gate added last round, which is the sibling I should have looked for then: activation now requires everything init depends on, not the one thing that was reported. And the allocated-slot figure read the fresh-allocation cursor, which misses a high row taken directly and later recycled - handed out again from the free list without the cursor ever reaching it. Diagnostic only, but it is the number that says how far the id-keyed arrays actually reach, so it now tracks the highest id in use wherever one becomes live. --- wurst/util/SpatialPartition.wurst | 12 ++++++++++-- wurst/util/UnitSpatialIndex.wurst | 8 +++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index 62e9aca9..25cde3fc 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -248,6 +248,8 @@ var freeEntryIdCount = 0 next two allocations hand the same row to two consumers, which is the exact failure the allocator exists to prevent. This is the single record of what is outstanding. */ boolean array entryIdReserved +/** Highest id in use, tracked wherever one becomes live so it covers directly-taken rows too. */ +var highestEntryId = 0 /** Reserves an entry id no other consumer will be handed. Release it with `spatialPartitionReleaseEntryId` after removing the entry, or ids are never reused. Answers 0 when @@ -263,6 +265,7 @@ public function spatialPartitionAllocateEntryId() returns int freeEntryIds[freeEntryIdCount] = 0 if not entryIdReserved[recycled] entryIdReserved[recycled] = true + highestEntryId = max(highestEntryId, recycled) return recycled // Same rule for a fresh id: step over rows a consumer took directly. Addressing a row by a known // id is legitimate, so the allocator works around those instead of pretending it is the only way @@ -276,6 +279,7 @@ public function spatialPartitionAllocateEntryId() returns int let fresh = nextEntryId nextEntryId++ entryIdReserved[fresh] = true + highestEntryId = max(highestEntryId, fresh) return fresh /** Whether an id is currently handed out to some consumer. */ @@ -300,9 +304,12 @@ public function spatialPartitionReleaseEntryId(int id) freeEntryIds[freeEntryIdCount] = id freeEntryIdCount++ -/** Ids handed out across every consumer. Reuse keeps this at peak concurrent population. */ +/** Highest id in use across every consumer, which is what the arrays keyed by an id are sized to. + Not the fresh-allocation cursor: a high row taken directly and later recycled is handed out again + from the free list without the cursor ever reaching it, so the cursor can read far below the rows + that are actually live. */ public function spatialPartitionAllocatedEntryIds() returns int - return nextEntryId - 1 + return highestEntryId /* Groups have the same problem as ids, one level up: a group is what a query filters by, so two consumers that both picked group 0 would each see the other's entries come back and would have no @@ -403,6 +410,7 @@ public function spatialPartitionSetPos(int id, real x, real y) // rather than asking the allocator, and that is fine - but the allocator must then never // hand the same row to somebody else, so the two ways in share one record of what is taken. entryIdReserved[id] = true + highestEntryId = max(highestEntryId, id) let cell = cellAt(x, y) for groupId = 0 to groupsInUse - 1 let slot = groupSlot(id, groupId) diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index 54a8acbc..929ca725 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -469,11 +469,13 @@ init + " The grid is shared, so set SPATIAL_PARTITION_CELL_SIZE to " + SPATIAL_INDEX_CELL_SIZE.toString() + " instead.") ensureUnitGroup() - // A group is what a query filters by, so without one every query would match nothing. Staying - // inactive says that honestly; activating would leave spatialIndexHealthy claiming an index that - // answers every question with an empty set. + // Everything init depends on has to have worked. A group is what a query filters by, and the grid + // is what it walks; without either, every query matches nothing, and activating would leave + // spatialIndexHealthy claiming an index that answers every question with an empty set - which is + // the failure where a map silently loses its range checks instead of falling back to natives. if isLua and USE_UNIT_SPATIAL_INDEX and unitGroup >= 0 rebuildSpatialIndexGrid(boundMin, boundMax) + if isLua and USE_UNIT_SPATIAL_INDEX and unitGroup >= 0 and spatialPartitionIsReady() onSpatialUnitRemove(() -> unregisterUnit(getSpatialRemovingUnit())) initializeSpatialRemovalTracking() From 5f045c24351a8262703a502a7b0fdca625324e2d Mon Sep 17 00:00:00 2001 From: Frotty Date: Fri, 4 Sep 2026 14:11:21 +0200 Subject: [PATCH 10/10] Stop the query paths returning early, so nothing threads a flag Measured against the merged compiler on a real map. An inlined early return becomes a boolean the caller tests before every statement that follows it, and four of these sat on the query path, so the innermost entry walk ran six of those tests per visited entry. None of the four returns was load-bearing. Both partition queries guarded a whole body and returned 0, but nothing has been pushed when that guard fails, so the count at the end is 0 regardless: a positive guard around the body says the same thing. The two result accessors and the enumeration filter are single expressions written as a branch and two returns. On zombie-defense, addRangeMatches goes from 791 lines with 23 guard tests to 636 with none, and the per-entry walk now runs its statements unconditionally. No behaviour change: `and` short-circuits, so a Locust unit still costs the one native it did before. --- wurst/util/SpatialPartition.wurst | 240 +++++++++++++++--------------- wurst/util/UnitSpatialIndex.wurst | 8 +- 2 files changed, 127 insertions(+), 121 deletions(-) diff --git a/wurst/util/SpatialPartition.wurst b/wurst/util/SpatialPartition.wurst index 25cde3fc..947b6cf8 100644 --- a/wurst/util/SpatialPartition.wurst +++ b/wurst/util/SpatialPartition.wurst @@ -562,62 +562,64 @@ public function spatialPartitionBeginRangeQuery(vec2 center, real radius, real m queryDepth++ lastQueryVisits = 0 lastQueryBlocksSkipped = 0 - if tooDeep or not spatialPartitionIsValidGroupId(groupId) or not spatialPartitionIsReady() - return 0 - - let reach = radius + maxDisplacement - // May go negative when the displacement exceeds the radius, in which case nothing can be decided - // from a cached position and every hit is flagged. Zero is still decisive, though: it means only an - // entry exactly on the point is certainly inside, which is what a point query asks for. - let certainlyIn = radius - maxDisplacement - let certainlyInSq = certainlyIn * certainlyIn - let certainlyOutSq = reach * reach - - let minCx = cellCoordX(center.x - reach) - let maxCx = cellCoordX(center.x + reach) - let minCy = cellCoordY(center.y - reach) - let maxCy = cellCoordY(center.y + reach) - - // Walk coarse blocks and descend only into those holding a member of this group. A large radius - // over mostly empty space is the case this exists for: thousands of empty-cell visits become tens - // of block tests. - let minBx = minCx div SPATIAL_PARTITION_COARSE_FACTOR - let maxBx = maxCx div SPATIAL_PARTITION_COARSE_FACTOR - let minBy = minCy div SPATIAL_PARTITION_COARSE_FACTOR - let maxBy = maxCy div SPATIAL_PARTITION_COARSE_FACTOR - - var by = minBy - while by <= maxBy - var bx = minBx - while bx <= maxBx - if coarseCount[coarseSlot(bx + by * coarseWidth, groupId)] <= 0 - lastQueryBlocksSkipped++ - else - let cellMinX = max(minCx, bx * SPATIAL_PARTITION_COARSE_FACTOR) - let cellMaxX = min(maxCx, - bx * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) - let cellMinY = max(minCy, by * SPATIAL_PARTITION_COARSE_FACTOR) - let cellMaxY = min(maxCy, - by * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) - var cy = cellMinY - while cy <= cellMaxY - let rowBase = cy * gridWidth - var cx = cellMinX - while cx <= cellMaxX - var id = cellHead[headSlot(rowBase + cx, groupId)] - while id != 0 - let next = nextInCell[groupSlot(id, groupId)] - lastQueryVisits++ - let dx = entryX[id] - center.x - let dy = entryY[id] - center.y - let distSq = dx * dx + dy * dy - if distSq <= certainlyOutSq - pushMatch(id, not (certainlyIn >= 0. and distSq <= certainlyInSq)) - id = next - cx++ - cy++ - bx++ - by++ + // A positive guard rather than an early return. An inlined return has to be threaded through + // the rest of the body as a flag tested before every statement, and this body is the hottest + // loop in the package. Nothing was pushed when the guard fails, so the count below is 0 anyway. + if not tooDeep and spatialPartitionIsValidGroupId(groupId) and spatialPartitionIsReady() + + let reach = radius + maxDisplacement + // May go negative when the displacement exceeds the radius, in which case nothing can be decided + // from a cached position and every hit is flagged. Zero is still decisive, though: it means only an + // entry exactly on the point is certainly inside, which is what a point query asks for. + let certainlyIn = radius - maxDisplacement + let certainlyInSq = certainlyIn * certainlyIn + let certainlyOutSq = reach * reach + + let minCx = cellCoordX(center.x - reach) + let maxCx = cellCoordX(center.x + reach) + let minCy = cellCoordY(center.y - reach) + let maxCy = cellCoordY(center.y + reach) + + // Walk coarse blocks and descend only into those holding a member of this group. A large radius + // over mostly empty space is the case this exists for: thousands of empty-cell visits become tens + // of block tests. + let minBx = minCx div SPATIAL_PARTITION_COARSE_FACTOR + let maxBx = maxCx div SPATIAL_PARTITION_COARSE_FACTOR + let minBy = minCy div SPATIAL_PARTITION_COARSE_FACTOR + let maxBy = maxCy div SPATIAL_PARTITION_COARSE_FACTOR + + var by = minBy + while by <= maxBy + var bx = minBx + while bx <= maxBx + if coarseCount[coarseSlot(bx + by * coarseWidth, groupId)] <= 0 + lastQueryBlocksSkipped++ + else + let cellMinX = max(minCx, bx * SPATIAL_PARTITION_COARSE_FACTOR) + let cellMaxX = min(maxCx, + bx * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) + let cellMinY = max(minCy, by * SPATIAL_PARTITION_COARSE_FACTOR) + let cellMaxY = min(maxCy, + by * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) + var cy = cellMinY + while cy <= cellMaxY + let rowBase = cy * gridWidth + var cx = cellMinX + while cx <= cellMaxX + var id = cellHead[headSlot(rowBase + cx, groupId)] + while id != 0 + let next = nextInCell[groupSlot(id, groupId)] + lastQueryVisits++ + let dx = entryX[id] - center.x + let dy = entryY[id] - center.y + let distSq = dx * dx + dy * dy + if distSq <= certainlyOutSq + pushMatch(id, not (certainlyIn >= 0. and distSq <= certainlyInSq)) + id = next + cx++ + cy++ + bx++ + by++ return snapshotTop - queryBase[queryDepth - 1] @@ -639,77 +641,79 @@ public function spatialPartitionBeginBoxQuery(vec2 boxMin, vec2 boxMax, real max queryDepth++ lastQueryVisits = 0 lastQueryBlocksSkipped = 0 - if tooDeep or not spatialPartitionIsValidGroupId(groupId) or not spatialPartitionIsReady() - return 0 - - let outMinX = boxMin.x - maxDisplacement - let outMaxX = boxMax.x + maxDisplacement - let outMinY = boxMin.y - maxDisplacement - let outMaxY = boxMax.y + maxDisplacement - // The inner box collapses, or inverts, once the displacement exceeds half an edge. An inverted - // one is still correct as a test - no position satisfies it - so every hit is simply flagged. - let inMinX = boxMin.x + maxDisplacement - let inMaxX = boxMax.x - maxDisplacement - let inMinY = boxMin.y + maxDisplacement - let inMaxY = boxMax.y - maxDisplacement - - let minCx = cellCoordX(outMinX) - let maxCx = cellCoordX(outMaxX) - let minCy = cellCoordY(outMinY) - let maxCy = cellCoordY(outMaxY) - - let minBx = minCx div SPATIAL_PARTITION_COARSE_FACTOR - let maxBx = maxCx div SPATIAL_PARTITION_COARSE_FACTOR - let minBy = minCy div SPATIAL_PARTITION_COARSE_FACTOR - let maxBy = maxCy div SPATIAL_PARTITION_COARSE_FACTOR - - var by = minBy - while by <= maxBy - var bx = minBx - while bx <= maxBx - if coarseCount[coarseSlot(bx + by * coarseWidth, groupId)] <= 0 - lastQueryBlocksSkipped++ - else - let cellMinX = max(minCx, bx * SPATIAL_PARTITION_COARSE_FACTOR) - let cellMaxX = min(maxCx, - bx * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) - let cellMinY = max(minCy, by * SPATIAL_PARTITION_COARSE_FACTOR) - let cellMaxY = min(maxCy, - by * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) - var cy = cellMinY - while cy <= cellMaxY - let rowBase = cy * gridWidth - var cx = cellMinX - while cx <= cellMaxX - var id = cellHead[headSlot(rowBase + cx, groupId)] - while id != 0 - let next = nextInCell[groupSlot(id, groupId)] - lastQueryVisits++ - let px = entryX[id] - let py = entryY[id] - if px >= outMinX and px <= outMaxX and py >= outMinY and py <= outMaxY - pushMatch(id, not (px >= inMinX and px <= inMaxX - and py >= inMinY and py <= inMaxY)) - id = next - cx++ - cy++ - bx++ - by++ + if not tooDeep and spatialPartitionIsValidGroupId(groupId) and spatialPartitionIsReady() + + let outMinX = boxMin.x - maxDisplacement + let outMaxX = boxMax.x + maxDisplacement + let outMinY = boxMin.y - maxDisplacement + let outMaxY = boxMax.y + maxDisplacement + // The inner box collapses, or inverts, once the displacement exceeds half an edge. An inverted + // one is still correct as a test - no position satisfies it - so every hit is simply flagged. + let inMinX = boxMin.x + maxDisplacement + let inMaxX = boxMax.x - maxDisplacement + let inMinY = boxMin.y + maxDisplacement + let inMaxY = boxMax.y - maxDisplacement + + let minCx = cellCoordX(outMinX) + let maxCx = cellCoordX(outMaxX) + let minCy = cellCoordY(outMinY) + let maxCy = cellCoordY(outMaxY) + + let minBx = minCx div SPATIAL_PARTITION_COARSE_FACTOR + let maxBx = maxCx div SPATIAL_PARTITION_COARSE_FACTOR + let minBy = minCy div SPATIAL_PARTITION_COARSE_FACTOR + let maxBy = maxCy div SPATIAL_PARTITION_COARSE_FACTOR + + var by = minBy + while by <= maxBy + var bx = minBx + while bx <= maxBx + if coarseCount[coarseSlot(bx + by * coarseWidth, groupId)] <= 0 + lastQueryBlocksSkipped++ + else + let cellMinX = max(minCx, bx * SPATIAL_PARTITION_COARSE_FACTOR) + let cellMaxX = min(maxCx, + bx * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) + let cellMinY = max(minCy, by * SPATIAL_PARTITION_COARSE_FACTOR) + let cellMaxY = min(maxCy, + by * SPATIAL_PARTITION_COARSE_FACTOR + SPATIAL_PARTITION_COARSE_FACTOR - 1) + var cy = cellMinY + while cy <= cellMaxY + let rowBase = cy * gridWidth + var cx = cellMinX + while cx <= cellMaxX + var id = cellHead[headSlot(rowBase + cx, groupId)] + while id != 0 + let next = nextInCell[groupSlot(id, groupId)] + lastQueryVisits++ + let px = entryX[id] + let py = entryY[id] + if px >= outMinX and px <= outMaxX and py >= outMinY and py <= outMaxY + pushMatch(id, not (px >= inMinX and px <= inMaxX + and py >= inMinY and py <= inMaxY)) + id = next + cx++ + cy++ + bx++ + by++ return snapshotTop - queryBase[queryDepth - 1] /** Entry id `i` of the innermost active query, or 0 if there is no such result. */ public function spatialPartitionQueryId(int i) returns int - if queryDepth <= 0 or i < 0 or queryBase[queryDepth - 1] + i >= snapshotTop - return 0 - return snapshotId[queryBase[queryDepth - 1] + i] + // One expression rather than a guard and a return. These two are read once per result, so they + // inline into the caller's result loop, and an inlined return has to be carried through the rest + // of that loop as a flag tested before every following statement. + return queryDepth <= 0 or i < 0 or queryBase[queryDepth - 1] + i >= snapshotTop + ? 0 + : snapshotId[queryBase[queryDepth - 1] + i] /** Whether entry `i` could not be decided from its cached position and needs a live check against the requested radius. Always false when the query passed a zero displacement. */ public function spatialPartitionQueryUncertain(int i) returns boolean - if queryDepth <= 0 or i < 0 or queryBase[queryDepth - 1] + i >= snapshotTop - return false - return snapshotUncertain[queryBase[queryDepth - 1] + i] + return queryDepth <= 0 or i < 0 or queryBase[queryDepth - 1] + i >= snapshotTop + ? false + : snapshotUncertain[queryBase[queryDepth - 1] + i] public function spatialPartitionEndQuery() if queryDepth > 0 diff --git a/wurst/util/UnitSpatialIndex.wurst b/wurst/util/UnitSpatialIndex.wurst index 929ca725..455fe394 100644 --- a/wurst/util/UnitSpatialIndex.wurst +++ b/wurst/util/UnitSpatialIndex.wurst @@ -315,10 +315,12 @@ public function spatialIndexGridHeight() returns int snapshotStack[snapshotTop] = u snapshotTop++ +/* One expression, for the same reason as the partition's result accessors: this is tested once per + match, and an inlined early return is carried through the rest of the caller's loop as a flag. + `and` still short-circuits, so a Locust unit costs the one native it did before. */ @inline function unit.passesEnumerationState() returns boolean - if this.getAbilityLevel(LOCUST_ID) != 0 - return false - return not SPATIAL_INDEX_CHECK_HIDDEN or not this.isHidden() + return this.getAbilityLevel(LOCUST_ID) == 0 + and (not SPATIAL_INDEX_CHECK_HIDDEN or not this.isHidden()) /** Reads an entry's live position, caches it and stamps the tick, so a later query in the same frame decides it from the cache instead of paying the two natives again.