Configurable water level, with water decided per tile #164

New issue

Open

opened 2026-10-11 20:23:50 +01:00 by liamjd · 0 comments

liamjd commented

2026-10-11 20:23:50 +01:00

Owner

Milestone 2, item 5 ("Make the water level configurable so that the has_water flag is more flexible").

Problem

Water is a single global rule in _kind_for(): has_water and level <= _datum_index. So:

The water line is the datum. There is no way to have a low sea or a high flood without moving the whole height range.
Water is all-or-nothing per preset, decided by height alone. The milestone's terrain goals (a canyon river, a crater lake, rivers across plains: #165, #166, #167) all need water somewhere specific, which a global height threshold cannot express. A river runs downhill; a crater lake sits above some dry land outside the rim.

Proposal

Two changes. The first is the configurable level asked for; the second is the seam the follow-on tickets need, and is cheap to put in now.

water_level on TerrainPreset, in world units like datum_level, decoupled from the datum. has_water stays as the on/off switch, or is replaced by a sentinel level; decide which reads better in the inspector.
Water becomes per-tile. Add a water mask filled during generation, and an is_water(cell) query. The global water level becomes the first thing that fills the mask; later tickets add rivers and lakes as other fillers.
- _kind_for(), is_walkable(), drawn_level_at() and _paint_cells() all ask is_water(cell) instead of re-deriving it from height.
- drawn_level_at() lifts a submerged tile to its water surface, not to _datum_index.

Decision needed

Is the mask a bool plus one global surface level, or a per-tile surface level? A crater lake (#166) sits at a different height from any sea, and a terraced river (#165) steps down in falls. Per-tile surface is the general answer; a bool is enough only if every water body shares one level. Recommend per-tile, since #165 and #166 both need it.

Acceptance criteria

water_level on the preset; changing it in the .tres moves the shoreline without changing the land shape.
is_water(cell) is the only place water is decided. Grep for Kind.WATER comparisons outside it as part of review.
Existing presets look identical with water_level set to their current datum. Pin with a fixed-seed test of the water cells.
Placement still rejects water (PlacementRules → WATER); navigation still excludes it (test_terrain_navigation.gd green).
Unit tests: raising the water level floods more tiles and never fewer; a tile below the level is not walkable.
Check #132 (entrance arrow sinks below the water) against the new drawn_level_at(). It may get fixed or changed for free; note which in the PR.

#30 (distance-to-water query for clay deposits) reads water. Building it on is_water() once this lands saves rewriting it.

Milestone 2, item 5 ("Make the water level configurable so that the `has_water` flag is more flexible"). ## Problem Water is a single global rule in `_kind_for()`: `has_water and level <= _datum_index`. So: - The water line **is** the datum. There is no way to have a low sea or a high flood without moving the whole height range. - Water is all-or-nothing per preset, decided by height alone. The milestone's terrain goals (a canyon river, a crater lake, rivers across plains: #165, #166, #167) all need water **somewhere specific**, which a global height threshold cannot express. A river runs downhill; a crater lake sits above some dry land outside the rim. ## Proposal Two changes. The first is the configurable level asked for; the second is the seam the follow-on tickets need, and is cheap to put in now. 1. **`water_level`** on `TerrainPreset`, in world units like `datum_level`, decoupled from the datum. `has_water` stays as the on/off switch, or is replaced by a sentinel level; decide which reads better in the inspector. 2. **Water becomes per-tile.** Add a water mask filled during generation, and an `is_water(cell)` query. The global water level becomes the first thing that fills the mask; later tickets add rivers and lakes as other fillers. - `_kind_for()`, `is_walkable()`, `drawn_level_at()` and `_paint_cells()` all ask `is_water(cell)` instead of re-deriving it from height. - `drawn_level_at()` lifts a submerged tile to **its** water surface, not to `_datum_index`. ### Decision needed Is the mask a bool plus one global surface level, or a per-tile surface level? A crater lake (#166) sits at a different height from any sea, and a terraced river (#165) steps down in falls. Per-tile surface is the general answer; a bool is enough only if every water body shares one level. Recommend per-tile, since #165 and #166 both need it. ## Acceptance criteria - [ ] `water_level` on the preset; changing it in the `.tres` moves the shoreline without changing the land shape. - [ ] `is_water(cell)` is the only place water is decided. Grep for `Kind.WATER` comparisons outside it as part of review. - [ ] Existing presets look identical with `water_level` set to their current datum. Pin with a fixed-seed test of the water cells. - [ ] Placement still rejects water (`PlacementRules` → WATER); navigation still excludes it (`test_terrain_navigation.gd` green). - [ ] Unit tests: raising the water level floods more tiles and never fewer; a tile below the level is not walkable. - [ ] Check #132 (entrance arrow sinks below the water) against the new `drawn_level_at()`. It may get fixed or changed for free; note which in the PR. ## Related #30 (distance-to-water query for clay deposits) reads water. Building it on `is_water()` once this lands saves rewriting it.

liamjd added this to the Terrain Generation milestone

2026-10-11 20:23:50 +01:00

liamjd added the

enhancement

label

2026-10-11 20:23:50 +01:00

liamjd referenced this issue

2026-10-11 20:23:50 +01:00

River along the canyon floor #165

liamjd referenced this issue

2026-10-11 20:23:51 +01:00

Lake in the crater bowl #166