Skip to content

fix(landscape): fail more gracefully when there are no occupied parcels#7954

Open
AnsisMalins wants to merge 4 commits intodevfrom
fix/zero-owned-parcels
Open

fix(landscape): fail more gracefully when there are no occupied parcels#7954
AnsisMalins wants to merge 4 commits intodevfrom
fix/zero-owned-parcels

Conversation

@AnsisMalins
Copy link
Copy Markdown
Contributor

Pull Request Description

What does this PR change?

This change adds input validation code in terrain generation so that more informative error messages are logged and the game proceeds with less breakage.

Test Instructions

Visit empty parcels in Genesis City and in worlds and see that the terrain is correct. In particular, test

  • teleport from city to world
  • teleport from world to city
  • teleport from world to world

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated (if required)
  • Performance impact has been considered
  • For SDK features: Test scene is included

Code Review Reference

Please review our Code Review Standards before submitting.

@AnsisMalins AnsisMalins requested review from a team as code owners March 31, 2026 08:18
@github-actions github-actions bot requested a review from DafGreco March 31, 2026 08:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 31, 2026

Claude finished @AnsisMalins's task in 1m 37s —— View job


Code Review

  • Gather context
  • Read changed files
  • Analyze diff
  • Post review

No blocking issues found. The changes correctly handle the zero-occupied-parcels edge case across all three files.

Summary of changes:

  • TerrainModel.csCalculateMinMaxParcels returns sentinel (int2(0,0), int2(-1,-1)) when empty, yielding sizeInParcels = (0,0) so terrain is centered at origin with size 2 × borderPadding. This is graceful.
  • TerrainGenerator.csCreateOccupancyMap returns null early (with a LogWarning) when ownedParcels.IsEmpty; callers already handle null by setting OccupancyMapSize = 0 and an empty NativeArray.
  • WorldTerrainGenerator.cs — Same null-handling pattern added consistently.

Traced the flat-terrain guarantee:
When no occupied parcels → OccupancyMap = nullOccupancyMapSize = 0GetParcelNoiseHeight uses occupancy = 0f. Both possible OccupancyFloor values (0 or 255) satisfy occupancy (0f) <= minValue, returning 0f. Flat terrain, as expected. ✓

One minor observation (no fix required):
CalculateMinMaxParcels returning (0,0)/(-1,-1) when borderPadding = 0 leads to sizeInParcelsWithPadding = (0,0), which causes Mathf.ClosestPowerOfTwo(0) = 0 and zero-sized chunks. This is a pre-existing edge case that is not worsened by this PR, and in practice borderPadding > 0 in all known configurations.

REVIEW_RESULT: PASS ✅
COMPLEXITY: SIMPLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved by Claude — simple fix/chore with no blocking issues. QA approval is still required.

@AnsisMalins
Copy link
Copy Markdown
Contributor Author

PR 7954

PR number: 7954
Windows change build
Windows baseline build
macOS change build
macOS baseline build

Intel Core i3

Change samples: 2670
Baseline samples: 2672

Metric Change Baseline Delta Improvement
CPU average 33.6 ms 33.6 ms 0.0 ms -0.0%
CPU 1% worst 54.6 ms 56.5 ms -1.9 ms 3.4% 🟢
CPU 0.1% worst 115.2 ms 116.1 ms -0.8 ms 0.7%
GPU average 16.1 ms 15.6 ms 0.5 ms -3.2% 🔴
GPU 1% worst 21.8 ms 21.5 ms 0.3 ms -1.3%
GPU 0.1% worst 25.2 ms 27.3 ms -2.2 ms 8.0% 🟢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant