[Audit][Medium] isInCoastalNoTreeZone uses wrong boolean logic (AND instead of OR)

[MichaelFisher1997]

🔍 Module Scanned

src/world/worldgen/ (automated audit scan)

📝 Summary

The isInCoastalNoTreeZone function in surface_builder.zig contains incorrect boolean logic. It uses AND (and) to combine three conditions when the intended logic (checking if a value is within a range OR above a minimum threshold) requires OR. The function is currently dead code (not called anywhere) but would produce incorrect results if used.

📍 Location

• File: src/world/worldgen/surface_builder.zig:240
• Function/Scope: SurfaceBuilder.isInCoastalNoTreeZone

🔴 Severity: Medium

• Medium: Incorrect logic that would cause wrong tree placement decisions if the function were used

💥 Impact

If isInCoastalNoTreeZone were called with a height value, the function would return incorrect results. For example, with default parameters (sea_level=64, coastal_no_tree_min=8, coastal_no_tree_max=18):

• isInCoastalNoTreeZone(70) (diff=6) returns false instead of true
• isInCoastalNoTreeZone(72) (diff=8) returns true (correct)

The function creates a gap in coverage from diff=1 to diff=7 where the function returns false even though those positions might need to be in the no-tree zone depending on design intent.

🔎 Evidence

Problem analysis:

• With parameters min=8, max=18:
  • diff=6: true and true and false = false (incorrect if the no-tree zone should start at diff=0)
  • diff=10: true and true and true = true (correct)
  • diff=25: true and false and true = false (correct)

The AND logic requires ALL three conditions to be true simultaneously, which creates an impossible constraint (diff >= 0 AND diff <= 18 AND diff >= 8 simplifies to diff >= 8 AND diff <= 18).

The intended logic (checking if diff is in range [0, max] OR >= min) requires OR between the range check and the minimum check.

🛠️ Proposed Fix

The fix depends on the intended semantics:

Option 1: If the no-tree zone is [0, max]:

Option 2: If the no-tree zone is [min, max]:

Option 3: If the no-tree zone is [0, max] OR >= min (i.e., NOT in gap):

The parameters suggest Option 2 (a bounded zone from 8 to 18) or Option 3 might be intended, since having coastal_no_tree_min = 8 suggests the zone doesn't start at sea level.

✅ Acceptance Criteria

• The isInCoastalNoTreeZone function returns correct values for all diff values
• If the function is not needed, it should be removed to avoid dead code confusion
• If the function is needed, it should have tests verifying its behavior

📚 References

• Related: coastal_no_tree_min and coastal_no_tree_max parameters in SurfaceParams (surface_builder.zig:49-50)
• Note: The coastal_plains biome in biome_registry.zig already handles coastal tree restrictions via biome-specific vegetation settings
• This function appears to be unused dead code that was likely intended to handle tree placement near coasts

[github-actions]

The issue is valid. The function at line 240 uses AND logic:

return diff >= 0 and diff <= p.coastal_no_tree_max and diff >= p.coastal_no_tree_min;

This simplifies to diff >= 8 and diff <= 18 with default params, making it a bounded range check rather than the intended logic. The grep confirms the function is never called — it's dead code.

Question for the author: Before suggesting a fix, clarification is needed on intended semantics:

• Option A: Bounded zone [min, max] — trees excluded only in range 8-18 → current AND logic is mostly correct (just redundant diff >= 0)
• Option B: Trees excluded below max OR above min — needs OR between range and min check
• Option C: Trees excluded below max AND above min simultaneously (current behavior)

Which semantics are intended? Given the parameter names coastal_no_tree_min/max, Option A seems most plausible, but the diff >= 0 check suggests a different intent. If dead code, removing the function entirely may be the cleanest path.

opencode session  |  github run