Skip to content

Modularize map generation and add four generators - #240

Open
genixpro wants to merge 8 commits into
masterfrom
codex/modular-map-generators
Open

Modularize map generation and add four generators#240
genixpro wants to merge 8 commits into
masterfrom
codex/modular-map-generators

Conversation

@genixpro

@genixpro genixpro commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Map generation was spread across central dispatch code, overloaded descriptor fields, global random state, and duplicated helpers, which made new generators hard to add safely. This follow-up to #238 introduces a registry-driven modular framework: each generator owns its named options, controls, and ordered recipe; the lobby, editor, compatibility adapter, lifecycle service, and study tools consume the same definitions.

It also incorporates the finalized Contested Commons, Lattice, Maze, and Fjord Continent generators. Integration fixes include real shoreline generation for Lattice and Maze, larger playable home areas, coarse 20/24/32-tile Maze cells, stone seams inside Maze water walls, clumped resources, and guaranteed wheat/wood deposits on both banks of every fjord. All 18 new strings have reviewed translations in every locale.

Validation:

  • Built the release game and map-generator test binaries.
  • Registry/control, legacy codec, deterministic RNG, failure diagnostics, lobby/editor history, and test-only noncontiguous generator checks pass.
  • Native custom-game, save/load, replay, and all 12 playable-generator lifecycle checks pass.
  • Translation structural/strict audit and translation regression tests pass.
  • 1,000 seeds per playable generator, with charts and fixed-seed previews retained locally.
  • 320 new-generator edge attempts cover control extremes, rectangular maps, solo Fjord, and crowded 64×64 configurations; impossible cases fail cleanly.

Rectangular map follow-up

Rectangular previews now crop thumbnail padding, preserve aspect ratio and align colony markers with terrain. Legacy seam-crossing start coordinates are normalized (including a Shattered Coast start recorded at y=-6); generated-world validation now checks start bounds. Native pixel checks cover 4:1 and 8:1 maps in both orientations at compact and larger window sizes.

A 960-case generation audit still reports 51 seed-specific placement failures (38/720 rectangular attempts and 13/240 square attempts). This preview/start-coordinate fix does not claim to resolve those layout-quality limitations.

Wide preview Tall preview
Wide Tall

Failures by generator (rectangular / square): River 3/1, Separate Islands 8/1, Shattered Coast 11/3, Contested Commons 1/6, Fjord Continent 15/2. The other seven generators had none in this sample. Failures involve small/crowded settlement or resource placement; five Commons 512×512 cases reached the dispersion evaluation limit.

Generated reports, study data and screenshot files are excluded from the branch tip and final source changes. Existing image embeds reference earlier review commits; future artifacts stay outside tracked source.

@genixpro
genixpro force-pushed the codex/map-generator-defaults branch from 9e9f0f1 to b74f6df Compare September 10, 2026 21:36
genixpro added a commit that referenced this pull request Sep 10, 2026
#238 (already merged) added CustomGamePreferences.h, which serializes
CustomGameSetup::generator by taking Sint32 MapGenerationDescriptor::*
member pointers directly. #240 replaced generator's type with
GenerationRequest, whose method-specific options live in a generic
std::map<std::string,int> instead of fixed struct fields — an
architecture change, not a rename — so the file no longer compiled.

- CustomGamePreferences::encode()/decode() now convert through
  toLegacyDescriptor()/fromLegacyDescriptor() (the compatibility
  adapter #240 already built for exactly this kind of interop), so the
  on-disk wire format and its corruption-recovery bounds are unchanged.
  decode()'s method-validity check now asks the live GeneratorRegistry
  instead of a hardcoded 1-8 range, so it stays correct as generators
  are added or retired.
- Widened several fields() bounds (terrain weights 0-64 -> 0-100,
  riverDiameter's max 64 -> 65, oldIslandSize 1-64 -> 1-70) to match
  the modular registry's current ranges. These are approximate,
  same as before: the reused legacy fields (e.g. riverDiameter also
  stands in for lake size/channel width/bridge width) don't have one
  true bound, so this is a safe envelope, not a tight per-method one.
  Without this, decode() could reject a preferences file that a normal
  save legitimately produced (oldIslandSize's own default already
  exceeded the old 1-64 bound for any method other than Isles/Old
  Islands).
- Found and fixed a related crash bug in the compatibility adapter
  itself while tracing this: Lattice and Maze register wheat/wood/
  stone/algae controls with no entry in legacyField()'s mapping table,
  so converting either method through toLegacyDescriptor/
  fromLegacyDescriptor threw an uncaught std::invalid_argument. Added
  the four missing mappings (they match pre-existing legacy struct
  fields exactly) and changed every other option with no legacy slot
  (loopiness, home-radius, cell-size, ...) from throwing to falling
  back to its control's default value, so a newer generator's full
  option set can never crash this adapter again.
- test/CustomGameSetupHarness.cpp: preferencesModel()/preferencesScreen()
  built a GenerationRequest via the same member-pointer approach;
  updated both to build a temporary MapGenerationDescriptor and convert.
  Switched preferencesModel()'s method from Old Islands to Crater Lakes
  (one of the four modern height-map generators that still exposes a
  repeat-landscape control; Old Islands never did in the new registry,
  so it always round-tripped back to 0). The per-field assertions in
  preferencesScreen() now compare against the same achievable
  conversion rather than raw field maximums, since only the options a
  method actually registers survive a GenerationRequest round trip.

Verified: full scons -j8 release=1 server=0 client build is clean.
CustomGameSetupHarness passes in default, preferences-write and
preferences-read modes. MapGeneratorDefaultsTest and
MapGeneratorStudy --catalog also pass, unaffected by the adapter fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KFxsZmLM4qsovemHqDrHGP
@genixpro
genixpro force-pushed the codex/modular-map-generators branch from e0508e0 to 2a0895c Compare September 10, 2026 22:14
@genixpro

Copy link
Copy Markdown
Contributor Author

Rebased onto the (also just-rebased) #238 branch — it forked from the same codex/custom-game-ux point as #238, so I used the same rebase --onto approach to replay only this branch's own 6 commits.

Most conflicts were pure append/append (translation files) or a stale-vs-final doc mismatch, resolved by taking this branch's own final version of docs/map-generators/README.md directly rather than trusting the intermediate per-commit auto-merge (one commit in this branch's own history trims that doc down over several later commits; replaying it against my manual resolution of an earlier commit briefly duplicated content — using the actual final committed version sidesteps that entirely). test/CustomGameSetupHarness.cpp had one large genuine conflict (model()/engine()/sessionReplay()/reload()/main()); after stripping whitespace, master's and this branch's versions turned out to be functionally identical except for the map-generator loop itself, which I merged by keeping master's already-independently-added content (two new test methods from #237's tail commits, extra AINames assertions) and splicing in this branch's GeneratorRegistry-based loop in place of the old hardcoded 8-method one.

The real find, after conflicts resolved: the branch didn't compile. #238 (merged in the meantime) added CustomGamePreferences.h, which serializes CustomGameSetup::generator via direct Sint32 MapGenerationDescriptor::* member pointers. This branch replaced generator's type with GenerationRequest, whose per-method options live in a generic std::map<std::string,int> — an architecture change, not a rename, so #238's file had no path forward. Fixed by routing encode()/decode() through toLegacyDescriptor()/fromLegacyDescriptor() (the compatibility adapter this PR already built for exactly this), keeping the on-disk format and corruption-recovery bounds unchanged, and widening a few of those bounds to match the registry's current ranges (they'd gone stale — e.g. oldIslandSize's own default already exceeded the field's declared max for any method other than Isles/Old Islands, so a completely ordinary preferences save could fail to load back).

While tracing that, I also found and fixed an unrelated crash in the compatibility adapter itself: Lattice and Maze register wheat/wood/stone/algae controls that legacyField()'s mapping table doesn't know about, so converting either method through the adapter threw an uncaught std::invalid_argument. Added the four mappings (they match pre-existing legacy struct fields exactly) and changed the fallback for every other option with no legacy slot (loopiness, home-radius, cell-size, ...) from throwing to using the control's default value, so a newer generator's full option set can't take the adapter down.

Verification on this head:

  • Full scons -j8 release=1 server=0 client build is clean.
  • CustomGameSetupHarness passes in default, preferences-write and preferences-read modes.
  • MapGeneratorDefaultsTest and MapGeneratorStudy --catalog pass, unaffected by the adapter fix.

Requesting review from @Giszmo and @kylelutze (same as #238, since this stacks directly on it). No merge performed.

genixpro added a commit that referenced this pull request Sep 10, 2026
Merging directly. CI green on all three platforms, no merge conflicts, no reviewer objections. Stacked map-generator work (#240) follows.
Base automatically changed from codex/map-generator-defaults to master September 10, 2026 23:23
genixpro and others added 8 commits September 10, 2026 19:25
#238 (already merged) added CustomGamePreferences.h, which serializes
CustomGameSetup::generator by taking Sint32 MapGenerationDescriptor::*
member pointers directly. #240 replaced generator's type with
GenerationRequest, whose method-specific options live in a generic
std::map<std::string,int> instead of fixed struct fields — an
architecture change, not a rename — so the file no longer compiled.

- CustomGamePreferences::encode()/decode() now convert through
  toLegacyDescriptor()/fromLegacyDescriptor() (the compatibility
  adapter #240 already built for exactly this kind of interop), so the
  on-disk wire format and its corruption-recovery bounds are unchanged.
  decode()'s method-validity check now asks the live GeneratorRegistry
  instead of a hardcoded 1-8 range, so it stays correct as generators
  are added or retired.
- Widened several fields() bounds (terrain weights 0-64 -> 0-100,
  riverDiameter's max 64 -> 65, oldIslandSize 1-64 -> 1-70) to match
  the modular registry's current ranges. These are approximate,
  same as before: the reused legacy fields (e.g. riverDiameter also
  stands in for lake size/channel width/bridge width) don't have one
  true bound, so this is a safe envelope, not a tight per-method one.
  Without this, decode() could reject a preferences file that a normal
  save legitimately produced (oldIslandSize's own default already
  exceeded the old 1-64 bound for any method other than Isles/Old
  Islands).
- Found and fixed a related crash bug in the compatibility adapter
  itself while tracing this: Lattice and Maze register wheat/wood/
  stone/algae controls with no entry in legacyField()'s mapping table,
  so converting either method through toLegacyDescriptor/
  fromLegacyDescriptor threw an uncaught std::invalid_argument. Added
  the four missing mappings (they match pre-existing legacy struct
  fields exactly) and changed every other option with no legacy slot
  (loopiness, home-radius, cell-size, ...) from throwing to falling
  back to its control's default value, so a newer generator's full
  option set can never crash this adapter again.
- test/CustomGameSetupHarness.cpp: preferencesModel()/preferencesScreen()
  built a GenerationRequest via the same member-pointer approach;
  updated both to build a temporary MapGenerationDescriptor and convert.
  Switched preferencesModel()'s method from Old Islands to Crater Lakes
  (one of the four modern height-map generators that still exposes a
  repeat-landscape control; Old Islands never did in the new registry,
  so it always round-tripped back to 0). The per-field assertions in
  preferencesScreen() now compare against the same achievable
  conversion rather than raw field maximums, since only the options a
  method actually registers survive a GenerationRequest round trip.

Verified: full scons -j8 release=1 server=0 client build is clean.
CustomGameSetupHarness passes in default, preferences-write and
preferences-read modes. MapGeneratorDefaultsTest and
MapGeneratorStudy --catalog also pass, unaffected by the adapter fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KFxsZmLM4qsovemHqDrHGP
Rebased onto master now that #238 has merged. That surfaced a real compile
break: the harness still called Map::oldMakeIslandsMap/Game::oldMakeIslandsMap,
which the registry-driven generator rewrite removed.

Route it through MapGenerator::generateMap(Game&, const MapGenerationDescriptor&,
seed) instead, which owns map sizing and the game association internally. Seed
explicitly since generation determinism no longer follows the global sync-rand
state.

Also start from setMethodDefaults() rather than hand-picked constants: #238's
defaults tuning tightened several control ranges (island-size moved to 50-70,
the shared "workers" control caps at 8), so the harness's old literals
(oldIslandSize=35, nbWorkers=48) now fail request validation. Defaulting first
and overriding only what this decorative colony actually needs to differ keeps
it from rotting the same way again as ranges keep moving.

Verified: full client and menu-colony-harness build clean; `check`, `navigation`,
and `generate` subcommands all pass (generate grows the colony from 8 to 56
units over the same 12,000-tick warmup, confirming the reduced starting worker
count doesn't defeat the decorative intent).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Le23P4h9QuDjwExuK6HU96
@genixpro
genixpro force-pushed the codex/modular-map-generators branch from 2a0895c to db88fb0 Compare September 10, 2026 23:35
@genixpro

Copy link
Copy Markdown
Contributor Author

Retargeted onto master and rebased now that #238 has merged. Master's tree was byte-identical to #238's old branch tip (clean squash merge), so git rebase --onto origin/master origin/codex/map-generator-defaults replayed this branch's own 7 commits with zero conflicts — no manual reconciliation needed this time.

That surfaced a real compile break in CI, though: tools/MenuColonyHarness.cpp still called Map::oldMakeIslandsMap/Game::oldMakeIslandsMap, which the registry-driven rewrite removed. Fixed by routing it through MapGenerator::generateMap(game, d, seed). Two follow-on issues surfaced once it actually ran end-to-end (CI only exercises check/navigation, not generate, so these were latent):

  • The shared workers control is now capped at 1–8; the harness's old nbWorkers=48 fails validation. Clamped to 8 — verified the 12,000-tick AI warmup still grows the decorative colony to 56 units/11 buildings by the end, so this doesn't defeat the intent.
  • island-size on Rugged Archipelago (the renamed Old Islands) is now 50–70; the old oldIslandSize=35 fails too. Switched to d.setMethodDefaults(...) first and only overriding what the tool actually needs to differ, so it stops rotting every time a generator's tuned ranges move.

Verified on this head (db88fb0c9):

  • Full scons -j8 release=1 server=0 client build is clean.
  • scons -j8 release=1 server=0 menu-colony-harness builds clean.
  • check, navigation, and generate subcommands all pass.

CI is running now. Requesting review from @Giszmo and @kylelutze (unchanged from the original description). No merge performed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant