Skip unnecessary defender water-crossing analysis (#8778)

src/main/cpp/net/eagle0/shardok/ai/AIDefenderStrategySelector.cpp

@@ -26,30 +26,11 @@ auto AIDefenderStrategySelector::BestDefenderStrategy(
         const LandRegions& landRegions,
         const BattalionTypeGetter& battalionTypeGetter) -> AIStrategy {
     uint32_t attackerNonUndeadUnitCount = 0;
-    uint32_t attackerNonUndeadUnitNotRequiringWaterCrossingCount = 0;
     int attackerTroops = 0;
     int defenderTroops = 0;
 
     bool canFlee = false;
 
-    vector<UnitId> attackerUnitIdsRequiringWaterCrossing{};
-    attackerUnitIdsRequiringWaterCrossing.reserve(gameState->units()->size());
-    for (const auto& player : *gameState->player_infos()) {
-        if (!player->is_defender()) {
-            const auto& unitIdsRequiringWaterCrossing = UnitIdsRequiringWaterCrossing(
-                    gameState,
-                    player->player_id(),
-                    criticalTileCoords,
-                    landRegions,
-                    apdCache,
-                    battalionTypeGetter);
-            attackerUnitIdsRequiringWaterCrossing.insert(
-                    attackerUnitIdsRequiringWaterCrossing.end(),
-                    unitIdsRequiringWaterCrossing.begin(),
-                    unitIdsRequiringWaterCrossing.end());
-        }
-    }
-
     for (const Unit* unit : *gameState->units()) {
         if (unit->status() != net::eagle0::shardok::storage::fb::UnitStatus_NORMAL_UNIT &&
             unit->status() != net::eagle0::shardok::storage::fb::UnitStatus_RESERVE_UNIT)
@@ -65,12 +46,6 @@ auto AIDefenderStrategySelector::BestDefenderStrategy(
                 if (unit->battalion().type() !=
                     net::eagle0::shardok::storage::fb::BattalionTypeId_UNDEAD) {
                     ++attackerNonUndeadUnitCount;
-
-                    if (!std::ranges::contains(
-                                attackerUnitIdsRequiringWaterCrossing,
-                                unit->unit_id())) {
-                        ++attackerNonUndeadUnitNotRequiringWaterCrossingCount;
-                    }
                 }
                 attackerTroops += unit->battalion().size();
             }
@@ -88,11 +63,51 @@ auto AIDefenderStrategySelector::BestDefenderStrategy(
     // other considerations we should have but do not presently:
     //  * the attacker is close to the defender / castles
 
-    if (canFlee && roundsRemaining > 2 &&
-        defenderTroops < MAXIMUM_RATIO_FOR_DEFENDER_TO_FLEE * attackerTroops &&
-        attackerNonUndeadUnitNotRequiringWaterCrossingCount >= criticalTileCoords.size()) {
-        chosenStrategy = FleeStrategy;
-    } else if (attackerNonUndeadUnitCount >= criticalTileCoords.size()) {
+    const bool waterCrossingCouldAffectStrategy =
+            canFlee && roundsRemaining > 2 &&
+            defenderTroops < MAXIMUM_RATIO_FOR_DEFENDER_TO_FLEE * attackerTroops &&
+            attackerNonUndeadUnitCount >= criticalTileCoords.size();
+    if (waterCrossingCouldAffectStrategy) {
+        vector<UnitId> attackerUnitIdsRequiringWaterCrossing{};
+        attackerUnitIdsRequiringWaterCrossing.reserve(gameState->units()->size());
+        for (const auto& player : *gameState->player_infos()) {
+            if (!player->is_defender()) {
+                const auto& unitIdsRequiringWaterCrossing = UnitIdsRequiringWaterCrossing(
+                        gameState,
+                        player->player_id(),
+                        criticalTileCoords,
+                        landRegions,
+                        apdCache,
+                        battalionTypeGetter);
+                attackerUnitIdsRequiringWaterCrossing.insert(
+                        attackerUnitIdsRequiringWaterCrossing.end(),
+                        unitIdsRequiringWaterCrossing.begin(),
+                        unitIdsRequiringWaterCrossing.end());
+            }
+        }
+
+        uint32_t attackerNonUndeadUnitNotRequiringWaterCrossingCount = 0;
+        for (const Unit* unit : *gameState->units()) {
+            if (unit->status() != net::eagle0::shardok::storage::fb::UnitStatus_NORMAL_UNIT &&
+                unit->status() != net::eagle0::shardok::storage::fb::UnitStatus_RESERVE_UNIT) {
+                continue;
+            }
+
+            const auto* pi = PlayerInfoForPid(gameState, unit->player_id());
+            if (pi != nullptr && !pi->is_defender() &&
+                unit->battalion().type() !=
+                        net::eagle0::shardok::storage::fb::BattalionTypeId_UNDEAD &&
+                !std::ranges::contains(attackerUnitIdsRequiringWaterCrossing, unit->unit_id())) {
+                ++attackerNonUndeadUnitNotRequiringWaterCrossingCount;
+            }
+        }
+
+        if (attackerNonUndeadUnitNotRequiringWaterCrossingCount >= criticalTileCoords.size()) {
+            return FleeStrategy;
+        }
+    }
+
+    if (attackerNonUndeadUnitCount >= criticalTileCoords.size()) {
         chosenStrategy = HoldCastlesStrategy;
     } else if (defenderTroops > MINIMUM_RATIO_FOR_DEFENDER_TO_HOLD * attackerTroops) {
         chosenStrategy = HoldCastlesStrategy;

src/test/cpp/net/eagle0/shardok/ai/AIDefenderStrategySelector_test.cpp

@@ -13,6 +13,18 @@
 #include "src/test/cpp/net/eagle0/shardok/library/GameStart_test_data.hpp"
 #include "src/test/cpp/net/eagle0/shardok/library/HexMap_test_data.hpp"
 
+namespace {
+
+auto AllWaterLandRegions(const HexMap* map) -> shardok::LandRegions {
+    std::vector<Terrain> terrain(map->terrain()->size());
+    for (auto& tile : terrain) {
+        tile.mutate_type(net::eagle0::shardok::storage::fb::Terrain_::Type_STILL_WATER);
+    }
+    return shardok::LandRegions(map->row_count(), map->column_count(), terrain);
+}
+
+}  // namespace
+
 class AIStrategySelectorTest : public ::testing::Test {
     void SetUp() override {
         InitializeGameSettings();
@@ -182,7 +194,7 @@ TEST_F(AIStrategySelectorTest,
 }
 
 TEST_F(AIStrategySelectorTest,
-       DefenderUnder15PercentOfAttacker_enoughAttackerUnits_returnsFleeStrategy) {
+       DefenderUnder15PercentOfAttacker_enoughAttackerUnits_checksWaterCrossingAndReturnsFlee) {
     CoordsSet criticalTileCoords(6, 6);
     criticalTileCoords.AddMapIndexUnchecked(0);
     criticalTileCoords.AddMapIndexUnchecked(1);
@@ -196,6 +208,9 @@ TEST_F(AIStrategySelectorTest,
 
     defender->mutable_battalion().mutate_size(134);
 
+    const auto disconnectedLandRegions = AllWaterLandRegions(hexMap);
+    int battalionTypeLookups = 0;
+
     EXPECT_EQ(
             AIStrategy::STRATEGY_FLEE,
             AIDefenderStrategySelector::BestDefenderStrategy(
@@ -203,11 +218,13 @@ TEST_F(AIStrategySelectorTest,
                     criticalTileCoords,
                     GetGameSettings()->GetGetter().Backing().max_rounds(),
                     apdCache,
-                    alCache->GetLandRegions(),
-                    [](BattalionTypeId typeId) {
+                    disconnectedLandRegions,
+                    [&battalionTypeLookups](BattalionTypeId typeId) {
+                        ++battalionTypeLookups;
                         return GetGameSettings()->GetGetter().GetBattalionType(typeId);
                     })
                     .strategyType);
+    EXPECT_GT(battalionTypeLookups, 0);
 }
 
 TEST_F(AIStrategySelectorTest,
@@ -241,7 +258,7 @@ TEST_F(AIStrategySelectorTest,
 }
 
 TEST_F(AIStrategySelectorTest,
-       DefenderUnder15PercentOfAttacker_enoughAttackerUnits_cannotFlee_returnsScatterStrategy) {
+       DefenderUnder15PercentOfAttacker_enoughAttackerUnits_cannotFlee_skipsWaterCrossing) {
     CoordsSet criticalTileCoords(6, 6);
     criticalTileCoords.AddMapIndexUnchecked(0);
     criticalTileCoords.AddMapIndexUnchecked(1);
@@ -256,6 +273,9 @@ TEST_F(AIStrategySelectorTest,
     defender->mutable_battalion().mutate_size(134);
     defender->mutate_can_flee(false);
 
+    const auto disconnectedLandRegions = AllWaterLandRegions(hexMap);
+    int battalionTypeLookups = 0;
+
     EXPECT_EQ(
             AIStrategy::STRATEGY_HOLD_CASTLES,
             AIDefenderStrategySelector::BestDefenderStrategy(
@@ -263,11 +283,13 @@ TEST_F(AIStrategySelectorTest,
                     criticalTileCoords,
                     GetGameSettings()->GetGetter().Backing().max_rounds(),
                     apdCache,
-                    alCache->GetLandRegions(),
-                    [](BattalionTypeId typeId) {
+                    disconnectedLandRegions,
+                    [&battalionTypeLookups](BattalionTypeId typeId) {
+                        ++battalionTypeLookups;
                         return GetGameSettings()->GetGetter().GetBattalionType(typeId);
                     })
                     .strategyType);
+    EXPECT_EQ(battalionTypeLookups, 0);
 }
 
 TEST_F(AIStrategySelectorTest,