fix(headless): actually check readiness in pregenerateChunks - #5377
fix(headless): actually check readiness in pregenerateChunks#5377soloturn wants to merge 1 commit into
Conversation
#5024: PrepareWorld.step() loops until worldRenderer.pregenerateChunks() returns true or 5 seconds elapse. HeadlessWorldRenderer.pregenerateChunks() was an unimplemented stub that always returned false, so headless runs (including every MTE test) always burned the full 5 second wait regardless of how quickly the world actually generated. ## Fix Implemented pregenerateChunks() using the same chunk-loaded/local-view check updateChunksInProximity() already performs for per-frame updates: advance chunkProvider, force a full rescan of the current view region, and report done once every chunk in it is loaded. The rescan has to be forced: updateChunksInProximity's own "did the camera move" shortcut is keyed off chunkPos, which never changes in headless (NullCamera reports a constant position), so an unforced call would never re-examine chunk state and permanently report "nothing pending" from its zero-value default - a different way to end up with the always-true failure mode keturn already ran into on a bare `return true`, without ever confirming anything actually loaded. ## Verification :engine:compileJava clean. Ran the full integrationenvironment test suite (engine-tests:integrationTest, 18 MTE test classes covering per-class and per-method lifecycles back-to-back in the same JVM, 34 tests total) - the exact scenario keturn's earlier attempt broke ("first test fine, next one had problems") - all 34 pass, 0 failures. Fixes #5024 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe headless renderer now refreshes the chunk provider, forces a proximity-chunk scan within view distance, and reports whether chunk pre-generation has completed. ChangesHeadless chunk generation
Estimated code review effort: 2 (Simple) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized fix makes headless world preparation stop once required chunks are ready instead of always waiting for the timeout. No actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Fixes #5024 -
PrepareWorldalways waits the full 5 seconds in headless, regardless of how quickly the world is actually ready.Root cause
PrepareWorld.step()loops untilworldRenderer.pregenerateChunks()returnstrueor 5 seconds elapse.HeadlessWorldRenderer.pregenerateChunks()was an unimplemented stub:Always
falsemeans the loop only ever exits via the timeout, so every headless run - including every MTE integration test - burns the full 5 seconds here no matter how small or fast-generating the test world is.Fix
Implemented it using the same chunk-loaded/local-view readiness check
updateChunksInProximity()already performs for per-frame updates: advancechunkProvider, force a full rescan of the current view region, and report done once every chunk in it is loaded (chunkProvider.getChunk(pos) != null && worldProvider.getLocalView(pos) != nullfor each position) - mirroring what the graphicalRenderableWorldImpl.pregenerateChunks()checks, minus the mesh-generation part that doesn't apply headless.The rescan has to be forced.
updateChunksInProximity's own "did the camera move" shortcut is keyed offchunkPos, which never changes in headless (NullCameraalways reports the same position) - an unforced call would never re-examine chunk state and would permanently report "nothing pending" from its zero-value field default. That's a second, more subtle way to land on the same always-true-without-checking-anything failure mode as a barereturn true, which is what @keturn's comment on the issue describes having already tried and found broke a following test.Verification
:engine:compileJavaclean. Ran the fullengine-tests:integrationTestsuite scoped toorg.terasology.engine.integrationenvironment.*- 18 MTE test classes, 34 tests total, covering both per-class and per-method lifecycles, run back-to-back in the same JVM. This is the exact scenario keturn's earlier attempt broke ("first test fine, next one had problems"). All 34 pass, 0 failures, 0 errors.No existing test exercises
HeadlessWorldRenderer.pregenerateChunks()directly, only indirectly through the full MTE bootstrap above.