From 6d8e64cdd3347d4cfd5dc764c42fc781b9979931 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 12 Sep 2026 10:13:41 +0200 Subject: [PATCH 1/5] Add a closure API over the unit spatial index. --- wurst/closures/SpatialIndexForUnits.wurst | 96 +++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index 725c38bc..f4efb510 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -1,5 +1,6 @@ package SpatialIndexForUnits import ArrayList +import ClosureForGroups import UnitSpatialIndex import Rect @@ -96,3 +97,98 @@ public function unitsOfPlayer(ArrayList result, player owner, UnitSpatialF public function unitsOfPlayer(ArrayList result, player owner) unitsOfPlayer(result, owner, null) + +// ============================================================================ +// Closure API +// ============================================================================ + +/* Almost every caller is doing the same three things: run a query, decide which matches count, + and act on those. The overloads above hand back a list and leave the last two to the caller. + These do the whole thing - the callback sees each match, and deciding is an `if` inside it. + + **This is a separate API, not a replacement for ClosureForGroups.** The index beats a Warcraft + group on Lua and loses to one on Jass, so which is faster depends on the backend. That choice + belongs to the caller, not to a silent substitution inside the function everyone already calls. + Behaviour is otherwise the same, including that the callback is destroyed afterwards - keep one + in a field and pass it repeatedly where the allocation matters. + + Matches are copied out and the query snapshot closed before the callback runs, for the reason + the list overloads do it: a callback is caller code. It can fail, which would leave the + snapshot open for good, and it can yield, which would let two queries corrupt each other. +*/ + +/** One scratch list per nesting level, so a callback may start another query. */ +ArrayList array scratchLists +var scratchDepth = 0 + +function borrowScratch() returns ArrayList + if scratchLists[scratchDepth] == null + scratchLists[scratchDepth] = new ArrayList() + let list = scratchLists[scratchDepth] + scratchDepth++ + return list + +function releaseScratch() + scratchDepth-- + +/** Runs the callback for every unit whose origin is within radius of center. */ +public function forEachUnitInRange(vec2 center, real radius, ForGroupCallback cb) + forEachUnitInRange(center, radius, false, cb) + +/** Runs the callback for every unit in range, optionally applying collision-size filtering. */ +public function forEachUnitInRange(vec2 center, real radius, bool collisionFiltering, + ForGroupCallback cb) + let matches = borrowScratch() + unitsInRange(matches, center, radius, collisionFiltering) + for i = 0 to matches.size() - 1 + cb.callback(matches.get(i)) + releaseScratch() + destroy cb + +/** Runs the callback for every unit whose origin is inside the rect. */ +public function forEachUnitInRect(rect area, ForGroupCallback cb) + let matches = borrowScratch() + unitsInRect(matches, area) + for i = 0 to matches.size() - 1 + cb.callback(matches.get(i)) + releaseScratch() + destroy cb + +/** Runs the callback for every unit owned by the player. */ +public function forEachUnitOfPlayer(player owner, ForGroupCallback cb) + let matches = borrowScratch() + unitsOfPlayer(matches, owner) + for i = 0 to matches.size() - 1 + cb.callback(matches.get(i)) + releaseScratch() + destroy cb + +/** As forEachUnitInRange, stopping as soon as the callback returns false. */ +public function forEachUnitInRangeUntil(vec2 center, real radius, ForGroupCallbackUntil cb) + let matches = borrowScratch() + unitsInRange(matches, center, radius) + for i = 0 to matches.size() - 1 + if not cb.callback(matches.get(i)) + break + releaseScratch() + destroy cb + +/** As forEachUnitInRect, stopping as soon as the callback returns false. */ +public function forEachUnitInRectUntil(rect area, ForGroupCallbackUntil cb) + let matches = borrowScratch() + unitsInRect(matches, area) + for i = 0 to matches.size() - 1 + if not cb.callback(matches.get(i)) + break + releaseScratch() + destroy cb + +/** As forEachUnitOfPlayer, stopping as soon as the callback returns false. */ +public function forEachUnitOfPlayerUntil(player owner, ForGroupCallbackUntil cb) + let matches = borrowScratch() + unitsOfPlayer(matches, owner) + for i = 0 to matches.size() - 1 + if not cb.callback(matches.get(i)) + break + releaseScratch() + destroy cb From a2098355c2941d71b87cd123ee0f802a6ec067ef Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 12 Sep 2026 10:16:25 +0200 Subject: [PATCH 2/5] Pin the closure API to the list API, early exit and nesting. --- wurst/StdlibIngameTests.wurst | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/wurst/StdlibIngameTests.wurst b/wurst/StdlibIngameTests.wurst index 17f17082..b84803ec 100644 --- a/wurst/StdlibIngameTests.wurst +++ b/wurst/StdlibIngameTests.wurst @@ -511,6 +511,24 @@ function describeDifference(group actual, group expected) returns string return "engine=" + expected.size() + " closure=" + actual.size() + " missed=" + missing + " spurious=" + extra +/** The closure API must see exactly what the list API sees, which parity above ties to the engine. */ +function checkClosureParity(vec2 pos, real radius, string label) + let viaClosure = CreateGroup() + let viaList = CreateGroup() + forEachUnitInRange(pos, radius) u -> + viaClosure.add(u) + unitsInRange(spatialResultScratch, pos, radius) + for i = 0 to spatialResultScratch.size() - 1 + viaList.add(spatialResultScratch.get(i)) + var equal = viaClosure.size() == viaList.size() + if equal + for i = 0 to viaClosure.size() - 1 + if not viaList.has(viaClosure.get(i)) + equal = false + check(equal, label + " (" + describeDifference(viaClosure, viaList) + ")") + viaClosure.destr() + viaList.destr() + function checkRangeParity(vec2 pos, real radius, bool collisionFiltering, string label) let actual = CreateGroup() let expected = CreateGroup() @@ -560,6 +578,31 @@ function testSpatialIndexParity() playerUnits.destr() checkRangeParity(SPATIAL_TEST_POS, 300., false, "range query matches the engine") + checkClosureParity(SPATIAL_TEST_POS, 300., "closure API sees the same units as the list API") + checkClosureParity(SPATIAL_TEST_POS, 2000., "closure API agrees on a wide radius") + + // Early exit must stop, and must stop after the callback that asked to. + var visited = 0 + forEachUnitInRangeUntil(SPATIAL_TEST_POS, 2000.) u -> + visited++ + return visited < 2 + check(visited == 2, "forEachUnitInRangeUntil stops when the callback says so (" + visited + ")") + + // A callback starting another query must not disturb the one it is inside: each nesting level + // takes its own scratch list. + var outer = 0 + var innerTotal = 0 + forEachUnitInRange(SPATIAL_TEST_POS, 300.) _u -> + outer++ + forEachUnitInRange(SPATIAL_TEST_POS, 300.) _v -> + innerTotal++ + let flat = CreateGroup() + collectViaSpatialIndex(SPATIAL_TEST_POS, 300., false, flat) + check(outer == flat.size(), "nested query leaves the outer iteration intact (" + + outer + " vs " + flat.size() + ")") + check(innerTotal == outer * flat.size(), "nested query runs in full each time (" + + innerTotal + ")") + flat.destr() checkRangeParity(SPATIAL_TEST_POS, 300., true, "collision-filtered query matches the engine") checkRangeParity(SPATIAL_TEST_POS, 121., false, "tight radius on a probe boundary") checkRangeParity(SPATIAL_TEST_POS, 2000., false, "wide radius") From a655bf027c61c10a25c52b8b50c4fe868a7ab949 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 12 Sep 2026 10:29:35 +0200 Subject: [PATCH 3/5] Hold scratch lists by owner, and stop advising a reuse the API forbids. The scratch lists were handed out by nesting depth, which assumes they come back in the order they went out. Callbacks may yield, and two that yield can resume in either order - the first to resume would then lower the depth while the second still held its list, and the next query would reset a list still being read. A slot is now owned until its own holder returns it. The documentation told callers to keep a callback in a field and pass it repeatedly, next to the sentence saying the call destroys it. Both cannot be true. The call destroys it, because the usual call site is a lambda written in place and nothing else would free it; a caller who wants no per-call allocation wants the list overloads, which is why they remain. The new tests counted through locals captured by a closure. Wurst captures those by value, so the counter read after the call would have stayed at zero and the checks would have failed whether or not the API worked. --- wurst/StdlibIngameTests.wurst | 39 +++++++----- wurst/closures/SpatialIndexForUnits.wurst | 74 +++++++++++++++-------- 2 files changed, 72 insertions(+), 41 deletions(-) diff --git a/wurst/StdlibIngameTests.wurst b/wurst/StdlibIngameTests.wurst index b84803ec..95455645 100644 --- a/wurst/StdlibIngameTests.wurst +++ b/wurst/StdlibIngameTests.wurst @@ -543,6 +543,11 @@ function checkRangeParity(vec2 pos, real radius, bool collisionFiltering, string actual.destr() expected.destr() +/** Mutated inside closures, so they cannot be locals - see the note at their use. */ +var closureVisited = 0 +var closureOuter = 0 +var closureInner = 0 + function testSpatialIndexParity() section("--- 13. spatial index vs engine enumeration ---") if isLua and USE_UNIT_SPATIAL_INDEX @@ -582,26 +587,30 @@ function testSpatialIndexParity() checkClosureParity(SPATIAL_TEST_POS, 2000., "closure API agrees on a wide radius") // Early exit must stop, and must stop after the callback that asked to. - var visited = 0 + // The counters are package level on purpose: a local captured by a closure is captured by + // value, so incrementing one inside the callback would leave the copy read here at zero and + // the check would fail whether or not the API worked. + closureVisited = 0 forEachUnitInRangeUntil(SPATIAL_TEST_POS, 2000.) u -> - visited++ - return visited < 2 - check(visited == 2, "forEachUnitInRangeUntil stops when the callback says so (" + visited + ")") - - // A callback starting another query must not disturb the one it is inside: each nesting level - // takes its own scratch list. - var outer = 0 - var innerTotal = 0 + closureVisited++ + return closureVisited < 2 + check(closureVisited == 2, "forEachUnitInRangeUntil stops when the callback says so (" + + closureVisited + ")") + + // A callback starting another query must not disturb the one it is inside: each takes its own + // scratch list, and gives it back itself. + closureOuter = 0 + closureInner = 0 forEachUnitInRange(SPATIAL_TEST_POS, 300.) _u -> - outer++ + closureOuter++ forEachUnitInRange(SPATIAL_TEST_POS, 300.) _v -> - innerTotal++ + closureInner++ let flat = CreateGroup() collectViaSpatialIndex(SPATIAL_TEST_POS, 300., false, flat) - check(outer == flat.size(), "nested query leaves the outer iteration intact (" - + outer + " vs " + flat.size() + ")") - check(innerTotal == outer * flat.size(), "nested query runs in full each time (" - + innerTotal + ")") + check(closureOuter == flat.size(), "nested query leaves the outer iteration intact (" + + closureOuter + " vs " + flat.size() + ")") + check(closureInner == closureOuter * flat.size(), "nested query runs in full each time (" + + closureInner + ")") flat.destr() checkRangeParity(SPATIAL_TEST_POS, 300., true, "collision-filtered query matches the engine") checkRangeParity(SPATIAL_TEST_POS, 121., false, "tight radius on a probe boundary") diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index f4efb510..3991355e 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -1,6 +1,7 @@ package SpatialIndexForUnits import ArrayList import ClosureForGroups +import ErrorHandling import UnitSpatialIndex import Rect @@ -109,27 +110,42 @@ public function unitsOfPlayer(ArrayList result, player owner) **This is a separate API, not a replacement for ClosureForGroups.** The index beats a Warcraft group on Lua and loses to one on Jass, so which is faster depends on the backend. That choice belongs to the caller, not to a silent substitution inside the function everyone already calls. - Behaviour is otherwise the same, including that the callback is destroyed afterwards - keep one - in a field and pass it repeatedly where the allocation matters. + Behaviour is otherwise the same, including ownership: **the callback is destroyed after the + call**, as ClosureForGroups does, because the usual call site is a lambda written in place and + nothing else would free it. Passing the same callback twice therefore uses a destroyed object. + Where the per-call closure is too much, use the list overloads above - they allocate nothing, + which is the reason they remain. Matches are copied out and the query snapshot closed before the callback runs, for the reason the list overloads do it: a callback is caller code. It can fail, which would leave the snapshot open for good, and it can yield, which would let two queries corrupt each other. */ -/** One scratch list per nesting level, so a callback may start another query. */ -ArrayList array scratchLists -var scratchDepth = 0 - -function borrowScratch() returns ArrayList - if scratchLists[scratchDepth] == null - scratchLists[scratchDepth] = new ArrayList() - let list = scratchLists[scratchDepth] - scratchDepth++ - return list +/* Scratch lists held by slot rather than by nesting depth. -function releaseScratch() - scratchDepth-- + A depth counter assumes borrows are returned in the order they were taken. Callbacks may yield, + and two that yield can resume in either order - at which point the first to resume would lower + the depth while the second still holds its list, and the next query would reset a list that is + still being read. A slot is owned until its own holder gives it back, whatever order that + happens in. */ +constant MAX_NESTED_QUERIES = 16 +ArrayList array scratchLists +bool array scratchInUse + +function borrowScratch() returns int + var slot = 0 + while slot < MAX_NESTED_QUERIES and scratchInUse[slot] + slot++ + if slot == MAX_NESTED_QUERIES + error("SpatialIndexForUnits: more than " + MAX_NESTED_QUERIES.toString() + + " queries are open at once; a callback is most likely never returning.") + if scratchLists[slot] == null + scratchLists[slot] = new ArrayList() + scratchInUse[slot] = true + return slot + +function releaseScratch(int slot) + scratchInUse[slot] = false /** Runs the callback for every unit whose origin is within radius of center. */ public function forEachUnitInRange(vec2 center, real radius, ForGroupCallback cb) @@ -138,57 +154,63 @@ public function forEachUnitInRange(vec2 center, real radius, ForGroupCallback cb /** Runs the callback for every unit in range, optionally applying collision-size filtering. */ public function forEachUnitInRange(vec2 center, real radius, bool collisionFiltering, ForGroupCallback cb) - let matches = borrowScratch() + let slot = borrowScratch() + let matches = scratchLists[slot] unitsInRange(matches, center, radius, collisionFiltering) for i = 0 to matches.size() - 1 cb.callback(matches.get(i)) - releaseScratch() + releaseScratch(slot) destroy cb /** Runs the callback for every unit whose origin is inside the rect. */ public function forEachUnitInRect(rect area, ForGroupCallback cb) - let matches = borrowScratch() + let slot = borrowScratch() + let matches = scratchLists[slot] unitsInRect(matches, area) for i = 0 to matches.size() - 1 cb.callback(matches.get(i)) - releaseScratch() + releaseScratch(slot) destroy cb /** Runs the callback for every unit owned by the player. */ public function forEachUnitOfPlayer(player owner, ForGroupCallback cb) - let matches = borrowScratch() + let slot = borrowScratch() + let matches = scratchLists[slot] unitsOfPlayer(matches, owner) for i = 0 to matches.size() - 1 cb.callback(matches.get(i)) - releaseScratch() + releaseScratch(slot) destroy cb /** As forEachUnitInRange, stopping as soon as the callback returns false. */ public function forEachUnitInRangeUntil(vec2 center, real radius, ForGroupCallbackUntil cb) - let matches = borrowScratch() + let slot = borrowScratch() + let matches = scratchLists[slot] unitsInRange(matches, center, radius) for i = 0 to matches.size() - 1 if not cb.callback(matches.get(i)) break - releaseScratch() + releaseScratch(slot) destroy cb /** As forEachUnitInRect, stopping as soon as the callback returns false. */ public function forEachUnitInRectUntil(rect area, ForGroupCallbackUntil cb) - let matches = borrowScratch() + let slot = borrowScratch() + let matches = scratchLists[slot] unitsInRect(matches, area) for i = 0 to matches.size() - 1 if not cb.callback(matches.get(i)) break - releaseScratch() + releaseScratch(slot) destroy cb /** As forEachUnitOfPlayer, stopping as soon as the callback returns false. */ public function forEachUnitOfPlayerUntil(player owner, ForGroupCallbackUntil cb) - let matches = borrowScratch() + let slot = borrowScratch() + let matches = scratchLists[slot] unitsOfPlayer(matches, owner) for i = 0 to matches.size() - 1 if not cb.callback(matches.get(i)) break - releaseScratch() + releaseScratch(slot) destroy cb From de977d8cae9eb124ed67f127f6ebf852d4225924 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 12 Sep 2026 10:39:44 +0200 Subject: [PATCH 4/5] Stop the call when no scratch list is free. error() reports and returns, so the guard printed its message and then carried on with the slot index one past the pool - handing every overflowing call the same list to reset and read, which is the sharing the slots were introduced to prevent. Borrowing now answers -1 when the pool is exhausted and each entry point returns, destroying the callback it was handed so ownership stays consistent with the ordinary path. --- wurst/closures/SpatialIndexForUnits.wurst | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index 3991355e..7d2f58e4 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -132,13 +132,17 @@ constant MAX_NESTED_QUERIES = 16 ArrayList array scratchLists bool array scratchInUse +/** A free slot, or -1 when none is left. `error` reports and returns, so the caller must check. */ function borrowScratch() returns int var slot = 0 while slot < MAX_NESTED_QUERIES and scratchInUse[slot] slot++ if slot == MAX_NESTED_QUERIES + // Reporting is not stopping: error() returns, and carrying on here would hand every + // overflowing call the same list to reset and read. error("SpatialIndexForUnits: more than " + MAX_NESTED_QUERIES.toString() + " queries are open at once; a callback is most likely never returning.") + return -1 if scratchLists[slot] == null scratchLists[slot] = new ArrayList() scratchInUse[slot] = true @@ -155,6 +159,10 @@ public function forEachUnitInRange(vec2 center, real radius, ForGroupCallback cb public function forEachUnitInRange(vec2 center, real radius, bool collisionFiltering, ForGroupCallback cb) let slot = borrowScratch() + if slot < 0 + // No list to borrow. The callback is still ours to free. + destroy cb + return let matches = scratchLists[slot] unitsInRange(matches, center, radius, collisionFiltering) for i = 0 to matches.size() - 1 @@ -165,6 +173,10 @@ public function forEachUnitInRange(vec2 center, real radius, bool collisionFilte /** Runs the callback for every unit whose origin is inside the rect. */ public function forEachUnitInRect(rect area, ForGroupCallback cb) let slot = borrowScratch() + if slot < 0 + // No list to borrow. The callback is still ours to free. + destroy cb + return let matches = scratchLists[slot] unitsInRect(matches, area) for i = 0 to matches.size() - 1 @@ -175,6 +187,10 @@ public function forEachUnitInRect(rect area, ForGroupCallback cb) /** Runs the callback for every unit owned by the player. */ public function forEachUnitOfPlayer(player owner, ForGroupCallback cb) let slot = borrowScratch() + if slot < 0 + // No list to borrow. The callback is still ours to free. + destroy cb + return let matches = scratchLists[slot] unitsOfPlayer(matches, owner) for i = 0 to matches.size() - 1 @@ -185,6 +201,10 @@ public function forEachUnitOfPlayer(player owner, ForGroupCallback cb) /** As forEachUnitInRange, stopping as soon as the callback returns false. */ public function forEachUnitInRangeUntil(vec2 center, real radius, ForGroupCallbackUntil cb) let slot = borrowScratch() + if slot < 0 + // No list to borrow. The callback is still ours to free. + destroy cb + return let matches = scratchLists[slot] unitsInRange(matches, center, radius) for i = 0 to matches.size() - 1 @@ -196,6 +216,10 @@ public function forEachUnitInRangeUntil(vec2 center, real radius, ForGroupCallba /** As forEachUnitInRect, stopping as soon as the callback returns false. */ public function forEachUnitInRectUntil(rect area, ForGroupCallbackUntil cb) let slot = borrowScratch() + if slot < 0 + // No list to borrow. The callback is still ours to free. + destroy cb + return let matches = scratchLists[slot] unitsInRect(matches, area) for i = 0 to matches.size() - 1 @@ -207,6 +231,10 @@ public function forEachUnitInRectUntil(rect area, ForGroupCallbackUntil cb) /** As forEachUnitOfPlayer, stopping as soon as the callback returns false. */ public function forEachUnitOfPlayerUntil(player owner, ForGroupCallbackUntil cb) let slot = borrowScratch() + if slot < 0 + // No list to borrow. The callback is still ours to free. + destroy cb + return let matches = scratchLists[slot] unitsOfPlayer(matches, owner) for i = 0 to matches.size() - 1 From 835ed44fd0d04b5035c81df97dfbd93580b7b8c6 Mon Sep 17 00:00:00 2001 From: Frotty Date: Sat, 12 Sep 2026 11:15:51 +0200 Subject: [PATCH 5/5] Give each query its own list instead of borrowing from a pool. A pool has to be handed back, and a callback which fails never hands anything back. The slot was then gone for the rest of the map, so sixteen recoverable failures would leave every later closure query rejected - and the API documents that a callback can fail, which is what makes that reachable rather than theoretical. Owning the list per call costs one allocation on a path which already allocates a closure, and a failed callback now costs that list rather than the ability to query at all. Destroying it also clears the unit references it collected, which a released pool slot did not - a wide query kept everything it returned reachable, in every slot it had used. The list overloads still allocate nothing, which is the reason they remain. --- wurst/closures/SpatialIndexForUnits.wurst | 95 ++++++----------------- 1 file changed, 22 insertions(+), 73 deletions(-) diff --git a/wurst/closures/SpatialIndexForUnits.wurst b/wurst/closures/SpatialIndexForUnits.wurst index 7d2f58e4..28f80ac5 100644 --- a/wurst/closures/SpatialIndexForUnits.wurst +++ b/wurst/closures/SpatialIndexForUnits.wurst @@ -1,7 +1,6 @@ package SpatialIndexForUnits import ArrayList import ClosureForGroups -import ErrorHandling import UnitSpatialIndex import Rect @@ -119,37 +118,17 @@ public function unitsOfPlayer(ArrayList result, player owner) Matches are copied out and the query snapshot closed before the callback runs, for the reason the list overloads do it: a callback is caller code. It can fail, which would leave the snapshot open for good, and it can yield, which would let two queries corrupt each other. -*/ -/* Scratch lists held by slot rather than by nesting depth. - - A depth counter assumes borrows are returned in the order they were taken. Callbacks may yield, - and two that yield can resume in either order - at which point the first to resume would lower - the depth while the second still holds its list, and the next query would reset a list that is - still being read. A slot is owned until its own holder gives it back, whatever order that - happens in. */ -constant MAX_NESTED_QUERIES = 16 -ArrayList array scratchLists -bool array scratchInUse - -/** A free slot, or -1 when none is left. `error` reports and returns, so the caller must check. */ -function borrowScratch() returns int - var slot = 0 - while slot < MAX_NESTED_QUERIES and scratchInUse[slot] - slot++ - if slot == MAX_NESTED_QUERIES - // Reporting is not stopping: error() returns, and carrying on here would hand every - // overflowing call the same list to reset and read. - error("SpatialIndexForUnits: more than " + MAX_NESTED_QUERIES.toString() - + " queries are open at once; a callback is most likely never returning.") - return -1 - if scratchLists[slot] == null - scratchLists[slot] = new ArrayList() - scratchInUse[slot] = true - return slot - -function releaseScratch(int slot) - scratchInUse[slot] = false + Each call owns the list it collects into, rather than borrowing from a pool. A pool has to be + handed back, and a callback which fails never hands anything back - a pooled slot would then be + gone for the rest of the map, and the API would stop working after enough failures. Here a + failed callback costs one list, the same way it already costs the closure it was called with, + and the next query is unaffected. Destroying the list also clears the unit references it + collected, so a wide query does not keep them reachable afterwards. + + That is one list per call on top of the closure. It is the price of the convenience: the list + overloads allocate nothing at all, which is why they remain. +*/ /** Runs the callback for every unit whose origin is within radius of center. */ public function forEachUnitInRange(vec2 center, real radius, ForGroupCallback cb) @@ -158,87 +137,57 @@ public function forEachUnitInRange(vec2 center, real radius, ForGroupCallback cb /** Runs the callback for every unit in range, optionally applying collision-size filtering. */ public function forEachUnitInRange(vec2 center, real radius, bool collisionFiltering, ForGroupCallback cb) - let slot = borrowScratch() - if slot < 0 - // No list to borrow. The callback is still ours to free. - destroy cb - return - let matches = scratchLists[slot] + let matches = new ArrayList() unitsInRange(matches, center, radius, collisionFiltering) for i = 0 to matches.size() - 1 cb.callback(matches.get(i)) - releaseScratch(slot) + destroy matches destroy cb /** Runs the callback for every unit whose origin is inside the rect. */ public function forEachUnitInRect(rect area, ForGroupCallback cb) - let slot = borrowScratch() - if slot < 0 - // No list to borrow. The callback is still ours to free. - destroy cb - return - let matches = scratchLists[slot] + let matches = new ArrayList() unitsInRect(matches, area) for i = 0 to matches.size() - 1 cb.callback(matches.get(i)) - releaseScratch(slot) + destroy matches destroy cb /** Runs the callback for every unit owned by the player. */ public function forEachUnitOfPlayer(player owner, ForGroupCallback cb) - let slot = borrowScratch() - if slot < 0 - // No list to borrow. The callback is still ours to free. - destroy cb - return - let matches = scratchLists[slot] + let matches = new ArrayList() unitsOfPlayer(matches, owner) for i = 0 to matches.size() - 1 cb.callback(matches.get(i)) - releaseScratch(slot) + destroy matches destroy cb /** As forEachUnitInRange, stopping as soon as the callback returns false. */ public function forEachUnitInRangeUntil(vec2 center, real radius, ForGroupCallbackUntil cb) - let slot = borrowScratch() - if slot < 0 - // No list to borrow. The callback is still ours to free. - destroy cb - return - let matches = scratchLists[slot] + let matches = new ArrayList() unitsInRange(matches, center, radius) for i = 0 to matches.size() - 1 if not cb.callback(matches.get(i)) break - releaseScratch(slot) + destroy matches destroy cb /** As forEachUnitInRect, stopping as soon as the callback returns false. */ public function forEachUnitInRectUntil(rect area, ForGroupCallbackUntil cb) - let slot = borrowScratch() - if slot < 0 - // No list to borrow. The callback is still ours to free. - destroy cb - return - let matches = scratchLists[slot] + let matches = new ArrayList() unitsInRect(matches, area) for i = 0 to matches.size() - 1 if not cb.callback(matches.get(i)) break - releaseScratch(slot) + destroy matches destroy cb /** As forEachUnitOfPlayer, stopping as soon as the callback returns false. */ public function forEachUnitOfPlayerUntil(player owner, ForGroupCallbackUntil cb) - let slot = borrowScratch() - if slot < 0 - // No list to borrow. The callback is still ours to free. - destroy cb - return - let matches = scratchLists[slot] + let matches = new ArrayList() unitsOfPlayer(matches, owner) for i = 0 to matches.size() - 1 if not cb.callback(matches.get(i)) break - releaseScratch(slot) + destroy matches destroy cb