implement async chunk generation (codex)#24
Conversation
- *Tx is explicitly invalid after the transaction ends. Storing it on World means World can keep a pointer to a closed transaction. - It hides a transaction dependency inside methods like w.chunk(). A caller cannot tell that chunk resolution may need the live transaction. - It makes public/non-transaction World methods accidentally depend on “whatever transaction happened last.” - It is fragile with async callbacks because the whole async design is about crossing transaction boundaries.
Constraint: Loader.Chunk reads loaded chunks under l.mu, so async callbacks must write through the same lock. Rejected: Keep Load holding l.mu across loadChunkAsync, callbacks may run synchronously and deadlock on the loader lock. Confidence: high Scope-risk: narrow Directive: Do not mutate Loader fields from chunk callbacks without holding l.mu.
Constraint: Async loader requests can fan out to hundreds of unique chunks at high view radii. Rejected: Spawn one goroutine per chunk request | lets loader radius directly control provider/generator concurrency. Confidence: high Scope-risk: moderate Directive: Keep raw chunk loading off the transaction path, but install chunks and invoke callbacks through World.Exec.
Constraint: Loader callbacks and bounded chunk workers both changed concurrency behavior in this branch. Rejected: Leave regression coverage uncommitted | would push concurrency fixes without local proof points. Confidence: high Scope-risk: narrow Directive: Keep these tests aligned with loader callback locking and ChunkLoadWorkers semantics. Tested: go test ./server/world; go test -race ./server/world; go test ./... Not-tested: live server profiling with high-radius clients
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Constraint: CI staticcheck flags the untagged switch in loadChunk as QF1002. Rejected: Keep switch form | adds no behavior and fails the build. Confidence: high Scope-risk: narrow Directive: Keep loadChunk error handling simple unless more cases are added. Tested: go test ./server/world Not-tested: staticcheck locally, installed binary was built with Go 1.25 while module requires Go 1.26
Constraint: ChunkLoadWorkers is configurable, so the unset default should be predictable. Rejected: Derive the default from GOMAXPROCS | couples world chunk loading policy to process CPU settings without a clear benefit. Confidence: high Scope-risk: narrow Directive: Tune async chunk loading through Config.ChunkLoadWorkers when a world needs a different limit. Tested: go test ./server/world Not-tested: live high-radius chunk loading profile
Constraint: PR branch should avoid adding a new world test file for this review fix. Rejected: Keep local regression tests | expands the branch beyond the requested code changes. Confidence: high Scope-risk: narrow Directive: Reintroduce focused tests separately if the project wants regression coverage for async chunk loading. Tested: go test ./server/world Not-tested: race detector after removing tests
Constraint: Async chunk request scheduling should be swappable without changing chunkRequest state handling. Rejected: Let chunkRequest call World queue internals directly | couples request coalescing to the worker-pool implementation. Confidence: high Scope-risk: narrow Directive: Keep chunk installation on World.Exec even if the request handler changes. Tested: go test ./server/world Not-tested: alternate chunk request handler implementation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1da801d. Configure here.
2 participants
Note
High Risk
High risk because it changes core chunk loading/generation paths and introduces new asynchronous, concurrent chunk installation/callback handling that can affect world consistency and shutdown behavior.
Overview
Implements asynchronous chunk load/generation via a new
chunkRequestsystem: concurrent workers load/generatechunk.Columns off-thread, then schedule installation back onto the world transaction queue and fan out callbacks to all waiters.World initialization now starts a configurable worker pool (
Config.ChunkLoadWorkers, default 4) and tracks in-flight loads inWorld.chunkRequests;Loader.Loadis updated to request chunks asynchronously and only attach/view them when the callback runs.Refactors several world read/write helpers to operate on
Tx(e.g., block/biome/liquid/weather/height queries) and adjusts lighting/sky-light behavior to avoid implicitly loading chunks for some reads (e.g.,World.lightreturns 0 if the chunk isn’t loaded).Reviewed by Cursor Bugbot for commit 1da801d. Bugbot is set up for automated code reviews on this repo. Configure here.