Remove terrain clamping at 50 height difference

[BlackYps]

FAF patched the black plane bug years ago, so we don't need this anymore.

Fixes #41

Summary by CodeRabbit

• Removed Features

  • Removed height difference warning popup that triggered for terrain height ranges exceeding 49.9 units
  • Removed HeightmapClamp toggle from editor settings
  • Removed terrain height clamping utility functions

• Refactor

  • Refactored terrain height constraint system to use normalized (0–1) range values instead of absolute height values

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes remove the HeightmapClamp feature and refactor the terrain height system from absolute Min/Max/Heightmap values to a normalized 0-1 range. Three public heightmap API methods are eliminated, and terrain height clamping logic is updated to operate on normalized values instead of absolute heights.

Changes

Cohort / File(s)	Summary
Height clamping API removal Assets/Scripts/Ozone SCMAP Code/ScmapEditor_Heightmap.cs	Removed three public methods: ClampTop(), ClampBottom(), and IsOverMinMaxDistance() that previously handled absolute height clamping and distance validation.
HeightmapClamp feature removal Assets/Scripts/UI/Windows/FafEditorSettings.cs	Removed HeightmapClamp toggle field, its initialization, persistence logic, and related PlayerPrefs accessors and internal state.
Height system normalization Assets/Scripts/UI/Tools/Terrain/TerrainInfo.cs	Replaced Min/Max/HeightmapMin/HeightmapMax/HeightmapDistance fields with NormalizedMin and NormalizedMax (0-1 range); updated all height calculations, clamping logic, and removed calls to RecalcTerrainClamp and legacy clamp methods; fixed typo in region comment.
Runtime height check removal Assets/Scripts/Ozone SCMAP Code/ScmapEditor.cs	Removed height difference validation that logged and warned when elevation range exceeded 49.9 units.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• TerrainInfo.cs — Most complex file with extensive calculation updates; verify all normalized range math is correct (division by height, division by 10f, and 0-1 range assumptions)
• API removal cross-references — Confirm no remaining calls to ClampTop(), ClampBottom(), or IsOverMinMaxDistance() exist in other files
• Normalized value semantics — Ensure NormalizedMin/NormalizedMax usage is consistent across paint, update menu, and height adjustment code paths

Poem

🐰 The heights are now normalized and neat,
No more clamping that made logic incomplete,
From absolute values to ranges of one,
The terrain brush now draws with proper run,
Fixed targets at last—your maps will be sweet! 🎨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing terrain clamping at 50 height difference, which aligns with the core objective of the PR.
Linked Issues check	✅ Passed	The PR removes heightmap clamping infrastructure and normalizes height ranges to 0-1, allowing the terrain brush to properly respect fixed height settings as required by issue #41.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to removing the heightmap clamping system and refactoring height handling to use normalized ranges, with no unrelated modifications.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch remove-heightmap-clamp

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b961e16 and ef6d51a.

⛔ Files ignored due to path filters (1)

• Assets/MapEditor.unity is excluded by !**/*.unity

📒 Files selected for processing (4)

• Assets/Scripts/Ozone SCMAP Code/ScmapEditor.cs (0 hunks)
• Assets/Scripts/Ozone SCMAP Code/ScmapEditor_Heightmap.cs (0 hunks)
• Assets/Scripts/UI/Tools/Terrain/TerrainInfo.cs (5 hunks)
• Assets/Scripts/UI/Windows/FafEditorSettings.cs (0 hunks)

💤 Files with no reviewable changes (3)

• Assets/Scripts/Ozone SCMAP Code/ScmapEditor_Heightmap.cs
• Assets/Scripts/Ozone SCMAP Code/ScmapEditor.cs
• Assets/Scripts/UI/Windows/FafEditorSettings.cs

🧰 Additional context used

🧬 Code graph analysis (1)

Assets/Scripts/UI/Tools/Terrain/TerrainInfo.cs (1)

Assets/Scripts/Ozone SCMAP Code/ScmapEditor.cs (1)

• ScmapEditor (13-1230)

🔇 Additional comments (4)

Assets/Scripts/UI/Tools/Terrain/TerrainInfo.cs (4)

115-115: LGTM: Typo correction.

The region comment typo has been fixed.

---

927-927: LGTM: This fixes issue #41.

The TargetHeight is now clamped between NormalizedMin and NormalizedMax, which ensures that when Target, Min, and Max are all set to the same value, the brush will respect that fixed height setting. This directly addresses the reported bug.

---

985-988: LGTM: Normalized bounds applied correctly.

The Sharp and default raise/lower brushes now use NormalizedMin and NormalizedMax for clamping, ensuring they respect user-specified height constraints. This is consistent with the new normalized height system.

---

387-399: Normalization formula is structurally correct but requires runtime verification of data scale assumptions.

The formula at lines 398-399 is applied consistently: both NormalizedMin/Max and TargetHeight (line 927) use the identical transformation value / Data.size.y / 10f. When Target, Min, and Max are set to the same SCMap value, they all normalize to the same value and get clamped to the same range, which correctly fixes issue #41.

However, the codebase contains two different Data.size.y assignments in ScmapEditor.cs (line 241: TerrainHeight, and line 805: yRes / 10.0f), suggesting different code paths or map formats. The formula's correctness depends on which assignment is active during TerrainInfo execution and whether the resulting scale properly maps SCMap units (0-256) to normalized terrain heights (0-1).

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}