Remove the AI unit growth bonuses and tune the income bonuses for different difficulty levels

src/fheroes2/ai/ai.h

@@ -105,6 +105,7 @@ namespace AI
 
         virtual void Reset();
         virtual void resetPathfinder() = 0;
+
         virtual bool isValidHeroObject( const Heroes & hero, const int32_t index, const bool underHero ) = 0;
 
         // Should be called at the beginning of the battle even if no AI-controlled players are

src/fheroes2/ai/normal/ai_normal.h

@@ -298,42 +298,30 @@ namespace AI
 
         void CastlePreBattle( Castle & castle ) override;
 
-        bool recruitHero( Castle & castle, bool buyArmy, bool underThreat );
-        void reinforceHeroInCastle( Heroes & hero, Castle & castle, const Funds & budget );
-        void evaluateRegionSafety();
-        std::vector<AICastle> getSortedCastleList( const VecCastles & castles, const std::set<int> & castlesInDanger );
-
         void resetPathfinder() override;
-        bool isValidHeroObject( const Heroes & hero, const int32_t index, const bool underHero ) override;
 
         void battleBegins() override;
 
         void tradingPostVisitEvent( Kingdom & kingdom ) override;
 
+        bool isValidHeroObject( const Heroes & hero, const int32_t index, const bool underHero ) override;
+
         double getObjectValue( const Heroes & hero, const int index, const int objectType, const double valueToIgnore, const uint32_t distanceToObject ) const;
         double getTargetArmyStrength( const Maps::Tiles & tile, const MP2::MapObjectType objectType );
 
-        bool isPriorityTask( const int32_t index ) const
-        {
-            return _priorityTargets.find( index ) != _priorityTargets.end();
-        }
-
-        bool isCriticalTask( const int32_t index ) const
-        {
-            const auto iter = _priorityTargets.find( index );
-            if ( iter == _priorityTargets.end() ) {
-                return false;
-            }
-
-            return iter->second.type == PriorityTaskType::ATTACK || iter->second.type == PriorityTaskType::DEFEND;
-        }
-
     private:
         void CastleTurn( Castle & castle, const bool defensiveStrategy );
 
         // Returns true if heroes can still do tasks but they have no move points.
         bool HeroesTurn( VecHeroes & heroes, const uint32_t startProgressValue, const uint32_t endProgressValue );
 
+        bool recruitHero( Castle & castle, bool buyArmy );
+        void reinforceHeroInCastle( Heroes & hero, Castle & castle, const Funds & budget );
+
+        void evaluateRegionSafety();
+
+        std::vector<AICastle> getSortedCastleList( const VecCastles & castles, const std::set<int> & castlesInDanger );
+
         int getPriorityTarget( const HeroToMove & heroInfo, double & maxPriority );
 
         double getGeneralObjectValue( const Heroes & hero, const int index, const double valueToIgnore, const uint32_t distanceToObject ) const;
@@ -360,17 +348,30 @@ namespace AI
         std::set<int> findCastlesInDanger( const Kingdom & kingdom );
 
         void updatePriorityForEnemyArmy( const Kingdom & kingdom, const EnemyArmy & enemyArmy );
-
         void updatePriorityForCastle( const Castle & castle );
 
         // Return true if the castle is in danger.
         // IMPORTANT!!! Do not call this method directly. Use other methods which call it internally.
         bool updateIndividualPriorityForCastle( const Castle & castle, const EnemyArmy & enemyArmy );
 
         void removePriorityAttackTarget( const int32_t tileIndex );
-
         void updatePriorityAttackTarget( const Kingdom & kingdom, const Maps::Tiles & tile );
 
+        bool isPriorityTask( const int32_t index ) const
+        {
+            return _priorityTargets.find( index ) != _priorityTargets.end();
+        }
+
+        bool isCriticalTask( const int32_t index ) const
+        {
+            const auto iter = _priorityTargets.find( index );
+            if ( iter == _priorityTargets.end() ) {
+                return false;
+            }
+
+            return iter->second.type == PriorityTaskType::ATTACK || iter->second.type == PriorityTaskType::DEFEND;
+        }
+
         // The following member variables should not be saved or serialized
         std::vector<IndexObject> _mapActionObjects;
         std::map<int32_t, PriorityTask> _priorityTargets;

src/fheroes2/ai/normal/ai_normal_battle.cpp

@@ -38,7 +38,6 @@
 
 #include "ai.h"
 #include "ai_normal.h"
-#include "army.h"
 #include "artifact.h"
 #include "artifact_info.h"
 #include "battle.h"
@@ -600,7 +599,6 @@ namespace AI
                 }
 
                 const int gameDifficulty = Game::getDifficulty();
-                const bool isGameCampaign = Game::isCampaign();
 
                 // TODO: consider taking speed/turn order into account in the future
                 if ( _myArmyStrength * Difficulty::getArmyStrengthRatioForAIRetreat( gameDifficulty ) >= _enemyArmyStrength ) {
@@ -620,98 +618,87 @@ namespace AI
 
                 const Kingdom & kingdom = actualHero->GetKingdom();
 
-                const bool considerRetreat = [this, &arena, actualHero, gameDifficulty, isGameCampaign, hasValuableArtifacts, &kingdom]() {
-                    if ( !Difficulty::allowAIToRetreat( gameDifficulty, isGameCampaign ) ) {
+                const bool isAbleToSurrender = [this, &arena, &kingdom]() {
+                    if ( !arena.CanSurrenderOpponent( _myColor ) ) {
                         return false;
                     }
 
-                    if ( !arena.CanRetreatOpponent( _myColor ) ) {
-                        return false;
-                    }
+                    return kingdom.AllowPayment( { Resource::GOLD, arena.getForce( _myColor ).GetSurrenderCost() } );
+                }();
 
-                    // If the hero has valuable artifacts, he should in any case consider retreating so that these artifacts do not end up at the disposal of the enemy,
-                    // especially in the case of an alliance war
-                    if ( hasValuableArtifacts ) {
+                const bool isPossibleToReHire = [actualHero, &kingdom]() {
+                    // If the hero is not the last in his kingdom, then we will assume that there is a possibility of re-hiring - even if there are no castles in the
+                    // kingdom, one of the remaining heroes can capture an enemy castle
+                    const VecHeroes & heroes = kingdom.GetHeroes();
+                    if ( heroes.size() > 1 ) {
                         return true;
                     }
 
-                    // Otherwise, if this hero is the last one, and the kingdom has no castles, then there is no point in retreating
-                    if ( kingdom.GetHeroes().size() == 1 ) {
-                        assert( kingdom.GetHeroes().at( 0 ) == actualHero );
-
-                        if ( kingdom.GetCastles().empty() ) {
-                            return false;
-                        }
-                    }
+                    assert( heroes.size() == 1 && heroes.at( 0 ) == actualHero );
 
-                    // Otherwise, if this hero is relatively experienced, then he should think about retreating so that he can be hired again later
-                    return actualHero->GetLevel() >= Difficulty::getMinHeroLevelForAIRetreat( gameDifficulty );
-                }();
-
-                const bool considerSurrender = [this, &arena, actualHero, gameDifficulty, isGameCampaign, hasValuableArtifacts, &kingdom]() {
-                    if ( !Difficulty::allowAIToSurrender( gameDifficulty, isGameCampaign ) ) {
+                    // Otherwise, if this hero is the last one, and there are no castles in the kingdom, then it will be impossible to re-hire this hero
+                    const VecCastles & castles = kingdom.GetCastles();
+                    if ( castles.empty() ) {
                         return false;
                     }
 
-                    if ( !arena.CanSurrenderOpponent( _myColor ) ) {
-                        return false;
-                    }
+                    // Otherwise, if this hero is defending the last castle, then it will be impossible to re-hire this hero
+                    const Castle * castle = actualHero->inCastle();
+                    if ( castle && castles.size() == 1 ) {
+                        assert( castles.at( 0 ) == castle );
 
-                    // If the hero has valuable artifacts, he should in any case consider surrendering so that these artifacts do not end up at the disposal of the enemy,
-                    // especially in the case of an alliance war
-                    if ( hasValuableArtifacts ) {
-                        return true;
+                        return false;
                     }
 
-                    // Otherwise, if this hero is the last one, and either the kingdom has no castles, or this hero is defending the last castle, then there is no point
-                    // in surrendering
-                    if ( kingdom.GetHeroes().size() == 1 ) {
-                        assert( kingdom.GetHeroes().at( 0 ) == actualHero );
-
-                        const VecCastles & castles = kingdom.GetCastles();
-                        if ( castles.empty() ) {
-                            return false;
-                        }
+                    // Otherwise, assume that there is a possibility of re-hiring
+                    return true;
+                }();
 
-                        const Castle * castle = actualHero->inCastle();
-                        if ( castle && castles.size() == 1 ) {
-                            assert( castles.at( 0 ) == castle );
+                const int minHeroTotalPrimarySkillLevelForRetreat = 10;
 
-                            return false;
-                        }
+                if ( !arena.CanRetreatOpponent( _myColor ) ) {
+                    if ( !isAbleToSurrender ) {
+                        return Outcome::ContinueBattle;
                     }
 
-                    // Otherwise, if this hero is relatively experienced, then he should think about surrendering so that he can be hired again later
-                    return actualHero->GetLevel() >= Difficulty::getMinHeroLevelForAIRetreat( gameDifficulty );
-                }();
-
-                const Force & force = arena.getForce( _myColor );
+                    // If the hero has valuable artifacts, he should surrender so that these artifacts do not end up at the disposal of the enemy, especially in the case
+                    // of an alliance war
+                    if ( hasValuableArtifacts ) {
+                        return Outcome::Surrender;
+                    }
 
-                if ( !considerRetreat ) {
-                    if ( !considerSurrender ) {
+                    // Otherwise, if this hero cannot be rehired, then there is no point in surrendering
+                    if ( !isPossibleToReHire ) {
                         return Outcome::ContinueBattle;
                     }
 
-                    if ( !kingdom.AllowPayment( { Resource::GOLD, force.GetSurrenderCost() } ) ) {
-                        return Outcome::ContinueBattle;
+                    // Otherwise, if this hero is relatively experienced, then he should surrender so that he can be hired again later
+                    if ( actualHero->getTotalPrimarySkillLevel() >= minHeroTotalPrimarySkillLevelForRetreat ) {
+                        return Outcome::Surrender;
                     }
 
-                    return Outcome::Surrender;
+                    // Otherwise, there is no point in surrendering
+                    return Outcome::ContinueBattle;
                 }
 
-                if ( !considerSurrender ) {
+                // If the hero has valuable artifacts, he should retreat so that these artifacts do not end up at the disposal of the enemy, especially in the case of an
+                // alliance war
+                if ( hasValuableArtifacts ) {
                     return Outcome::Retreat;
                 }
 
-                if ( force.getStrengthOfArmyRemainingInCaseOfSurrender() < Army::getStrengthOfAverageStartingArmy( actualHero ) ) {
-                    return Outcome::Retreat;
+                // Otherwise, if this hero cannot be rehired, then there is no point in retreating
+                if ( !isPossibleToReHire ) {
+                    return Outcome::ContinueBattle;
                 }
 
-                if ( !kingdom.AllowPayment( Funds{ Resource::GOLD, force.GetSurrenderCost() } * Difficulty::getGoldReserveRatioForAISurrender( gameDifficulty ) ) ) {
+                // Otherwise, if this hero is relatively experienced, then he should retreat so that he can be hired again later
+                if ( actualHero->getTotalPrimarySkillLevel() >= minHeroTotalPrimarySkillLevelForRetreat ) {
                     return Outcome::Retreat;
                 }
 
-                return Outcome::Surrender;
+                // Otherwise, there is no point in retreating or surrendering
+                return Outcome::ContinueBattle;
             }();
 
             const auto farewellSpellcast = [this, &arena, &currentUnit, &actions]() {

src/fheroes2/ai/normal/ai_normal_castle.cpp

@@ -28,8 +28,6 @@
 #include "ai.h"
 #include "ai_normal.h"
 #include "army.h"
-#include "army_troop.h"
-#include "battle_tower.h"
 #include "castle.h"
 #include "difficulty.h"
 #include "game.h"
@@ -314,31 +312,26 @@ namespace AI
     void Normal::CastleTurn( Castle & castle, const bool defensiveStrategy )
     {
         if ( defensiveStrategy ) {
-            // Avoid building monster dwellings when defensive as they might fall into enemy's hands, unless we have a lot of resources.
-            const Kingdom & kingdom = castle.GetKingdom();
+            const Funds & kingdomFunds = castle.GetKingdom().GetFunds();
 
-            // TODO: check if we can upgrade monsters. It is much cheaper (except Giants into Titans) to upgrade monsters than buy new ones.
+            // If the castle is potentially under threat, then it makes sense to try to hire the maximum number of troops so that the enemy cannot hire them even if he
+            // captures the castle, therefore, it is worth starting with hiring.
+            castle.recruitBestAvailable( kingdomFunds );
 
-            Troops possibleReinforcement = castle.getAvailableArmy( kingdom.GetFunds() );
-            double possibleReinforcementStrength = possibleReinforcement.GetStrength();
+            Army & garrison = castle.GetArmy();
 
-            // A very rough estimation of strength. We measure the strength of possible army to hire with the strength of purchasing a turret.
-            const Battle::Tower tower( castle, Battle::TowerType::TWR_RIGHT, 0 );
-            const Troop towerMonster( Monster::ARCHER, tower.GetCount() );
-            const double towerStrength = towerMonster.GetStrength();
-            if ( possibleReinforcementStrength > towerStrength ) {
-                castle.recruitBestAvailable( kingdom.GetFunds() );
-                OptimizeTroopsOrder( castle.GetArmy() );
-            }
+            // Then we try to upgrade the existing units in the castle garrison...
+            garrison.UpgradeTroops( castle );
 
-            if ( castle.GetActualArmy().getTotalCount() > 0 ) {
-                Build( castle, defensiveStructures );
-            }
+            // ... and then we try to hire troops again, because after upgrading the existing troops, there could be a place for new units.
+            castle.recruitBestAvailable( kingdomFunds );
 
-            castle.recruitBestAvailable( kingdom.GetFunds() );
-            OptimizeTroopsOrder( castle.GetArmy() );
+            OptimizeTroopsOrder( garrison );
 
+            // Avoid building monster dwellings when defensive as they might fall into enemy's hands, unless we have a lot of resources. Instead, try to build defensive
+            // structures if there is at least some kind of garrison in the castle.
             if ( castle.GetActualArmy().getTotalCount() > 0 ) {
+                Build( castle, defensiveStructures );
                 Build( castle, supportingDefensiveStructures );
             }
         }

src/fheroes2/ai/normal/ai_normal_kingdom.cpp

@@ -269,7 +269,7 @@ namespace
 
 namespace AI
 {
-    bool Normal::recruitHero( Castle & castle, bool buyArmy, bool underThreat )
+    bool Normal::recruitHero( Castle & castle, bool buyArmy )
     {
         Kingdom & kingdom = castle.GetKingdom();
         const Recruits & rec = kingdom.GetRecruits();
@@ -310,7 +310,6 @@ namespace AI
         }
 
         if ( buyArmy ) {
-            CastleTurn( castle, underThreat );
             reinforceHeroInCastle( *recruit, castle, kingdom.GetFunds() );
         }
         else {
@@ -481,12 +480,16 @@ namespace AI
     std::vector<AICastle> Normal::getSortedCastleList( const VecCastles & castles, const std::set<int> & castlesInDanger )
     {
         std::vector<AICastle> sortedCastleList;
+        sortedCastleList.reserve( castles.size() );
+
         for ( Castle * castle : castles ) {
-            if ( !castle )
+            if ( castle == nullptr ) {
                 continue;
+            }
 
             const int32_t castleIndex = castle->GetIndex();
             const uint32_t regionID = world.GetTiles( castleIndex ).GetRegion();
+
             sortedCastleList.emplace_back( castle, castlesInDanger.count( castleIndex ) > 0, _regions[regionID].safetyFactor, castle->getBuildingValue() );
         }
 
@@ -588,7 +591,8 @@ namespace AI
         const uint32_t threatDistanceLimit = 3000;
 
         const int32_t castleIndex = castle.GetIndex();
-        // skip precise distance check if army is too far to be a threat
+
+        // Skip precise distance check if army is too far to be a threat
         if ( Maps::GetApproximateDistance( enemyArmy.index, castleIndex ) * Maps::Ground::fastestMovePenalty > threatDistanceLimit ) {
             return false;
         }
@@ -920,6 +924,8 @@ namespace AI
         // Sync the list of castles (if new ones were captured during the turn)
         if ( castles.size() != sortedCastleList.size() ) {
             evaluateRegionSafety();
+
+            castlesInDanger = findCastlesInDanger( kingdom );
             sortedCastleList = getSortedCastleList( castles, castlesInDanger );
         }
 
@@ -946,8 +952,8 @@ namespace AI
     bool Normal::purchaseNewHeroes( const std::vector<AICastle> & sortedCastleList, const std::set<int> & castlesInDanger, const int32_t availableHeroCount,
                                     const bool moreTasksForHeroes )
     {
-        const bool slowEarlyGame = world.CountDay() < 5 && sortedCastleList.size() == 1;
-        const int32_t heroLimit = slowEarlyGame ? 2 : world.w() / Maps::SMALL + 2;
+        const bool isEarlyGameWithSingleCastle = world.CountDay() < 5 && sortedCastleList.size() == 1;
+        const int32_t heroLimit = isEarlyGameWithSingleCastle ? 2 : world.w() / Maps::SMALL + 2;
 
         if ( availableHeroCount >= heroLimit ) {
             return false;
@@ -990,7 +996,7 @@ namespace AI
         }
 
         // target found, buy hero
-        return recruitmentCastle && recruitHero( *recruitmentCastle, !slowEarlyGame, false );
+        return recruitmentCastle && recruitHero( *recruitmentCastle, !isEarlyGameWithSingleCastle );
     }
 
     void Normal::tradingPostVisitEvent( Kingdom & /*kingdom*/ )