Skip to content

[godot4] [AI] Fix Issue 94 - Improve SettlerLocationAI #430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: godot4
Choose a base branch
from
Open

[godot4] [AI] Fix Issue 94 - Improve SettlerLocationAI #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b46e3f5
Adding BFC Calculations
Sep 22, 2023
d96d1de
Adding structure for tileimprovement score
Sep 22, 2023
b252df1
Fix Lake Logic and apply to production
benskywalker Sep 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion C7Engine/AI/CityProductionAI.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static IProducible GetNextItemToBeProduced(City city, IProducible lastPro


//Exclude naval units from land-only cities
if (unitPrototype.categories.Contains("Sea") && !city.location.NeighborsWater()) {
if (unitPrototype.categories.Contains("Sea") && !city.location.Coastal()) {
flatAdjustedScore = 0.0f;
}

Expand Down
8 changes: 4 additions & 4 deletions C7Engine/AI/StrategicAI/ExpansionPriority.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ public override void CalculateWeightAndMetadata(Player player) {
if (player.cities.Count < 2) {
this.calculatedWeight = 1000;
} else {
int score = UtilityCalculations.CalculateAvailableLandScore(player);
double score = UtilityCalculations.CalculateAvailableLandScore(player);
score = ApplyEarlyGameMultiplier(score);
score = ApplyNationTraitMultiplier(score, player);

this.calculatedWeight = score;
this.calculatedWeight = (float) score;
}
}

Expand Down Expand Up @@ -60,7 +60,7 @@ public override string ToString() {
return "ExpansionPriority";
}

private int ApplyEarlyGameMultiplier(int score)
private double ApplyEarlyGameMultiplier(double score)
{
//If it's early game, multiply this score.
//TODO: We haven't implemented the part for "how many turns does the game have?" yet. So this is hard-coded.
Expand All @@ -72,7 +72,7 @@ private int ApplyEarlyGameMultiplier(int score)
return score;
}

private int ApplyNationTraitMultiplier(int score, Player player) {
private double ApplyNationTraitMultiplier(double score, Player player) {
// TODO: The "Expansionist" trait should give a higher priority to this strategic priority.
return score;
}
Expand Down
8 changes: 4 additions & 4 deletions C7Engine/AI/StrategicAI/UtilityCalculations.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ public class UtilityCalculations {
private static readonly int POSSIBLE_CITY_LOCATION_SCORE = 2; //how much weight to give to each possible city location
private static readonly int TILE_SCORE_DIVIDER = 10; //how much to divide each location's tile score by

public static int CalculateAvailableLandScore(Player player)
public static double CalculateAvailableLandScore(Player player)
{
//Figure out if there's land to settle, and how much
Dictionary<Tile, int> possibleLocations = SettlerLocationAI.GetScoredSettlerCandidates(player.cities[0].location, player);
int score = possibleLocations.Count * POSSIBLE_CITY_LOCATION_SCORE;
foreach (int i in possibleLocations.Values) {
Dictionary<Tile, double> possibleLocations = SettlerLocationAI.GetScoredSettlerCandidates(player.cities[0].location, player);
double score = possibleLocations.Count * POSSIBLE_CITY_LOCATION_SCORE;
foreach (double i in possibleLocations.Values) {
score += i / TILE_SCORE_DIVIDER;
}
return score;
Expand Down
2 changes: 1 addition & 1 deletion C7Engine/AI/StrategicAI/WarPriority.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public override void CalculateWeightAndMetadata(Player player) {
if (player.cities.Count < 2) {
this.calculatedWeight = 0;
} else {
int landScore = UtilityCalculations.CalculateAvailableLandScore(player);
double landScore = UtilityCalculations.CalculateAvailableLandScore(player);
//N.B. Eventually this won't be an all-or-nothing proposition; if land is getting tight but not quite zero,
//the AI may decide it's time for the next phrase of the game, especially if it's aggressive.
if (landScore == 0) { //nowhere else to expand
Expand Down
64 changes: 45 additions & 19 deletions C7Engine/AI/UnitAI/SettlerLocationAI.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ public class SettlerLocationAI {

//Figures out where to plant Settlers
public static Tile findSettlerLocation(Tile start, Player player) {
Dictionary<Tile, int> scores = GetScoredSettlerCandidates(start, player);
Dictionary<Tile, double> scores = GetScoredSettlerCandidates(start, player);
if (scores.Count == 0 || scores.Values.Max() <= 0) {
return Tile.NONE; //nowhere to settle
}

IOrderedEnumerable<KeyValuePair<Tile, int> > orderedScores = scores.OrderByDescending(t => t.Value);
IOrderedEnumerable<KeyValuePair<Tile, double> > orderedScores = scores.OrderByDescending(t => t.Value);
log.Debug("Top city location candidates from " + start + ":");
Tile returnValue = null;
foreach (KeyValuePair<Tile, int> kvp in orderedScores.Take(5))
foreach (KeyValuePair<Tile, double> kvp in orderedScores.Take(5))
{
if (returnValue == null) {
returnValue = kvp.Key;
Expand All @@ -39,36 +39,44 @@ public static Tile findSettlerLocation(Tile start, Player player) {
return returnValue;
}

public static Dictionary<Tile, int> GetScoredSettlerCandidates(Tile start, Player player) {
public static Dictionary<Tile, double> GetScoredSettlerCandidates(Tile start, Player player) {
List<MapUnit> playerUnits = player.units;
IEnumerable<Tile> candidates = player.tileKnowledge.AllKnownTiles().Where(t => !IsInvalidCityLocation(t));
Dictionary<Tile, int> scores = AssignTileScores(start, player, candidates, playerUnits.FindAll(u => u.unitType.name == "Settler"));
Dictionary<Tile, double> scores = AssignTileScores(start, player, candidates, playerUnits.FindAll(u => u.unitType.name == "Settler"));
return scores;
}

private static Dictionary<Tile, int> AssignTileScores(Tile startTile, Player player, IEnumerable<Tile> candidates, List<MapUnit> playerSettlers)
private static Dictionary<Tile, double> AssignTileScores(Tile startTile, Player player, IEnumerable<Tile> candidates, List<MapUnit> playerSettlers)
{
Dictionary<Tile, int> scores = new Dictionary<Tile, int>();
Dictionary<Tile, double> scores = new Dictionary<Tile, double>();
candidates = candidates.Where(t => !SettlerAlreadyMovingTowardsTile(t, playerSettlers) && t.IsAllowCities());
foreach (Tile t in candidates) {
int score = GetTileYieldScore(t, player);
double score = GetTileYieldScore(t, player);
//For simplicity's sake, I'm only going to look at immediate neighbors here, but
//a lot more things should be considered over time.
foreach (Tile nt in t.neighbors.Values) {
score += GetTileYieldScore(nt, player);
double improvementScore = GetTileImprovementScore(nt, player);
double yieldScore = GetTileYieldScore(nt, player);
log.Information("Neighbor tile has score of " + yieldScore);
log.Information("Neighbor tile has improvement score of " + improvementScore);
score += yieldScore;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the improvementScore be included in this? I would argue that it should have a lesser weight than the yieldScore, since in most cases the improvements won't exist when the tile is being settled, but greater than zero weight.

}
//Also look at the next ring out, with lower weights.
foreach (Tile outerTile in t.neighbors.Values)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this block, improvement score is included, but isn't it still looping over the same tiles (t.neighbors.Values in both cases)?

{
double outerTileScore = (GetTileYieldScore(outerTile, player) + GetTileImprovementScore(outerTile, player)) / 3;
score += outerTileScore;
log.Information("Outer ring tile has yield score of " + outerTileScore);
}
//TODO: Also look at the next ring out, with lower weights.

//Prefer hills for defense, and coast for boats and such.
if (t.baseTerrainType.Key == "hills") {
score += 10;
}
if (t.NeighborsWater()) {
score += 10;
}
// In this scale, the defense bonus of hills adds up to a bonus of +10, which is equivalent to the previous hardcoded bonus. This just opens up possibilities with editing terrain.
score += t.baseTerrainType.defenseBonus.amount * 20.0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely like that conceptually, and it will be useful in scenarios which allow settling on volcanoes and mountains, or which set different bonuses for tundra/plains/etc.

One gotcha that should be noted in a comment for now is that forests and jungles provide defensive bonuses, but are removed when a city is built on them - thus the bonus should not factor in here. We don't yet have a place where we capture that "disappears when city is built on them" attribute as it isn't moddable in Civ3 and is just an intrinsic property of the terrain.


// Need to add a way to check freshwater source, and separately to check if coast is lake or coast tile. This score would not apply if the city only borders a lake, although we still need a freshwater bonus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of having checks for both freshwater sources and coasts. The initial bonus was to encourage the AI to settle on coasts instead of one tile off the coasts, which it does to a baffling degree in Civ3, preventing it from both building ships and as importantly, harbors which boost the coastal food yield.

I would advise leaving the water bonus in for now as it's likely to encourage the coastal pattern, and if it also encourages settling by lakes for now, that's not a bad thing as it guarantees fresh water (once irrigation is implemented). But with a comment about making this more nuanced in the future.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: This can just use the new Coastal() method, right? And then have a comment about eventually checking for nearby freshwater (which IMO could be both less weighty and more complex, as it can be chained across cities so local fresh water sources aren't necessarily that important).


//Lower scores if they are far away
int preDistanceScore = score;
double preDistanceScore = score;
int distance = startTile.distanceTo(t);
if (distance > 4) {
score -= distance * 2;
Expand All @@ -80,14 +88,17 @@ private static Dictionary<Tile, int> AssignTileScores(Tile startTile, Player pla
}
if (score > 0)
scores[t] = score;
log.Information("Tile score for settling is " + score);
}
return scores;
}
private static int GetTileYieldScore(Tile t, Player owner)
private static double GetTileYieldScore(Tile t, Player owner)
{
int score = t.foodYield(owner) * 5;
double score = t.foodYield(owner) * 5;
score += t.productionYield(owner) * 3;
score += t.commerceYield(owner) * 2;

// TODO: Add multipliers for resource rarity, utility, and whether this player has a surplus
if (t.Resource.Category == ResourceCategory.STRATEGIC) {
score += STRATEGIC_RESOURCE_BONUS;
}
Expand All @@ -97,6 +108,21 @@ private static int GetTileYieldScore(Tile t, Player owner)
return score;
}

private static double GetTileImprovementScore (Tile t, Player owner)
{
double irrigationBonus = t.irrigationYield(owner);
double mineBonus = t.miningYield();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but the tradeBonus should also be considered. It's disabled by default for volcanoes, and can be disabled or increased in mods as well. The commerce yield weight in the tile score is 2 which seems largely appropriate.


// Food is more important than production
// Irrigation only applies to freshwater tiles, although we should add a check for electricity later.
// Also this doesn't account for the ability to chain irrigation.
double irrigationValue = irrigationBonus * 5 * (t.providesFreshWater() ? 1 : 0);
double mineValue = mineBonus * 3;

// Since we can only irrigate OR mine, we just return the max of the two
return Math.Max(irrigationValue,mineValue);
}

public static bool IsInvalidCityLocation(Tile tile) {
if (tile.HasCity) {
log.Verbose("Tile " + tile + " is invalid due to existing city of " + tile.cityAtTile.name);
Expand Down
2 changes: 2 additions & 0 deletions C7GameData/GameData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ public void PerformPostLoadActions()
{
//Let each tile know who its neighbors are. It needs to know this so its graphics can be selected appropriately.
map.computeNeighbors();
// Compute outer ring for settler ai
map.computeOuterRing();
}

/**
Expand Down
26 changes: 26 additions & 0 deletions C7GameData/GameMap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,32 @@ public void computeNeighbors() {
}
}

// Compute the outer ring of the BFC of tiles. Might not be necessary for tiles which can't be settled?
public void computeOuterRing()
{
// List of all the BFC tiles NOT including direct Neighbors
// Essentially, this is every outer tile except North->North, South->South, West->West,East->East
(TileDirection direction1, TileDirection direction2) [] outerRingDirections = {(TileDirection.NORTHWEST, TileDirection.NORTH),(TileDirection.NORTHWEST, TileDirection.NORTHWEST),(TileDirection.NORTHWEST, TileDirection.WEST),(TileDirection.SOUTHWEST, TileDirection.WEST),(TileDirection.SOUTHWEST, TileDirection.SOUTHWEST),(TileDirection.SOUTHWEST, TileDirection.SOUTH),(TileDirection.SOUTHEAST, TileDirection.EAST),(TileDirection.SOUTHEAST, TileDirection.SOUTHEAST),(TileDirection.SOUTHEAST, TileDirection.SOUTH),(TileDirection.NORTHEAST, TileDirection.NORTH),(TileDirection.NORTHEAST, TileDirection.NORTHEAST),(TileDirection.NORTHEAST, TileDirection.EAST)};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this solution to the outer ring problem. Certainly more elegant than how I did it in my editor, in a language that (at the time) didn't have tuple support.

One minor suggestion for readability would be having each of the tiles on its own line here, e.g.:

(TileDirection direction1, TileDirection direction2) [] outerRingDirections = {
    (TileDirection.NORTHWEST, TileDirection.NORTH),
    (TileDirection.NORTHWEST, TileDirection.NORTHWEST)
    ...
}

That would make it easier to verify that the expected 12 are there. Which they are... but I did validate it just to make sure there weren't any erroneous tiles in the ring.


foreach (Tile tile in tiles)
{
Dictionary<(TileDirection,TileDirection), Tile> outerRing = new Dictionary<(TileDirection,TileDirection), Tile>();
foreach((TileDirection dir1, TileDirection dir2) directions in outerRingDirections)
{
Tile inner = tileNeighbor(tile,directions.dir1);
if(inner != Tile.NONE)
{
Tile outer = tileNeighbor(inner, directions.dir2);
if(outer != Tile.NONE)
{
outerRing[(directions.dir1,directions.dir2)] = outer;
}
}
}
tile.outerRing = outerRing;
}
}

// This method verifies that the conversion between tile index and coords is consistent for all possible valid inputs. It's not called
// anywhere but I'm keeping it around in case we ever need to work on the conversion methods again.
public void testTileIndexComputation()
Expand Down
8 changes: 7 additions & 1 deletion C7GameData/TerrainType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ public class TerrainType
public int baseShieldProduction {get; set; }
public int baseCommerceProduction {get; set; }
public int movementCost {get; set; }
public bool allowCities { get; set; } = true;
public bool allowCities {get; set; } = true;

public int miningBonus {get; set; }

public int irrigationBonus {get; set; }
public StrengthBonus defenseBonus;

//some stuff about graphics would probably make sense, too
Expand Down Expand Up @@ -51,6 +55,8 @@ public static TerrainType ImportFromCiv3(int civ3Index, TERR civ3Terrain)
c7Terrain.baseFoodProduction = civ3Terrain.Food;
c7Terrain.baseShieldProduction = civ3Terrain.Shields;
c7Terrain.baseCommerceProduction = civ3Terrain.Commerce;
c7Terrain.irrigationBonus = civ3Terrain.IrrigationBonus;
c7Terrain.miningBonus = civ3Terrain.MiningBonus;
c7Terrain.movementCost = civ3Terrain.MovementCost;
c7Terrain.allowCities = civ3Terrain.AllowCities != 0;
c7Terrain.defenseBonus = new StrengthBonus {
Expand Down
Loading