From c3c4501c5762bfea4c76171a3fc7c7c1cbe1bbc8 Mon Sep 17 00:00:00 2001 From: Enn3Developer Date: Tue, 25 Mar 2025 20:01:21 +0100 Subject: [PATCH 01/10] initial generation --- OpenPolytopia.Common/City.cs | 2 + OpenPolytopia.Common/Grid.cs | 22 +++++- OpenPolytopia.Common/Player.cs | 4 ++ OpenPolytopia.Common/TerrainGeneration.cs | 83 +++++++++++++++++++++++ OpenPolytopia/test/src/GridTest.cs | 6 ++ 5 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 OpenPolytopia.Common/Player.cs create mode 100644 OpenPolytopia.Common/TerrainGeneration.cs diff --git a/OpenPolytopia.Common/City.cs b/OpenPolytopia.Common/City.cs index a0e53e87..2a31e10e 100644 --- a/OpenPolytopia.Common/City.cs +++ b/OpenPolytopia.Common/City.cs @@ -11,6 +11,8 @@ namespace OpenPolytopia.Common; public class CityManager(Grid grid) { private readonly List _cities = []; + public IEnumerable Cities => _cities; + /// /// Access to the /// diff --git a/OpenPolytopia.Common/Grid.cs b/OpenPolytopia.Common/Grid.cs index a81c3b75..b34ca5fb 100644 --- a/OpenPolytopia.Common/Grid.cs +++ b/OpenPolytopia.Common/Grid.cs @@ -64,6 +64,19 @@ public Tile this[uint index] { [MethodImpl(MethodImplOptions.AggressiveInlining)] public uint GridPositionToIndex(int x, int y) => (uint)((y * size) + x); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public Vector2I IndexToGridPosition(uint index) { + var x = index % size; + var y = (index - x) / size; + return new Vector2I((int)x, (int)y); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void IndexToGridPosition(uint index, out uint x, out uint y) { + x = index % size; + y = (index - x) / size; + } + /// /// Modifies a given tile /// @@ -215,7 +228,7 @@ public static Type GetModifier(TileKind kind) => /// Using 0 as the left-most bit and 63 as the right-most bit /// /// 0 -> 1: has road; 0 doesn't have any road; (bridge if on water) - /// 1 -> 1: has ancient ruin; 0 doesn't have any ruin + /// 1 -> 2: has ancient ruin; 0 doesn't have any ruin /// [2, 4] -> /// [5, 6] -> Tile modifier /// [7, 9] -> Tile buildings @@ -251,7 +264,12 @@ public bool Ruin { /// /// The type of the tile /// - public TileKind Kind => (TileKind)_inner.GetBits(THREE_BITS, TILEKIND_POSITION); + public TileKind Kind { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => (TileKind)_inner.GetBits(THREE_BITS, TILEKIND_POSITION); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + set => _inner.SetBits((ulong)value, THREE_BITS, TILEKIND_POSITION); + } /// /// The tile modifier castable to the corresponding enum diff --git a/OpenPolytopia.Common/Player.cs b/OpenPolytopia.Common/Player.cs new file mode 100644 index 00000000..3f6915e6 --- /dev/null +++ b/OpenPolytopia.Common/Player.cs @@ -0,0 +1,4 @@ +namespace OpenPolytopia.Common; + +public record Player(TribeType Tribe, int Id) { +} diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs new file mode 100644 index 00000000..b212544e --- /dev/null +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -0,0 +1,83 @@ +namespace OpenPolytopia.Common; + +using System.Runtime.CompilerServices; + +public class TerrainGeneration( + Grid grid, + CityManager cityManager, + TribeManager tribeManager, + Player[] players, + int? seed = null) { + // create the random number generator from a seed if set + private readonly Random _rng = seed == null ? new Random() : new Random(seed.Value); + + /// + /// Generates the map ready to use in-game + /// + /// + /// + /// var terrainGeneration = new TerrainGeneration(grid, cityManager, tribeManager); + /// await terrainGeneration.GenerateMapAsync(); + /// + /// + public async Task GenerateMapAsync() { + await GenerateInitialCitiesAsync(); + await GenerateTerrainAsync(); + await GenerateCitiesAsync(); + await GenerateResourcesAsync(); + } + + private async Task GenerateInitialCitiesAsync() { + // for each player + foreach (var player in players) { + // yield to the task executor + await Task.Yield(); + + // compute the index position of a random tile in the grid + var index = (uint)_rng.Next(0, (int)(grid.Size * grid.Size)); + + // check if the computed index is valid + var valid = false; + while (!valid) { + if (IsCityValid(index)) { + valid = true; + } + else { + index = (uint)_rng.Next(0, (int)(grid.Size * grid.Size)); + } + } + + // register the city + var cityId = cityManager.RegisterCity(index); + + // set all data for the tile in the grid + grid.ModifyTile(index, (ref Tile tile) => { + tile.Owner = player.Id; + tile.Kind = TileKind.Village; + tile.Modifier = (int)VillageTileModifier.City; + }); + + // set all data for the city + cityManager.ModifyCity(cityId, (ref CityData city) => { + city.Capital = true; + city.Owner = player.Id; + city.Level = 1; + }); + } + } + + private async Task GenerateTerrainAsync() { + } + + private async Task GenerateCitiesAsync() { + } + + private async Task GenerateResourcesAsync() { + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private bool IsCityValid(uint index) { + grid.IndexToGridPosition(index, out var x, out var y); + return grid[index].Owner == 0 && x > 0 && y > 0 && x < grid.Size - 1 && y < grid.Size - 1; + } +} diff --git a/OpenPolytopia/test/src/GridTest.cs b/OpenPolytopia/test/src/GridTest.cs index 0a7a2ecc..fc18e712 100644 --- a/OpenPolytopia/test/src/GridTest.cs +++ b/OpenPolytopia/test/src/GridTest.cs @@ -12,6 +12,12 @@ public void TestPositionToIndex() { grid.GridPositionToIndex(new Vector2I(2, 2)).ShouldBe(22u); } + [Test] + public void TestIndexToPosition() { + var grid = new Grid(10); + grid.IndexToGridPosition(22u).ShouldBe(new Vector2I(2, 2)); + } + [Test] public void TestModifyTile() { var grid = new Grid(10); From 18506b24f8c94b216d272e6502cad30d7da089d0 Mon Sep 17 00:00:00 2001 From: Enn3Developer Date: Wed, 15 Oct 2025 16:49:41 +0200 Subject: [PATCH 02/10] added doc comments --- OpenPolytopia.Common/TerrainGeneration.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index b212544e..90912dbb 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -2,6 +2,14 @@ namespace OpenPolytopia.Common; using System.Runtime.CompilerServices; +/// +/// Terrain generation system +/// +/// the grid to use +/// the city manager to use +/// the tribe manager to use +/// all the players in the game +/// the optional random seed public class TerrainGeneration( Grid grid, CityManager cityManager, From d3cebb81d7c190a4cb50fdbdd94698970de597ca Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 14:19:23 +0000 Subject: [PATCH 03/10] feat: implement Polytopia-style terrain generation pipeline Implements the full map generation pipeline in TerrainGeneration, following the algorithm used by The Battle of Polytopia: - land generation: random land seeding followed by cellular-automata smoothing passes, tunable via InitialLand, Smoothing and Relief - capital placement: capitals spawn on land away from the map border, maximizing the distance between each other, with progressively relaxed requirements so placement never fails on tiny or watery maps - terrain assignment: every tile gets the biome of the nearest capital and rolls mountain/forest/water using the tribe terrain rates, with ocean adjacent to land becoming shallow water - village placement: villages spawn on free land at least 2 tiles away from any other city or village until no free tile remains - resource spawning: resources spawn only within 2 tiles of a city or village, at a third of the rate on the outer ring, using the tribe spawn rates as multipliers - ruins: one ancient ruin every 40 tiles, away from villages, never adjacent to each other and with at most a third of them on water Generation is fully deterministic for a given seed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/TerrainGeneration.cs | 521 +++++++++++++++++- .../test/src/TerrainGenerationTest.cs | 166 ++++++ 2 files changed, 672 insertions(+), 15 deletions(-) create mode 100644 OpenPolytopia/test/src/TerrainGenerationTest.cs diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index 90912dbb..b4d78798 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -3,7 +3,19 @@ namespace OpenPolytopia.Common; using System.Runtime.CompilerServices; /// -/// Terrain generation system +/// Terrain generation system. +/// +/// Implements the Polytopia map generation pipeline: +/// +/// Land generation: random land seeding followed by cellular-automata smoothing passes +/// Capital placement: capitals are placed on land maximizing the distance between each other +/// Terrain assignment: every tile gets the biome of the nearest capital and rolls +/// mountain/forest/water using the tribe terrain rates +/// Village placement: villages are placed on free land at least 2 tiles away from other villages +/// Resource spawning: resources spawn only within 2 tiles of a city or village, +/// with reduced rates on the outer ring +/// Ruins: tiles / 40 ancient ruins, at most a third of them on water +/// /// /// the grid to use /// the city manager to use @@ -16,45 +28,189 @@ public class TerrainGeneration( TribeManager tribeManager, Player[] players, int? seed = null) { + // base terrain rates, multiplied by every tribe's TerrainRate + private const float BASE_FOREST_RATE = 0.38f; + private const float BASE_MOUNTAIN_RATE = 0.14f; + + // base resource rates, multiplied by every tribe's SpawnRate + private const float BASE_FRUIT_RATE = 0.18f; + private const float BASE_CROP_RATE = 0.18f; + private const float BASE_ANIMAL_RATE = 0.19f; + private const float BASE_FISH_RATE = 0.5f; + private const float BASE_MINERAL_RATE = 0.11f; + + // stars have no rate in SpawnRate, so they only use the base rate + private const float BASE_STAR_RATE = 0.4f; + + // resources on the border expansion ring (2 tiles away from a city) spawn at a third of the rate + private const float BORDER_EXPANSION_RATE = 1f / 3f; + + // capitals can't spawn closer than this to the map border + private const int CAPITAL_EDGE_MARGIN = 2; + + // one ruin every RUINS_DIVISOR tiles, at most a third of them on water + private const int RUINS_DIVISOR = 40; + + // Tile.City is 8 bits so there can't be more than 255 cities in a grid + private const int MAX_CITIES = 255; + + // zone map values, used for village spacing and resource spawning + private const byte ZONE_FREE = 0; + private const byte ZONE_BORDER = 1; + private const byte ZONE_TERRITORY = 2; + private const byte ZONE_CITY = 3; + // create the random number generator from a seed if set private readonly Random _rng = seed == null ? new Random() : new Random(seed.Value); + // distance-to-city zones; rebuilt while placing capitals and villages + private readonly byte[] _zoneMap = new byte[grid.Size * grid.Size]; + + // grid indexes of the capitals, aligned with players + private readonly List _capitals = new(players.Length); + + private int _citiesCount; + + /// + /// Fraction of the map converted to land before the smoothing passes + /// + public float InitialLand { get; init; } = 0.5f; + + /// + /// Number of cellular-automata smoothing passes applied to the initial land + /// + public int Smoothing { get; init; } = 3; + + /// + /// Land/water balance of the smoothing passes. + /// + /// A cell stays land when at most Relief of the 9 cells around it are water, + /// so lower values produce more compact continents while higher values produce rougher coasts + /// + public int Relief { get; init; } = 4; + + /// + /// Base probability of a land tile converting to water, multiplied by the tribe's + /// + /// + public float WaterRate { get; init; } = 0.05f; + /// /// Generates the map ready to use in-game /// /// /// - /// var terrainGeneration = new TerrainGeneration(grid, cityManager, tribeManager); + /// var terrainGeneration = new TerrainGeneration(grid, cityManager, tribeManager, players); /// await terrainGeneration.GenerateMapAsync(); /// /// public async Task GenerateMapAsync() { + await GenerateLandAsync(); await GenerateInitialCitiesAsync(); await GenerateTerrainAsync(); await GenerateCitiesAsync(); await GenerateResourcesAsync(); + await GenerateRuinsAsync(); + } + + /// + /// Generates the land/ocean layout. + /// + /// Starts from a full ocean map, converts random tiles to land until + /// is reached, then applies cellular-automata passes where a tile stays + /// land when at most of the 9 cells around it are water + /// + private async Task GenerateLandAsync() { + var size = (int)grid.Size; + var cells = size * size; + + for (var i = 0u; i < cells; i++) { + grid.ModifyTile(i, (ref Tile tile) => tile.Kind = TileKind.Ocean); + } + + // seed random land tiles until the initial land fraction is reached + var target = (int)(cells * Math.Clamp(InitialLand, 0f, 1f)); + var landCount = 0; + while (landCount < target) { + var index = (uint)_rng.Next(0, cells); + if (grid[index].Kind != TileKind.Ocean) { + continue; + } + + grid.ModifyTile(index, (ref Tile tile) => tile.Kind = TileKind.Field); + landCount++; + } + + // smooth the noise into continuous landmasses + var relief = Math.Clamp(Relief, 0, 8); + var snapshot = new TileKind[cells]; + for (var pass = 0; pass < Smoothing; pass++) { + // yield to the task executor + await Task.Yield(); + + for (var i = 0u; i < cells; i++) { + snapshot[i] = grid[i].Kind; + } + + for (var y = 0; y < size; y++) { + for (var x = 0; x < size; x++) { + // count water cells in the 3x3 neighborhood; out-of-bounds cells count as water + var waterCount = 0; + for (var dy = -1; dy <= 1; dy++) { + for (var dx = -1; dx <= 1; dx++) { + var nx = x + dx; + var ny = y + dy; + if (nx < 0 || ny < 0 || nx >= size || ny >= size || + snapshot[(ny * size) + nx] == TileKind.Ocean) { + waterCount++; + } + } + } + + var kind = waterCount <= relief ? TileKind.Field : TileKind.Ocean; + grid.ModifyTile(grid.GridPositionToIndex(x, y), (ref Tile tile) => tile.Kind = kind); + } + } + } } + /// + /// Places a capital for every player. + /// + /// Capitals only spawn on land at least tiles away from the + /// map border; every capital is placed on the tile maximizing the minimum distance to the + /// already placed capitals so players don't start too close to one another + /// private async Task GenerateInitialCitiesAsync() { - // for each player foreach (var player in players) { // yield to the task executor await Task.Yield(); - // compute the index position of a random tile in the grid - var index = (uint)_rng.Next(0, (int)(grid.Size * grid.Size)); + var candidates = CollectCapitalCandidates(); - // check if the computed index is valid - var valid = false; - while (!valid) { - if (IsCityValid(index)) { - valid = true; + // keep the tiles with the maximum distance to the closest already placed capital + var best = new List(); + var bestScore = -1; + foreach (var candidate in candidates) { + var score = int.MaxValue; + foreach (var capital in _capitals) { + score = Math.Min(score, Distance(candidate, capital)); } - else { - index = (uint)_rng.Next(0, (int)(grid.Size * grid.Size)); + + if (score > bestScore) { + bestScore = score; + best.Clear(); + best.Add(candidate); + } + else if (score == bestScore) { + best.Add(candidate); } } + var index = best[_rng.Next(best.Count)]; + _capitals.Add(index); + _citiesCount++; + // register the city var cityId = cityManager.RegisterCity(index); @@ -63,6 +219,7 @@ private async Task GenerateInitialCitiesAsync() { tile.Owner = player.Id; tile.Kind = TileKind.Village; tile.Modifier = (int)VillageTileModifier.City; + tile.Biome = player.Tribe; }); // set all data for the city @@ -71,21 +228,355 @@ private async Task GenerateInitialCitiesAsync() { city.Owner = player.Id; city.Level = 1; }); + + // claim the starting 3x3 territory + ForEachInRadius(index, 1, neighbour => grid.ModifyTile(neighbour, (ref Tile tile) => { + tile.Owner = player.Id; + tile.City = (int)cityId; + })); + + MarkCityZone(index); + } + } + + /// + /// Collects the tiles where a capital can spawn. + /// + /// Starts with free land at least tiles away from the map + /// border and progressively relaxes the requirements so capitals can always be placed, even + /// on tiny or very watery maps + /// + /// the indexes of the valid spawn tiles + private List CollectCapitalCandidates() { + var cells = grid.Size * grid.Size; + var candidates = new List(); + + (int margin, byte maxZone)[] tiers = [(CAPITAL_EDGE_MARGIN, ZONE_FREE), (1, ZONE_FREE), (0, ZONE_BORDER)]; + foreach (var (margin, maxZone) in tiers) { + for (var i = 0u; i < cells; i++) { + grid.IndexToGridPosition(i, out var x, out var y); + if (grid[i].Kind == TileKind.Field && grid[i].Owner == 0 && _zoneMap[i] <= maxZone && + x >= margin && y >= margin && x < grid.Size - margin && y < grid.Size - margin) { + candidates.Add(i); + } + } + + if (candidates.Count > 0) { + return candidates; + } + } + + // last resort: raise a random tile from the ocean + var index = (uint)_rng.Next(0, (int)cells); + while (grid[index].Kind == TileKind.Village) { + index = (uint)_rng.Next(0, (int)cells); } + + grid.ModifyTile(index, (ref Tile tile) => tile.Kind = TileKind.Field); + candidates.Add(index); + return candidates; } + /// + /// Assigns biomes and terrain types. + /// + /// Every tile gets the biome of the nearest capital; every land tile then rolls water, + /// mountain and forest in this order using the biome tribe's , + /// defaulting to field. Finally, ocean tiles adjacent to land become shallow water + /// private async Task GenerateTerrainAsync() { + var size = (int)grid.Size; + var cells = (uint)(size * size); + + for (var i = 0u; i < cells; i++) { + if (i % grid.Size == 0) { + // yield to the task executor once per row + await Task.Yield(); + } + + var biome = NearestCapitalTribe(i); + grid.ModifyTile(i, (ref Tile tile) => tile.Biome = biome); + + if (grid[i].Kind != TileKind.Field) { + continue; + } + + var rates = tribeManager[biome]?.TerrainRate; + var waterRate = WaterRate * (rates?.WaterRate ?? 1f); + var mountainRate = BASE_MOUNTAIN_RATE * (rates?.MountainRate ?? 1f); + var forestRate = BASE_FOREST_RATE * (rates?.ForestRate ?? 1f); + + // city territory never converts to water, otherwise a capital could end up without land + if (grid[i].Owner == 0 && _rng.NextSingle() < waterRate) { + grid.ModifyTile(i, (ref Tile tile) => tile.Kind = TileKind.Water); + continue; + } + + // mountain first, then forest; fields are just the remainder + var roll = _rng.NextSingle(); + if (roll < mountainRate) { + grid.ModifyTile(i, (ref Tile tile) => tile.Kind = TileKind.Mountain); + } + else if (roll < mountainRate + forestRate) { + grid.ModifyTile(i, (ref Tile tile) => tile.Kind = TileKind.Forest); + } + } + + // ocean adjacent to land becomes shallow water + var snapshot = new TileKind[cells]; + for (var i = 0u; i < cells; i++) { + snapshot[i] = grid[i].Kind; + } + + for (var y = 0; y < size; y++) { + for (var x = 0; x < size; x++) { + if (snapshot[(y * size) + x] != TileKind.Ocean) { + continue; + } + + var nearLand = (x > 0 && IsLand(snapshot[(y * size) + x - 1])) || + (x < size - 1 && IsLand(snapshot[(y * size) + x + 1])) || + (y > 0 && IsLand(snapshot[((y - 1) * size) + x])) || + (y < size - 1 && IsLand(snapshot[((y + 1) * size) + x])); + if (nearLand) { + grid.ModifyTile(grid.GridPositionToIndex(x, y), (ref Tile tile) => tile.Kind = TileKind.Water); + } + } + } } + /// + /// Places the villages. + /// + /// Villages spawn on free field/forest tiles at least 1 tile away from the map border and + /// 2 tiles away from any other city or village, until no free tile remains + /// private async Task GenerateCitiesAsync() { + var size = (int)grid.Size; + var cells = (uint)(size * size); + + // collect all the tiles where a village can spawn + var candidates = new List(); + for (var i = 0u; i < cells; i++) { + grid.IndexToGridPosition(i, out var x, out var y); + if (_zoneMap[i] == ZONE_FREE && grid[i].Kind is TileKind.Field or TileKind.Forest && + x > 0 && y > 0 && x < size - 1 && y < size - 1) { + candidates.Add(i); + } + } + + while (candidates.Count > 0 && _citiesCount < MAX_CITIES) { + // yield to the task executor + await Task.Yield(); + + var position = _rng.Next(candidates.Count); + var index = candidates[position]; + candidates.RemoveAt(position); + + // a previously placed village may have claimed this tile + if (_zoneMap[index] != ZONE_FREE) { + continue; + } + + // register the village; the default CityData already means "village" (no owner, level 0) + cityManager.RegisterCity(index); + _citiesCount++; + + grid.ModifyTile(index, (ref Tile tile) => { + tile.Kind = TileKind.Village; + tile.Modifier = (int)VillageTileModifier.Village; + }); + + MarkCityZone(index); + } } + /// + /// Spawns the resources. + /// + /// Resources only spawn within 2 tiles of a city or village, at full rate inside city + /// territory and at a third of the rate on the outer ring, using the biome tribe's + /// as multiplier + /// private async Task GenerateResourcesAsync() { + var cells = grid.Size * grid.Size; + + for (var i = 0u; i < cells; i++) { + if (i % grid.Size == 0) { + // yield to the task executor once per row + await Task.Yield(); + } + + var zone = _zoneMap[i]; + if (zone is not ZONE_TERRITORY and not ZONE_BORDER) { + continue; + } + + var multiplier = zone == ZONE_TERRITORY ? 1f : BORDER_EXPANSION_RATE; + var tile = grid[i]; + var rates = tribeManager[tile.Biome]?.SpawnRate; + + switch (tile.Kind) { + case TileKind.Field: { + // fruit first, then crop; both can't spawn on the same tile + var fruitRate = BASE_FRUIT_RATE * (rates?.FruitRate ?? 1f) * multiplier; + var cropRate = BASE_CROP_RATE * (rates?.CropRate ?? 1f) * multiplier; + var roll = _rng.NextSingle(); + if (roll < fruitRate) { + grid.ModifyTile(i, (ref Tile t) => t.SetTileModifier(FieldTileModifier.Fruit)); + } + else if (roll < fruitRate + cropRate) { + grid.ModifyTile(i, (ref Tile t) => t.SetTileModifier(FieldTileModifier.Crop)); + } + + break; + } + case TileKind.Forest: + if (_rng.NextSingle() < BASE_ANIMAL_RATE * (rates?.AnimalRate ?? 1f) * multiplier) { + grid.ModifyTile(i, (ref Tile t) => t.SetTileModifier(ForestTileModifier.Animal)); + } + + break; + case TileKind.Mountain: + if (_rng.NextSingle() < BASE_MINERAL_RATE * (rates?.MineralRate ?? 1f) * multiplier) { + grid.ModifyTile(i, (ref Tile t) => t.SetTileModifier(MountainTileModifier.Ore)); + } + + break; + case TileKind.Water: + if (_rng.NextSingle() < BASE_FISH_RATE * (rates?.FishRate ?? 1f) * multiplier) { + grid.ModifyTile(i, (ref Tile t) => t.SetTileModifier(WaterTileModifier.Fish)); + } + + break; + case TileKind.Ocean: + if (_rng.NextSingle() < BASE_STAR_RATE * multiplier) { + grid.ModifyTile(i, (ref Tile t) => t.SetTileModifier(OceanTileModifier.Star)); + } + + break; + case TileKind.Village: + default: + break; + } + } } + /// + /// Places the ancient ruins. + /// + /// Spawns tiles / 40 ruins away from cities and villages, never adjacent to another + /// ruin and with at most a third of them on water + /// + private async Task GenerateRuinsAsync() { + // yield to the task executor + await Task.Yield(); + + var cells = (int)(grid.Size * grid.Size); + var total = cells / RUINS_DIVISOR; + var maxOnWater = total / 3; + + var placed = 0; + var placedOnWater = 0; + // random placement can fail, so bound the number of attempts + var attempts = total * 30; + while (placed < total && attempts-- > 0) { + var index = (uint)_rng.Next(0, cells); + var tile = grid[index]; + + // keep ruins away from cities, villages and their territory + if (tile.Ruin || tile.Kind == TileKind.Village || _zoneMap[index] >= ZONE_TERRITORY) { + continue; + } + + var onWater = tile.Kind is TileKind.Water or TileKind.Ocean; + if (onWater && placedOnWater >= maxOnWater) { + continue; + } + + // never place two ruins next to each other + var nearRuin = false; + ForEachInRadius(index, 1, neighbour => nearRuin |= grid[neighbour].Ruin); + if (nearRuin) { + continue; + } + + grid.ModifyTile(index, (ref Tile t) => t.Ruin = true); + placed++; + if (onWater) { + placedOnWater++; + } + } + } + + /// + /// Marks the zones around a new city or village: the tile itself as city, the tiles within + /// 1 tile as territory and the tiles within 2 tiles as border expansion + /// + /// index of the city tile + private void MarkCityZone(uint index) { + _zoneMap[index] = ZONE_CITY; + ForEachInRadius(index, 2, + neighbour => _zoneMap[neighbour] = Math.Max(_zoneMap[neighbour], ZONE_BORDER)); + ForEachInRadius(index, 1, + neighbour => _zoneMap[neighbour] = Math.Max(_zoneMap[neighbour], ZONE_TERRITORY)); + } + + /// + /// Returns the tribe of the capital closest to a tile + /// + /// index of the tile + /// the tribe of the nearest capital, or the first player's tribe if there are no capitals + private TribeType NearestCapitalTribe(uint index) { + var tribe = players[0].Tribe; + var bestDistance = int.MaxValue; + for (var i = 0; i < _capitals.Count; i++) { + var distance = Distance(index, _capitals[i]); + if (distance < bestDistance) { + bestDistance = distance; + tribe = players[i].Tribe; + } + } + + return tribe; + } + + /// + /// Runs a callback for every tile within a radius, excluding the center tile itself + /// + /// index of the center tile + /// the radius + /// callback run with the index of every neighbour + private void ForEachInRadius(uint index, int radius, Action action) { + var size = (int)grid.Size; + grid.IndexToGridPosition(index, out var centerX, out var centerY); + for (var dy = -radius; dy <= radius; dy++) { + for (var dx = -radius; dx <= radius; dx++) { + if (dx == 0 && dy == 0) { + continue; + } + + var x = (int)centerX + dx; + var y = (int)centerY + dy; + if (x < 0 || y < 0 || x >= size || y >= size) { + continue; + } + + action(grid.GridPositionToIndex(x, y)); + } + } + } + + /// + /// Chebyshev distance between two tiles, so diagonals count as 1 + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - private bool IsCityValid(uint index) { - grid.IndexToGridPosition(index, out var x, out var y); - return grid[index].Owner == 0 && x > 0 && y > 0 && x < grid.Size - 1 && y < grid.Size - 1; + private int Distance(uint a, uint b) { + grid.IndexToGridPosition(a, out var ax, out var ay); + grid.IndexToGridPosition(b, out var bx, out var by); + return Math.Max(Math.Abs((int)ax - (int)bx), Math.Abs((int)ay - (int)by)); } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsLand(TileKind kind) => kind is not TileKind.Water and not TileKind.Ocean; } diff --git a/OpenPolytopia/test/src/TerrainGenerationTest.cs b/OpenPolytopia/test/src/TerrainGenerationTest.cs new file mode 100644 index 00000000..bba9820a --- /dev/null +++ b/OpenPolytopia/test/src/TerrainGenerationTest.cs @@ -0,0 +1,166 @@ +namespace OpenPolytopia; + +using System.Threading.Tasks; +using Chickensoft.GoDotTest; +using Common; +using Godot; +using Shouldly; + +public class TerrainGenerationTest(Node testScene) : TestClass(testScene) { + private const uint SIZE = 16; + private const int SEED = 42; + + private static async Task<(Grid, CityManager, Player[])> GenerateMapAsync() { + var grid = new Grid(SIZE); + var cityManager = new CityManager(grid); + var tribeManager = new TribeManager(); + tribeManager.RegisterTribe(TribeType.Imperius, + new Tribe { + StartingTech = new StartingTech { Branch = BranchType.Organization, Id = "organization" }, + SpawnRate = new SpawnRate { + FruitRate = 2.0f, CropRate = 1.0f, AnimalRate = 0.5f, MineralRate = 1.0f, FishRate = 1.0f + }, + TerrainRate = new TerrainRate { ForestRate = 1.0f, MountainRate = 1.0f, WaterRate = 1.0f }, + StartingStars = 7 + }); + var players = new[] { new Player(TribeType.Imperius, 1), new Player(TribeType.Imperius, 2) }; + + var terrainGeneration = new TerrainGeneration(grid, cityManager, tribeManager, players, SEED); + await terrainGeneration.GenerateMapAsync(); + return (grid, cityManager, players); + } + + [Test] + public async Task TestCapitals() { + var (grid, cityManager, players) = await GenerateMapAsync(); + + var capitals = 0; + foreach (var index in cityManager.Cities) { + var tile = grid[index]; + tile.Kind.ShouldBe(TileKind.Village); + + var cityData = tile.GetCustomData(); + if (!cityData.Capital) { + continue; + } + + capitals++; + cityData.Level.ShouldBe(1); + cityData.Owner.ShouldBeGreaterThan(0); + tile.GetTileModifier().ShouldBe(VillageTileModifier.City); + + // capitals can't spawn on the map border + grid.IndexToGridPosition(index, out var x, out var y); + x.ShouldBeInRange(2u, SIZE - 3); + y.ShouldBeInRange(2u, SIZE - 3); + } + + capitals.ShouldBe(players.Length); + } + + [Test] + public async Task TestTerrain() { + var (grid, _, _) = await GenerateMapAsync(); + + // a default map should have both land and water + var land = 0; + var water = 0; + for (var i = 0u; i < SIZE * SIZE; i++) { + if (grid[i].Kind is TileKind.Water or TileKind.Ocean) { + water++; + } + else { + land++; + } + } + + land.ShouldBeGreaterThan(0); + water.ShouldBeGreaterThan(0); + } + + [Test] + public async Task TestVillages() { + var (grid, cityManager, players) = await GenerateMapAsync(); + + var villages = 0; + foreach (var index in cityManager.Cities) { + if (grid[index].GetCustomData().Capital) { + continue; + } + + villages++; + + // villages have no owner until captured + grid[index].GetCustomData().Owner.ShouldBe(0); + grid[index].GetCustomData().Level.ShouldBe(0); + + // villages can't spawn next to another city or village + foreach (var other in cityManager.Cities) { + if (other == index) { + continue; + } + + grid.IndexToGridPosition(index, out var x, out var y); + grid.IndexToGridPosition(other, out var otherX, out var otherY); + var distance = Mathf.Max(Mathf.Abs((int)x - (int)otherX), Mathf.Abs((int)y - (int)otherY)); + distance.ShouldBeGreaterThanOrEqualTo(2); + } + } + + villages.ShouldBeGreaterThan(0); + } + + [Test] + public async Task TestResourcesNearCities() { + var (grid, cityManager, _) = await GenerateMapAsync(); + + for (var i = 0u; i < SIZE * SIZE; i++) { + var tile = grid[i]; + if (tile.Kind == TileKind.Village || tile.Modifier == 0) { + continue; + } + + // resources only spawn within 2 tiles of a city or village + grid.IndexToGridPosition(i, out var x, out var y); + var nearCity = false; + foreach (var index in cityManager.Cities) { + grid.IndexToGridPosition(index, out var cityX, out var cityY); + var distance = Mathf.Max(Mathf.Abs((int)x - (int)cityX), Mathf.Abs((int)y - (int)cityY)); + if (distance <= 2) { + nearCity = true; + break; + } + } + + nearCity.ShouldBeTrue($"resource at ({x}, {y}) is more than 2 tiles away from any city"); + } + } + + [Test] + public async Task TestRuins() { + var (grid, _, _) = await GenerateMapAsync(); + + var ruins = 0; + for (var i = 0u; i < SIZE * SIZE; i++) { + if (grid[i].Ruin) { + ruins++; + } + } + + // one ruin every 40 tiles + ruins.ShouldBeGreaterThan(0); + ruins.ShouldBeLessThanOrEqualTo((int)(SIZE * SIZE / 40)); + } + + [Test] + public async Task TestDeterministicSeed() { + var (first, _, _) = await GenerateMapAsync(); + var (second, _, _) = await GenerateMapAsync(); + + for (var i = 0u; i < SIZE * SIZE; i++) { + first[i].Kind.ShouldBe(second[i].Kind); + first[i].Modifier.ShouldBe(second[i].Modifier); + first[i].Ruin.ShouldBe(second[i].Ruin); + } + } +} From 56806eabdf2be8fd8c6753081cdc4a325d305c41 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 14:22:48 +0000 Subject: [PATCH 04/10] fix: resolve spellcheck CI failures Use the American spelling "neighbor" to match the rest of the codebase, replace Mathf with System.Math in the terrain generation test and add "Chebyshev" to the cspell dictionary. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/TerrainGeneration.cs | 10 +++++----- OpenPolytopia/test/src/TerrainGenerationTest.cs | 5 +++-- cspell.json | 1 + 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index b4d78798..3db6d329 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -230,7 +230,7 @@ private async Task GenerateInitialCitiesAsync() { }); // claim the starting 3x3 territory - ForEachInRadius(index, 1, neighbour => grid.ModifyTile(neighbour, (ref Tile tile) => { + ForEachInRadius(index, 1, neighbor => grid.ModifyTile(neighbor, (ref Tile tile) => { tile.Owner = player.Id; tile.City = (int)cityId; })); @@ -496,7 +496,7 @@ private async Task GenerateRuinsAsync() { // never place two ruins next to each other var nearRuin = false; - ForEachInRadius(index, 1, neighbour => nearRuin |= grid[neighbour].Ruin); + ForEachInRadius(index, 1, neighbor => nearRuin |= grid[neighbor].Ruin); if (nearRuin) { continue; } @@ -517,9 +517,9 @@ private async Task GenerateRuinsAsync() { private void MarkCityZone(uint index) { _zoneMap[index] = ZONE_CITY; ForEachInRadius(index, 2, - neighbour => _zoneMap[neighbour] = Math.Max(_zoneMap[neighbour], ZONE_BORDER)); + neighbor => _zoneMap[neighbor] = Math.Max(_zoneMap[neighbor], ZONE_BORDER)); ForEachInRadius(index, 1, - neighbour => _zoneMap[neighbour] = Math.Max(_zoneMap[neighbour], ZONE_TERRITORY)); + neighbor => _zoneMap[neighbor] = Math.Max(_zoneMap[neighbor], ZONE_TERRITORY)); } /// @@ -546,7 +546,7 @@ private TribeType NearestCapitalTribe(uint index) { /// /// index of the center tile /// the radius - /// callback run with the index of every neighbour + /// callback run with the index of every neighbor private void ForEachInRadius(uint index, int radius, Action action) { var size = (int)grid.Size; grid.IndexToGridPosition(index, out var centerX, out var centerY); diff --git a/OpenPolytopia/test/src/TerrainGenerationTest.cs b/OpenPolytopia/test/src/TerrainGenerationTest.cs index bba9820a..710b8d0f 100644 --- a/OpenPolytopia/test/src/TerrainGenerationTest.cs +++ b/OpenPolytopia/test/src/TerrainGenerationTest.cs @@ -1,5 +1,6 @@ namespace OpenPolytopia; +using System; using System.Threading.Tasks; using Chickensoft.GoDotTest; using Common; @@ -102,7 +103,7 @@ public async Task TestVillages() { grid.IndexToGridPosition(index, out var x, out var y); grid.IndexToGridPosition(other, out var otherX, out var otherY); - var distance = Mathf.Max(Mathf.Abs((int)x - (int)otherX), Mathf.Abs((int)y - (int)otherY)); + var distance = Math.Max(Math.Abs((int)x - (int)otherX), Math.Abs((int)y - (int)otherY)); distance.ShouldBeGreaterThanOrEqualTo(2); } } @@ -125,7 +126,7 @@ public async Task TestResourcesNearCities() { var nearCity = false; foreach (var index in cityManager.Cities) { grid.IndexToGridPosition(index, out var cityX, out var cityY); - var distance = Mathf.Max(Mathf.Abs((int)x - (int)cityX), Mathf.Abs((int)y - (int)cityY)); + var distance = Math.Max(Math.Abs((int)x - (int)cityX), Math.Abs((int)y - (int)cityY)); if (distance <= 2) { nearCity = true; break; diff --git a/cspell.json b/cspell.json index ee429a1b..bc4af743 100644 --- a/cspell.json +++ b/cspell.json @@ -98,6 +98,7 @@ "Vengir", "Bardur", "Oumaji", + "Chebyshev", "smithery", "aquatism", "stdb", From 1257bcffa1730308453e810661814c52c4248ac9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 14:54:14 +0000 Subject: [PATCH 05/10] feat: add TerrainGenerationNode to generate maps from a scene Adds a GlobalClass node wrapping TerrainGeneration so a map can be generated directly from a scene: all generation parameters (grid size, seed, initial land, smoothing, relief, water rate, player tribes) are exported to the inspector, the map can generate automatically when the node is ready or on demand via GenerateMapAsync, and the MapGenerated signal notifies when the grid and city manager are available. The node registers the tribes from the embedded tribes.json automatically; the file was updated to match the shape expected by TribeGenerationContext (tribe_type key and starting_stars inside the tribe object) since it previously failed to deserialize. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/resources/tribes.json | 6 +- OpenPolytopia/src/TerrainGenerationNode.cs | 180 ++++++++++++++++++ .../test/src/TerrainGenerationNodeTest.cs | 66 +++++++ 3 files changed, 249 insertions(+), 3 deletions(-) create mode 100644 OpenPolytopia/src/TerrainGenerationNode.cs create mode 100644 OpenPolytopia/test/src/TerrainGenerationNodeTest.cs diff --git a/OpenPolytopia.Common/resources/tribes.json b/OpenPolytopia.Common/resources/tribes.json index 4abdcb64..351d9fe2 100644 --- a/OpenPolytopia.Common/resources/tribes.json +++ b/OpenPolytopia.Common/resources/tribes.json @@ -1,8 +1,7 @@ { "tribes": [ { - "type": "imperius", - "starting_stars": 7, + "tribe_type": "imperius", "tribe": { "starting_tech": { "branch": "organization", @@ -19,7 +18,8 @@ "forest_rate": 1.0, "mountain_rate": 1.0, "water_rate": 1.0 - } + }, + "starting_stars": 7 } } ] diff --git a/OpenPolytopia/src/TerrainGenerationNode.cs b/OpenPolytopia/src/TerrainGenerationNode.cs new file mode 100644 index 00000000..e5d96cc0 --- /dev/null +++ b/OpenPolytopia/src/TerrainGenerationNode.cs @@ -0,0 +1,180 @@ +namespace OpenPolytopia; + +using System; +using System.Text.Json; +using System.Text.Unicode; +using System.Threading.Tasks; +using Common; +using Godot; + +/// +/// Node wrapper around so a map can be generated +/// directly from a scene. +/// +/// Add it to a scene, configure the generation parameters from the inspector and either let it +/// generate the map when it's ready (see ) or call +/// yourself; the generated map is available through +/// and once is emitted +/// +[GlobalClass] +public partial class TerrainGenerationNode : Node { + private static readonly JsonSerializerOptions _jsonOptions = new() { + Encoder = System.Text.Encodings.Web.JavaScriptEncoder.Create(UnicodeRanges.All), + TypeInfoResolver = TribeGenerationContext.Default, + PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower + }; + + /// + /// Emitted when the map has been generated + /// + [Signal] + public delegate void MapGeneratedEventHandler(); + + /// + /// Width of the squared grid to generate + /// + [Export(PropertyHint.Range, "4,64,1")] + public int GridSize { get; set; } = 16; + + /// + /// Random seed used by the generation; 0 means a random seed + /// + [Export] + public int Seed { get; set; } + + /// + /// Fraction of the map converted to land before the smoothing passes + /// + [Export(PropertyHint.Range, "0,1,0.01")] + public float InitialLand { get; set; } = 0.5f; + + /// + /// Number of cellular-automata smoothing passes applied to the initial land + /// + [Export(PropertyHint.Range, "0,8,1")] + public int Smoothing { get; set; } = 3; + + /// + /// Land/water balance of the smoothing passes; lower values produce more compact + /// continents while higher values produce rougher coasts + /// + [Export(PropertyHint.Range, "0,8,1")] + public int Relief { get; set; } = 4; + + /// + /// Base probability of a land tile converting to water, multiplied by the tribe's + /// + /// + [Export(PropertyHint.Range, "0,1,0.01")] + public float WaterRate { get; set; } = 0.05f; + + /// + /// Tribes of the players as values; the player at index + /// i gets id i + 1 + /// + [Export] + public int[] PlayerTribes { get; set; } = [(int)TribeType.Imperius, (int)TribeType.Bardur]; + + /// + /// Whether to generate the map as soon as the node is ready + /// + [Export] + public bool GenerateOnReady { get; set; } = true; + + /// + /// The generated grid; null until the first generation + /// + public Grid? Grid { get; private set; } + + /// + /// The city manager holding the generated cities and villages; null until the first generation + /// + public CityManager? CityManager { get; private set; } + + /// + /// The players of the generated map; null until the first generation + /// + public Player[]? Players { get; private set; } + + /// + /// The tribe manager used by the generation. + /// + /// If no tribe is registered when the generation starts, the tribes from the embedded + /// tribes.json are registered automatically; set your own populated manager to + /// override this behavior + /// + public TribeManager TribeManager { get; set; } = new(); + + public override void _Ready() { + if (GenerateOnReady) { + _ = GenerateAndReportAsync(); + } + } + + /// + /// Generates a new map with the current parameters. + /// + /// A fresh and are created on every call, so it + /// can be called again to regenerate the map; emits when done + /// + /// + /// if is empty or has more than 15 players + /// + /// + /// + /// var node = GetNode<TerrainGenerationNode>("TerrainGenerationNode"); + /// await node.GenerateMapAsync(); + /// var grid = node.Grid!; + /// + /// + public async Task GenerateMapAsync() { + // Tile.Owner is 4 bits, so there can't be more than 15 players + if (PlayerTribes.Length is 0 or > 15) { + throw new InvalidOperationException( + $"invalid number of players: {PlayerTribes.Length}; must be between 1 and 15"); + } + + if (TribeManager.Tribes.Count == 0) { + RegisterEmbeddedTribes(); + } + + var players = new Player[PlayerTribes.Length]; + for (var i = 0; i < PlayerTribes.Length; i++) { + players[i] = new Player((TribeType)PlayerTribes[i], i + 1); + } + + Players = players; + Grid = new Grid((uint)Math.Max(GridSize, 1)); + CityManager = new CityManager(Grid); + + var generation = new TerrainGeneration(Grid, CityManager, TribeManager, players, + Seed == 0 ? null : Seed) { + InitialLand = InitialLand, Smoothing = Smoothing, Relief = Relief, WaterRate = WaterRate + }; + await generation.GenerateMapAsync(); + + EmitSignal(SignalName.MapGenerated); + } + + /// + /// Registers the tribes from the embedded tribes.json + /// + private void RegisterEmbeddedTribes() { + var tribes = JsonSerializer.Deserialize(EmbeddedResources.TribesData, _jsonOptions); + if (tribes == null) { + GD.PushWarning("no tribes data found; terrain generation will use the base rates"); + return; + } + + TribeManager.RegisterTribes(tribes); + } + + private async Task GenerateAndReportAsync() { + try { + await GenerateMapAsync(); + } + catch (Exception e) { + GD.PushError($"terrain generation failed: {e}"); + } + } +} diff --git a/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs new file mode 100644 index 00000000..59edbdc5 --- /dev/null +++ b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs @@ -0,0 +1,66 @@ +namespace OpenPolytopia; + +using System.Threading.Tasks; +using Chickensoft.GoDotTest; +using Common; +using Godot; +using Shouldly; + +public class TerrainGenerationNodeTest(Node testScene) : TestClass(testScene) { + [Test] + public async Task TestGenerateMap() { + var node = new TerrainGenerationNode { + GenerateOnReady = false, + GridSize = 16, + Seed = 42, + PlayerTribes = [(int)TribeType.Imperius, (int)TribeType.Bardur] + }; + + var emitted = false; + node.MapGenerated += () => emitted = true; + + await node.GenerateMapAsync(); + + emitted.ShouldBeTrue(); + node.Grid.ShouldNotBeNull(); + node.CityManager.ShouldNotBeNull(); + node.Players.ShouldNotBeNull(); + node.Players.Length.ShouldBe(2); + + // the tribes from the embedded tribes.json are registered automatically + node.TribeManager[TribeType.Imperius].ShouldNotBeNull(); + + var capitals = 0; + foreach (var index in node.CityManager.Cities) { + if (node.Grid[index].GetCustomData().Capital) { + capitals++; + } + } + + capitals.ShouldBe(2); + node.Free(); + } + + [Test] + public async Task TestRegenerate() { + var node = new TerrainGenerationNode { + GenerateOnReady = false, GridSize = 16, Seed = 42, PlayerTribes = [(int)TribeType.Imperius] + }; + + await node.GenerateMapAsync(); + var firstGrid = node.Grid; + + // every generation creates a fresh grid + await node.GenerateMapAsync(); + node.Grid.ShouldNotBeSameAs(firstGrid); + node.Free(); + } + + [Test] + public async Task TestInvalidPlayers() { + var node = new TerrainGenerationNode { GenerateOnReady = false, PlayerTribes = [] }; + + await Should.ThrowAsync(node.GenerateMapAsync); + node.Free(); + } +} From 217bac35e150c706e2516e8c90af47f7529dea91 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 06:51:41 +0000 Subject: [PATCH 06/10] style: align doc comments with the repo style Move detailed explanations from multi-paragraph summaries into remarks blocks and drop trailing periods from summaries, matching the style of the existing doc comments Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/TerrainGeneration.cs | 67 +++++++++++++--------- OpenPolytopia/src/TerrainGenerationNode.cs | 53 +++++++++++------ 2 files changed, 75 insertions(+), 45 deletions(-) diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index 3db6d329..bc63e0d5 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -3,8 +3,9 @@ namespace OpenPolytopia.Common; using System.Runtime.CompilerServices; /// -/// Terrain generation system. -/// +/// Terrain generation system +/// +/// /// Implements the Polytopia map generation pipeline: /// /// Land generation: random land seeding followed by cellular-automata smoothing passes @@ -16,7 +17,7 @@ namespace OpenPolytopia.Common; /// with reduced rates on the outer ring /// Ruins: tiles / 40 ancient ruins, at most a third of them on water /// -/// +/// /// the grid to use /// the city manager to use /// the tribe manager to use @@ -82,11 +83,12 @@ public class TerrainGeneration( public int Smoothing { get; init; } = 3; /// - /// Land/water balance of the smoothing passes. - /// + /// Land/water balance of the smoothing passes + /// + /// /// A cell stays land when at most Relief of the 9 cells around it are water, /// so lower values produce more compact continents while higher values produce rougher coasts - /// + /// public int Relief { get; init; } = 4; /// @@ -114,12 +116,13 @@ public async Task GenerateMapAsync() { } /// - /// Generates the land/ocean layout. - /// + /// Generates the land/ocean layout + /// + /// /// Starts from a full ocean map, converts random tiles to land until /// is reached, then applies cellular-automata passes where a tile stays /// land when at most of the 9 cells around it are water - /// + /// private async Task GenerateLandAsync() { var size = (int)grid.Size; var cells = size * size; @@ -175,12 +178,13 @@ private async Task GenerateLandAsync() { } /// - /// Places a capital for every player. - /// + /// Places a capital for every player + /// + /// /// Capitals only spawn on land at least tiles away from the /// map border; every capital is placed on the tile maximizing the minimum distance to the /// already placed capitals so players don't start too close to one another - /// + /// private async Task GenerateInitialCitiesAsync() { foreach (var player in players) { // yield to the task executor @@ -240,12 +244,13 @@ private async Task GenerateInitialCitiesAsync() { } /// - /// Collects the tiles where a capital can spawn. - /// + /// Collects the tiles where a capital can spawn + /// + /// /// Starts with free land at least tiles away from the map /// border and progressively relaxes the requirements so capitals can always be placed, even /// on tiny or very watery maps - /// + /// /// the indexes of the valid spawn tiles private List CollectCapitalCandidates() { var cells = grid.Size * grid.Size; @@ -278,12 +283,15 @@ private List CollectCapitalCandidates() { } /// - /// Assigns biomes and terrain types. - /// + /// Assigns biomes and terrain types + /// + /// /// Every tile gets the biome of the nearest capital; every land tile then rolls water, /// mountain and forest in this order using the biome tribe's , - /// defaulting to field. Finally, ocean tiles adjacent to land become shallow water - /// + /// defaulting to field. + ///
+ /// Finally, ocean tiles adjacent to land become shallow water + /// private async Task GenerateTerrainAsync() { var size = (int)grid.Size; var cells = (uint)(size * size); @@ -346,11 +354,12 @@ private async Task GenerateTerrainAsync() { } /// - /// Places the villages. - /// + /// Places the villages + /// + /// /// Villages spawn on free field/forest tiles at least 1 tile away from the map border and /// 2 tiles away from any other city or village, until no free tile remains - /// + /// private async Task GenerateCitiesAsync() { var size = (int)grid.Size; var cells = (uint)(size * size); @@ -392,12 +401,13 @@ private async Task GenerateCitiesAsync() { } /// - /// Spawns the resources. - /// + /// Spawns the resources + /// + /// /// Resources only spawn within 2 tiles of a city or village, at full rate inside city /// territory and at a third of the rate on the outer ring, using the biome tribe's /// as multiplier - /// + /// private async Task GenerateResourcesAsync() { var cells = grid.Size * grid.Size; @@ -463,11 +473,12 @@ private async Task GenerateResourcesAsync() { } /// - /// Places the ancient ruins. - /// + /// Places the ancient ruins + /// + /// /// Spawns tiles / 40 ruins away from cities and villages, never adjacent to another /// ruin and with at most a third of them on water - /// + /// private async Task GenerateRuinsAsync() { // yield to the task executor await Task.Yield(); diff --git a/OpenPolytopia/src/TerrainGenerationNode.cs b/OpenPolytopia/src/TerrainGenerationNode.cs index e5d96cc0..1889e1b7 100644 --- a/OpenPolytopia/src/TerrainGenerationNode.cs +++ b/OpenPolytopia/src/TerrainGenerationNode.cs @@ -9,13 +9,14 @@ namespace OpenPolytopia; /// /// Node wrapper around so a map can be generated -/// directly from a scene. -/// +/// directly from a scene +/// +/// /// Add it to a scene, configure the generation parameters from the inspector and either let it /// generate the map when it's ready (see ) or call /// yourself; the generated map is available through /// and once is emitted -/// +/// [GlobalClass] public partial class TerrainGenerationNode : Node { private static readonly JsonSerializerOptions _jsonOptions = new() { @@ -37,8 +38,11 @@ public partial class TerrainGenerationNode : Node { public int GridSize { get; set; } = 16; /// - /// Random seed used by the generation; 0 means a random seed + /// Random seed used by the generation /// + /// + /// If 0, a random seed is used + /// [Export] public int Seed { get; set; } @@ -55,9 +59,11 @@ public partial class TerrainGenerationNode : Node { public int Smoothing { get; set; } = 3; /// - /// Land/water balance of the smoothing passes; lower values produce more compact - /// continents while higher values produce rougher coasts + /// Land/water balance of the smoothing passes /// + /// + /// Lower values produce more compact continents while higher values produce rougher coasts + /// [Export(PropertyHint.Range, "0,8,1")] public int Relief { get; set; } = 4; @@ -69,9 +75,11 @@ public partial class TerrainGenerationNode : Node { public float WaterRate { get; set; } = 0.05f; /// - /// Tribes of the players as values; the player at index - /// i gets id i + 1 + /// Tribes of the players as values /// + /// + /// The player at index i gets id i + 1 + /// [Export] public int[] PlayerTribes { get; set; } = [(int)TribeType.Imperius, (int)TribeType.Bardur]; @@ -82,27 +90,37 @@ public partial class TerrainGenerationNode : Node { public bool GenerateOnReady { get; set; } = true; /// - /// The generated grid; null until the first generation + /// The generated grid /// + /// + /// Null until the first generation + /// public Grid? Grid { get; private set; } /// - /// The city manager holding the generated cities and villages; null until the first generation + /// The city manager holding the generated cities and villages /// + /// + /// Null until the first generation + /// public CityManager? CityManager { get; private set; } /// - /// The players of the generated map; null until the first generation + /// The players of the generated map /// + /// + /// Null until the first generation + /// public Player[]? Players { get; private set; } /// - /// The tribe manager used by the generation. - /// + /// The tribe manager used by the generation + /// + /// /// If no tribe is registered when the generation starts, the tribes from the embedded /// tribes.json are registered automatically; set your own populated manager to /// override this behavior - /// + /// public TribeManager TribeManager { get; set; } = new(); public override void _Ready() { @@ -112,11 +130,12 @@ public override void _Ready() { } /// - /// Generates a new map with the current parameters. - /// + /// Generates a new map with the current parameters + /// + /// /// A fresh and are created on every call, so it /// can be called again to regenerate the map; emits when done - /// + /// /// /// if is empty or has more than 15 players /// From e66dc1f0fa7bad7fec203597fd869f61eb7951a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 07:40:58 +0000 Subject: [PATCH 07/10] fix: address terrain generation review findings - validate the player count in TerrainGeneration itself so every consumer gets it, not just the Godot node - validate PlayerTribes values in the node so an out-of-range tribe can't corrupt the biome bits - mask the data in SetBits so overflows can't bleed into neighboring bit fields - never steal already claimed tiles when a capital claims its starting territory and make the ocean fallback reject claimed tiles too, throwing instead of looping forever when no tile is left - guard GenerateMapAsync against racing the GenerateOnReady run - yield once per placed village instead of once per candidate - revert the wrong ruin bit change in the Grid docs - add the missing .uid files for the new scripts Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/Extensions.cs | 6 +-- OpenPolytopia.Common/Grid.cs | 2 +- OpenPolytopia.Common/TerrainGeneration.cs | 41 +++++++++++++++---- OpenPolytopia/src/TerrainGenerationNode.cs | 33 ++++++++++++++- .../src/TerrainGenerationNode.cs.uid | 1 + .../test/src/TerrainGenerationNodeTest.cs | 9 ++++ .../test/src/TerrainGenerationNodeTest.cs.uid | 1 + .../test/src/TerrainGenerationTest.cs | 8 ++++ .../test/src/TerrainGenerationTest.cs.uid | 1 + 9 files changed, 88 insertions(+), 14 deletions(-) create mode 100644 OpenPolytopia/src/TerrainGenerationNode.cs.uid create mode 100644 OpenPolytopia/test/src/TerrainGenerationNodeTest.cs.uid create mode 100644 OpenPolytopia/test/src/TerrainGenerationTest.cs.uid diff --git a/OpenPolytopia.Common/Extensions.cs b/OpenPolytopia.Common/Extensions.cs index 0ad8fff5..4a9c7d05 100644 --- a/OpenPolytopia.Common/Extensions.cs +++ b/OpenPolytopia.Common/Extensions.cs @@ -44,7 +44,7 @@ public static class ULongExtensions { /// the position where to set bits starting from the right [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void SetBits(this ref ulong value, ulong data, ulong bits, int position) => - value = value.ClearBits(bits, position) | (data << position); + value = value.ClearBits(bits, position) | ((data & bits) << position); /// /// Get the bits @@ -77,7 +77,7 @@ public static class UIntExtensions { /// the position where to set bits starting from the right [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void SetBits(this ref uint value, uint data, uint bits, int position) => - value = value.ClearBits(bits, position) | (data << position); + value = value.ClearBits(bits, position) | ((data & bits) << position); /// /// Get the bits @@ -110,7 +110,7 @@ public static class IntExtensions { /// the position where to set bits starting from the right [MethodImpl(MethodImplOptions.AggressiveInlining)] public static void SetBits(this ref int value, int data, int bits, int position) => - value = value.ClearBits(bits, position) | (data << position); + value = value.ClearBits(bits, position) | ((data & bits) << position); /// /// Get the bits diff --git a/OpenPolytopia.Common/Grid.cs b/OpenPolytopia.Common/Grid.cs index b34ca5fb..df40e5ef 100644 --- a/OpenPolytopia.Common/Grid.cs +++ b/OpenPolytopia.Common/Grid.cs @@ -228,7 +228,7 @@ public static Type GetModifier(TileKind kind) => /// Using 0 as the left-most bit and 63 as the right-most bit /// /// 0 -> 1: has road; 0 doesn't have any road; (bridge if on water) - /// 1 -> 2: has ancient ruin; 0 doesn't have any ruin + /// 1 -> 1: has ancient ruin; 0 doesn't have any ruin /// [2, 4] -> /// [5, 6] -> Tile modifier /// [7, 9] -> Tile buildings diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index bc63e0d5..88b9ed88 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -100,6 +100,7 @@ public class TerrainGeneration( /// /// Generates the map ready to use in-game /// + /// if there are no players or more than 15 players /// /// /// var terrainGeneration = new TerrainGeneration(grid, cityManager, tribeManager, players); @@ -107,6 +108,12 @@ public class TerrainGeneration( /// /// public async Task GenerateMapAsync() { + // Tile.Owner is 4 bits, so there can't be more than 15 players + if (players.Length is 0 or > 15) { + throw new InvalidOperationException( + $"invalid number of players: {players.Length}; must be between 1 and 15"); + } + await GenerateLandAsync(); await GenerateInitialCitiesAsync(); await GenerateTerrainAsync(); @@ -233,8 +240,12 @@ private async Task GenerateInitialCitiesAsync() { city.Level = 1; }); - // claim the starting 3x3 territory + // claim the starting 3x3 territory; never steal tiles already claimed by another capital ForEachInRadius(index, 1, neighbor => grid.ModifyTile(neighbor, (ref Tile tile) => { + if (tile.Owner != 0 || tile.Kind == TileKind.Village) { + return; + } + tile.Owner = player.Id; tile.City = (int)cityId; })); @@ -271,12 +282,26 @@ private List CollectCapitalCandidates() { } } - // last resort: raise a random tile from the ocean - var index = (uint)_rng.Next(0, (int)cells); - while (grid[index].Kind == TileKind.Village) { - index = (uint)_rng.Next(0, (int)cells); + // last resort: raise a random unclaimed tile from the ocean, preferring free zones + var free = new List(); + var unclaimed = new List(); + for (var i = 0u; i < cells; i++) { + if (grid[i].Kind == TileKind.Village || grid[i].Owner != 0) { + continue; + } + + unclaimed.Add(i); + if (_zoneMap[i] == ZONE_FREE) { + free.Add(i); + } + } + + var pool = free.Count > 0 ? free : unclaimed; + if (pool.Count == 0) { + throw new InvalidOperationException("not enough tiles to place all the capitals"); } + var index = pool[_rng.Next(pool.Count)]; grid.ModifyTile(index, (ref Tile tile) => tile.Kind = TileKind.Field); candidates.Add(index); return candidates; @@ -375,9 +400,6 @@ private async Task GenerateCitiesAsync() { } while (candidates.Count > 0 && _citiesCount < MAX_CITIES) { - // yield to the task executor - await Task.Yield(); - var position = _rng.Next(candidates.Count); var index = candidates[position]; candidates.RemoveAt(position); @@ -387,6 +409,9 @@ private async Task GenerateCitiesAsync() { continue; } + // yield to the task executor once per placed village + await Task.Yield(); + // register the village; the default CityData already means "village" (no owner, level 0) cityManager.RegisterCity(index); _citiesCount++; diff --git a/OpenPolytopia/src/TerrainGenerationNode.cs b/OpenPolytopia/src/TerrainGenerationNode.cs index 1889e1b7..bb27b2bc 100644 --- a/OpenPolytopia/src/TerrainGenerationNode.cs +++ b/OpenPolytopia/src/TerrainGenerationNode.cs @@ -113,6 +113,9 @@ public partial class TerrainGenerationNode : Node { /// public Player[]? Players { get; private set; } + // currently running generation, so concurrent calls can't race each other + private Task? _generation; + /// /// The tribe manager used by the generation /// @@ -134,10 +137,13 @@ public override void _Ready() { /// /// /// A fresh and are created on every call, so it - /// can be called again to regenerate the map; emits when done + /// can be called again to regenerate the map; emits when done. + ///
+ /// If a generation is already running (for example the one started by + /// ), this waits for it instead of starting another one /// /// - /// if is empty or has more than 15 players + /// if is empty, has more than 15 players or contains an invalid tribe /// /// /// @@ -147,12 +153,35 @@ public override void _Ready() { /// /// public async Task GenerateMapAsync() { + // if a generation is already running, wait for it instead of racing it + if (_generation != null) { + await _generation; + return; + } + + _generation = GenerateInternalAsync(); + try { + await _generation; + } + finally { + _generation = null; + } + } + + private async Task GenerateInternalAsync() { // Tile.Owner is 4 bits, so there can't be more than 15 players if (PlayerTribes.Length is 0 or > 15) { throw new InvalidOperationException( $"invalid number of players: {PlayerTribes.Length}; must be between 1 and 15"); } + // an out-of-range tribe would corrupt the biome bits of every tile + foreach (var tribe in PlayerTribes) { + if (!Enum.IsDefined((TribeType)tribe)) { + throw new InvalidOperationException($"invalid tribe: {tribe}"); + } + } + if (TribeManager.Tribes.Count == 0) { RegisterEmbeddedTribes(); } diff --git a/OpenPolytopia/src/TerrainGenerationNode.cs.uid b/OpenPolytopia/src/TerrainGenerationNode.cs.uid new file mode 100644 index 00000000..012a9bfa --- /dev/null +++ b/OpenPolytopia/src/TerrainGenerationNode.cs.uid @@ -0,0 +1 @@ +uid://xqi84zuz87ju diff --git a/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs index 59edbdc5..a7a1b4f2 100644 --- a/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs +++ b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs @@ -63,4 +63,13 @@ public async Task TestInvalidPlayers() { await Should.ThrowAsync(node.GenerateMapAsync); node.Free(); } + + [Test] + public async Task TestInvalidTribe() { + // an out-of-range tribe would overflow the 5-bit biome field + var node = new TerrainGenerationNode { GenerateOnReady = false, PlayerTribes = [40] }; + + await Should.ThrowAsync(node.GenerateMapAsync); + node.Free(); + } } diff --git a/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs.uid b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs.uid new file mode 100644 index 00000000..448562d5 --- /dev/null +++ b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs.uid @@ -0,0 +1 @@ +uid://b434c82va4lfe diff --git a/OpenPolytopia/test/src/TerrainGenerationTest.cs b/OpenPolytopia/test/src/TerrainGenerationTest.cs index 710b8d0f..f4e2ae37 100644 --- a/OpenPolytopia/test/src/TerrainGenerationTest.cs +++ b/OpenPolytopia/test/src/TerrainGenerationTest.cs @@ -153,6 +153,14 @@ public async Task TestRuins() { ruins.ShouldBeLessThanOrEqualTo((int)(SIZE * SIZE / 40)); } + [Test] + public async Task TestInvalidPlayers() { + var grid = new Grid(SIZE); + var terrainGeneration = new TerrainGeneration(grid, new CityManager(grid), new TribeManager(), []); + + await Should.ThrowAsync(terrainGeneration.GenerateMapAsync); + } + [Test] public async Task TestDeterministicSeed() { var (first, _, _) = await GenerateMapAsync(); diff --git a/OpenPolytopia/test/src/TerrainGenerationTest.cs.uid b/OpenPolytopia/test/src/TerrainGenerationTest.cs.uid new file mode 100644 index 00000000..06733810 --- /dev/null +++ b/OpenPolytopia/test/src/TerrainGenerationTest.cs.uid @@ -0,0 +1 @@ +uid://bcebes3h2jpca From bbcb62517b70ebe5880f657fc6a32ed6d8975645 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 20:56:32 +0000 Subject: [PATCH 08/10] fix: address second round of terrain generation review findings - guard GenerateMapAsync against being called twice on the same instance, since the grid and city manager keep the generated data - validate player ids (1-15, unique) and tribes in TerrainGeneration, dropping the duplicated checks from the node so every consumer gets the same validation - never spawn a ruin on a tile that already has a resource - publish Grid/CityManager/Players from the node only after the generation completes, so consumers never see a partial map - simplify IndexToGridPosition: (index - x) / size is just index / size Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/Grid.cs | 4 +- OpenPolytopia.Common/TerrainGeneration.cs | 43 +++++++++++++++++-- OpenPolytopia/src/TerrainGenerationNode.cs | 25 ++++------- .../test/src/TerrainGenerationTest.cs | 14 ++++++ 4 files changed, 64 insertions(+), 22 deletions(-) diff --git a/OpenPolytopia.Common/Grid.cs b/OpenPolytopia.Common/Grid.cs index df40e5ef..dbc7f142 100644 --- a/OpenPolytopia.Common/Grid.cs +++ b/OpenPolytopia.Common/Grid.cs @@ -67,14 +67,14 @@ public Tile this[uint index] { [MethodImpl(MethodImplOptions.AggressiveInlining)] public Vector2I IndexToGridPosition(uint index) { var x = index % size; - var y = (index - x) / size; + var y = index / size; return new Vector2I((int)x, (int)y); } [MethodImpl(MethodImplOptions.AggressiveInlining)] public void IndexToGridPosition(uint index, out uint x, out uint y) { x = index % size; - y = (index - x) / size; + y = index / size; } /// diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index 88b9ed88..0fd956d6 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -71,6 +71,7 @@ public class TerrainGeneration( private readonly List _capitals = new(players.Length); private int _citiesCount; + private bool _generated; /// /// Fraction of the map converted to land before the smoothing passes @@ -100,7 +101,15 @@ public class TerrainGeneration( /// /// Generates the map ready to use in-game /// - /// if there are no players or more than 15 players + /// + /// An instance can only generate one map because the grid and the city manager keep the + /// generated data; create a new to generate another map + /// + /// + /// if the map has already been generated or the players are invalid: there must be + /// between 1 and 15 players, every player id must be unique and between 1 and 15 and + /// every tribe must be a defined + /// /// /// /// var terrainGeneration = new TerrainGeneration(grid, cityManager, tribeManager, players); @@ -108,12 +117,40 @@ public class TerrainGeneration( /// /// public async Task GenerateMapAsync() { + // the grid and the city manager keep the generated data, so an instance is single-use + if (_generated) { + throw new InvalidOperationException( + "the map has already been generated; create a new TerrainGeneration to generate another map"); + } + // Tile.Owner is 4 bits, so there can't be more than 15 players if (players.Length is 0 or > 15) { throw new InvalidOperationException( $"invalid number of players: {players.Length}; must be between 1 and 15"); } + var seenIds = 0; + foreach (var player in players) { + // player ids must fit in the 4-bit Tile.Owner field, where 0 means no owner + if (player.Id is < 1 or > 15) { + throw new InvalidOperationException($"invalid player id: {player.Id}; must be between 1 and 15"); + } + + // a duplicate id would merge the territories of two players + if ((seenIds & (1 << player.Id)) != 0) { + throw new InvalidOperationException($"duplicate player id: {player.Id}"); + } + + seenIds |= 1 << player.Id; + + // an undefined tribe would silently fall back to the base rates + if (!Enum.IsDefined(player.Tribe)) { + throw new InvalidOperationException($"invalid tribe: {player.Tribe}"); + } + } + + _generated = true; + await GenerateLandAsync(); await GenerateInitialCitiesAsync(); await GenerateTerrainAsync(); @@ -520,8 +557,8 @@ private async Task GenerateRuinsAsync() { var index = (uint)_rng.Next(0, cells); var tile = grid[index]; - // keep ruins away from cities, villages and their territory - if (tile.Ruin || tile.Kind == TileKind.Village || _zoneMap[index] >= ZONE_TERRITORY) { + // keep ruins away from cities, villages, their territory and tiles with a resource + if (tile.Ruin || tile.Modifier != 0 || tile.Kind == TileKind.Village || _zoneMap[index] >= ZONE_TERRITORY) { continue; } diff --git a/OpenPolytopia/src/TerrainGenerationNode.cs b/OpenPolytopia/src/TerrainGenerationNode.cs index bb27b2bc..831d571c 100644 --- a/OpenPolytopia/src/TerrainGenerationNode.cs +++ b/OpenPolytopia/src/TerrainGenerationNode.cs @@ -169,38 +169,29 @@ public async Task GenerateMapAsync() { } private async Task GenerateInternalAsync() { - // Tile.Owner is 4 bits, so there can't be more than 15 players - if (PlayerTribes.Length is 0 or > 15) { - throw new InvalidOperationException( - $"invalid number of players: {PlayerTribes.Length}; must be between 1 and 15"); - } - - // an out-of-range tribe would corrupt the biome bits of every tile - foreach (var tribe in PlayerTribes) { - if (!Enum.IsDefined((TribeType)tribe)) { - throw new InvalidOperationException($"invalid tribe: {tribe}"); - } - } - if (TribeManager.Tribes.Count == 0) { RegisterEmbeddedTribes(); } + // the players are validated by TerrainGeneration.GenerateMapAsync var players = new Player[PlayerTribes.Length]; for (var i = 0; i < PlayerTribes.Length; i++) { players[i] = new Player((TribeType)PlayerTribes[i], i + 1); } - Players = players; - Grid = new Grid((uint)Math.Max(GridSize, 1)); - CityManager = new CityManager(Grid); + var grid = new Grid((uint)Math.Max(GridSize, 1)); + var cityManager = new CityManager(grid); - var generation = new TerrainGeneration(Grid, CityManager, TribeManager, players, + var generation = new TerrainGeneration(grid, cityManager, TribeManager, players, Seed == 0 ? null : Seed) { InitialLand = InitialLand, Smoothing = Smoothing, Relief = Relief, WaterRate = WaterRate }; await generation.GenerateMapAsync(); + // publish the new map only when it's fully generated, so consumers never see a partial grid + Players = players; + Grid = grid; + CityManager = cityManager; EmitSignal(SignalName.MapGenerated); } diff --git a/OpenPolytopia/test/src/TerrainGenerationTest.cs b/OpenPolytopia/test/src/TerrainGenerationTest.cs index f4e2ae37..028d4510 100644 --- a/OpenPolytopia/test/src/TerrainGenerationTest.cs +++ b/OpenPolytopia/test/src/TerrainGenerationTest.cs @@ -145,6 +145,9 @@ public async Task TestRuins() { for (var i = 0u; i < SIZE * SIZE; i++) { if (grid[i].Ruin) { ruins++; + + // ruins never spawn on a tile with a resource + grid[i].Modifier.ShouldBe(0); } } @@ -161,6 +164,17 @@ public async Task TestInvalidPlayers() { await Should.ThrowAsync(terrainGeneration.GenerateMapAsync); } + [Test] + public async Task TestSingleUse() { + var grid = new Grid(SIZE); + var players = new[] { new Player(TribeType.Imperius, 1) }; + var terrainGeneration = new TerrainGeneration(grid, new CityManager(grid), new TribeManager(), players); + await terrainGeneration.GenerateMapAsync(); + + // an instance can only generate one map + await Should.ThrowAsync(terrainGeneration.GenerateMapAsync); + } + [Test] public async Task TestDeterministicSeed() { var (first, _, _) = await GenerateMapAsync(); From 65f131935fe12c4614b4cda85a21d4e72a0a2c3f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 05:39:16 +0000 Subject: [PATCH 09/10] fix: address third round of terrain generation review findings - allow villages on border expansion tiles so the spacing matches the documented 2-tile minimum distance - expose CityManager.Cities as IReadOnlyList so the internal list can't be modified from outside - hoist the json serialization options into EmbeddedResources with LoadTribes/LoadTroops helpers, removing the three duplicated copies - replace the capturing lambdas in the generation loops with the grid indexer to avoid per-tile closure allocations Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/City.cs | 9 ++++++- OpenPolytopia.Common/EmbeddedResources.cs | 24 +++++++++++++++++++ OpenPolytopia.Common/TerrainGeneration.cs | 26 ++++++++++++++------- OpenPolytopia/src/TerrainGenerationNode.cs | 10 +------- OpenPolytopia/test/src/TroopManagerTest.cs | 11 +-------- OpenPolytopia/test/src/TroopMovementTest.cs | 11 +-------- 6 files changed, 53 insertions(+), 38 deletions(-) diff --git a/OpenPolytopia.Common/City.cs b/OpenPolytopia.Common/City.cs index 2a31e10e..a66f21bb 100644 --- a/OpenPolytopia.Common/City.cs +++ b/OpenPolytopia.Common/City.cs @@ -11,7 +11,14 @@ namespace OpenPolytopia.Common; public class CityManager(Grid grid) { private readonly List _cities = []; - public IEnumerable Cities => _cities; + /// + /// Grid indexes of the registered cities + /// + /// + /// The city with id i is at position i - 1; don't modify this list because the + /// ids are positions in it, use to add a city + /// + public IReadOnlyList Cities => _cities; /// /// Access to the diff --git a/OpenPolytopia.Common/EmbeddedResources.cs b/OpenPolytopia.Common/EmbeddedResources.cs index cacc2353..afc606b6 100644 --- a/OpenPolytopia.Common/EmbeddedResources.cs +++ b/OpenPolytopia.Common/EmbeddedResources.cs @@ -1,8 +1,18 @@ namespace OpenPolytopia.Common; using System.Reflection; +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; +using System.Text.Unicode; public static class EmbeddedResources { + // serialization conventions shared by all the json resources + private static readonly JsonSerializerOptions _jsonOptions = new() { + Encoder = System.Text.Encodings.Web.JavaScriptEncoder.Create(UnicodeRanges.All), + TypeInfoResolver = JsonTypeInfoResolver.Combine(TribeGenerationContext.Default, TroopGenerationContext.Default), + PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower + }; + /// /// Get the troops data from the json file /// @@ -13,6 +23,20 @@ public static class EmbeddedResources { /// public static string TribesData => GetResource("OpenPolytopia.Common.resources.tribes.json"); + /// + /// Deserializes the troops from the embedded json file + /// + /// the troops data, or null if the json is empty + public static TroopsSerializedData? LoadTroops() => + JsonSerializer.Deserialize(TroopsData, _jsonOptions); + + /// + /// Deserializes the tribes from the embedded json file + /// + /// the tribes data, or null if the json is empty + public static TribesSerializedData? LoadTribes() => + JsonSerializer.Deserialize(TribesData, _jsonOptions); + /// /// Returns the content of an embedded resource /// diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index 0fd956d6..bc924f2c 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -214,8 +214,11 @@ private async Task GenerateLandAsync() { } } - var kind = waterCount <= relief ? TileKind.Field : TileKind.Ocean; - grid.ModifyTile(grid.GridPositionToIndex(x, y), (ref Tile tile) => tile.Kind = kind); + // the indexer avoids allocating a closure capturing the computed kind for every tile + var index = grid.GridPositionToIndex(x, y); + var tile = grid[index]; + tile.Kind = waterCount <= relief ? TileKind.Field : TileKind.Ocean; + grid[index] = tile; } } } @@ -364,8 +367,11 @@ private async Task GenerateTerrainAsync() { await Task.Yield(); } + // the indexer avoids allocating a closure capturing the biome for every tile var biome = NearestCapitalTribe(i); - grid.ModifyTile(i, (ref Tile tile) => tile.Biome = biome); + var biomeTile = grid[i]; + biomeTile.Biome = biome; + grid[i] = biomeTile; if (grid[i].Kind != TileKind.Field) { continue; @@ -409,7 +415,10 @@ private async Task GenerateTerrainAsync() { (y > 0 && IsLand(snapshot[((y - 1) * size) + x])) || (y < size - 1 && IsLand(snapshot[((y + 1) * size) + x])); if (nearLand) { - grid.ModifyTile(grid.GridPositionToIndex(x, y), (ref Tile tile) => tile.Kind = TileKind.Water); + var index = grid.GridPositionToIndex(x, y); + var tile = grid[index]; + tile.Kind = TileKind.Water; + grid[index] = tile; } } } @@ -426,11 +435,12 @@ private async Task GenerateCitiesAsync() { var size = (int)grid.Size; var cells = (uint)(size * size); - // collect all the tiles where a village can spawn + // collect all the tiles where a village can spawn; border expansion tiles are allowed + // so villages can be exactly 2 tiles away from another city var candidates = new List(); for (var i = 0u; i < cells; i++) { grid.IndexToGridPosition(i, out var x, out var y); - if (_zoneMap[i] == ZONE_FREE && grid[i].Kind is TileKind.Field or TileKind.Forest && + if (_zoneMap[i] <= ZONE_BORDER && grid[i].Kind is TileKind.Field or TileKind.Forest && x > 0 && y > 0 && x < size - 1 && y < size - 1) { candidates.Add(i); } @@ -441,8 +451,8 @@ private async Task GenerateCitiesAsync() { var index = candidates[position]; candidates.RemoveAt(position); - // a previously placed village may have claimed this tile - if (_zoneMap[index] != ZONE_FREE) { + // a previously placed village may have claimed this tile as its territory + if (_zoneMap[index] > ZONE_BORDER) { continue; } diff --git a/OpenPolytopia/src/TerrainGenerationNode.cs b/OpenPolytopia/src/TerrainGenerationNode.cs index 831d571c..8e16a6d1 100644 --- a/OpenPolytopia/src/TerrainGenerationNode.cs +++ b/OpenPolytopia/src/TerrainGenerationNode.cs @@ -1,8 +1,6 @@ namespace OpenPolytopia; using System; -using System.Text.Json; -using System.Text.Unicode; using System.Threading.Tasks; using Common; using Godot; @@ -19,12 +17,6 @@ namespace OpenPolytopia; /// [GlobalClass] public partial class TerrainGenerationNode : Node { - private static readonly JsonSerializerOptions _jsonOptions = new() { - Encoder = System.Text.Encodings.Web.JavaScriptEncoder.Create(UnicodeRanges.All), - TypeInfoResolver = TribeGenerationContext.Default, - PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower - }; - /// /// Emitted when the map has been generated /// @@ -199,7 +191,7 @@ private async Task GenerateInternalAsync() { /// Registers the tribes from the embedded tribes.json /// private void RegisterEmbeddedTribes() { - var tribes = JsonSerializer.Deserialize(EmbeddedResources.TribesData, _jsonOptions); + var tribes = EmbeddedResources.LoadTribes(); if (tribes == null) { GD.PushWarning("no tribes data found; terrain generation will use the base rates"); return; diff --git a/OpenPolytopia/test/src/TroopManagerTest.cs b/OpenPolytopia/test/src/TroopManagerTest.cs index bd483f91..05c5e0c8 100644 --- a/OpenPolytopia/test/src/TroopManagerTest.cs +++ b/OpenPolytopia/test/src/TroopManagerTest.cs @@ -1,26 +1,17 @@ namespace OpenPolytopia.test.src; -using System.Text.Json; -using System.Text.Unicode; using Chickensoft.GoDotTest; using Godot; using Common; using Shouldly; public class TroopManagerTest(Node testScene) : TestClass(testScene) { - private static readonly JsonSerializerOptions _options = new() { - Encoder = System.Text.Encodings.Web.JavaScriptEncoder.Create(UnicodeRanges.All), - TypeInfoResolver = TroopGenerationContext.Default, - PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower - }; - private TroopManager _troopManager = null!; [Setup] public void Setup() { _troopManager = new TroopManager(10); - var content = EmbeddedResources.TroopsData; - var troops = JsonSerializer.Deserialize(content, _options); + var troops = EmbeddedResources.LoadTroops(); troops.ShouldNotBeNull(); _troopManager.RegisterTroops(troops); } diff --git a/OpenPolytopia/test/src/TroopMovementTest.cs b/OpenPolytopia/test/src/TroopMovementTest.cs index 345970ac..fcfdfc52 100644 --- a/OpenPolytopia/test/src/TroopMovementTest.cs +++ b/OpenPolytopia/test/src/TroopMovementTest.cs @@ -1,7 +1,5 @@ namespace OpenPolytopia; -using System.Text.Json; -using System.Text.Unicode; using System.Threading.Tasks; using Chickensoft.GoDotTest; using Godot; @@ -9,19 +7,12 @@ namespace OpenPolytopia; using Shouldly; public class TroopMovementTest(Node testScene) : TestClass(testScene) { - private static readonly JsonSerializerOptions _options = new() { - Encoder = System.Text.Encodings.Web.JavaScriptEncoder.Create(UnicodeRanges.All), - TypeInfoResolver = TroopGenerationContext.Default, - PropertyNamingPolicy = JsonNamingPolicy.SnakeCaseLower - }; - private TroopManager _troopManager = null!; [Setup] public void Setup() { _troopManager = new TroopManager(10); - var content = EmbeddedResources.TroopsData; - var troops = JsonSerializer.Deserialize(content, _options); + var troops = EmbeddedResources.LoadTroops(); troops.ShouldNotBeNull(); _troopManager.RegisterTroops(troops); } From 0c1ae562bffa58f9322f8f4d230347d96a2dc00e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 11:59:11 +0000 Subject: [PATCH 10/10] fix: address fourth round of terrain generation review findings - bound village placement to about one village every 9 eligible tiles instead of maximally packing the land, which also frees up land for ruins to reach the documented tiles / 40 count - queue a new generation after the running one in TerrainGenerationNode so an explicit call never returns a map built from stale parameters - fix the inverted Relief remarks and narrow its exported range to 3-6 so a degenerate all-ocean map can't be selected from the inspector - assert the ruin count lower bound and the water ruins cap in TestRuins and add a queued generation test Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01JpezEueE6SxbBAWziamygM --- OpenPolytopia.Common/TerrainGeneration.cs | 21 +++++++--- OpenPolytopia/src/TerrainGenerationNode.cs | 41 +++++++++++++------ .../test/src/TerrainGenerationNodeTest.cs | 19 +++++++++ .../test/src/TerrainGenerationTest.cs | 12 ++++-- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/OpenPolytopia.Common/TerrainGeneration.cs b/OpenPolytopia.Common/TerrainGeneration.cs index bc924f2c..a38f63cf 100644 --- a/OpenPolytopia.Common/TerrainGeneration.cs +++ b/OpenPolytopia.Common/TerrainGeneration.cs @@ -12,7 +12,8 @@ namespace OpenPolytopia.Common; /// Capital placement: capitals are placed on land maximizing the distance between each other /// Terrain assignment: every tile gets the biome of the nearest capital and rolls /// mountain/forest/water using the tribe terrain rates -/// Village placement: villages are placed on free land at least 2 tiles away from other villages +/// Village placement: about one village every 9 eligible tiles, at least 2 tiles away +/// from any other city or village /// Resource spawning: resources spawn only within 2 tiles of a city or village, /// with reduced rates on the outer ring /// Ruins: tiles / 40 ancient ruins, at most a third of them on water @@ -52,6 +53,9 @@ public class TerrainGeneration( // one ruin every RUINS_DIVISOR tiles, at most a third of them on water private const int RUINS_DIVISOR = 40; + // one village every VILLAGE_DIVISOR eligible tiles + private const int VILLAGE_DIVISOR = 9; + // Tile.City is 8 bits so there can't be more than 255 cities in a grid private const int MAX_CITIES = 255; @@ -87,8 +91,9 @@ public class TerrainGeneration( /// Land/water balance of the smoothing passes /// /// - /// A cell stays land when at most Relief of the 9 cells around it are water, - /// so lower values produce more compact continents while higher values produce rougher coasts + /// A cell stays land when at most Relief of the 9 cells around it are water, so lower + /// values erode the land into the ocean while higher values expand it; below 3 the smoothing + /// passes can erode the whole map into ocean /// public int Relief { get; init; } = 4; @@ -429,7 +434,8 @@ private async Task GenerateTerrainAsync() { /// /// /// Villages spawn on free field/forest tiles at least 1 tile away from the map border and - /// 2 tiles away from any other city or village, until no free tile remains + /// 2 tiles away from any other city or village; about one village every 9 eligible tiles + /// is placed so villages don't saturate the land /// private async Task GenerateCitiesAsync() { var size = (int)grid.Size; @@ -446,7 +452,11 @@ private async Task GenerateCitiesAsync() { } } - while (candidates.Count > 0 && _citiesCount < MAX_CITIES) { + // bound the number of villages, otherwise the random placement would maximally pack the + // land with a village every 2 tiles + var target = Math.Max(1, candidates.Count / VILLAGE_DIVISOR); + var placed = 0; + while (placed < target && candidates.Count > 0 && _citiesCount < MAX_CITIES) { var position = _rng.Next(candidates.Count); var index = candidates[position]; candidates.RemoveAt(position); @@ -469,6 +479,7 @@ private async Task GenerateCitiesAsync() { }); MarkCityZone(index); + placed++; } } diff --git a/OpenPolytopia/src/TerrainGenerationNode.cs b/OpenPolytopia/src/TerrainGenerationNode.cs index 8e16a6d1..8b629292 100644 --- a/OpenPolytopia/src/TerrainGenerationNode.cs +++ b/OpenPolytopia/src/TerrainGenerationNode.cs @@ -54,9 +54,10 @@ public partial class TerrainGenerationNode : Node { /// Land/water balance of the smoothing passes /// /// - /// Lower values produce more compact continents while higher values produce rougher coasts + /// Lower values erode the land into the ocean while higher values expand it; the range is + /// limited because values outside it erode or flood the whole map /// - [Export(PropertyHint.Range, "0,8,1")] + [Export(PropertyHint.Range, "3,6,1")] public int Relief { get; set; } = 4; /// @@ -105,7 +106,7 @@ public partial class TerrainGenerationNode : Node { /// public Player[]? Players { get; private set; } - // currently running generation, so concurrent calls can't race each other + // latest queued generation, so concurrent calls chain instead of racing each other private Task? _generation; /// @@ -132,7 +133,8 @@ public override void _Ready() { /// can be called again to regenerate the map; emits when done. ///
/// If a generation is already running (for example the one started by - /// ), this waits for it instead of starting another one + /// ), this waits for it to finish and then generates another + /// map, so the parameters set before this call are always used /// /// /// if is empty, has more than 15 players or contains an invalid tribe @@ -145,21 +147,34 @@ public override void _Ready() { /// /// public async Task GenerateMapAsync() { - // if a generation is already running, wait for it instead of racing it - if (_generation != null) { - await _generation; - return; - } - - _generation = GenerateInternalAsync(); + // queue this generation after the running one, so this call always generates a fresh map + // with the current parameters instead of returning a map built from stale ones + var current = GenerateAfterAsync(_generation); + _generation = current; try { - await _generation; + await current; } finally { - _generation = null; + // a queued generation may have replaced this one already + if (_generation == current) { + _generation = null; + } } } + private async Task GenerateAfterAsync(Task? previous) { + if (previous != null) { + try { + await previous; + } + catch (Exception) { + // the previous generation already reported its failure to its own caller + } + } + + await GenerateInternalAsync(); + } + private async Task GenerateInternalAsync() { if (TribeManager.Tribes.Count == 0) { RegisterEmbeddedTribes(); diff --git a/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs index a7a1b4f2..0664d431 100644 --- a/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs +++ b/OpenPolytopia/test/src/TerrainGenerationNodeTest.cs @@ -56,6 +56,25 @@ public async Task TestRegenerate() { node.Free(); } + [Test] + public async Task TestQueuedGeneration() { + var node = new TerrainGenerationNode { + GenerateOnReady = false, GridSize = 16, Seed = 42, PlayerTribes = [(int)TribeType.Imperius] + }; + + var generated = 0; + node.MapGenerated += () => generated++; + + // a call made while a generation is running must generate its own map + var first = node.GenerateMapAsync(); + var second = node.GenerateMapAsync(); + await first; + await second; + + generated.ShouldBe(2); + node.Free(); + } + [Test] public async Task TestInvalidPlayers() { var node = new TerrainGenerationNode { GenerateOnReady = false, PlayerTribes = [] }; diff --git a/OpenPolytopia/test/src/TerrainGenerationTest.cs b/OpenPolytopia/test/src/TerrainGenerationTest.cs index 028d4510..0344c1e4 100644 --- a/OpenPolytopia/test/src/TerrainGenerationTest.cs +++ b/OpenPolytopia/test/src/TerrainGenerationTest.cs @@ -142,18 +142,24 @@ public async Task TestRuins() { var (grid, _, _) = await GenerateMapAsync(); var ruins = 0; + var waterRuins = 0; for (var i = 0u; i < SIZE * SIZE; i++) { if (grid[i].Ruin) { ruins++; + if (grid[i].Kind is TileKind.Water or TileKind.Ocean) { + waterRuins++; + } // ruins never spawn on a tile with a resource grid[i].Modifier.ShouldBe(0); } } - // one ruin every 40 tiles - ruins.ShouldBeGreaterThan(0); - ruins.ShouldBeLessThanOrEqualTo((int)(SIZE * SIZE / 40)); + // one ruin every 40 tiles, at most a third of them on water; at least the land ruins + // must always be placed + var total = (int)(SIZE * SIZE / 40); + ruins.ShouldBeInRange(total - (total / 3), total); + waterRuins.ShouldBeLessThanOrEqualTo(total / 3); } [Test]