[Engine] Overhaul simulation correctness, world generation, and chunk sleeping#13
Open
Kakapio wants to merge 1 commit into
Open
[Engine] Overhaul simulation correctness, world generation, and chunk sleeping#13Kakapio wants to merge 1 commit into
Kakapio wants to merge 1 commit into
Conversation
… sleeping Restructure as lib + bin so integration tests can use the crate directly. Simulation: make simulators pure deciders (calculate_step -> StepAction) with the engine owning all writes. Cross-chunk moves and all interactions become DeferredSteps, applied serially with re-validation, fixing: - cross-chunk Replace placing the result at the source cell - cross-chunk Preserve interactions silently doing nothing - inter-chunk move conflicts destroying the losing particle - iteration-order-dependent in-chunk interactions Per-(tick, chunk) seeded RNG + ordered apply make simulation fully deterministic; DashMap dependency removed. Generation: replace UnsafeCell + manual threads (a real data race via vein spill-over across thread boundaries) with rayon per-chunk generation and serial spill application. Zero unsafe remains. Seeded generation via Map::generate_with_seed. Flat row-major chunk storage with one shared index convention fixes the square-map-only chunk layout. Performance: chunk versions only bump on real change (no more 80 Hz GPU re-uploads of still water) and settled chunks sleep, woken by mutations and neighbor border activity. Dirty-flag refresh machinery deleted. Tests: 24 passing - mass conservation, cross-chunk interaction regressions, sleep/wake, determinism, generator invariants, and insta snapshots of terrain and a settled pool. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A deep-dive review of the core engine found several correctness bugs (including one genuine data race), constant GPU re-upload churn, and a move pipeline that every future particle type would have to re-implement. This PR fixes all of it in three pieces, with a regression test suite to keep it fixed.
1. Move pipeline: one pure decision path, one application path
Simulatorimplementations are now pure:FluidSimulator::calculate_stepreads aStepContextand returns aStepAction(Stay,MoveTo,ReplaceTarget,ConvertTarget) — the engine owns all writes via a singleapply_local_step. In-chunk empty-cell moves apply immediately; cross-chunk moves and all interactions becomeDeferredSteps, applied serially in deterministic order with both ends re-validated. A step that lost its race simply doesn't happen and the particle stays put.This one mechanism fixes four bugs:
Simulation is now fully deterministic: per-(tick, chunk) seeded
SmallRngplus the ordered apply phase make thread scheduling irrelevant. Thedashmapdependency is gone.2. World generation: no more
unsafe, seeded, layout-correctThe old generator shared a
Vec<Chunk>across manually-spawned threads throughUnsafeCell+unsafe impl Sync, partitioned by column ranges — but ore veins write at ±1 offsets across partition boundaries, so this was a real data race (UB). It's replaced by rayon per-chunk generation with vein spill-over collected and applied serially. Zerounsaferemains in the codebase.Map::generate_with_seedmakes worlds reproducible (generatestill picks a random seed)Vecbehind one sharedcoords::chunk_indexconvention, fixing a latent bug where the chunk layout was only correct for square mapsroll_special_particleno longer allocates aVecand sorts per cell3. Chunk sleeping and conditional versions
Chunk::versiononly bumps when a pass actually changes something, so the renderer stops re-uploading 4 KB materials at 80 Hz for still waterset_particle_at, including facing neighbors for border cells) or when a neighbor's border changes (BorderActivity). Simulation cost now scales with moving liquid, not total liquiddirty/trigger_refresh/update_dirty_chunksmachinery became redundant and was deleted — painting now activates simulation without depending onUpdate-schedule orderingTests
Restructured as lib + bin (
src/lib.rs) so integration tests usecavernborn::directly instead of the previous#[path]include hack. 24 tests:instasnapshots of generated terrain and a settled pool (64 particles → exactly two flat rows)Deliberately preserved quirks (follow-up candidates)
.max(0)edge clamp lets lone droplets wander sideways along the floor row (pre-existing)🤖 Generated with Claude Code