Write live positions back into the spatial index cache during a query - #473
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8767364c23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The annulus branch of spatialIndexBeginQuery already pays GetUnitX and GetUnitY for a unit whose cached position is stale by up to maxDisp. It used the live position for the distance test and threw it away, so every further query in the same frame read it again. Twelve synchronized range queries over the same crowd paid the two natives twelve times per boundary unit. The live position is now stored back, together with the current sweep tick, but only while the unit is still in its linked cell. Relinking mid-walk is not safe: the unit could move into a cell this walk has not reached yet and be visited and pushed a second time. The same-cell case covers nearly everything, since displacement is bounded well under one cell edge.
8767364 to
b00ec53
Compare
The write-back stamps lastSweepTick, but nothing read it back: certainlyIn and certainlyOut come from the global maxDisp, which is the worst staleness of any entry in the registry. A stationary unit in the annulus therefore landed in the annulus again on the next query of the same center and radius, and paid GetUnitX and GetUnitY once more. The write-back saved nothing in precisely the case it was written for. The annulus branch now re-tests against the padding the entry itself needs, tickDisplacement times age plus one, before deciding to read. The extra tick covers the write and the query sitting at opposite ends of their own ticks, the same slack displacementBoundTicks already carries over a sweep cycle. Both directions stay conservative: within radius minus that padding the true position cannot be outside, beyond radius plus it cannot be inside. For an entry written back a moment earlier the age is zero, so the band is one tick of movement wide instead of a whole sweep cycle. With 500 units at 128 per tick the bound is five ticks, so the second query's annulus is a fifth as wide and nearly every boundary unit is decided from the cache. The refinement sits inside the annulus branch alone. The cell window and the outer bands stay on the global bound: the window has to be chosen before any entry is known, and the outer bands are the cheap reject that keeps entries out of this arithmetic entirely.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea3f8c49e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
What
In
spatialIndexBeginQuery, the annulus branch (cached distance betweencertainlyInandcertainlyOut) reads the unit's live position to decide the match. That position was discarded. It is now written back intolastX/lastYwith the current sweep tick, so the next query in the same frame decides that unit from the cache and skips the two natives.Per-entry padding
The stamp is only worth writing if something reads it.
certainlyIn/certainlyOutcome from the globalmaxDisp, the worst staleness of any entry in the registry, so on its own the write-back changed nothing: a stationary boundary unit fell into the annulus again on the next query with the same center and radius and read both natives a second time.The annulus branch therefore re-tests against the padding the entry itself needs,
tickDisplacement() * (age + 1), before deciding to read. The extra tick covers the cache write and the query sitting at opposite ends of their own ticks, which is the slackdisplacementBoundTicksalready carries overrequiredSweepCycleTicks();currentMaxDisplacement()is now that same expression at the worst age, so the two bounds cannot drift apart.After a write-back the age is 0, so the band is one tick of movement wide rather than a whole sweep cycle. At 500 units and 128 per tick
displacementBoundTicksis 5, so the second query in the frame sees an annulus a fifth as wide, about ±16 world units instead of ±82.The cell window and the outer bands stay on the global bound: the window is chosen before any entry is known, and the outer bands are the cheap reject that keeps most entries out of this arithmetic.
spatialIndexBeginBoxQueryhas the same split and the same discarded live read but no write-back, so giving it one is a separate change.Why not relink
Calling
refreshUnithere would be wrong: a relink can move the unit into a cell the walk has not visited yet, and it would be visited and pushed twice. The write-back therefore happens only while the unit is still in its linked cell. Displacement is bounded well under one cell edge, so this covers nearly every case, and the invariant that a cached position lies in its linked cell holds.Context
Found while reading the emitted Lua for the synchronized-cast case (12+ range queries over 500+ units in one frame). The larger costs there are compiler-side and are specified in
LUA_HOT_PATH_SPEC.mdin the WurstScript repo (wurstscript/WurstScript#1284). This is the one stdlib-side change that is independent of those.Checks
grill typecheck: succeededgrill test(full suite): all tests succeededUnitSpatialIndexTestscovers grid arithmetic only and every query path is asserted inStdlibIngameTestsStdlibIngameTests) need a running map and were not run here.