Jonas/planetsim
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix Territory/Culture/Wealth view toggles reverting instantly + orphaned overlay lines

Browse Source 
Two independent bugs, both found by reading the raygui source rather than
relying on live clicks:

1. The toolbar's view-mode dropdown re-applied its (stale) selected index back
   onto Viewer::mode every single frame whenever mode was Territory/Culture/
   Wealth/Ecoregion/Habitability -- none of which are in the dropdown's plain
   13-entry list, so this fired unconditionally and reverted the view within
   the same frame it was toggled on, regardless of pause state. Fixed by only
   applying a mode change when the dropdown's active index actually changed
   as a result of that frame's GuiDropdownBox call (verified against raygui's
   own source: it only writes back *active on a genuine item click).

2. showNationBorders/showCultureBorders/showTradeRoutes were separate bools
   set true only inside their own P/X/Z toggle method, with nothing resetting
   them when the user switched views by any other means (number keys, the
   dropdown, another civ toggle) -- so realm borders, war fronts, alliance
   arcs and trade routes, once ever turned on, kept drawing over every other
   view forever. Fixed by deriving all three from `mode` instead of storing
   them separately, removing the possibility of drift entirely.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TMfyZv91tonDnPJqbaTVJE
This commit is contained in:
Jonas Reith 2026-08-30 18:28:21 +02:00
parent 7c0dcb7843
commit 7cc44cda84
5 changed files with 57 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
CLAUDE.md
View File

@ -386,6 +386,26 @@ on the Live World clock). **Steps 1–7 of the roadmap are done (plus a derived
exported; a gazetteer should read the same everywhere. Saves a timestamped
`atlas_export_YYYY-MM-DD_HHMMSS.png`; the status line reports how many labels made it in. Purely
`src/render` — no save-format or `src/sim` change.
- **Bugfix: Territory/Culture/Wealth (`P`/`X`/`Z`) toggles didn't stick, and their border/route
overlays never turned off** — two independent bugs, both in `src/render`, found by reading the
actual raygui source rather than trusting live clicks (this session's X11 input became severely
delayed/queued after enough interaction; see the `gui-verification-xdotool` memory). (1) The
toolbar's view-mode `GuiDropdownBox` sync code re-derived `v.mode` from `toolbarModeActive` **every
frame**, not just on a real pick — `Territory`/`Culture`/`Wealth`/`Ecoregion`/`Habitability` aren't
in the dropdown's 13-entry list, so whenever `mode` was one of those, the code found no match, left
`toolbarModeActive` at its last plain-mode index, and then unconditionally forced `v.mode` back to
that stale plain mode and called `recolor()` — reverting the just-toggled view **within the same
frame**, independent of pause state (`drawToolbar()` runs every render frame regardless). Fixed by
only applying the picked mode when `toolbarModeActive` actually changed as a result of *this* call
(before/after compare, confirmed against raygui's `GuiDropdownBox` source: it only writes `*active`
on a genuine item click). (2) `showNationBorders`/`showCultureBorders`/`showTradeRoutes` were
separate `bool` fields set `true` only inside their own P/X/Z toggle method and never reset when
switching away by any *other* means (a number key, the dropdown, another civ toggle) — so realm
borders / war fronts / alliance arcs / trade routes, once ever turned on, stayed drawn over every
subsequent view forever. Fixed by making all three **derived** from `mode` (`showNationBorders()`
etc., now zero-arg methods on `Viewer`, e.g. `mode == ColorMode::Territory`) instead of stored,
independently-mutable state — the two can no longer drift apart because there's only one source of
truth. `src/render` only, no save-format or `src/sim` change; ctest unaffected (`src/sim` untouched).
## Current state

14
src/render/Toolbar.cpp
View File

@ -240,14 +240,22 @@ void drawToolbar(Viewer& v) {
// View-mode dropdown, drawn LAST so its open list renders on top of the scroll body below it
// (see Toolbar.hpp / the gotcha this avoids: raygui doesn't auto-exclude overlapping controls).
// v.mode can be one of 5 "special" side-effecting views (Territory/Culture/Wealth/Ecoregion/
// Habitability) that aren't in kViewModes at all -- the sync loop below then finds no match and
// leaves toolbarModeActive at whatever plain index it last held. That used to be applied back
// unconditionally every frame ("if picked != v.mode, force v.mode = picked") -- which reverted a
// just-toggled special view back off within the very same frame, since it never matches any
// dropdown entry. Now it's only applied when the dropdown itself just changed (a real pick),
// tracked via before/after like the toggle-button shadow pattern elsewhere in this file.
if (!v.toolbarModeEditMode) {
for (int i = 0; i < kViewModeCount; ++i) if (kViewModes[i] == v.mode) { v.toolbarModeActive = i; break; }
}
int activeBefore = v.toolbarModeActive;
if (GuiDropdownBox(dropRect, kViewModeList, &v.toolbarModeActive, v.toolbarModeEditMode))
v.toolbarModeEditMode = !v.toolbarModeEditMode;
if (!v.toolbarModeEditMode) {
ColorMode picked = kViewModes[std::clamp(v.toolbarModeActive, 0, kViewModeCount - 1)];
if (picked != v.mode) { v.mode = picked; v.recolor(); }
if (!v.toolbarModeEditMode && v.toolbarModeActive != activeBefore) {
v.mode = kViewModes[std::clamp(v.toolbarModeActive, 0, kViewModeCount - 1)];
v.recolor();
}
if (wasModal) GuiEnable();

7
src/render/Viewer.cpp
View File

@ -348,10 +348,10 @@ void Viewer::regenWorld() { // after generate(): geometry change
liveWorld = false; followId = 0; wxUndo.clear(); events.clear(); nextEventId = 1; // reseed/regen drops back to World Creation
liveInfoTab = 0; eventRowRects.clear(); eventRowIndices.clear();
// Civilization overlays are viewer-side segment lists; buildGeometry() cleared the engine data but not
// these, so drop them (and their toggles) on a reseed or they keep drawing over the new world.
// these, so drop them on a reseed or they keep drawing over the new world (their draw toggles are
// derived from `mode`, reset right below, so nothing separate needs resetting here).
nationBorders.clear(); cultureBorders.clear(); warFrontier.clear();
allyLinks.clear(); rivalLinks.clear(); tradeSea.clear(); tradeLand.clear();
showNationBorders = showCultureBorders = showTradeRoutes = false;
lastTerritoryYear = -1;
if (mode == ColorMode::Territory || mode == ColorMode::Culture || mode == ColorMode::Wealth)
mode = ColorMode::Biome; // civ colour views have no data until settlements are placed
@ -913,7 +913,6 @@ void Viewer::toggleTerritoryView() { // P: territory/realms colour vie
if (!planet.settlementsPlaced()) { setStatus("Press U for the dawn of civilization first"); return; }
if (!planet.nationsBuilt() || (int)planet.cellNation().size() != (int)planet.cells.size()) rebuildTerritory();
mode = (mode == ColorMode::Territory) ? ColorMode::Biome : ColorMode::Territory;
showNationBorders = (mode == ColorMode::Territory);
recolor();
setStatus(mode == ColorMode::Territory ? "Territory / realms on" : "Territory off");
}
@ -923,7 +922,6 @@ void Viewer::toggleCultureView() { // X: culture/faiths colour view
if (!planet.settlementsPlaced()) { setStatus("Press U for the dawn of civilization first"); return; }
if (!planet.culturesBuilt() || (int)planet.cellCulture().size() != (int)planet.cells.size()) rebuildTerritory();
mode = (mode == ColorMode::Culture) ? ColorMode::Biome : ColorMode::Culture;
showCultureBorders = (mode == ColorMode::Culture);
recolor();
setStatus(mode == ColorMode::Culture ? "Cultures / faiths on" : "Cultures off");
}
@ -933,7 +931,6 @@ void Viewer::toggleTradeView() { // Z: wealth/trade colour view +
if (!planet.settlementsPlaced()) { setStatus("Press U for the dawn of civilization first"); return; }
if (!planet.tradeBuilt() || (int)planet.cellWealth().size() != (int)planet.cells.size()) rebuildTerritory();
mode = (mode == ColorMode::Wealth) ? ColorMode::Biome : ColorMode::Wealth;
showTradeRoutes = (mode == ColorMode::Wealth);
recolor();
setStatus(mode == ColorMode::Wealth ? "Wealth / trade on" : "Wealth off");
}

14
src/render/Viewer.hpp
View File

@ -107,14 +107,22 @@ struct Viewer {
bool showSettlements = true; // civilization settlement markers (key U seeds + toggles)
std::vector<int> atlasRowCells; // cell to focus per visible Atlas/Eco/Civ/Realms-tab row (parallel to the list)
std::vector<Vector3> nationBorders; // political border segments (rebuilt on year tick / placement / load)
bool showNationBorders = false; // draw nation/realm borders (on with the Territory view)
std::vector<Vector3> cultureBorders; // cultural-region border segments (civ Step 4, rebuilt with territory)
bool showCultureBorders = false; // draw cultural-region borders (on with the Culture view)
std::vector<Vector3> warFrontier; // red frontier segments between realms currently at war (civ Step 5)
std::vector<Vector3> allyLinks, rivalLinks; // civ Step 6: capital-to-capital arcs (allies green, rivals red)
std::vector<Vector3> tradeSea, tradeLand; // civ Step 7: trade routes (sea cyan / river+land amber)
bool showTradeRoutes = false; // draw trade routes (on with the Wealth view)
long lastTerritoryYear = -1; // sim year territory was last recomputed (recompute when it ticks)
// Whether to draw the political/cultural/trade overlays is *derived* from `mode` -- deliberately
// not a separately-stored bool. It used to be (showNationBorders et al., set only inside the P/X/Z
// toggle methods), and every other way of changing `mode` (number keys, the toolbar's view-mode
// dropdown) left it stale: switching away from the Territory view by any means other than pressing
// P again left showNationBorders stuck true forever (realm borders/war fronts/alliance arcs kept
// drawing over every other view), while pressing P/X/Z never actually stuck because of a second,
// compounding bug in the dropdown sync (see drawToolbar()) -- so it looked like the toggle
// "immediately turned back off". Deriving it removes the class of bug entirely.
bool showNationBorders() const { return mode == ColorMode::Territory; }
bool showCultureBorders() const { return mode == ColorMode::Culture; }
bool showTradeRoutes() const { return mode == ColorMode::Wealth; }
// World event journal: currently Live World events, shaped to be reused by later phases.
struct WorldEvent {

26
src/render/ViewerRender.cpp
View File

@ -84,7 +84,7 @@ void Viewer::renderGlobe3D() {
}
rlEnd(); rlSetLineWidth(1.0f);
}
if (showNationBorders && !nationBorders.empty()) { // political / realm borders (dark, over the tint)
if (showNationBorders() && !nationBorders.empty()) { // political / realm borders (dark, over the tint)
rlSetLineWidth(2.5f); rlBegin(RL_LINES); rlColor4ub(18, 18, 26, 235);
for (size_t i = 0; i + 1 < nationBorders.size(); i += 2) {
rlVertex3f(nationBorders[i].x, nationBorders[i].y, nationBorders[i].z);
@ -92,7 +92,7 @@ void Viewer::renderGlobe3D() {
}
rlEnd(); rlSetLineWidth(1.0f);
}
if (showCultureBorders && !cultureBorders.empty()) { // cultural-region borders (pale, distinct from political)
if (showCultureBorders() && !cultureBorders.empty()) { // cultural-region borders (pale, distinct from political)
rlSetLineWidth(3.0f); rlBegin(RL_LINES); rlColor4ub(245, 240, 220, 220);
for (size_t i = 0; i + 1 < cultureBorders.size(); i += 2) {
rlVertex3f(cultureBorders[i].x, cultureBorders[i].y, cultureBorders[i].z);
@ -100,7 +100,7 @@ void Viewer::renderGlobe3D() {
}
rlEnd(); rlSetLineWidth(1.0f);
}
if (showNationBorders && !warFrontier.empty()) { // civ Step 5: war fronts (bright red, with the territory view)
if (showNationBorders() && !warFrontier.empty()) { // civ Step 5: war fronts (bright red, with the territory view)
rlSetLineWidth(3.5f); rlBegin(RL_LINES); rlColor4ub(235, 40, 30, 255);
for (size_t i = 0; i + 1 < warFrontier.size(); i += 2) {
rlVertex3f(warFrontier[i].x, warFrontier[i].y, warFrontier[i].z);
@ -108,7 +108,7 @@ void Viewer::renderGlobe3D() {
}
rlEnd(); rlSetLineWidth(1.0f);
}
if (showNationBorders) { // civ Step 6: diplomacy arcs (allies green, rivals dark red)
if (showNationBorders()) { // civ Step 6: diplomacy arcs (allies green, rivals dark red)
rlSetLineWidth(2.0f); rlBegin(RL_LINES);
rlColor4ub(70, 220, 120, 200);
for (size_t i = 0; i + 1 < allyLinks.size(); i += 2) {
@ -122,7 +122,7 @@ void Viewer::renderGlobe3D() {
}
rlEnd(); rlSetLineWidth(1.0f);
}
if (showTradeRoutes) { // civ Step 7: trade routes (sea cyan, land/river amber)
if (showTradeRoutes()) { // civ Step 7: trade routes (sea cyan, land/river amber)
rlSetLineWidth(1.5f); rlBegin(RL_LINES);
rlColor4ub(90, 200, 235, 200);
for (size_t i = 0; i + 1 < tradeSea.size(); i += 2) {
@ -391,13 +391,13 @@ void Viewer::drawMapOverlays(Rectangle vr, double lonOffset, float scale, float
if (showGrat) { drawGraticule2D(graticule, vr, lonOffset); drawGraticuleLabels2D(vr, lonOffset); }
if (showBorders && !borders.empty()) drawSegments2D(borders, Color{255, 235, 90, 255}, 2.0f * scale, vr, lonOffset);
if (showBorders && !ridgeBorders.empty()) drawSegments2D(ridgeBorders, Color{220, 70, 60, 255}, 2.0f * scale, vr, lonOffset);
if (showNationBorders && !nationBorders.empty()) drawSegments2D(nationBorders, Color{18, 18, 26, 235}, 2.0f * scale, vr, lonOffset);
if (showCultureBorders && !cultureBorders.empty()) drawSegments2D(cultureBorders, Color{245, 240, 220, 230}, 2.5f * scale, vr, lonOffset);
if (showNationBorders && !warFrontier.empty()) drawSegments2D(warFrontier, Color{235, 40, 30, 255}, 2.5f * scale, vr, lonOffset);
if (showNationBorders && !allyLinks.empty()) drawSegments2D(allyLinks, Color{70, 220, 120, 220}, 1.5f * scale, vr, lonOffset);
if (showNationBorders && !rivalLinks.empty()) drawSegments2D(rivalLinks, Color{150, 40, 60, 220}, 1.5f * scale, vr, lonOffset);
if (showTradeRoutes && !tradeSea.empty()) drawSegments2D(tradeSea, Color{90, 200, 235, 220}, 1.2f * scale, vr, lonOffset);
if (showTradeRoutes && !tradeLand.empty()) drawSegments2D(tradeLand, Color{230, 180, 90, 220}, 1.2f * scale, vr, lonOffset);
if (showNationBorders() && !nationBorders.empty()) drawSegments2D(nationBorders, Color{18, 18, 26, 235}, 2.0f * scale, vr, lonOffset);
if (showCultureBorders() && !cultureBorders.empty()) drawSegments2D(cultureBorders, Color{245, 240, 220, 230}, 2.5f * scale, vr, lonOffset);
if (showNationBorders() && !warFrontier.empty()) drawSegments2D(warFrontier, Color{235, 40, 30, 255}, 2.5f * scale, vr, lonOffset);
if (showNationBorders() && !allyLinks.empty()) drawSegments2D(allyLinks, Color{70, 220, 120, 220}, 1.5f * scale, vr, lonOffset);
if (showNationBorders() && !rivalLinks.empty()) drawSegments2D(rivalLinks, Color{150, 40, 60, 220}, 1.5f * scale, vr, lonOffset);
if (showTradeRoutes() && !tradeSea.empty()) drawSegments2D(tradeSea, Color{90, 200, 235, 220}, 1.2f * scale, vr, lonOffset);
if (showTradeRoutes() && !tradeLand.empty()) drawSegments2D(tradeLand, Color{230, 180, 90, 220}, 1.2f * scale, vr, lonOffset);
if (showDrift && !driftArrows.empty()) drawSegments2D(driftArrows, Color{90, 230, 255, 255}, 2.0f * scale, vr, lonOffset);
if (liveWorld && showTides && !coastCols.empty()) drawColoredSegments2D(coast, coastCols, 2.0f * scale, vr, lonOffset);
if (showCurrents && !currentCols.empty()) drawColoredSegments2D(currentSegs, currentCols, 1.6f * scale, vr, lonOffset);
@ -1190,7 +1190,7 @@ void Viewer::renderFrame() {
}
}
// 3D realm labels (with the territory view): name kingdoms/empires at their capital.
if (showNationBorders && !planet.nationList().empty()) {
if (showNationBorders() && !planet.nationList().empty()) {
Vec3 camPos{cam.position.x, cam.position.y, cam.position.z};
Vec3 forward = (Vec3{cam.target.x, cam.target.y, cam.target.z} - camPos).normalized();
Vec3 right = forward.cross(Vec3{cam.up.x, cam.up.y, cam.up.z}).normalized();