fix(simulation): a river course was one D8 hop, not a river (T-1237)
D-261 asked for contiguous river strokes, and the client got them by chaining hops back together after the fact. That could not work: each hop was warped independently, so a shared confluence point arrived as two points that no longer coincided -- 375 hops rejoined into 260 pieces, and Ferrath's Global map showed scratches rather than watercourses. The join belongs before invention, so it now happens on the server. river_course::build_paths walks the D8 cell graph into whole rivers from headwater to mouth, edge-drain, or junction with an already-walked river (including the joint cell, so a tributary visibly meets its trunk). step_canvas emits one course per river instead of one per cell, which also drops the per-hop warp and resampling -- a path's shape is the terrain's, so there is nothing left to invent. It is cheaper too: one point per river cell rather than three. The client's _chain_runs() and its endpoint index are deleted. Runs survive only for the reason D-261 gives them -- water splits a course, and a river crossing a lake is genuinely two strokes that must not be rejoined. Pinned by a real-terrain test on GJ380c rather than a synthetic graph, because the bug was caught by eye on real terrain: a lake must have an outflow that runs to sea level, and no such line existed. It asserts the property the eye was checking -- rivers are long, at least one reaches the sea, and every path is a contiguous walk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -289,8 +289,6 @@ func _prepare_courses() -> void:
|
||||
# cut short by water and have no mouth of their own.
|
||||
_flush_run(run, run_is_first, first_world_m, terminus)
|
||||
|
||||
# Chain the runs into rivers BEFORE culling — see _chain_runs().
|
||||
_prepared_courses = _chain_runs(_prepared_courses)
|
||||
_cull_short(_prepared_courses)
|
||||
|
||||
|
||||
@@ -314,89 +312,26 @@ func _flush_run(
|
||||
|
||||
|
||||
|
||||
## Join runs that meet end-to-end into whole rivers, then cull.
|
||||
## Drop runs too short to read as a line (D-261).
|
||||
##
|
||||
## THE UNIT PROBLEM. A server "course" is an EDGE of the river network — the
|
||||
## stretch between two confluences — not a river. Culling per course therefore
|
||||
## culls per segment, and a 1,000 km river assembled from fifty 20 km edges
|
||||
## vanishes entirely because no single edge clears the threshold. Measured on
|
||||
## Ferrath's Global canvas: 375 courses, 180 surviving the water clip, and
|
||||
## ZERO surviving a per-course length cull.
|
||||
## THE UNIT PROBLEM, and why there is no chaining step here any more. This layer
|
||||
## briefly joined runs end-to-end, because a server "course" used to be a single
|
||||
## D8 hop and culling per course culled per hop — a 1,000 km river assembled from
|
||||
## fifty 20 km fragments vanished entirely, since no fragment cleared the
|
||||
## threshold. Measured on Ferrath Global: 375 courses, 180 surviving the water
|
||||
## clip, ZERO surviving the length cull.
|
||||
##
|
||||
## Chaining also delivers the other half of D-261's "contiguous" requirement.
|
||||
## Per-course contiguity only makes each edge unbroken; it is the joining that
|
||||
## makes a river read as one line rather than a row of dashes.
|
||||
## Chaining was the wrong fix and only half-worked (375 hops rejoined into 260
|
||||
## pieces) because the server warped each hop independently, so a shared
|
||||
## confluence point arrived as two points that no longer coincided. The join
|
||||
## belongs before invention, and now happens there: the server emits one course
|
||||
## per RIVER (river_course::build_paths). A course is therefore already a whole
|
||||
## watercourse when it gets here, and this cull measures a river — which is what
|
||||
## D-261 says it measures.
|
||||
##
|
||||
## Greedy: take any unused run, extend it downstream while some unused run
|
||||
## starts where it ends, then upstream likewise. O(n) lookups via an endpoint
|
||||
## index rather than an O(n^2) scan.
|
||||
func _chain_runs(runs: Array) -> Array:
|
||||
if runs.size() < 2:
|
||||
return runs
|
||||
var starts: Dictionary = {} # quantised start point -> [run indices]
|
||||
for i in range(runs.size()):
|
||||
var key: String = _point_key((runs[i]["points"] as PackedVector2Array)[0])
|
||||
if not starts.has(key):
|
||||
starts[key] = []
|
||||
(starts[key] as Array).append(i)
|
||||
|
||||
var used: Dictionary = {}
|
||||
var chained: Array = []
|
||||
for i in range(runs.size()):
|
||||
if used.has(i):
|
||||
continue
|
||||
used[i] = true
|
||||
var entry: Dictionary = runs[i]
|
||||
var pts := PackedVector2Array(entry["points"])
|
||||
# Extend downstream: repeatedly find an unused run beginning where this
|
||||
# one ends. The terminus travels with the LAST link, since that is the
|
||||
# end that actually reaches the sea.
|
||||
var terminus: String = str(entry["terminus"])
|
||||
while true:
|
||||
var next_i: int = _take_run_starting_at(starts, used, pts[pts.size() - 1])
|
||||
if next_i < 0:
|
||||
break
|
||||
var nxt: Dictionary = runs[next_i]
|
||||
var npts: PackedVector2Array = nxt["points"]
|
||||
for j in range(1, npts.size()): # skip the shared joint
|
||||
pts.append(npts[j])
|
||||
terminus = str(nxt["terminus"])
|
||||
chained.append(
|
||||
{
|
||||
"points": pts,
|
||||
"terminus": terminus,
|
||||
# Taper only if this chain BEGINS at a true source. Extending
|
||||
# downstream never changes where the chain starts, so the head
|
||||
# run's own judgement still holds.
|
||||
"taper": bool(entry["taper"]),
|
||||
}
|
||||
)
|
||||
return chained
|
||||
|
||||
|
||||
## First unused run whose start coincides with `at`, or -1.
|
||||
static func _take_run_starting_at(starts: Dictionary, used: Dictionary, at: Vector2) -> int:
|
||||
var key: String = _point_key(at)
|
||||
if not starts.has(key):
|
||||
return -1
|
||||
for idx_raw: Variant in (starts[key] as Array):
|
||||
var idx: int = idx_raw
|
||||
if not used.has(idx):
|
||||
used[idx] = true
|
||||
return idx
|
||||
return -1
|
||||
|
||||
|
||||
## Quantise a screen point to a joinable key. Confluence endpoints come from
|
||||
## the same server coordinate and survive an identical linear projection, so
|
||||
## they agree closely; rounding to a tenth of a pixel absorbs float drift
|
||||
## without gluing genuinely separate rivers together.
|
||||
static func _point_key(p: Vector2) -> String:
|
||||
return "%d:%d" % [roundi(p.x * 10.0), roundi(p.y * 10.0)]
|
||||
|
||||
|
||||
## Drop chains too short to read as a line (D-261). Done AFTER chaining, so the
|
||||
## measurement is of a river rather than of one of its segments.
|
||||
## Runs still exist, but only for the reason D-261 gives them: water splits a
|
||||
## course. A river crossing a lake is genuinely two visible strokes, and those
|
||||
## must NOT be rejoined.
|
||||
func _cull_short(chains: Array) -> void:
|
||||
var keep: Array = []
|
||||
for entry_raw: Variant in chains:
|
||||
|
||||
@@ -241,6 +241,127 @@ pub fn build_edges(rn: &RiverNetwork) -> Vec<RiverEdge> {
|
||||
/// step is only used to identify the neighbor cell for `Interior` edges,
|
||||
/// where the offset is by construction in-bounds — it came from the same D8
|
||||
/// walk `extract_river_network` already validated).
|
||||
/// A whole river: the ordered cell path from a headwater down to a mouth, an
|
||||
/// edge-drain, or a junction with an already-walked river.
|
||||
pub struct RiverPath {
|
||||
pub cells: Vec<RiverCell>,
|
||||
pub class: u8,
|
||||
pub terminus: EdgeTerminusKind,
|
||||
}
|
||||
|
||||
/// Assemble the D8 cell graph into whole rivers (D-261, 2026-07-28).
|
||||
///
|
||||
/// WHY THIS EXISTS. [`build_edges`] emits one edge PER RIVER CELL — a single
|
||||
/// D8 step — so "a course" was never a river, it was one hop. Rendering each
|
||||
/// hop independently produced exactly what you would expect: on Ferrath's
|
||||
/// Global canvas, 375 courses of 1-4 points each (median 3), drawn as ~70 km
|
||||
/// straight stubs, with no continuous line anywhere. Jeroen spotted it from
|
||||
/// the map alone: a lake must have an outflow that runs to sea level, and no
|
||||
/// such line existed.
|
||||
///
|
||||
/// Chaining the hops back together on the CLIENT does not work, because
|
||||
/// `invent_course` warps each hop independently — the shared confluence cell
|
||||
/// receives a different offset in each, so the endpoints that ought to be one
|
||||
/// point are two. Measured: 375 hops chained into only 260 pieces. The join
|
||||
/// has to happen before invention, which is here.
|
||||
///
|
||||
/// A tributary stops at the first cell already claimed by another river, but
|
||||
/// INCLUDES that cell, so it visibly meets the trunk instead of stopping one
|
||||
/// cell short of it.
|
||||
pub fn build_paths(rn: &RiverNetwork) -> Vec<RiverPath> {
|
||||
// BTreeMap, not HashMap — D-030 bans the hashed containers in simulation
|
||||
// code. This one is pure lookup, never iterated, so order would not have
|
||||
// bitten us; the ban is blanket and the walk runs once per canvas.
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
let mut index: BTreeMap<RiverCell, usize> = BTreeMap::new();
|
||||
for (i, &cell) in rn.river_cells.iter().enumerate() {
|
||||
index.insert(cell, i);
|
||||
}
|
||||
|
||||
// A headwater is a cell nothing flows into.
|
||||
let mut has_upstream = vec![false; rn.river_cells.len()];
|
||||
for (i, &cell) in rn.river_cells.iter().enumerate() {
|
||||
let Some(&sentinel) = rn.river_downstream.get(i) else {
|
||||
continue;
|
||||
};
|
||||
if sentinel < 8 {
|
||||
let (dr, dc) = d8_offset(sentinel);
|
||||
if let Some(&j) = index.get(&step_cell(cell, dr, dc)) {
|
||||
has_upstream[j] = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let mut claimed = vec![false; rn.river_cells.len()];
|
||||
let mut paths: Vec<RiverPath> = Vec::new();
|
||||
|
||||
// Headwaters first so main stems are walked before the leftovers pass.
|
||||
let order = (0..rn.river_cells.len())
|
||||
.filter(|&i| !has_upstream[i])
|
||||
.chain(0..rn.river_cells.len());
|
||||
|
||||
for start in order {
|
||||
if claimed[start] {
|
||||
continue;
|
||||
}
|
||||
let mut cells: Vec<RiverCell> = Vec::new();
|
||||
let mut terminus = EdgeTerminusKind::Interior;
|
||||
let mut i = start;
|
||||
loop {
|
||||
claimed[i] = true;
|
||||
cells.push(rn.river_cells[i]);
|
||||
let Some(&sentinel) = rn.river_downstream.get(i) else {
|
||||
break;
|
||||
};
|
||||
if sentinel == RIVER_DOWNSTREAM_TERMINAL {
|
||||
break;
|
||||
}
|
||||
if sentinel == RIVER_DOWNSTREAM_MOUTH {
|
||||
// The seaward cell is off-network; append it so the line
|
||||
// actually reaches the water rather than stopping inland.
|
||||
let seaward = rn.river_seaward.get(i).copied().unwrap_or((0, 0));
|
||||
if seaward != (0, 0) {
|
||||
cells.push(seaward);
|
||||
}
|
||||
terminus = EdgeTerminusKind::Mouth;
|
||||
break;
|
||||
}
|
||||
if sentinel == RIVER_DOWNSTREAM_EDGE_DRAIN {
|
||||
terminus = EdgeTerminusKind::EdgeDrain;
|
||||
break;
|
||||
}
|
||||
let (dr, dc) = d8_offset(sentinel);
|
||||
let Some(&next) = index.get(&step_cell(rn.river_cells[i], dr, dc)) else {
|
||||
break;
|
||||
};
|
||||
if claimed[next] {
|
||||
// Junction with a river already walked — include the joint so
|
||||
// the tributary visibly meets it, then stop.
|
||||
cells.push(rn.river_cells[next]);
|
||||
break;
|
||||
}
|
||||
i = next;
|
||||
}
|
||||
if cells.len() < 2 {
|
||||
continue;
|
||||
}
|
||||
// Class of the DOWNSTREAM end: a river reads by what it becomes, not
|
||||
// by the trickle it starts as.
|
||||
let class = rn
|
||||
.river_class
|
||||
.get(i)
|
||||
.copied()
|
||||
.unwrap_or_else(|| rn.river_class.get(start).copied().unwrap_or(0));
|
||||
paths.push(RiverPath {
|
||||
cells,
|
||||
class,
|
||||
terminus,
|
||||
});
|
||||
}
|
||||
paths
|
||||
}
|
||||
|
||||
fn step_cell(cell: RiverCell, dr: i32, dc: i32) -> RiverCell {
|
||||
let r = (cell.0 as i32 + dr).max(0) as u16;
|
||||
let c = (cell.1 as i32 + dc).max(0) as u16;
|
||||
@@ -1243,4 +1364,62 @@ mod tests {
|
||||
0.0
|
||||
));
|
||||
}
|
||||
|
||||
/// The bug Jeroen caught by eye, pinned on the terrain he caught it on.
|
||||
///
|
||||
/// His tell was precise: *a lake should have a run in (which may be too
|
||||
/// short to show) and a run out towards sealevel that should definitely be
|
||||
/// long enough.* No such line existed on Ferrath's Global map, because
|
||||
/// every "course" was one D8 hop — `build_edges` emits one edge per river
|
||||
/// cell. 375 courses, median 3 points, all of them stubs.
|
||||
///
|
||||
/// So this asserts the property the eye was checking, not the mechanism:
|
||||
/// real terrain must yield rivers that are LONG, and at least one of them
|
||||
/// must actually reach the sea. A per-hop regression fails both — its
|
||||
/// longest path would be 2 cells and no path would be a Mouth.
|
||||
#[test]
|
||||
fn real_terrain_yields_long_rivers_that_reach_the_sea() {
|
||||
let src = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
|
||||
.join("../wiki/star-systems/GJ-380/bodies/GJ380c/heightmap.png");
|
||||
let heightmap = crate::atlas::heightmap::load_heightmap_png(&src, "GJ380c", 0.3)
|
||||
.expect("decode GJ380c heightmap");
|
||||
let small = heightmap.downsample(512, 256);
|
||||
let dr = drainage::analyze(&small.data, small.width, small.height, small.sea_level);
|
||||
|
||||
let paths = build_paths(&dr.river_network);
|
||||
assert!(!paths.is_empty(), "GJ380c must have rivers at all");
|
||||
|
||||
let longest = paths.iter().map(|p| p.cells.len()).max().unwrap_or(0);
|
||||
assert!(
|
||||
longest >= 20,
|
||||
"longest river is only {longest} cells — rivers are being cut into hops again \
|
||||
(the whole-body map then shows scratches, not watercourses)"
|
||||
);
|
||||
|
||||
let to_sea = paths
|
||||
.iter()
|
||||
.filter(|p| p.terminus == EdgeTerminusKind::Mouth)
|
||||
.count();
|
||||
assert!(
|
||||
to_sea > 0,
|
||||
"no river on GJ380c reaches the sea — an outflow to sea level must exist"
|
||||
);
|
||||
|
||||
// Every path is a walk of adjacent cells, so a gap means the chain
|
||||
// broke and the stroke would jump across the map.
|
||||
for p in &paths {
|
||||
for w in p.cells.windows(2) {
|
||||
let (dr_, dc_) = (
|
||||
(w[1].0 as i64 - w[0].0 as i64).abs(),
|
||||
(w[1].1 as i64 - w[0].1 as i64).abs(),
|
||||
);
|
||||
assert!(
|
||||
dr_ <= 1 && dc_ <= 1,
|
||||
"river path jumps from {:?} to {:?} — not a contiguous walk",
|
||||
w[0],
|
||||
w[1]
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -649,14 +649,19 @@ fn fixed_canvas_world_rect(
|
||||
/// return.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn invent_courses_for_canvas(
|
||||
seed: SeedChain,
|
||||
// Unused since D-261 moved rivers from per-hop invention to whole cell
|
||||
// paths: a path's shape IS the terrain's, so there is no warp to seed and
|
||||
// no resampling wavelength to cut off. Kept in the signature because the
|
||||
// deferred polish (T-1238) reintroduces a per-river warp, and threading
|
||||
// them back through later is worse than leaving the plumbing in place.
|
||||
_seed: SeedChain,
|
||||
params: &BodyParams,
|
||||
ta: &TerrainAnalysis,
|
||||
river_network: &RiverNetwork,
|
||||
canvas_rect: (f64, f64, f64, f64),
|
||||
rung: StepCanvasRung,
|
||||
step_m: f64,
|
||||
min_wavelength_m: f64,
|
||||
_step_m: f64,
|
||||
_min_wavelength_m: f64,
|
||||
) -> Vec<InventedCourse> {
|
||||
// D-261: courses are invented at EVERY rung, Global included. They used to
|
||||
// be withheld from the orbital rung, which is why the whole-body map had no
|
||||
@@ -676,52 +681,63 @@ fn invent_courses_for_canvas(
|
||||
// direction — a District canvas spans ~3.6 km post-inversion, so a 2,048 m
|
||||
// station pitch would place two stations across the whole view and render
|
||||
// every river as a straight line.
|
||||
let station_spacing_m = step_m;
|
||||
let (win_x0, win_y0, win_x1, win_y1) = canvas_rect;
|
||||
|
||||
let edges = river_course::build_edges(river_network);
|
||||
// D-261: one course per RIVER, not per D8 hop.
|
||||
//
|
||||
// build_edges() emits one edge per river cell — a single step — so every
|
||||
// "course" used to be one hop, inflated by invent_course into a ~70 km
|
||||
// straight stub. Measured on Ferrath's Global canvas: 375 courses of 1-4
|
||||
// points each, median 3. Nothing on that map read as a watercourse, and a
|
||||
// lake's outflow to the sea — which must exist and must be long — was
|
||||
// nowhere to be seen.
|
||||
//
|
||||
// A path IS the river, so its points are the river's own cells in order:
|
||||
// no resampling, no per-hop warp, and the shape is a function of the
|
||||
// terrain rather than of the zoom level (which is what D-261 asks for).
|
||||
// It is also CHEAPER — the whole network becomes one point per river cell
|
||||
// instead of three per cell.
|
||||
let paths = river_course::build_paths(river_network);
|
||||
let mut courses = Vec::new();
|
||||
for edge in &edges {
|
||||
let anchor_a = crate::atlas::district_profile::pixel_to_world_m(
|
||||
edge.upstream.1 as f64,
|
||||
edge.upstream.0 as f64,
|
||||
ta.w,
|
||||
ta.h,
|
||||
params.body_radius_km,
|
||||
);
|
||||
let anchor_b = crate::atlas::district_profile::pixel_to_world_m(
|
||||
edge.downstream.1 as f64,
|
||||
edge.downstream.0 as f64,
|
||||
ta.w,
|
||||
ta.h,
|
||||
params.body_radius_km,
|
||||
);
|
||||
let chord_m =
|
||||
((anchor_a.0 - anchor_b.0).powi(2) + (anchor_a.1 - anchor_b.1).powi(2)).sqrt();
|
||||
// Same inflation fraction layer_proxy::COURSE_BBOX_INFLATION_FRACTION
|
||||
// uses — mirrors the Stage-B peak-amplitude bound exactly (0.08 =
|
||||
// STAGE_B_PEAK_FRACTION_OF_CHORD, pinned equal by a const assert in
|
||||
// layer_proxy.rs).
|
||||
let inflate_m = chord_m * 0.08;
|
||||
let (bx0, bx1) = (
|
||||
anchor_a.0.min(anchor_b.0) - inflate_m,
|
||||
anchor_a.0.max(anchor_b.0) + inflate_m,
|
||||
);
|
||||
let (by0, by1) = (
|
||||
anchor_a.1.min(anchor_b.1) - inflate_m,
|
||||
anchor_a.1.max(anchor_b.1) + inflate_m,
|
||||
);
|
||||
for (path_idx, path) in paths.iter().enumerate() {
|
||||
let points: Vec<river_course::CoursePoint> = path
|
||||
.cells
|
||||
.iter()
|
||||
.map(|&cell| {
|
||||
crate::atlas::district_profile::pixel_to_world_m(
|
||||
cell.1 as f64,
|
||||
cell.0 as f64,
|
||||
ta.w,
|
||||
ta.h,
|
||||
params.body_radius_km,
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
if points.len() < 2 {
|
||||
continue;
|
||||
}
|
||||
// Cull whole rivers that miss this canvas entirely, cheaply, before
|
||||
// any per-point work downstream.
|
||||
let mut bx0 = f64::MAX;
|
||||
let mut bx1 = f64::MIN;
|
||||
let mut by0 = f64::MAX;
|
||||
let mut by1 = f64::MIN;
|
||||
for p in &points {
|
||||
bx0 = bx0.min(p.0);
|
||||
bx1 = bx1.max(p.0);
|
||||
by0 = by0.min(p.1);
|
||||
by1 = by1.max(p.1);
|
||||
}
|
||||
if bx1 < win_x0 || bx0 > win_x1 || by1 < win_y0 || by0 > win_y1 {
|
||||
continue;
|
||||
}
|
||||
courses.push(river_course::invent_course(
|
||||
seed,
|
||||
edge,
|
||||
ta,
|
||||
params,
|
||||
station_spacing_m,
|
||||
min_wavelength_m,
|
||||
));
|
||||
courses.push(InventedCourse {
|
||||
edge_id: path_idx as u32,
|
||||
class: path.class,
|
||||
terminus: path.terminus,
|
||||
bbox: (bx0, by0, bx1, by1),
|
||||
points,
|
||||
});
|
||||
}
|
||||
|
||||
courses
|
||||
|
||||
Reference in New Issue
Block a user