refactoring RecruitHeroBehavior + test sample; renamed more Nullkiller variables consistently

AI/Nullkiller2/Analyzers/BuildAnalyzer.h

@@ -79,7 +79,7 @@ class DLL_EXPORT BuildAnalyzer
 	Nullkiller * aiNk;
 
 public:
-	explicit BuildAnalyzer(Nullkiller * ai) : aiNk(ai) {}
+	explicit BuildAnalyzer(Nullkiller * aiNk) : aiNk(aiNk) {}
 	void update();
 
 	TResources getResourcesRequiredNow() const;

AI/Nullkiller2/Analyzers/DangerHitMapAnalyzer.h

@@ -79,7 +79,7 @@ private:
 	std::map<ObjectInstanceID, std::vector<HitMapInfo>> townThreats;
 
 public:
-	DangerHitMapAnalyzer(const Nullkiller * ai) :aiNk(ai) {}
+	DangerHitMapAnalyzer(const Nullkiller * aiNk) :aiNk(aiNk) {}
 
 	void updateHitMap();
 	void calculateTileOwners();

AI/Nullkiller2/Analyzers/HeroManager.cpp

@@ -97,6 +97,7 @@ float HeroManager::evaluateSpeciality(const CGHeroInstance * hero) const
 
 float HeroManager::evaluateFightingStrength(const CGHeroInstance * hero) const
 {
+	// TODO: Mircea: Shouldn't we count bonuses from artifacts when generating the fighting strength? That could make a huge difference
 	return evaluateSpeciality(hero) + wariorSkillsScores.evaluateSecSkills(hero) + hero->getBasePrimarySkillValue(PrimarySkill::ATTACK) + hero->getBasePrimarySkillValue(PrimarySkill::DEFENSE) + hero->getBasePrimarySkillValue(PrimarySkill::SPELL_POWER) + hero->getBasePrimarySkillValue(PrimarySkill::KNOWLEDGE);
 }
 
@@ -127,37 +128,37 @@ void HeroManager::update()
 	}
 
 	std::sort(myHeroes.begin(), myHeroes.end(), scoreSort);
-	heroRoles.clear();
+	heroToRoleMap.clear();
 
 	for(auto hero : myHeroes)
 	{
 		if(hero->patrol.patrolling)
 		{
-			heroRoles[hero] = HeroRole::MAIN;
+			heroToRoleMap[hero] = HeroRole::MAIN;
 		}
 		else
 		{
-			heroRoles[hero] = (globalMainCount--) > 0 ? HeroRole::MAIN : HeroRole::SCOUT;
+			heroToRoleMap[hero] = (globalMainCount--) > 0 ? HeroRole::MAIN : HeroRole::SCOUT;
 		}
 	}
 
 	for(auto hero : myHeroes)
 	{
-		logAi->trace("Hero %s has role %s", hero->getNameTranslated(), heroRoles[hero] == HeroRole::MAIN ? "main" : "scout");
+		logAi->trace("Hero %s has role %s", hero->getNameTranslated(), heroToRoleMap[hero] == HeroRole::MAIN ? "main" : "scout");
 	}
 }
 
 HeroRole HeroManager::getHeroRole(const HeroPtr & hero) const
 {
-	if (heroRoles.find(hero) != heroRoles.end())
-		return heroRoles.at(hero);
+	if (heroToRoleMap.find(hero) != heroToRoleMap.end())
+		return heroToRoleMap.at(hero);
 	else
 		return HeroRole::SCOUT;
 }
 
-const std::map<HeroPtr, HeroRole> & HeroManager::getHeroRoles() const
+const std::map<HeroPtr, HeroRole> & HeroManager::getHeroToRoleMap() const
 {
-	return heroRoles;
+	return heroToRoleMap;
 }
 
 int HeroManager::selectBestSkill(const HeroPtr & hero, const std::vector<SecondarySkill> & skills) const

AI/Nullkiller2/Analyzers/HeroManager.h

@@ -44,12 +44,12 @@ private:
 
 	CCallback * cc; //this is enough, but we downcast from CCallback
 	const Nullkiller * aiNk;
-	std::map<HeroPtr, HeroRole> heroRoles;
+	std::map<HeroPtr, HeroRole> heroToRoleMap;
 	std::map<ObjectInstanceID, float> knownFightingStrength;
 
 public:
-	HeroManager(CCallback * cc, const Nullkiller * ai) : cc(cc), aiNk(ai) {}
-	const std::map<HeroPtr, HeroRole> & getHeroRoles() const;
+	HeroManager(CCallback * cc, const Nullkiller * aiNk) : cc(cc), aiNk(aiNk) {}
+	const std::map<HeroPtr, HeroRole> & getHeroToRoleMap() const;
 	HeroRole getHeroRole(const HeroPtr & hero) const;
 	int selectBestSkill(const HeroPtr & hero, const std::vector<SecondarySkill> & skills) const;
 	void update();

AI/Nullkiller2/Analyzers/ObjectClusterizer.h

@@ -81,7 +81,7 @@ public:
 	const CGObjectInstance * getBlocker(const AIPath & path) const;
 	std::optional<const CGObjectInstance *> getBlocker(const AIPathNodeInfo & node) const;
 
-	ObjectClusterizer(const Nullkiller * ai): aiNk(ai), valueEvaluator(ai), isUpToDate(false){}
+	ObjectClusterizer(const Nullkiller * aiNk): aiNk(aiNk), valueEvaluator(aiNk), isUpToDate(false){}
 
 	void validateObjects();
 	void onObjectRemoved(ObjectInstanceID id);

AI/Nullkiller2/Behaviors/BuyArmyBehavior.h

@@ -24,7 +24,7 @@ namespace Goals
 		std::string toString() const override;
 		bool operator==(const BuyArmyBehavior & other) const override
 		{
-			return true;
+			return true; // TODO: Mircea: how does this make sense?
 		}
 	};
 }

AI/Nullkiller2/Behaviors/RecruitHeroBehavior.cpp

@@ -9,6 +9,8 @@
 */
 #include "StdInc.h"
 #include "RecruitHeroBehavior.h"
+
+#include <algorithm>
 #include "../AIGateway.h"
 #include "../AIUtility.h"
 #include "../Goals/RecruitHero.h"
@@ -28,124 +30,119 @@ Goals::TGoalVec RecruitHeroBehavior::decompose(const Nullkiller * aiNk) const
 {
 	Goals::TGoalVec tasks;
 	const auto ourTowns = aiNk->cc->getTownsInfo();
-	const auto ourHeroes = aiNk->heroManager->getHeroRoles();
-	auto minScoreToHireMain = std::numeric_limits<float>::max();
-	int currentArmyValue = 0;
-
-	for(const auto & hero : ourHeroes)
-	{
-		currentArmyValue += hero.first->getArmyCost();
-		if(hero.second != HeroRole::MAIN)
-			continue;
-
-		const auto newScore = aiNk->heroManager->evaluateHero(hero.first.get());
-
-		if(minScoreToHireMain > newScore)
-		{
-			// weakest main hero score
-			minScoreToHireMain = newScore;
-		}
-	}
-
-	// If we don't have any heroes, lower our expectations.
-	if (ourHeroes.empty())
-		minScoreToHireMain = 0;
-
-	const CGHeroInstance* bestHeroToHire = nullptr;
-	const CGTownInstance* bestTownToHireFrom = nullptr;
-	float bestScore = 0;
+	const auto ourHeroes = aiNk->heroManager->getHeroToRoleMap();
+	RecruitHeroChoice bestChoice;
 	bool haveCapitol = false;
+	int treasureSourcesCount = 0;
 
 	// Simplification: Moved this call before getting into the decomposer
 	// aiNk->dangerHitMap->updateHitMap();
 
-	int treasureSourcesCount = 0;
-	int bestClosestThreat = UINT8_MAX;
-	
 	for(const auto * town : ourTowns)
 	{
-		uint8_t closestThreat = UINT8_MAX;
-		for (const auto & threat : aiNk->dangerHitMap->getTownThreats(town))
+		if(town->getVisitingHero() && town->getGarrisonHero())
+			continue;
+
+		uint8_t closestThreatTurn = UINT8_MAX;
+		for(const auto & threat : aiNk->dangerHitMap->getTownThreats(town))
 		{
-			closestThreat = std::min(closestThreat, threat.turn);
+			closestThreatTurn = std::min(closestThreatTurn, threat.turn);
 		}
 
-		if (town->getVisitingHero() && town->getGarrisonHero())
-			continue;
-
-		float visitability = 0;
-		for (const auto & hero : ourHeroes)
+		float visitabilityRatio = 0;
+		for(const auto & [hero, role] : ourHeroes)
 		{
-			if (aiNk->dangerHitMap->getClosestTown(hero.first.get()->visitablePos()) == town)
-				visitability++;
+			if(aiNk->dangerHitMap->getClosestTown(hero.get()->visitablePos()) == town)
+				visitabilityRatio += 1.0f / ourHeroes.size();
 		}
 
 		if(aiNk->heroManager->canRecruitHero(town))
 		{
-			auto availableHeroes = aiNk->cc->getAvailableHeroes(town);
-			
-			for (const auto * obj : aiNk->objectClusterizer->getNearbyObjects())
-			{
-				if (obj->ID == Obj::RESOURCE
-					|| obj->ID == Obj::TREASURE_CHEST
-					|| obj->ID == Obj::CAMPFIRE
-					|| isWeeklyRevisitable(aiNk->playerID, obj)
-					|| obj->ID == Obj::ARTIFACT)
-				{
-					auto tile = obj->visitablePos();
-					if (town == aiNk->dangerHitMap->getClosestTown(tile))
-						treasureSourcesCount++; // TODO: Mircea: Shouldn't it be used to determine the best town?
-				}
-			}
-
-			for(const auto hero : availableHeroes)
-			{
-				if ((town->getVisitingHero() || town->getGarrisonHero()) 
-					&& closestThreat < 1
-					&& hero->getArmyCost() < GameConstants::HERO_GOLD_COST / 3.0)
-					continue;
-
-				auto score = aiNk->heroManager->evaluateHero(hero);
-				if(score > minScoreToHireMain)
-				{
-					score *= score / minScoreToHireMain;
-				}
-				score *= hero->getArmyCost() + currentArmyValue;
-
-				if (hero->getFactionID() == town->getFactionID())
-					score *= 1.5;
-				if (vstd::isAlmostZero(visitability))
-					score *= 30 * town->getTownLevel();
-				else
-					score *= town->getTownLevel() / visitability;
-
-				if (score > bestScore)
-				{
-					bestScore = score;
-					bestHeroToHire = hero;
-					bestTownToHireFrom = town; // TODO: Mircea: Seems to be no logic to choose the right town?
-					bestClosestThreat = closestThreat;
-				}
-			}
+			calculateTreasureSources(aiNk->objectClusterizer->getNearbyObjects(), aiNk->playerID, *aiNk->dangerHitMap, treasureSourcesCount, town);
+			calculateBestHero(aiNk->cc->getAvailableHeroes(town),
+			                  *aiNk->heroManager,
+			                  bestChoice,
+			                  town,
+			                  closestThreatTurn,
+			                  visitabilityRatio);
 		}
 
-		if (town->hasCapitol())
+		if(town->hasCapitol())
 			haveCapitol = true;
 	}
 
-	if (bestHeroToHire && bestTownToHireFrom)
+	if(!vstd::isAlmostZero(bestChoice.score))
 	{
-		if (aiNk->cc->getHeroesInfo().size() == 0
-			|| treasureSourcesCount > aiNk->cc->getHeroesInfo().size() * 5
-			|| (bestHeroToHire->getArmyCost() > GameConstants::HERO_GOLD_COST / 2.0 && (bestClosestThreat < 1 || !aiNk->buildAnalyzer->isGoldPressureOverMax()))
-			|| (aiNk->getFreeResources()[EGameResID::GOLD] > 10000 && !aiNk->buildAnalyzer->isGoldPressureOverMax() && haveCapitol)
-			|| (aiNk->getFreeResources()[EGameResID::GOLD] > 30000 && !aiNk->buildAnalyzer->isGoldPressureOverMax()))
+		if(ourHeroes.empty()
+		   || treasureSourcesCount > ourHeroes.size() * 5
+		   // TODO: Mircea: The next condition should always consider a hero if under attack especially if it has towers
+		   || (bestChoice.hero->getArmyCost() > GameConstants::HERO_GOLD_COST / 2.0 && (
+			       bestChoice.closestThreat < 1 || !aiNk->buildAnalyzer->isGoldPressureOverMax()))
+		   || (aiNk->getFreeResources()[EGameResID::GOLD] > 10000 && !aiNk->buildAnalyzer->isGoldPressureOverMax() && haveCapitol)
+		   || (aiNk->getFreeResources()[EGameResID::GOLD] > 30000 && !aiNk->buildAnalyzer->isGoldPressureOverMax()))
 		{
-			tasks.push_back(Goals::sptr(Goals::RecruitHero(bestTownToHireFrom, bestHeroToHire).setpriority((float)3 / (ourHeroes.size() + 1))));
+			tasks.push_back(Goals::sptr(Goals::RecruitHero(bestChoice.town, bestChoice.hero).setpriority((float)3 / (ourHeroes.size() + 1))));
 		}
 	}
 
 	return tasks;
 }
 
+void RecruitHeroBehavior::calculateTreasureSources(const std::vector<const CGObjectInstance *> & nearbyObjects,
+                                                   const PlayerColor & playerID,
+                                                   const DangerHitMapAnalyzer & dangerHitMap,
+                                                   int & treasureSourcesCount,
+                                                   const CGTownInstance * town)
+{
+	for(const auto * obj : nearbyObjects)
+	{
+		if(obj->ID == Obj::RESOURCE
+		   || obj->ID == Obj::TREASURE_CHEST
+		   || obj->ID == Obj::CAMPFIRE
+		   || isWeeklyRevisitable(playerID, obj)
+		   || obj->ID == Obj::ARTIFACT)
+		{
+			if(town == dangerHitMap.getClosestTown(obj->visitablePos()))
+				treasureSourcesCount++; // TODO: Mircea: Shouldn't it be used to determine the best town?
+		}
+	}
+}
+
+void RecruitHeroBehavior::calculateBestHero(const std::vector<const CGHeroInstance *> & availableHeroes,
+                                            const HeroManager & heroManager,
+                                            const RecruitHeroChoice & bestChoice,
+                                            const CGTownInstance * town,
+                                            const uint8_t closestThreatTurn,
+                                            const float visitabilityRatio)
+{
+
+	for(const auto * const hero : availableHeroes)
+	{
+		if((town->getVisitingHero() || town->getGarrisonHero())
+		   && closestThreatTurn < 1
+		   && hero->getArmyCost() < GameConstants::HERO_GOLD_COST / 3.0)
+			continue;
+
+		const float heroScore = heroManager.evaluateHero(hero);
+		float totalScore = heroScore + hero->getArmyCost();
+
+		// TODO: Mircea: Score higher if ballista/tent/ammo cart by the cost in gold? Or should that be covered in evaluateHero?
+		// getArtifactScoreForHero(hero, ...) ArtifactID::BALLISTA
+
+		if(hero->getFactionID() == town->getFactionID())
+			totalScore += heroScore * 1.5;
+
+		// prioritize a more developed town especially if no heroes can visit it (smaller ratio, bigger score)
+		totalScore += heroScore * town->getTownLevel() * (1 - visitabilityRatio);
+
+		if(totalScore > bestChoice.score)
+		{
+			bestChoice.score = totalScore;
+			bestChoice.hero = hero;
+			bestChoice.town = town;
+			bestChoice.closestThreat = closestThreatTurn;
+		}
+	}
+}
+
 }

AI/Nullkiller2/Behaviors/RecruitHeroBehavior.h

@@ -12,27 +12,51 @@
 #include "lib/GameLibrary.h"
 #include "../Goals/CGoal.h"
 #include "../AIUtility.h"
+#include "../Analyzers/DangerHitMapAnalyzer.h"
+#include "../Analyzers/HeroManager.h"
 
 namespace NK2AI
 {
 namespace Goals
 {
-	class RecruitHeroBehavior : public CGoal<RecruitHeroBehavior>
+struct RecruitHeroChoice
+{
+	mutable float score = 0;
+	mutable const CGHeroInstance * hero = nullptr;
+	mutable const CGTownInstance * town = nullptr;
+	mutable int closestThreat = 0;
+};
+
+class RecruitHeroBehavior : public CGoal<RecruitHeroBehavior>
+{
+public:
+	RecruitHeroBehavior()
+		: CGoal(RECRUIT_HERO_BEHAVIOR)
+	{
+	}
+
+	~RecruitHeroBehavior() override = default;
+
+	TGoalVec decompose(const Nullkiller * aiNk) const override;
+	std::string toString() const override;
+
+	bool operator==(const RecruitHeroBehavior & other) const override
 	{
-	public:
-		RecruitHeroBehavior()
-			:CGoal(RECRUIT_HERO_BEHAVIOR)
-		{
-		}
-
-		TGoalVec decompose(const Nullkiller * aiNk) const override;
-		std::string toString() const override;
-
-		bool operator==(const RecruitHeroBehavior & other) const override
-		{
-			return true;
-		}
-	};
+		return true; // TODO: Mircea: How does that make sense?
+	}
+
+	static void calculateTreasureSources(const std::vector<const CGObjectInstance *> & nearbyObjects,
+	                                     const PlayerColor & playerID,
+	                                     const DangerHitMapAnalyzer & dangerHitMap,
+	                                     int & treasureSourcesCount,
+	                                     const CGTownInstance * town);
+	static void calculateBestHero(const std::vector<const CGHeroInstance *> & availableHeroes,
+	                              const HeroManager & heroManager,
+	                              const RecruitHeroChoice & bestChoice,
+	                              const CGTownInstance * town,
+	                              uint8_t closestThreatTurn,
+	                              float visitabilityRatio);
+};
 }
 
 }

AI/Nullkiller2/Engine/DeepDecomposer.cpp

@@ -27,7 +27,7 @@ namespace NK2AI
 using namespace Goals;
 
 DeepDecomposer::DeepDecomposer(const Nullkiller * aiNk)
-	:ai(aiNk), depth(0)
+	:aiNk(aiNk), depth(0)
 {
 }
 
@@ -139,7 +139,7 @@ Goals::TSubgoal DeepDecomposer::aggregateGoals(int startDepth, TSubgoal last)
 
 Goals::TSubgoal DeepDecomposer::unwrapComposition(Goals::TSubgoal goal)
 {
-	return goal->goalType == Goals::COMPOSITION ? goal->decompose(ai).back() : goal;
+	return goal->goalType == Goals::COMPOSITION ? goal->decompose(aiNk).back() : goal;
 }
 
 bool isEquivalentGoals(TSubgoal goal1, TSubgoal goal2)
@@ -159,7 +159,7 @@ bool isEquivalentGoals(TSubgoal goal1, TSubgoal goal2)
 
 bool DeepDecomposer::isCompositionLoop(TSubgoal goal)
 {
-	auto goalsToTest = goal->goalType == Goals::COMPOSITION ? goal->decompose(ai) : TGoalVec{goal};
+	auto goalsToTest = goal->goalType == Goals::COMPOSITION ? goal->decompose(aiNk) : TGoalVec{goal};
 
 	for(auto goalToTest : goalsToTest)
 	{
@@ -209,7 +209,7 @@ TGoalVec DeepDecomposer::decomposeCached(TSubgoal goal, bool & fromCache)
 
 	fromCache = false;
 
-	return goal->decompose(ai);
+	return goal->decompose(aiNk);
 }
 
 void DeepDecomposer::addToCache(TSubgoal goal)

AI/Nullkiller2/Engine/DeepDecomposer.h

@@ -30,7 +30,7 @@ private:
 	std::vector<Goals::TGoalVec> goals;
 	std::vector<TGoalHashSet> decompositionCache;
 	int depth;
-	const Nullkiller * ai;
+	const Nullkiller * aiNk;
 
 public:
 	DeepDecomposer(const Nullkiller * aiNk);

AI/Nullkiller2/Engine/FuzzyHelper.h

@@ -22,7 +22,7 @@ private:
 	TacticalAdvantageEngine tacticalAdvantageEngine;
 
 public:
-	FuzzyHelper(const Nullkiller * ai): aiNk(ai) {}
+	FuzzyHelper(const Nullkiller * aiNk): aiNk(aiNk) {}
 
 	ui64 evaluateDanger(const CGObjectInstance * obj);
 	ui64 evaluateDanger(const int3 & tile, const CGHeroInstance * visitor, bool checkGuards = true);

AI/Nullkiller2/Engine/Nullkiller.h

@@ -107,14 +107,15 @@ public:
 	std::unique_ptr<Settings> settings;
 	/// Same value as AIGateway->playerID
 	PlayerColor playerID;
+	/// Same value as AIGateway->cc
 	std::shared_ptr<CCallback> cc;
 	std::mutex aiStateMutex;
 	mutable ThreadInterruption makingTurnInterrupption;
 
 	Nullkiller();
-	~Nullkiller();
+	virtual ~Nullkiller();
 	void init(std::shared_ptr<CCallback> cb, AIGateway * aiGw);
-	void makeTurn();
+	virtual void makeTurn();
 	bool makeTurnHelperPriorityPass(Goals::TGoalVec& tempResults, int passIndex);
 	bool isActive(const CGHeroInstance * hero) const { return activeHero == hero; }
 	bool isHeroLocked(const CGHeroInstance * hero) const;

AI/Nullkiller2/Engine/PriorityEvaluator.cpp

@@ -37,7 +37,7 @@ namespace NK2AI
 
 constexpr float MAX_CRITICAL_VALUE = 2.0f;
 
-EvaluationContext::EvaluationContext(const Nullkiller* ai)
+EvaluationContext::EvaluationContext(const Nullkiller* aiNk)
 	: movementCost(0.0),
 	manaCost(0),
 	danger(0),
@@ -52,7 +52,7 @@ EvaluationContext::EvaluationContext(const Nullkiller* ai)
 	turn(0),
 	strategicalValue(0),
 	conquestValue(0),
-	evaluator(ai),
+	evaluator(aiNk),
 	enemyHeroDangerRatio(0),
 	threat(0),
 	armyGrowth(0),
@@ -929,7 +929,7 @@ private:
 	const Nullkiller * aiNk;
 
 public:
-	ExecuteHeroChainEvaluationContextBuilder(const Nullkiller * ai) : aiNk(ai) {}
+	ExecuteHeroChainEvaluationContextBuilder(const Nullkiller * aiNk) : aiNk(aiNk) {}
 
 	void buildEvaluationContext(EvaluationContext & evaluationContext, Goals::TSubgoal task) const override
 	{
@@ -1045,7 +1045,7 @@ public:
 class ClusterEvaluationContextBuilder : public IEvaluationContextBuilder
 {
 public:
-	ClusterEvaluationContextBuilder(const Nullkiller * ai) {}
+	ClusterEvaluationContextBuilder(const Nullkiller * aiNk) {}
 
 	void buildEvaluationContext(EvaluationContext & evaluationContext, Goals::TSubgoal task) const override
 	{
@@ -1132,7 +1132,7 @@ private:
 	const Nullkiller * aiNk;
 
 public:
-	DismissHeroContextBuilder(const Nullkiller * ai) : aiNk(ai) {}
+	DismissHeroContextBuilder(const Nullkiller * aiNk) : aiNk(aiNk) {}
 
 	void buildEvaluationContext(EvaluationContext & evaluationContext, Goals::TSubgoal task) const override
 	{
@@ -1264,18 +1264,18 @@ uint64_t RewardEvaluator::getUpgradeArmyReward(const CGTownInstance * town, cons
 	return upgradedPower - creaturesToUpgrade.power;
 }
 
-PriorityEvaluator::PriorityEvaluator(const Nullkiller * ai)
-	:aiNk(ai)
+PriorityEvaluator::PriorityEvaluator(const Nullkiller * aiNk)
+	:aiNk(aiNk)
 {
 	initVisitTile();
-	evaluationContextBuilders.push_back(std::make_shared<ExecuteHeroChainEvaluationContextBuilder>(ai));
+	evaluationContextBuilders.push_back(std::make_shared<ExecuteHeroChainEvaluationContextBuilder>(aiNk));
 	evaluationContextBuilders.push_back(std::make_shared<BuildThisEvaluationContextBuilder>());
-	evaluationContextBuilders.push_back(std::make_shared<ClusterEvaluationContextBuilder>(ai));
+	evaluationContextBuilders.push_back(std::make_shared<ClusterEvaluationContextBuilder>(aiNk));
 	evaluationContextBuilders.push_back(std::make_shared<HeroExchangeEvaluator>());
 	evaluationContextBuilders.push_back(std::make_shared<ArmyUpgradeEvaluator>());
 	evaluationContextBuilders.push_back(std::make_shared<DefendTownEvaluator>());
 	evaluationContextBuilders.push_back(std::make_shared<ExchangeSwapTownHeroesContextBuilder>());
-	evaluationContextBuilders.push_back(std::make_shared<DismissHeroContextBuilder>(ai));
+	evaluationContextBuilders.push_back(std::make_shared<DismissHeroContextBuilder>(aiNk));
 	evaluationContextBuilders.push_back(std::make_shared<StayAtTownManaRecoveryEvaluator>());
 	evaluationContextBuilders.push_back(std::make_shared<ExplorePointEvaluator>());
 }

AI/Nullkiller2/Engine/PriorityEvaluator.h

@@ -32,7 +32,7 @@ class RewardEvaluator
 public:
 	const Nullkiller * aiNk;
 
-	RewardEvaluator(const Nullkiller * ai) : aiNk(ai) {}
+	RewardEvaluator(const Nullkiller * aiNk) : aiNk(aiNk) {}
 
 	uint64_t getArmyReward(const CGObjectInstance * target, const CGHeroInstance * hero, const CCreatureSet * army, bool checkGold) const;
 	uint64_t getArmyGrowth(const CGObjectInstance * target, const CGHeroInstance * hero, const CCreatureSet * army) const;
@@ -86,7 +86,7 @@ struct DLL_EXPORT EvaluationContext
 	int explorePriority;
 	float powerRatio;
 
-	EvaluationContext(const Nullkiller * ai);
+	EvaluationContext(const Nullkiller * aiNk);
 
 	void addNonCriticalStrategicalValue(float value);
 };
@@ -103,7 +103,7 @@ class Nullkiller;
 class PriorityEvaluator
 {
 public:
-	PriorityEvaluator(const Nullkiller * ai);
+	PriorityEvaluator(const Nullkiller * aiNk);
 	~PriorityEvaluator();
 	void initVisitTile();
 

AI/Nullkiller2/Goals/AbstractGoal.cpp

@@ -71,20 +71,15 @@ std::string AbstractGoal::toString() const
 	return desc;
 }
 
-bool AbstractGoal::operator==(const AbstractGoal & g) const
-{
-	return false;
-}
-
 //TODO: find out why the following are not generated automatically on MVS?
 bool TSubgoal::operator==(const TSubgoal & rhs) const
 {
 	return *get() == *rhs.get(); //comparison for Goals is overloaded, so they don't need to be identical to match
 }
 
-bool AbstractGoal::invalid() const
+bool TSubgoal::operator<(const TSubgoal & rhs) const
 {
-	return goalType == EGoals::INVALID;
+	return false;
 }
 
 }

AI/Nullkiller2/Goals/AbstractGoal.h

@@ -112,10 +112,12 @@ namespace Goals
 		const CGHeroInstance * hero; SETTER(CGHeroInstance *, hero)
 		const CGTownInstance *town; SETTER(CGTownInstance *, town)
 		int bid; SETTER(int, bid)
+		EGoals goalType;
 
-		AbstractGoal(EGoals goal = EGoals::INVALID): goalType(goal)
+		explicit AbstractGoal(EGoals goal = EGoals::INVALID): goalType(goal)
 		{
-			isAbstract = false;
+			// isAbstract = false;
+			isAbstract = true;
 			value = 0;
 			aid = -1;
 			resID = -1;
@@ -126,25 +128,31 @@ namespace Goals
 			bid = -1;
 			goldCost = 0;
 		}
-		virtual ~AbstractGoal() {}
+
+		virtual ~AbstractGoal() = default;
 		//FIXME: abstract goal should be abstract, but serializer fails to instantiate subgoals in such case
 		virtual AbstractGoal * clone() const
 		{
 			return const_cast<AbstractGoal *>(this);
+			// auto * copy = new AbstractGoal(*this);
+			// return copy;
 		}
 
-		virtual TGoalVec decompose(const Nullkiller * ai) const
+		virtual TGoalVec decompose(const Nullkiller * aiNk) const
 		{
 			return TGoalVec();
 		}
 
-		EGoals goalType;
-
 		virtual std::string toString() const;
 
-		bool invalid() const;
+		// virtual bool invalid() const;
+		virtual bool invalid() const
+		{
+			return goalType == EGoals::INVALID;
+		}
 		
-		virtual bool operator==(const AbstractGoal & g) const;
+		// virtual bool operator==(const AbstractGoal & g) const;
+		virtual bool operator==(const AbstractGoal & g) const { return false; }
 
 		virtual bool isElementar() const { return false; }
 
@@ -175,7 +183,7 @@ namespace Goals
 		virtual void accept(AIGateway * aiGw) = 0; //unhandled goal will report standard error
 		virtual std::string toString() const = 0;
 		virtual const CGHeroInstance * getHero() const = 0;
-		virtual ~ITask() {}
+		virtual ~ITask() = default;
 		virtual int getHeroExchangeCount() const = 0;
 		virtual bool isObjectAffected(ObjectInstanceID h) const = 0;
 		virtual std::vector<ObjectInstanceID> getAffectedObjects() const = 0;

AI/Nullkiller2/Goals/CGoal.h

@@ -22,17 +22,19 @@ namespace Goals
 	class DLL_EXPORT CGoal : public AbstractGoal
 	{
 	public:
-		CGoal(EGoals goal = INVALID) : AbstractGoal(goal)
+		explicit CGoal(EGoals goal = INVALID) : AbstractGoal(goal)
 		{
-			isAbstract = true;
-			value = 0;
-			aid = -1;
-			objid = -1;
-			resID = -1;
-			tile = int3(-1, -1, -1);
-			town = nullptr;
+			// isAbstract = true;
+			// value = 0;
+			// aid = -1;
+			// objid = -1;
+			// resID = -1;
+			// tile = int3(-1, -1, -1);
+			// town = nullptr;
 		}
 
+		~CGoal() override = default;
+
 		CGoal * clone() const override
 		{
 			return new T(static_cast<T const &>(*this)); //casting enforces template instantiation
@@ -48,9 +50,9 @@ namespace Goals
 
 		virtual bool operator==(const T & other) const = 0;
 
-		TGoalVec decompose(const Nullkiller * ai) const override
+		TGoalVec decompose(const Nullkiller * aiNk) const override
 		{
-			TSubgoal single = decomposeSingle(ai);
+			TSubgoal single = decomposeSingle(aiNk);
 
 			if(!single || single->invalid())
 				return {};
@@ -59,7 +61,7 @@ namespace Goals
 		}
 
 	protected:
-		virtual TSubgoal decomposeSingle(const Nullkiller * ai) const
+		virtual TSubgoal decomposeSingle(const Nullkiller * aiNk) const
 		{
 			return TSubgoal();
 		}

AI/Nullkiller2/Goals/CompleteQuest.cpp

@@ -31,37 +31,37 @@ std::string CompleteQuest::toString() const
 	return "Complete quest " + questToString();
 }
 
-TGoalVec CompleteQuest::decompose(const Nullkiller * ai) const
+TGoalVec CompleteQuest::decompose(const Nullkiller * aiNk) const
 {
 	if(isKeyMaster(q))
 	{
-		return missionKeymaster(ai);
+		return missionKeymaster(aiNk);
 	}
 
 	logAi->debug("Trying to realize quest: %s", questToString());
 	auto quest = q.getQuest(ccTl);
 
 	if(!quest->mission.artifacts.empty())
-		return missionArt(ai);
+		return missionArt(aiNk);
 
 	if(!quest->mission.heroes.empty())
-		return missionHero(ai);
+		return missionHero(aiNk);
 
 	if(!quest->mission.creatures.empty())
-		return missionArmy(ai);
+		return missionArmy(aiNk);
 
 	if(quest->mission.resources.nonZero())
-		return missionResources(ai);
+		return missionResources(aiNk);
 
 	if(quest->killTarget != ObjectInstanceID::NONE)
-		return missionDestroyObj(ai);
+		return missionDestroyObj(aiNk);
 
 	for(auto & s : quest->mission.primary)
 		if(s)
-			return missionIncreasePrimaryStat(ai);
+			return missionIncreasePrimaryStat(aiNk);
 
 	if(quest->mission.heroLevel > 0)
-		return missionLevel(ai);
+		return missionLevel(aiNk);
 
 	return TGoalVec();
 }
@@ -118,9 +118,9 @@ TGoalVec CompleteQuest::tryCompleteQuest(const Nullkiller * aiNk) const
 	return CaptureObjectsBehavior::getVisitGoals(paths, aiNk, q.getObject(ccTl));
 }
 
-TGoalVec CompleteQuest::missionArt(const Nullkiller * ai) const
+TGoalVec CompleteQuest::missionArt(const Nullkiller * aiNk) const
 {
-	TGoalVec solutions = tryCompleteQuest(ai);
+	TGoalVec solutions = tryCompleteQuest(aiNk);
 
 	if(!solutions.empty())
 		return solutions;
@@ -135,9 +135,9 @@ TGoalVec CompleteQuest::missionArt(const Nullkiller * ai) const
 	return solutions;
 }
 
-TGoalVec CompleteQuest::missionHero(const Nullkiller * ai) const
+TGoalVec CompleteQuest::missionHero(const Nullkiller * aiNk) const
 {
-	TGoalVec solutions = tryCompleteQuest(ai);
+	TGoalVec solutions = tryCompleteQuest(aiNk);
 
 	if(solutions.empty())
 	{
@@ -160,31 +160,31 @@ TGoalVec CompleteQuest::missionArmy(const Nullkiller * aiNk) const
 	return CaptureObjectsBehavior::getVisitGoals(paths, aiNk, q.getObject(ccTl));
 }
 
-TGoalVec CompleteQuest::missionIncreasePrimaryStat(const Nullkiller * ai) const
+TGoalVec CompleteQuest::missionIncreasePrimaryStat(const Nullkiller * aiNk) const
 {
-	return tryCompleteQuest(ai);
+	return tryCompleteQuest(aiNk);
 }
 
-TGoalVec CompleteQuest::missionLevel(const Nullkiller * ai) const
+TGoalVec CompleteQuest::missionLevel(const Nullkiller * aiNk) const
 {
-	return tryCompleteQuest(ai);
+	return tryCompleteQuest(aiNk);
 }
 
-TGoalVec CompleteQuest::missionKeymaster(const Nullkiller * ai) const
+TGoalVec CompleteQuest::missionKeymaster(const Nullkiller * aiNk) const
 {
-	if(isObjectPassable(ai, q.getObject(ccTl)))
+	if(isObjectPassable(aiNk, q.getObject(ccTl)))
 	{
-		return CaptureObjectsBehavior(q.getObject(ccTl)).decompose(ai);
+		return CaptureObjectsBehavior(q.getObject(ccTl)).decompose(aiNk);
 	}
 	else
 	{
-		return CaptureObjectsBehavior().ofType(Obj::KEYMASTER, q.getObject(ccTl)->subID).decompose(ai);
+		return CaptureObjectsBehavior().ofType(Obj::KEYMASTER, q.getObject(ccTl)->subID).decompose(aiNk);
 	}
 }
 
-TGoalVec CompleteQuest::missionResources(const Nullkiller * ai) const
+TGoalVec CompleteQuest::missionResources(const Nullkiller * aiNk) const
 {
-	TGoalVec solutions = tryCompleteQuest(ai);
+	TGoalVec solutions = tryCompleteQuest(aiNk);
 	return solutions;
 }
 

AI/Nullkiller2/Goals/CompleteQuest.h

@@ -28,7 +28,7 @@ namespace Goals
 		{
 		}
 
-		Goals::TGoalVec decompose(const Nullkiller * ai) const override;
+		Goals::TGoalVec decompose(const Nullkiller * aiNk) const override;
 		std::string toString() const override;
 		bool hasHash() const override { return true; }
 		uint64_t getHash() const override;
@@ -37,14 +37,14 @@ namespace Goals
 
 	private:
 		TGoalVec tryCompleteQuest(const Nullkiller * aiNk) const;
-		TGoalVec missionArt(const Nullkiller * ai) const;
-		TGoalVec missionHero(const Nullkiller * ai) const;
+		TGoalVec missionArt(const Nullkiller * aiNk) const;
+		TGoalVec missionHero(const Nullkiller * aiNk) const;
 		TGoalVec missionArmy(const Nullkiller * aiNk) const;
-		TGoalVec missionResources(const Nullkiller * ai) const;
+		TGoalVec missionResources(const Nullkiller * aiNk) const;
 		TGoalVec missionDestroyObj(const Nullkiller * aiNk) const;
-		TGoalVec missionIncreasePrimaryStat(const Nullkiller * ai) const;
-		TGoalVec missionLevel(const Nullkiller * ai) const;
-		TGoalVec missionKeymaster(const Nullkiller * ai) const;
+		TGoalVec missionIncreasePrimaryStat(const Nullkiller * aiNk) const;
+		TGoalVec missionLevel(const Nullkiller * aiNk) const;
+		TGoalVec missionKeymaster(const Nullkiller * aiNk) const;
 		std::string questToString() const;
 	};
 }

AI/Nullkiller2/Goals/Invalid.h

@@ -27,7 +27,7 @@ namespace Goals
 		{
 			priority = -1;
 		}
-		TGoalVec decompose(const Nullkiller * ai) const override
+		TGoalVec decompose(const Nullkiller * aiNk) const override
 		{
 			return TGoalVec();
 		}

AI/Nullkiller2/Helpers/ArmyFormation.h

@@ -28,7 +28,7 @@ private:
 	std::shared_ptr<CCallback> cb; //this is enough, but we downcast from CCallback
 
 public:
-	ArmyFormation(std::shared_ptr<CCallback> CB, const Nullkiller * ai): cb(CB) {}
+	ArmyFormation(std::shared_ptr<CCallback> CB, const Nullkiller * aiNk): cb(CB) {}
 
 	void rearrangeArmyForSiege(const CGTownInstance * town, const CGHeroInstance * attacker);
 

AI/Nullkiller2/Pathfinding/AIPathfinder.cpp

@@ -18,8 +18,8 @@ namespace NK2AI
 
 std::map<ObjectInstanceID, std::unique_ptr<GraphPaths>>  AIPathfinder::heroGraphs;
 
-AIPathfinder::AIPathfinder(CPlayerSpecificInfoCallback * cb, Nullkiller * ai)
-	:cb(cb), aiNk(ai)
+AIPathfinder::AIPathfinder(CPlayerSpecificInfoCallback * cb, Nullkiller * aiNk)
+	:cb(cb), aiNk(aiNk)
 {
 }
 

AI/Nullkiller2/Pathfinding/AIPathfinder.h

@@ -44,7 +44,7 @@ private:
 	static std::map<ObjectInstanceID, std::unique_ptr<GraphPaths>>  heroGraphs;
 
 public:
-	AIPathfinder(CPlayerSpecificInfoCallback * cb, Nullkiller * ai);
+	AIPathfinder(CPlayerSpecificInfoCallback * cb, Nullkiller * aiNk);
 	void calculatePathInfo(std::vector<AIPath> & paths, const int3 & tile, bool includeGraph = false) const;
 	bool isTileAccessible(const HeroPtr & hero, const int3 & tile) const;
 	void updatePaths(const std::map<const CGHeroInstance *, HeroRole> & heroes, PathfinderSettings pathfinderSettings);

AI/Nullkiller2/Pathfinding/AIPathfinderConfig.cpp

@@ -23,17 +23,17 @@ namespace AIPathfinding
 {
 	std::vector<std::shared_ptr<IPathfindingRule>> makeRuleset(
 		CPlayerSpecificInfoCallback * cb,
-		Nullkiller * ai,
+		Nullkiller * aiNk,
 		std::shared_ptr<AINodeStorage> nodeStorage,
 		bool allowBypassObjects)
 	{
 			std::vector<std::shared_ptr<IPathfindingRule>> rules = {
-				std::make_shared<AILayerTransitionRule>(cb, ai, nodeStorage),
+				std::make_shared<AILayerTransitionRule>(cb, aiNk, nodeStorage),
 				std::make_shared<DestinationActionRule>(),
 				std::make_shared<AIMovementToDestinationRule>(nodeStorage, allowBypassObjects),
 				std::make_shared<MovementCostRule>(),
 				std::make_shared<AIPreviousNodeRule>(nodeStorage),
-				std::make_shared<AIMovementAfterDestinationRule>(ai, cb, nodeStorage, allowBypassObjects)
+				std::make_shared<AIMovementAfterDestinationRule>(aiNk, cb, nodeStorage, allowBypassObjects)
 			};
 
 		return rules;

AI/Nullkiller2/Pathfinding/Actions/AdventureSpellCastMovementActions.cpp

@@ -56,7 +56,7 @@ namespace AIPathfinding
 		Goals::AdventureSpellCast(hero, spellToCast).accept(aiGw);
 	}
 
-	bool AdventureCastAction::canAct(const Nullkiller * ai, const AIPathNode * source) const
+	bool AdventureCastAction::canAct(const Nullkiller * aiNk, const AIPathNode * source) const
 	{
 		assert(hero == this->hero);
 

AI/Nullkiller2/Pathfinding/Actions/AdventureSpellCastMovementActions.h

@@ -38,7 +38,7 @@ namespace AIPathfinding
 			AIPathNode * dstMode,
 			const AIPathNode * srcNode) const override;
 
-		bool canAct(const Nullkiller * ai, const AIPathNode * source) const override;
+		bool canAct(const Nullkiller * aiNk, const AIPathNode * source) const override;
 
 		std::string toString() const override;
 	};

AI/Nullkiller2/Pathfinding/Actions/BoatActions.cpp

@@ -37,7 +37,7 @@ namespace AIPathfinding
 		return Goals::sptr(Goals::Invalid());
 	}
 
-	bool BuildBoatAction::canAct(const Nullkiller * ai, const CGHeroInstance * hero, const TResources & reservedResources) const
+	bool BuildBoatAction::canAct(const Nullkiller * aiNk, const CGHeroInstance * hero, const TResources & reservedResources) const
 	{
 		if(cpsic->getPlayerRelations(hero->tempOwner, shipyard->getObject()->getOwner()) == PlayerRelations::ENEMIES)
 		{
@@ -63,16 +63,16 @@ namespace AIPathfinding
 		return true;
 	}
 
-	bool BuildBoatAction::canAct(const Nullkiller * ai, const AIPathNode * source) const
+	bool BuildBoatAction::canAct(const Nullkiller * aiNk, const AIPathNode * source) const
 	{
-		return canAct(ai, source->actor->hero, source->actor->armyCost);
+		return canAct(aiNk, source->actor->hero, source->actor->armyCost);
 	}
 
-	bool BuildBoatAction::canAct(const Nullkiller * ai, const AIPathNodeInfo & source) const
+	bool BuildBoatAction::canAct(const Nullkiller * aiNk, const AIPathNodeInfo & source) const
 	{
 		TResources res;
 
-		return canAct(ai, source.targetHero, res);
+		return canAct(aiNk, source.targetHero, res);
 	}
 
 	const CGObjectInstance * BuildBoatAction::targetObject() const

AI/Nullkiller2/Pathfinding/Actions/BoatActions.h

@@ -62,9 +62,9 @@ namespace AIPathfinding
 		{
 		}
 
-		bool canAct(const Nullkiller * ai, const AIPathNode * source) const override;
-		bool canAct(const Nullkiller * ai, const AIPathNodeInfo & source) const override;
-		bool canAct(const Nullkiller * ai, const CGHeroInstance * hero, const TResources & reservedResources) const;
+		bool canAct(const Nullkiller * aiNk, const AIPathNode * source) const override;
+		bool canAct(const Nullkiller * aiNk, const AIPathNodeInfo & source) const override;
+		bool canAct(const Nullkiller * aiNk, const CGHeroInstance * hero, const TResources & reservedResources) const;
 
 		void execute(AIGateway * aiGw, const CGHeroInstance * hero) const override;
 

AI/Nullkiller2/Pathfinding/Actions/BuyArmyAction.h

@@ -21,7 +21,7 @@ namespace AIPathfinding
 	private:
 
 	public:
-		bool canAct(const Nullkiller * ai, const AIPathNode * source) const override
+		bool canAct(const Nullkiller * aiNk, const AIPathNode * source) const override
 		{
 			return true;
 		}

AI/Nullkiller2/Pathfinding/Actions/QuestAction.cpp

@@ -19,14 +19,14 @@ namespace NK2AI
 
 namespace AIPathfinding
 {
-	bool QuestAction::canAct(const Nullkiller * ai, const AIPathNode * node) const
+	bool QuestAction::canAct(const Nullkiller * aiNk, const AIPathNode * node) const
 	{
-		return canAct(ai, node->actor->hero);
+		return canAct(aiNk, node->actor->hero);
 	}
 
-	bool QuestAction::canAct(const Nullkiller * ai, const AIPathNodeInfo & node) const
+	bool QuestAction::canAct(const Nullkiller * aiNk, const AIPathNodeInfo & node) const
 	{
-		return canAct(ai, node.targetHero);
+		return canAct(aiNk, node.targetHero);
 	}
 
 	bool QuestAction::canAct(const Nullkiller * aiNk, const CGHeroInstance * hero) const
@@ -45,7 +45,7 @@ namespace AIPathfinding
 			|| quest->checkQuest(hero);
 	}
 
-	Goals::TSubgoal QuestAction::decompose(const Nullkiller * ai, const CGHeroInstance * hero) const
+	Goals::TSubgoal QuestAction::decompose(const Nullkiller * aiNk, const CGHeroInstance * hero) const
 	{
 		return Goals::sptr(Goals::CompleteQuest(questInfo));
 	}

AI/Nullkiller2/Pathfinding/Actions/QuestAction.h

@@ -28,11 +28,11 @@ namespace AIPathfinding
 		{
 		}
 
-		bool canAct(const Nullkiller * ai, const AIPathNode * node) const override;
-		bool canAct(const Nullkiller * ai, const AIPathNodeInfo & node) const override;
+		bool canAct(const Nullkiller * aiNk, const AIPathNode * node) const override;
+		bool canAct(const Nullkiller * aiNk, const AIPathNodeInfo & node) const override;
 		bool canAct(const Nullkiller * aiNk, const CGHeroInstance * hero) const;
 
-		Goals::TSubgoal decompose(const Nullkiller * ai, const CGHeroInstance * hero) const override;
+		Goals::TSubgoal decompose(const Nullkiller * aiNk, const CGHeroInstance * hero) const override;
 
 		void execute(AIGateway * aiGw, const CGHeroInstance * hero) const override;
 

AI/Nullkiller2/Pathfinding/Actions/SpecialAction.cpp

@@ -17,7 +17,7 @@
 namespace NK2AI
 {
 
-Goals::TSubgoal SpecialAction::decompose(const Nullkiller * ai, const CGHeroInstance * hero) const
+Goals::TSubgoal SpecialAction::decompose(const Nullkiller * aiNk, const CGHeroInstance * hero) const
 {
 	return Goals::sptr(Goals::Invalid());
 }
@@ -27,26 +27,26 @@ void SpecialAction::execute(AIGateway * aiGw, const CGHeroInstance * hero) const
 	throw cannotFulfillGoalException("Can not execute " + toString());
 }
 
-bool CompositeAction::canAct(const Nullkiller * ai, const AIPathNode * source) const
+bool CompositeAction::canAct(const Nullkiller * aiNk, const AIPathNode * source) const
 {
 	for(auto part : parts)
 	{
-		if(!part->canAct(ai, source)) return false;
+		if(!part->canAct(aiNk, source)) return false;
 	}
 
 	return true;
 }
 
-Goals::TSubgoal CompositeAction::decompose(const Nullkiller * ai, const CGHeroInstance * hero) const
+Goals::TSubgoal CompositeAction::decompose(const Nullkiller * aiNk, const CGHeroInstance * hero) const
 {
 	for(auto part : parts)
 	{
-		auto goal = part->decompose(ai, hero);
+		auto goal = part->decompose(aiNk, hero);
 
 		if(!goal->invalid()) return goal;
 	}
 
-	return SpecialAction::decompose(ai, hero);
+	return SpecialAction::decompose(aiNk, hero);
 }
 
 void CompositeAction::execute(AIGateway * aiGw, const CGHeroInstance * hero) const

AI/Nullkiller2/Pathfinding/Actions/SpecialAction.h

@@ -30,17 +30,17 @@ class SpecialAction
 public:
 	virtual ~SpecialAction() = default;
 
-	virtual bool canAct(const Nullkiller * ai, const AIPathNode * source) const
+	virtual bool canAct(const Nullkiller * aiNk, const AIPathNode * source) const
 	{
 		return true;
 	}
 
-	virtual bool canAct(const Nullkiller * ai, const AIPathNodeInfo & source) const
+	virtual bool canAct(const Nullkiller * aiNk, const AIPathNodeInfo & source) const
 	{
 		return true;
 	}
 
-	virtual Goals::TSubgoal decompose(const Nullkiller * ai, const CGHeroInstance * hero) const;
+	virtual Goals::TSubgoal decompose(const Nullkiller * aiNk, const CGHeroInstance * hero) const;
 
 	virtual void execute(AIGateway * aiGw, const CGHeroInstance * hero) const;
 
@@ -76,11 +76,11 @@ private:
 public:
 	CompositeAction(std::vector<std::shared_ptr<const SpecialAction>> parts) : parts(parts) {}
 
-	bool canAct(const Nullkiller * ai, const AIPathNode * source) const override;
+	bool canAct(const Nullkiller * aiNk, const AIPathNode * source) const override;
 	void execute(AIGateway * aiGw, const CGHeroInstance * hero) const override;
 	std::string toString() const override;
 	const CGObjectInstance * targetObject() const override;
-	Goals::TSubgoal decompose(const Nullkiller * ai, const CGHeroInstance * hero) const override;
+	Goals::TSubgoal decompose(const Nullkiller * aiNk, const CGHeroInstance * hero) const override;
 
 	std::vector<std::shared_ptr<const SpecialAction>> getParts() const override
 	{
@@ -98,7 +98,7 @@ public:
 class ISpecialActionFactory
 {
 public:
-	virtual std::shared_ptr<SpecialAction> create(const Nullkiller * ai) = 0;
+	virtual std::shared_ptr<SpecialAction> create(const Nullkiller * aiNk) = 0;
 	virtual ~ISpecialActionFactory() = default;
 };
 

AI/Nullkiller2/Pathfinding/Actors.cpp

@@ -97,10 +97,10 @@ std::string ObjectActor::toString() const
 	return object->getObjectName() + " at " + object->visitablePos().toString();
 }
 
-HeroActor::HeroActor(const CGHeroInstance * hero, HeroRole heroRole, uint64_t chainMask, const Nullkiller * ai)
+HeroActor::HeroActor(const CGHeroInstance * hero, HeroRole heroRole, uint64_t chainMask, const Nullkiller * aiNk)
 	:ChainActor(hero, heroRole, chainMask)
 {
-	exchangeMap.reset(new HeroExchangeMap(this, ai));
+	exchangeMap.reset(new HeroExchangeMap(this, aiNk));
 	setupSpecialActors();
 }
 
@@ -108,10 +108,10 @@ HeroActor::HeroActor(
 	const ChainActor * carrier,
 	const ChainActor * other,
 	const HeroExchangeArmy * army,
-	const Nullkiller * ai)
+	const Nullkiller * aiNk)
 	:ChainActor(carrier, other, army)
 {
-	exchangeMap.reset(new HeroExchangeMap(this, ai));
+	exchangeMap.reset(new HeroExchangeMap(this, aiNk));
 	armyCost += army->armyCost;
 	actorAction = army->getActorAction();
 	setupSpecialActors();
@@ -185,8 +185,8 @@ ExchangeResult HeroActor::tryExchangeNoLock(const ChainActor * specialActor, con
 	return result;
 }
 
-HeroExchangeMap::HeroExchangeMap(const HeroActor * actor, const Nullkiller * ai)
-	:actor(actor), aiNk(ai), sync()
+HeroExchangeMap::HeroExchangeMap(const HeroActor * actor, const Nullkiller * aiNk)
+	:actor(actor), aiNk(aiNk), sync()
 {
 }
 

AI/Nullkiller2/Pathfinding/Actors.h

@@ -96,7 +96,7 @@ private:
 	std::shared_mutex sync;
 
 public:
-	HeroExchangeMap(const HeroActor * actor, const Nullkiller * ai);
+	HeroExchangeMap(const HeroActor * actor, const Nullkiller * aiNk);
 	~HeroExchangeMap();
 
 	ExchangeResult tryExchangeNoLock(const ChainActor * other);
@@ -121,8 +121,8 @@ public:
 	std::shared_ptr<SpecialAction> exchangeAction;
 	// chain flags, can be combined meaning hero exchange and so on
 
-	HeroActor(const CGHeroInstance * hero, HeroRole heroRole, uint64_t chainMask, const Nullkiller * ai);
-	HeroActor(const ChainActor * carrier, const ChainActor * other, const HeroExchangeArmy * army, const Nullkiller * ai);
+	HeroActor(const CGHeroInstance * hero, HeroRole heroRole, uint64_t chainMask, const Nullkiller * aiNk);
+	HeroActor(const ChainActor * carrier, const ChainActor * other, const HeroExchangeArmy * army, const Nullkiller * aiNk);
 
 protected:
 	ExchangeResult tryExchangeNoLock(const ChainActor * specialActor, const ChainActor * other) const override;

AI/Nullkiller2/Pathfinding/GraphPaths.cpp

@@ -33,14 +33,14 @@ GraphPaths::GraphPaths()
 }
 
 std::shared_ptr<SpecialAction> getCompositeAction(
-	const Nullkiller * ai,
+	const Nullkiller * aiNk,
 	std::shared_ptr<ISpecialActionFactory> linkActionFactory,
 	std::shared_ptr<SpecialAction> transitionAction)
 {
 	if(!linkActionFactory)
 		return transitionAction;
 
-	auto linkAction = linkActionFactory->create(ai);
+	auto linkAction = linkActionFactory->create(aiNk);
 
 	if(!transitionAction)
 		return linkAction;

AI/Nullkiller2/Pathfinding/ObjectGraphCalculator.cpp

@@ -20,8 +20,8 @@
 namespace NK2AI
 {
 
-ObjectGraphCalculator::ObjectGraphCalculator(ObjectGraph * target, const Nullkiller * ai)
-	:aiNk(ai), target(target), syncLock()
+ObjectGraphCalculator::ObjectGraphCalculator(ObjectGraph * target, const Nullkiller * aiNk)
+	:aiNk(aiNk), target(target), syncLock()
 {
 }
 

AI/Nullkiller2/Pathfinding/ObjectGraphCalculator.h

@@ -37,7 +37,7 @@ private:
 	std::vector<std::unique_ptr<CGHeroInstance>> temporaryActorHeroes;
 
 public:
-	ObjectGraphCalculator(ObjectGraph * target, const Nullkiller * ai);
+	ObjectGraphCalculator(ObjectGraph * target, const Nullkiller * aiNk);
 	void setGraphObjects();
 	void calculateConnections();
 	float getNeighborConnectionsCost(const int3 & pos, std::vector<AIPath> & pathCache);

AI/Nullkiller2/Pathfinding/Rules/AILayerTransitionRule.cpp

@@ -21,9 +21,9 @@ namespace AIPathfinding
 {
 	AILayerTransitionRule::AILayerTransitionRule(
 		CPlayerSpecificInfoCallback * cb,
-		Nullkiller * ai,
+		Nullkiller * aiNk,
 		std::shared_ptr<AINodeStorage> nodeStorage)
-		:cb(cb), aiNk(ai), nodeStorage(nodeStorage)
+		:cb(cb), aiNk(aiNk), nodeStorage(nodeStorage)
 	{
 		setup();
 	}

AI/Nullkiller2/Pathfinding/Rules/AILayerTransitionRule.h

@@ -35,7 +35,7 @@ namespace AIPathfinding
 	public:
 		AILayerTransitionRule(
 			CPlayerSpecificInfoCallback * cb,
-			Nullkiller * ai,
+			Nullkiller * aiNk,
 			std::shared_ptr<AINodeStorage> nodeStorage);
 
 		virtual void process(

AI/Nullkiller2/Pathfinding/Rules/AIMovementAfterDestinationRule.cpp

@@ -23,11 +23,11 @@ namespace NK2AI
 namespace AIPathfinding
 {
 	AIMovementAfterDestinationRule::AIMovementAfterDestinationRule(
-		const Nullkiller * ai,
+		const Nullkiller * aiNk,
 		CPlayerSpecificInfoCallback * cb, 
 		std::shared_ptr<AINodeStorage> nodeStorage,
 		bool allowBypassObjects)
-		:aiNk(ai), cb(cb), nodeStorage(nodeStorage), allowBypassObjects(allowBypassObjects)
+		:aiNk(aiNk), cb(cb), nodeStorage(nodeStorage), allowBypassObjects(allowBypassObjects)
 	{
 	}
 

AI/Nullkiller2/Pathfinding/Rules/AIMovementAfterDestinationRule.h

@@ -29,7 +29,7 @@ namespace AIPathfinding
 
 	public:
 		AIMovementAfterDestinationRule(
-			const Nullkiller * ai,
+			const Nullkiller * aiNk,
 			CPlayerSpecificInfoCallback * cb,
 			std::shared_ptr<AINodeStorage> nodeStorage,
 			bool allowBypassObjects);

test/CMakeLists.txt

@@ -72,7 +72,7 @@ set(test_SRCS
 		spells/targetConditions/ResistanceConditionTest.cpp
  		spells/targetConditions/SpellEffectConditionTest.cpp
  		spells/targetConditions/TargetConditionItemFixture.cpp
-		
+
 		mock/BattleFake.cpp
 		mock/mock_IGameEventCallback.cpp
  		mock/mock_MapService.cpp
@@ -116,8 +116,8 @@ if(ENABLE_LUA)
 	)
 endif()
 
-if(ENABLE_ERM) 
-	list(APPEND test_SRCS 
+if(ENABLE_ERM)
+	list(APPEND test_SRCS
 		erm/ERM_BM.cpp
 		erm/ERM_BU.cpp
 		erm/ERM_FU.cpp
@@ -140,6 +140,15 @@ if(ENABLE_ERM)
 	)
 endif()
 
+if(ENABLE_NULLKILLER_AI)
+	list(APPEND test_SRCS
+			nullkiller2/Behaviors/RecruitHeroBehaviorTest.cpp
+	)
+
+	list(APPEND test_HEADERS
+	)
+endif()
+
 assign_source_group(${test_SRCS} ${test_HEADERS})
 
 set(mock_HEADERS
@@ -182,6 +191,12 @@ target_link_libraries(vcmitest PRIVATE gtest gmock vcmi ${SYSTEM_LIBS})
 if(ENABLE_LUA)
 	target_link_libraries(vcmitest PRIVATE vcmiLua)
 endif()
+if(ENABLE_NULLKILLER_AI)
+	target_link_libraries(vcmitest PRIVATE vcmi fuzzylite::fuzzylite TBB::tbb)
+	# TODO: Mircea: fix code to eliminate all linking issues, then remove the options below
+	target_link_options(vcmitest PRIVATE -Wl,--warn-unresolved-symbols)
+	#	target_link_libraries(vcmitest PUBLIC Nullkiller2)
+endif()
 
 target_include_directories(vcmitest
 		PUBLIC	${CMAKE_CURRENT_SOURCE_DIR}

test/nullkiller2/Behaviors/RecruitHeroBehaviorTest.cpp

@@ -0,0 +1,31 @@
+/*
+ * PriorityEvaluatorTest.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#include "AI/Nullkiller2/Behaviors/RecruitHeroBehavior.h"
+#include "AI/Nullkiller2/Engine/Nullkiller.h"
+
+class MockNullkiller : public NK2AI::Nullkiller
+{
+public:
+	~MockNullkiller() override = default;
+	MOCK_METHOD(void, makeTurn, (), (override));
+};
+
+TEST(Nullkiller2_Behaviors_RecruitHeroBehavior, calculateBestHero)
+{
+	EXPECT_EQ(1, 1);
+	auto behavior = NK2AI::Goals::RecruitHeroBehavior();
+	EXPECT_FALSE(behavior.invalid());
+	EXPECT_EQ(1, 1);
+	auto * const aiNk = new MockNullkiller();
+	EXPECT_CALL(*aiNk, makeTurn()).Times(1);
+	aiNk->makeTurn();
+	delete aiNk;
+}