fix(render): water climbs the bank on every lake shore — a 5-unit waterline is one under the cliff threshold #292

New issue

Open

opened 2026-08-09 18:12:44 +02:00 by panda · 1 comment

panda commented 2026-08-09 18:12:44 +02:00

Collaborator

Copy link

On a lake shore the water texture is painted UP the bank: a wide, slope-shaded band of water colour sitting above the waterline. The ocean does not do it. Both facts have the same cause and the ocean is the control.

What the map actually says

Transect across the swamp lake shore (authored data, read through CompactWorldMap):

(3874,4162) dirty_water   z 0  walkable False
(3872,4160) marsh         z 5  walkable True

The waterline is a single-cell drop of exactly 5. No bank cells, no ramp — the ground is authored crisp.

The ocean beach, same probe:

(4554,5338) beach_sand    z 0  walkable True
(4558,5338) water         z 0  walkable False

No step at all — beach_sand is the one land terrain in content/tiledata.txt with no baseZ, so it sits at 0 like the water. That is why the ocean shore looks right and the lakes do not.

Root cause

Two things compose, and the margin on the first is one unit.

1. The waterline is smoothed into a ramp. GroundBlendRenderer.Display uses CliffStart: WorldRules.MaxWalkStepZ + 1f = 6, and TerrainDisplayHeight.CliffGate returns 0 — "walkable terrace, keep it smooth" — where the largest 8-neighbour relief is <= CliffStart. The shoreline's relief is 5. So every waterline against baseZ 5 land is classified as a terrace and gets the full Sigma: 1.4 Gaussian, which spreads a one-cell drop over several cells of ramp.

2. The blend has no idea there is a bank. TerrainBlendField.Compute(width, height, biomeAt, blendWidth, chunkSize, voidId) takes a 2-D biome grid and no Z whatsoever. A water cell's SDF influence therefore reaches BlendWidth = 3 cells inland regardless of whether those cells are 5 units higher — and inland is uphill.

Water texture, over a ramp, shaded by the slope: water climbing the bank.

Direction (needs a design review before implementing)

The shoreline is a structural feature, not a height magnitude — the same argument the terrain cutaway already makes about itself. The targeted fix is to force the cliff gate open where a cell's neighbourhood contains both a fluid and a non-fluid terrain, instead of deciding by Z. Terrain Walkable == false looks like exactly the fluid set today, so it may need no new data.

Stopping the blend from crossing the waterline (the same shape as "a road cell keeps weight 1 and its neighbours feather into it") is complementary and probably wanted too, but on its own it leaves the water SURFACE tilting up at the edge, because the smoothing moves the vertices, not just the colours.

Explicitly do NOT just lower CliffStart to MaxWalkStepZ: it is one character and it re-classifies every 5-unit terrace in the world, not only shores.

Definition of Done

• A lake shore screenshot shows the water surface ending at the waterline, with the bank face in the land's own material.
• The ocean/beach shore (no step) is unchanged — same screenshot before and after.
• A unit test on the display-height gate: a one-cell 5-unit drop between a fluid and a non-fluid stays crisp, while a one-cell 5-unit drop between two land terrains still smooths as it does today.

On a lake shore the water texture is painted UP the bank: a wide, slope-shaded band of water colour sitting above the waterline. The ocean does not do it. Both facts have the same cause and the ocean is the control. ## What the map actually says Transect across the swamp lake shore (authored data, read through `CompactWorldMap`): ``` (3874,4162) dirty_water z 0 walkable False (3872,4160) marsh z 5 walkable True ``` The waterline is a **single-cell drop of exactly 5**. No bank cells, no ramp — the ground is authored crisp. The ocean beach, same probe: ``` (4554,5338) beach_sand z 0 walkable True (4558,5338) water z 0 walkable False ``` **No step at all** — `beach_sand` is the one land terrain in `content/tiledata.txt` with no `baseZ`, so it sits at 0 like the water. That is why the ocean shore looks right and the lakes do not. ## Root cause Two things compose, and the margin on the first is one unit. 1. **The waterline is smoothed into a ramp.** `GroundBlendRenderer.Display` uses `CliffStart: WorldRules.MaxWalkStepZ + 1f` = **6**, and `TerrainDisplayHeight.CliffGate` returns 0 — "walkable terrace, keep it smooth" — where the largest 8-neighbour relief is <= CliffStart. The shoreline's relief is **5**. So every waterline against `baseZ 5` land is classified as a terrace and gets the full `Sigma: 1.4` Gaussian, which spreads a one-cell drop over several cells of ramp. 2. **The blend has no idea there is a bank.** `TerrainBlendField.Compute(width, height, biomeAt, blendWidth, chunkSize, voidId)` takes a **2-D biome grid and no Z whatsoever**. A water cell's SDF influence therefore reaches `BlendWidth = 3` cells inland regardless of whether those cells are 5 units higher — and inland is uphill. Water texture, over a ramp, shaded by the slope: water climbing the bank. ## Direction (needs a design review before implementing) The shoreline is a *structural* feature, not a height magnitude — the same argument the terrain cutaway already makes about itself. The targeted fix is to force the cliff gate open where a cell's neighbourhood contains both a fluid and a non-fluid terrain, instead of deciding by Z. Terrain `Walkable == false` looks like exactly the fluid set today, so it may need no new data. Stopping the blend from crossing the waterline (the same shape as "a road cell keeps weight 1 and its neighbours feather into it") is complementary and probably wanted too, but on its own it leaves the water SURFACE tilting up at the edge, because the smoothing moves the vertices, not just the colours. Explicitly do NOT just lower `CliffStart` to `MaxWalkStepZ`: it is one character and it re-classifies every 5-unit terrace in the world, not only shores. ## Definition of Done - A lake shore screenshot shows the water surface ending at the waterline, with the bank face in the land's own material. - The ocean/beach shore (no step) is unchanged — same screenshot before and after. - A unit test on the display-height gate: a one-cell 5-unit drop between a fluid and a non-fluid stays crisp, while a one-cell 5-unit drop between two land terrains still smooths as it does today.

panda added the

area:render

area:world

type:bug

labels 2026-08-09 18:12:44 +02:00

panda commented 2026-08-09 18:24:33 +02:00

Author

Collaborator

Copy link

Design review — agreed 2026-08-09

Owner approved all three open decisions ("via"): (1) beach-against-inland becomes a bank too, (2) the
threshold is WorldRules.MaxWalkStepZ (ramp from half, full at 5), (3) the display-Z cliff gate reading
the same concept is a separate change.

The design

A pure TerrainBank in IsoMmo.Client.Core: an argine is the step between two adjacent pieces of ground
that are not the same surface, derived from the authoritative Z, never declared. Continuity(deltaZ)
returns 1 for coplanar ground and 0 at a full bank, ramping from MaxWalkStepZ / 2 to MaxWalkStepZ.

TerrainBlendField.Compute gains a Func<int,int,int> groundZAt (padded like the biome grid, clamped at
the border so the map edge is not a false bank). Per cell, after SmoothstepWeight and BEFORE
Normalize, a biome's weight is multiplied by TerrainBank.Continuity(dz) where dz is the minimum
|deltaZ| between the cell and the cells of that biome within blendWidth — a 7x7 search, because the
weight is already zero beyond blendWidth. The cell's own biome always has dz 0, so no cell is left
without weight.

Chosen over: "water never blends with non-water" (a special case the extend-by-type gate exists to
refuse, and it does nothing for future constructions); geodesic SDF / terrace components (a new algorithm,
and components clipped by the window edge would seam between rebuilds); fixing only the cliff gate (it
removes no water colour from the bank).

BuildWindow already holds snapshot.GroundZAt — the AUTHORITATIVE Z, not the smoothed DisplayZ — so
no new data channel is needed.

Invariants Check

Invariant	Verdict
Scope	OK — only the blend learns the bank; the display-Z cliff gate is declared a follow-up
Server-authoritative	OK — no intent; derived client-side from the same authored map the server reads
GM authorization	N/A — adds no admin action, does not touch CanExecute
Identity model	N/A — no accounts, characters or JWT claims
Protocol versioned	N/A — no wire shape; ProtocolVersion.Current stays 21
String catalog	N/A — no player-facing text
Single-threaded sim	OK — lives on the BuildWindow worker, which reads only the snapshot; World uninvolved
World.cs HARD GATE	OK — zero lines in World.cs
Screen HARD GATE	OK — logic in TerrainBlendField/TerrainBank (Core), glue in GroundBlendRenderer
Client engine-independence	OK — TerrainBank and the extended Compute are in Client.Core, no MonoGame, unit-tested
Gameplay/Networking separation	N/A — no game rule and no transport; this is rendering
Act on the instance	N/A — no entity and no per-instance state; a pure function over a grid
Extend by type, not switch	OK — no switch on biome or terrain: one threshold on deltaZ, so a new terrain (or a foundation) is covered with no code
Typed content-def fields	OK — adds no tiledata.txt field; the bank is derived, so there is no free string to type
Server-paced actions	N/A — no timed action
Persistence (GameServer)	OK — recomputed per window, never serialized; no blob changes
Persistence (Auth)	N/A — no EF Core, no schema
Process separation	N/A — client only
Typed options	OK — Start/Full are rendering constants derived from WorldRules.MaxWalkStepZ, siblings of BlendWidth/Display/Relief; not configuration, so NOT on GameOptions (server-side), declared in one place
Broadcasts / AoI	N/A — emits no message, has no observers
Multi-platform	OK — plain C# in Client.Core, no platform API
Assets required	OK — no new asset; uses the atlas already in the pack, introduces no placeholder shape
Asset naming HARD GATE	N/A — adds no art, static or terrain id
Walkability by slope	OK — does not touch MaxSlopeZ or TileMapRules; reads MaxWalkStepZ as a constant only
Occupancy contains render	OK — input is the AUTHORITATIVE snapshot.GroundZAt, not the smoothed DisplayZ, as the cutaway rule requires; output is presentation-only
Cutaway is structural	OK — same shape: deduced from terrain structure (a step between two cells), never from an absolute Z, a window threshold, or the player's position
ModernUO as reference	OK — ModernUO has no answer to copy here: its land is a flat per-tile tiledata with no blend field and no mesh, so the problem does not exist. We diverge deliberately because we chose a continuous surface; the precedent we follow is internal (GroundTileSelector: derived, deterministic, display-only)
Docs & DoD in the same change	OK — the bank rule goes into CLAUDE.md beside "Occupancy contains render" in the same change, plus its ## Design checklist bullet

Hard gates with no checklist bullet (the index needs them): Content-scripting "authored in Lua" → N/A
(engine primitive, which that section assigns to C#); "Filesystem = namespace" → N/A (no script);
"API self-describing" → N/A (does not touch IScriptContext); Code style "one type per file" → OK.

Server-side validation

N/A, and not as a shortcut: there is no player intent. The field is derived deterministically from the
authored map that client and server read alike. A modified client that alters its own blend changes only
its own pixels — walkability still comes from TileMapRules server-side, which knows nothing about this
field. Same category as GroundTileSelector.

Verification plan

• Unit (IsoMmo.Client.Core.Tests, pure logic — the level this file already uses): two coplanar
  biomes produce weights bit-identical to today (the ocean non-regression); the same grid with a
  5-step drops the foreign biome to 0 and the own biome to 1; a 1-2 step is unchanged; a 3x3 pedestal
  raised by 5 inside one biome (the future foundation) does not bleed past its edge; and a cell whose
  only weight is its own biome, pinning that Normalize never divides by zero.
• TerrainBank.Continuity: monotone, 1 at dz 0, 0 at dz >= Full, no NaN.
• Screenshots (required — visible change): fresh DB, just dev, the three shores already captured as
  BEFORE — /tp 3872 4160 (swamp lake), /tp 2102 8200 (redwood lake), /tp 4544 5338 (ocean). The
  first two must change; the third must be identical. Plus a beach-against-inland boundary to judge
  decision (1).
• Perf: the FRAMES ... build Xms line the client already prints in DEBUG, same framing before and
  after. Baseline measured during the tour: build median 431 ms, worst 558 ms.

## Design review — agreed 2026-08-09 Owner approved all three open decisions ("via"): (1) beach-against-inland becomes a bank too, (2) the threshold is `WorldRules.MaxWalkStepZ` (ramp from half, full at 5), (3) the display-Z cliff gate reading the same concept is a separate change. ### The design A pure `TerrainBank` in `IsoMmo.Client.Core`: an argine is the step between two adjacent pieces of ground that are not the same surface, **derived from the authoritative Z, never declared**. `Continuity(deltaZ)` returns 1 for coplanar ground and 0 at a full bank, ramping from `MaxWalkStepZ / 2` to `MaxWalkStepZ`. `TerrainBlendField.Compute` gains a `Func<int,int,int> groundZAt` (padded like the biome grid, clamped at the border so the map edge is not a false bank). Per cell, after `SmoothstepWeight` and BEFORE `Normalize`, a biome's weight is multiplied by `TerrainBank.Continuity(dz)` where dz is the **minimum |deltaZ| between the cell and the cells of that biome within `blendWidth`** — a 7x7 search, because the weight is already zero beyond `blendWidth`. The cell's own biome always has dz 0, so no cell is left without weight. Chosen over: "water never blends with non-water" (a special case the extend-by-type gate exists to refuse, and it does nothing for future constructions); geodesic SDF / terrace components (a new algorithm, and components clipped by the window edge would seam between rebuilds); fixing only the cliff gate (it removes no water colour from the bank). `BuildWindow` already holds `snapshot.GroundZAt` — the AUTHORITATIVE Z, not the smoothed `DisplayZ` — so no new data channel is needed. ### Invariants Check | Invariant | Verdict | | --- | --- | | Scope | OK — only the blend learns the bank; the display-Z cliff gate is declared a follow-up | | Server-authoritative | OK — no intent; derived client-side from the same authored map the server reads | | GM authorization | N/A — adds no admin action, does not touch `CanExecute` | | Identity model | N/A — no accounts, characters or JWT claims | | Protocol versioned | N/A — no wire shape; `ProtocolVersion.Current` stays 21 | | String catalog | N/A — no player-facing text | | Single-threaded sim | OK — lives on the `BuildWindow` worker, which reads only the snapshot; `World` uninvolved | | `World.cs` HARD GATE | OK — zero lines in `World.cs` | | `Screen` HARD GATE | OK — logic in `TerrainBlendField`/`TerrainBank` (Core), glue in `GroundBlendRenderer` | | Client engine-independence | OK — `TerrainBank` and the extended `Compute` are in Client.Core, no MonoGame, unit-tested | | Gameplay/Networking separation | N/A — no game rule and no transport; this is rendering | | Act on the instance | N/A — no entity and no per-instance state; a pure function over a grid | | Extend by type, not switch | OK — no switch on biome or terrain: one threshold on deltaZ, so a new terrain (or a foundation) is covered with no code | | Typed content-def fields | OK — adds no `tiledata.txt` field; the bank is derived, so there is no free string to type | | Server-paced actions | N/A — no timed action | | Persistence (GameServer) | OK — recomputed per window, never serialized; no blob changes | | Persistence (Auth) | N/A — no EF Core, no schema | | Process separation | N/A — client only | | Typed options | OK — `Start`/`Full` are rendering constants derived from `WorldRules.MaxWalkStepZ`, siblings of `BlendWidth`/`Display`/`Relief`; not configuration, so NOT on `GameOptions` (server-side), declared in one place | | Broadcasts / AoI | N/A — emits no message, has no observers | | Multi-platform | OK — plain C# in Client.Core, no platform API | | Assets required | OK — no new asset; uses the atlas already in the pack, introduces no placeholder shape | | Asset naming HARD GATE | N/A — adds no art, static or terrain id | | Walkability by slope | OK — does not touch `MaxSlopeZ` or `TileMapRules`; reads `MaxWalkStepZ` as a constant only | | Occupancy contains render | OK — input is the AUTHORITATIVE `snapshot.GroundZAt`, not the smoothed `DisplayZ`, as the cutaway rule requires; output is presentation-only | | Cutaway is structural | OK — same shape: deduced from terrain structure (a step between two cells), never from an absolute Z, a window threshold, or the player's position | | ModernUO as reference | OK — ModernUO has no answer to copy here: its land is a flat per-tile tiledata with no blend field and no mesh, so the problem does not exist. We diverge deliberately because we chose a continuous surface; the precedent we follow is internal (`GroundTileSelector`: derived, deterministic, display-only) | | Docs & DoD in the same change | OK — the bank rule goes into `CLAUDE.md` beside "Occupancy contains render" in the same change, plus its `## Design checklist` bullet | Hard gates with no checklist bullet (the index needs them): Content-scripting "authored in Lua" → N/A (engine primitive, which that section assigns to C#); "Filesystem = namespace" → N/A (no script); "API self-describing" → N/A (does not touch `IScriptContext`); Code style "one type per file" → OK. ### Server-side validation N/A, and not as a shortcut: there is no player intent. The field is derived deterministically from the authored map that client and server read alike. A modified client that alters its own blend changes only its own pixels — walkability still comes from `TileMapRules` server-side, which knows nothing about this field. Same category as `GroundTileSelector`. ### Verification plan - **Unit (`IsoMmo.Client.Core.Tests`, pure logic — the level this file already uses):** two coplanar biomes produce weights **bit-identical** to today (the ocean non-regression); the same grid with a 5-step drops the foreign biome to 0 and the own biome to 1; a 1-2 step is unchanged; a 3x3 pedestal raised by 5 inside one biome (the future foundation) does not bleed past its edge; and a cell whose only weight is its own biome, pinning that `Normalize` never divides by zero. - **`TerrainBank.Continuity`:** monotone, 1 at dz 0, 0 at dz >= Full, no NaN. - **Screenshots (required — visible change):** fresh DB, `just dev`, the three shores already captured as BEFORE — `/tp 3872 4160` (swamp lake), `/tp 2102 8200` (redwood lake), `/tp 4544 5338` (ocean). The first two must change; **the third must be identical**. Plus a beach-against-inland boundary to judge decision (1). - **Perf:** the `FRAMES ... build Xms` line the client already prints in DEBUG, same framing before and after. Baseline measured during the tour: build median 431 ms, worst 558 ms.

panda referenced this issue from a pull request that will close it, 2026-08-09 18:55:24 +02:00

fix(client): a biome stops painting where the ground steps away from it #293

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

renovate/opentelemetry-dotnet-monorepo

renovate/sixlabors.imagesharp-4.x

renovate/sonaranalyzer.csharp-10.x

renovate/dotnet-monorepo

renovate/roslynator.analyzers-4.x

fix/blend-knows-a-bank

docs/rendering-open-problems

feat/shaped-cutaway

spike/confine-unwalkable-blend

fix/sprite-depth

design/3d-terrain

feat/p2a-solid-terrain

feat/relief-overlay-occlusion

feat/cliff-face-legibility

renovate/patch-dependencies

chore/sonaranalyzer-10.30

feat/server-compact-worldmap

feat/world-new-biomes

fix/biome-seams

feat/vitals-hud

fix/marsh-creature-parity

feat/biome-props

feat/world-terrain

0.2.1

0.2.0

0.1.5

0.1.4

0.1.2

v0.1.1

v0.1.0

0.1.3

Labels

Clear labels   

alpha:wave-0

Alpha — de-risk, do first

  

alpha:wave-1

Alpha — foundations

  

alpha:wave-2

Alpha — features

  

alpha:wave-3

Alpha — refine & stabilize

  

area:assets

Art, sound, the assets.isoa pack and its tools

  

area:combat

Combat system, weapons, balance

  

area:ecology

Natural nodes, harvesting, spawners

  

area:infra

CI, analyzers, dependencies, performance, ops

  

area:render

Client rendering: terrain mesh, shading, sprites, cutaway

  

area:scripting

Lua content scripting host and API

  

area:ui

Client UI, HUD, input, targeting

  

area:world

World model, map format, editor, topology

  

enhancement

New feature or improvement

  

epic

Milestone-level roadmap epic

  

migration

Platform or framework migration

  

post-alpha

Explicitly out of the alpha scope

  

roadmap

Roadmap tracker

  

tech-debt

Accepted debt to revisit

  

type:bug

Something is wrong and reproducible

  

type:chore

Housekeeping: no behaviour change

  

type:design

Design to agree before any implementation

No labels

alpha:wave-0

alpha:wave-1

alpha:wave-2

alpha:wave-3

area:assets

area:combat

area:ecology

area:infra

area:render

area:scripting

area:ui

area:world

enhancement

epic

migration

post-alpha

roadmap

tech-debt

type:bug

type:chore

type:design

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

marco/IsoMmo#292

No description provided.