Unify map generator controls and tune playable defaults - #238
Merged
Conversation
genixpro
force-pushed
the
codex/map-generator-defaults
branch
from
September 10, 2026 21:36
9e9f0f1 to
b74f6df
Compare
Contributor
Author
|
Rebased onto current master (
Verification on this head (
Requesting review from @Giszmo and @kylelutze — this is still marked draft; let me know if it should come off draft. No merge performed. |
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
marked this pull request as ready for review
September 10, 2026 23:23
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
added a commit
that referenced
this pull request
Sep 10, 2026
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Random-map defaults now leave more grassy building room while retaining each generator's shape. The lobby, map editor, command-line stress tool and study tooling share one definition of generator names, available controls, ranges, steps and defaults.
Stacked on #237 (
codex/custom-game-ux, 82943d9). The diff contains only the generator follow-up.Changes
Validation
Both baseline cohorts include the same legacy correctness fixes. Placement failures remain: River has 18/1,000 (versus lobby 4 and editor 145), and Shattered Coast 14/1,000 (versus lobby 0 and editor 2), alongside substantially better building room and starting-resource access. Small crowded maps remain a limitation; this is not a full-match balance study.
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.