Repository navigation
Fix building height when datapck enabled - #1294
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The logic change is targeted and validated with new tests, with only a minor documentation clarity nit noted.
Pull request overview
Adjusts building start-Y offset calculation so that, when terrain generation is enabled and the terrain base is lower than the configured --ground-level (e.g., with an extended floor/datapack), buildings follow the actual terrain instead of being clamped to the configured ground level.
Changes:
- Change
calculate_start_y_offsetto derivemax_ground_levelpurely from sampled terrain levels (falling back toargs.ground_levelonly if no samples exist). - Add unit tests validating behavior for “sunk” terrain bases and for vanilla/default terrain levels.
File summaries
| File | Description |
|---|---|
| src/element_processing/buildings.rs | Updates start-Y offset logic for terrain-based placement and adds regression tests for low terrain bases. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
⏱️ Benchmark run finished in 0m 42s 📈 Compared against baseline: 18s time, 1160 MB memory 🟢 Generation time is unchanged. 📅 Last benchmark: 2026-08-23 10:08:01 UTC You can retrigger the benchmark by commenting |
…_level (upstream port) Hand-port of upstream 7c3c188 + a08c23a (PR louis-e#1294), the last three commits this fork was behind. calculate_start_y_offset seeded its max with args.ground_level, so any terrain base below it - the deep-floor datapack upstream fixed this for, or a scaled render that sinks the base - stranded every low-lying building up at the requested ground level. The max now reduces over the terrain samples alone and falls back to args.ground_level only when no sample has terrain, merged under this fork's tile-invariant corner sampling. Upstream's regression tests adapted to the fork's test editor, plus a fallback-path test. Rides along from the same upstream range: the chunks_exact -> as_chunks refactors in floodfill.rs and voxelize.rs, behaviour-neutral, taken to keep the files diffable against upstream. Golden gate: all five fixtures unchanged (flat-ground fixtures sit exactly at ground_level, where old and new agree). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ours-merge, zero content change: 7c3c188/a08c23ad (PR louis-e#1294 building start-Y) landed as 7ade707 in 3.1.5, hand-merged under the fork's tile-invariant sampling. Recording the upstream commits as ancestors clears the cosmetic 'commits behind' counter without rebasing the fork trunk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
No description provided.