overall refactoring, HeroPtr adjustments, AiNodeStorage remove unnecessary callback

2025-11-25 22:42:04 +02:00 · 2025-09-04 00:07:12 +02:00
parent 19076ee29d
commit 7a40981a95
22 changed files with 209 additions and 208 deletions
--- a/AI/Nullkiller2/AIGateway.cpp
+++ b/AI/Nullkiller2/AIGateway.cpp
@@ -194,7 +194,7 @@ void AIGateway::gameOver(PlayerColor player, const EVictoryLossCheckResult & vic
 			logAi->debug("AIGateway: Player %d (%s) lost. It's me. What a disappointment! :(", player, player.toString());
 		}

-		nullkiller->makingTurnInterrupption.interruptThread();
+		nullkiller->makingTurnInterruption.interruptThread();
 	}
 }

@@ -572,7 +572,7 @@ void AIGateway::yourTurn(QueryID queryID)
 	executeActionAsync("yourTurn", [this, queryID](){ answerQuery(queryID, 0); });
 	status.startedTurn();

-	nullkiller->makingTurnInterrupption.reset();
+	nullkiller->makingTurnInterruption.reset();

 	asyncTasks->run([this]()
 	{
@@ -593,11 +593,11 @@ void AIGateway::heroGotLevel(const CGHeroInstance * hero, PrimarySkill pskill, s
 	{
 		int sel = 0;

-		if(heroPtr.isValid())
+		if(heroPtr.isVerified())
 		{
 			std::unique_lock lockGuard(nullkiller->aiStateMutex);
 			nullkiller->heroManager->update();
-			sel = nullkiller->heroManager->selectBestSkill(heroPtr, skills);
+			sel = nullkiller->heroManager->selectBestSkillIndex(heroPtr, skills);
 		}

 		answerQuery(queryID, sel);
@@ -630,7 +630,7 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 			bool answer = true;
 			auto objects = cc->getVisitableObjs(target);

-			if(heroPtr.isValid() && target.isValid() && !objects.empty())
+			if(heroPtr.isVerified() && target.isValid() && !objects.empty())
 			{
 				auto topObj = objects.front()->id == heroPtr->id ? objects.back() : objects.front();
 				auto objType = topObj->ID; // top object should be our hero
@@ -683,10 +683,10 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 				std::unique_lock mxLock(nullkiller->aiStateMutex);

 				// TODO: Find better way to understand it is Chest of Treasures
-				if(heroPtr.isValid()
+				if(heroPtr.isVerified()
 					&& components.size() == 2
 					&& components.front().type == ComponentType::RESOURCE
-					&& (nullkiller->heroManager->getHeroRole(heroPtr) != HeroRole::MAIN
+					&& (nullkiller->heroManager->getHeroRoleOrDefault(heroPtr) != HeroRole::MAIN
 						|| nullkiller->buildAnalyzer->isGoldPressureOverMax()))
 				{
 					sel = 1;
@@ -873,7 +873,7 @@ void AIGateway::performObjectInteraction(const CGObjectInstance * obj, HeroPtr h
 			if(!heroPtr->getVisitedTown()->getGarrisonHero() || !nullkiller->isHeroLocked(heroPtr->getVisitedTown()->getGarrisonHero()))
 				moveCreaturesToHero(heroPtr->getVisitedTown());

-			if(nullkiller->heroManager->getHeroRole(heroPtr) == HeroRole::MAIN && !heroPtr->hasSpellbook()
+			if(nullkiller->heroManager->getHeroRoleOrDefault(heroPtr) == HeroRole::MAIN && !heroPtr->hasSpellbook()
 				&& nullkiller->getFreeGold() >= GameConstants::SPELLBOOK_GOLD_COST)
 			{
 				if(heroPtr->getVisitedTown()->hasBuilt(BuildingID::MAGES_GUILD_1))
@@ -1406,7 +1406,7 @@ void AIGateway::buildArmyIn(const CGTownInstance * t)

 void AIGateway::finish()
 {
-	nullkiller->makingTurnInterrupption.interruptThread();
+	nullkiller->makingTurnInterruption.interruptThread();

 	if (asyncTasks)
 	{
@@ -1429,13 +1429,13 @@ void AIGateway::executeActionAsync(const std::string & description, const std::f
 	});
 }

-void AIGateway::lostHero(HeroPtr h)
+void AIGateway::lostHero(const HeroPtr & heroPtr) const
 {
-	logAi->debug("I lost my hero %s. It's best to forget and move on.", h.nameOrDefault());
+	logAi->debug("I lost my hero %s. It's best to forget and move on.", heroPtr.nameOrDefault());
 	nullkiller->invalidatePathfinderData();
 }

-void AIGateway::answerQuery(QueryID queryID, int selection)
+void AIGateway::answerQuery(const QueryID queryID, const int selection) const
 {
 	logAi->debug("I'll answer the query %d giving the choice %d", queryID, selection);
 	if(queryID != QueryID(-1))
@@ -1564,7 +1564,7 @@ void AIStatus::waitTillFree()
 	while(battle != NO_BATTLE || !remainingQueries.empty() || !objectsBeingVisited.empty() || ongoingHeroMovement)
 	{
 		cv.wait_for(lock, std::chrono::milliseconds(10));
-		aiGw->nullkiller->makingTurnInterrupption.interruptionPoint();
+		aiGw->nullkiller->makingTurnInterruption.interruptionPoint();
 	}
 }

--- a/AI/Nullkiller2/AIGateway.h
+++ b/AI/Nullkiller2/AIGateway.h
@@ -207,7 +207,7 @@ public:
 	bool moveHeroToTile(int3 dst, const HeroPtr & heroPtr);
 	void buildStructure(const CGTownInstance * t, BuildingID building);

-	void lostHero(HeroPtr h); //should remove all references to hero (assigned tasks and so on)
+	void lostHero(const HeroPtr & heroPtr) const; //should remove all references to hero (assigned tasks and so on)
 	void waitTillFree();

 	void validateObject(const CGObjectInstance * obj); //checks if object is still visible and if not, removes references to it
@@ -215,7 +215,7 @@ public:
 	virtual std::vector<const CGObjectInstance *> getFlaggedObjects() const;

 	void requestSent(const CPackForServer * pack, int requestID) override;
-	void answerQuery(QueryID queryID, int selection);
+	void answerQuery(QueryID queryID, int selection) const;
 	//special function that can be called ONLY from game events handling thread and will send request ASAP
 	void executeActionAsync(const std::string & description, const std::function<void()> & whatToDo);

--- a/AI/Nullkiller2/AIUtility.cpp
+++ b/AI/Nullkiller2/AIUtility.cpp
@@ -78,17 +78,24 @@ ObjectInstanceID HeroPtr::idOrNone() const
 std::string HeroPtr::nameOrDefault() const
 {
 	if (hero)
-		return hero->getNameTextID();
+		return hero->getNameTextID() + "(" + hero->getNameTranslated() + ")";
 	return "<NO HERO>";
 }

+// enforces isVerified()
 const CGHeroInstance * HeroPtr::get() const
 {
-	assert(isValid());
+	assert(isVerified());
 	return hero;
 }

-bool HeroPtr::validate() const
+// does not enforce isVerified()
+const CGHeroInstance * HeroPtr::getUnverified() const
+{
+	return hero;
+}
+
+bool HeroPtr::verify() const
 {
 	// TODO: check if these all assertions every time we get info about hero affect efficiency
 	// behave terribly when attempting unauthorized access to hero that is not ours (or was lost)
@@ -101,7 +108,6 @@ bool HeroPtr::validate() const
 		if(!obj)
 			return false;

-		assert(hero == obj);
 		return true;
 		//assert(owned);
 	}
@@ -109,23 +115,27 @@ bool HeroPtr::validate() const
 	return false;
 }

+// enforces isVerified()
 const CGHeroInstance * HeroPtr::operator->() const
 {
-	assert(isValid());
+	assert(isVerified());
 	return hero;
 }

-bool HeroPtr::isValid() const
+// Should be called before using the hero inside this HeroPtr, that's the point of this
+bool HeroPtr::isVerified() const
 {
-	return validate();
+	return verify();
 }

+// enforces isVerified()
 const CGHeroInstance * HeroPtr::operator*() const
 {
-	assert(isValid());
+	assert(isVerified());
 	return hero;
 }

+// enforces isVerified() on input
 bool HeroPtr::operator==(const HeroPtr & rhs) const
 {
 	return hero == rhs.get();
@@ -712,7 +722,7 @@ bool shouldVisit(const Nullkiller * aiNk, const CGHeroInstance * hero, const CGO
 		break;
 	case Obj::TREE_OF_KNOWLEDGE:
 	{
-		if(aiNk->heroManager->getHeroRole(hero) == HeroRole::SCOUT)
+		if(aiNk->heroManager->getHeroRoleOrDefaultInefficient(hero) == HeroRole::SCOUT)
 			return false;

 		TResources myRes = aiNk->getFreeResources();
--- a/AI/Nullkiller2/AIUtility.h
+++ b/AI/Nullkiller2/AIUtility.h
@@ -79,14 +79,14 @@ struct DLL_EXPORT HeroPtr
 private:
 	const CGHeroInstance * hero;
 	std::shared_ptr<CPlayerSpecificInfoCallback> cpsic;
-	bool validate() const;
+	bool verify() const;

 public:
 	explicit HeroPtr(const CGHeroInstance * input, std::shared_ptr<CPlayerSpecificInfoCallback> cpsic);

 	explicit operator bool() const
 	{
-		return isValid();
+		return isVerified();
 	}

 	bool operator<(const HeroPtr & rhs) const;
@@ -101,7 +101,8 @@ public:
 	ObjectInstanceID idOrNone() const;
 	std::string nameOrDefault() const;
 	const CGHeroInstance * get() const;
-	bool isValid() const;
+	const CGHeroInstance * getUnverified() const;
+	bool isVerified() const;
 };

 enum BattleState
--- a/AI/Nullkiller2/Analyzers/DangerHitMapAnalyzer.cpp
+++ b/AI/Nullkiller2/Analyzers/DangerHitMapAnalyzer.cpp
@@ -30,7 +30,7 @@ void logHitmap(PlayerColor playerID, DangerHitMapAnalyzer & data)
 					auto & threat = data.getTileThreat(pos).maximumDanger;
 					b.addText(pos, std::to_string(threat.danger));

-					if(threat.heroPtr.isValid())
+					if(threat.heroPtr.isVerified())
 					{
 						b.addText(pos, std::to_string(threat.turn));
 						b.addText(pos, threat.heroPtr->getNameTranslated());
@@ -45,7 +45,7 @@ void logHitmap(PlayerColor playerID, DangerHitMapAnalyzer & data)
 					auto & threat = data.getTileThreat(pos).fastestDanger;
 					b.addText(pos, std::to_string(threat.danger));

-					if(threat.heroPtr.isValid())
+					if(threat.heroPtr.isVerified())
 					{
 						b.addText(pos, std::to_string(threat.turn));
 						b.addText(pos, threat.heroPtr->getNameTranslated());
@@ -119,7 +119,7 @@ void DangerHitMapAnalyzer::updateHitMap()

 		aiNk->pathfinder->updatePaths(pair.second, ps);

-		aiNk->makingTurnInterrupption.interruptionPoint();
+		aiNk->makingTurnInterruption.interruptionPoint();

 		pforeachTilePaths(mapSize, aiNk, [&](const int3 & pos, const std::vector<AIPath> & paths)
 		{
--- a/AI/Nullkiller2/Analyzers/DangerHitMapAnalyzer.h
+++ b/AI/Nullkiller2/Analyzers/DangerHitMapAnalyzer.h
@@ -77,7 +77,9 @@ public:
 	const HitMapNode & getTileThreat(const int3 & tile) const;
 	std::set<const CGObjectInstance *> getOneTurnAccessibleObjects(const CGHeroInstance * enemy) const;
 	void resetHitmap() {hitMapUpToDate = false;}
+	bool isHitMapUpToDate() const { return hitMapUpToDate; }
 	void resetTileOwners() { tileOwnersUpToDate = false; }
+	bool isTileOwnersUpToDate() const { return tileOwnersUpToDate; }
 	PlayerColor getTileOwner(const int3 & tile) const;
 	const CGTownInstance * getClosestTown(const int3 & tile) const;
 	const std::vector<HitMapInfo> & getTownThreats(const CGTownInstance * town) const;
--- a/AI/Nullkiller2/Analyzers/HeroManager.cpp
+++ b/AI/Nullkiller2/Analyzers/HeroManager.cpp
@@ -18,7 +18,7 @@
 namespace NK2AI
 {

-const SecondarySkillEvaluator HeroManager::wariorSkillsScores = SecondarySkillEvaluator(
+const SecondarySkillEvaluator HeroManager::mainSkillsEvaluator = SecondarySkillEvaluator(
 	{
 		std::make_shared<SecondarySkillScoreMap>(
 			std::map<SecondarySkill, float>
@@ -47,7 +47,7 @@ const SecondarySkillEvaluator HeroManager::wariorSkillsScores = SecondarySkillEv
 		std::make_shared<AtLeastOneMagicRule>()
 	});

-const SecondarySkillEvaluator HeroManager::scountSkillsScores = SecondarySkillEvaluator(
+const SecondarySkillEvaluator HeroManager::scoutSkillsEvaluator = SecondarySkillEvaluator(
 	{
 		std::make_shared<SecondarySkillScoreMap>(
 			std::map<SecondarySkill, float>
@@ -62,12 +62,12 @@ const SecondarySkillEvaluator HeroManager::scountSkillsScores = SecondarySkillEv

 float HeroManager::evaluateSecSkill(SecondarySkill skill, const CGHeroInstance * hero) const
 {
-	auto role = getHeroRole(hero);
+	auto role = getHeroRoleOrDefaultInefficient(hero);

 	if(role == HeroRole::MAIN)
-		return wariorSkillsScores.evaluateSecSkill(hero, skill);
+		return mainSkillsEvaluator.evaluateSecSkill(hero, skill);

-	return scountSkillsScores.evaluateSecSkill(hero, skill);
+	return scoutSkillsEvaluator.evaluateSecSkill(hero, skill);
 }

 float HeroManager::evaluateSpeciality(const CGHeroInstance * hero) const
@@ -85,7 +85,7 @@ float HeroManager::evaluateSpeciality(const CGHeroInstance * hero) const
 		if(hasBonus)
 		{
 			SecondarySkill bonusSkill = bonus->sid.as<SecondarySkill>();
-			float bonusScore = wariorSkillsScores.evaluateSecSkill(hero, bonusSkill);
+			float bonusScore = mainSkillsEvaluator.evaluateSecSkill(hero, bonusSkill);

 			if(bonusScore > 0)
 				specialityScore += bonusScore * bonusScore * bonusScore;
@@ -98,7 +98,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);
+	return evaluateSpeciality(hero) + mainSkillsEvaluator.evaluateSecSkills(hero) + hero->getBasePrimarySkillValue(PrimarySkill::ATTACK) + hero->getBasePrimarySkillValue(PrimarySkill::DEFENSE) + hero->getBasePrimarySkillValue(PrimarySkill::SPELL_POWER) + hero->getBasePrimarySkillValue(PrimarySkill::KNOWLEDGE);
 }

 void HeroManager::update()
@@ -120,64 +120,58 @@ void HeroManager::update()
 	};

 	int globalMainCount = std::min(((int)myHeroes.size() + 2) / 3, cc->getMapSize().x / 50 + 1);
-
 	//vstd::amin(globalMainCount, 1 + (cb->getTownsInfo().size() / 3));
 	if(cc->getTownsInfo().size() < 4 && globalMainCount > 2)
 	{
 		globalMainCount = 2;
 	}
+	// TODO: Mircea: Make it dependent on myHeroes.size() or better?
+	logAi->trace("Max number of main heroes (globalMainCount) is %d", globalMainCount);

 	std::sort(myHeroes.begin(), myHeroes.end(), scoreSort);
 	heroToRoleMap.clear();

-	for(auto hero : myHeroes)
+	for(const auto hero : myHeroes)
 	{
+		HeroPtr heroPtr(hero, aiNk->cc);
+		HeroRole role;
 		if(hero->patrol.patrolling)
 		{
-			heroToRoleMap[HeroPtr(hero, aiNk->cc)] = HeroRole::MAIN;
+			role = MAIN;
 		}
 		else
 		{
-			heroToRoleMap[HeroPtr(hero, aiNk->cc)] = (globalMainCount--) > 0 ? HeroRole::MAIN : HeroRole::SCOUT;
+			role = globalMainCount-- > 0 ? MAIN : SCOUT;
+		}
+
+		heroToRoleMap[heroPtr] = role;
+		logAi->trace("Hero %s has role %s", heroPtr.nameOrDefault(), role == MAIN ? "main" : "scout");
 	}
 }

-	for(auto hero : myHeroes)
+HeroRole HeroManager::getHeroRoleOrDefaultInefficient(const  CGHeroInstance * hero) const
 {
-		logAi->trace("Hero %s has role %s", hero->getNameTranslated(), heroToRoleMap[HeroPtr(hero, aiNk->cc)] == HeroRole::MAIN ? "main" : "scout");
-	}
-}
-
-HeroRole HeroManager::getHeroRole(const  CGHeroInstance * hero) const
-{
-	return getHeroRole(HeroPtr(hero, aiNk->cc));
+	return getHeroRoleOrDefault(HeroPtr(hero, aiNk->cc));
 }

 // TODO: Mircea: Do we need this map on HeroPtr or is enough just on hero?
-HeroRole HeroManager::getHeroRole(const HeroPtr & heroPtr) const
+HeroRole HeroManager::getHeroRoleOrDefault(const HeroPtr & heroPtr) const
 {
 	if(heroToRoleMap.find(heroPtr) != heroToRoleMap.end())
 		return heroToRoleMap.at(heroPtr);
 	return HeroRole::SCOUT;
 }

-const std::map<HeroPtr, HeroRole> & HeroManager::getHeroToRoleMap() const
+int HeroManager::selectBestSkillIndex(const HeroPtr & heroPtr, const std::vector<SecondarySkill> & skills) const
 {
-	return heroToRoleMap;
-}
-
-int HeroManager::selectBestSkill(const HeroPtr & heroPtr, const std::vector<SecondarySkill> & skills) const
-{
-	auto role = getHeroRole(heroPtr);
-	auto & evaluator = role == HeroRole::MAIN ? wariorSkillsScores : scountSkillsScores;
-
+	const auto role = getHeroRoleOrDefault(heroPtr);
+	const auto & evaluator = role == MAIN ? mainSkillsEvaluator : scoutSkillsEvaluator;
 	int result = 0;
 	float resultScore = -100;

 	for(int i = 0; i < skills.size(); i++)
 	{
-		auto score = evaluator.evaluateSecSkill(heroPtr.get(), skills[i]);
-
+		const auto score = evaluator.evaluateSecSkill(heroPtr.getUnverified(), skills[i]);
 		if(score > resultScore)
 		{
 			resultScore = score;
@@ -185,7 +179,7 @@ int HeroManager::selectBestSkill(const HeroPtr & heroPtr, const std::vector<Seco
 		}

 		logAi->trace(
-			"Hero %s is proposed to learn %d with score %f",
+			"Hero %s is offered to learn %d with score %f",
 			heroPtr.nameOrDefault(),
 			skills[i].toEnum(),
 			score);
@@ -307,7 +301,7 @@ const CGHeroInstance * HeroManager::findWeakHeroToDismiss(uint64_t armyLimit, co
 	{
 		if(aiNk->getHeroLockedReason(existingHero) == HeroLockedReason::DEFENCE
 			|| existingHero->getArmyStrength() >armyLimit
-			|| getHeroRole(existingHero) == HeroRole::MAIN
+			|| getHeroRoleOrDefaultInefficient(existingHero) == HeroRole::MAIN
 			|| existingHero->movementPointsRemaining()
 			|| (townToSpare != nullptr && existingHero->getVisitedTown() == townToSpare)
 			|| existingHero->artifactsWorn.size() > (existingHero->hasSpellbook() ? 2 : 1))
--- a/AI/Nullkiller2/Analyzers/HeroManager.h
+++ b/AI/Nullkiller2/Analyzers/HeroManager.h
@@ -39,20 +39,19 @@ public:
 class DLL_EXPORT HeroManager
 {
 private:
-	static const SecondarySkillEvaluator wariorSkillsScores;
-	static const SecondarySkillEvaluator scountSkillsScores;
+	static const SecondarySkillEvaluator mainSkillsEvaluator;
+	static const SecondarySkillEvaluator scoutSkillsEvaluator;

 	CCallback * cc; //this is enough, but we downcast from CCallback
 	const Nullkiller * aiNk;
-	std::map<HeroPtr, HeroRole> heroToRoleMap;
+	std::map<HeroPtr, HeroRole> heroToRoleMap; // can get out of sync with cc->getHeroesInfo() and include lost heroes
 	std::map<ObjectInstanceID, float> knownFightingStrength;

 public:
 	HeroManager(CCallback * cc, const Nullkiller * aiNk) : cc(cc), aiNk(aiNk) {}
-	const std::map<HeroPtr, HeroRole> & getHeroToRoleMap() const;
-	HeroRole getHeroRole(const CGHeroInstance * hero) const;
-	HeroRole getHeroRole(const HeroPtr & heroPtr) const;
-	int selectBestSkill(const HeroPtr & heroPtr, const std::vector<SecondarySkill> & skills) const;
+	HeroRole getHeroRoleOrDefaultInefficient(const CGHeroInstance * hero) const;
+	HeroRole getHeroRoleOrDefault(const HeroPtr & heroPtr) const;
+	int selectBestSkillIndex(const HeroPtr & heroPtr, const std::vector<SecondarySkill> & skills) const;
 	void update();
 	float evaluateSecSkill(SecondarySkill skill, const CGHeroInstance * hero) const;
 	float evaluateHero(const CGHeroInstance * hero) const;
--- a/AI/Nullkiller2/Analyzers/ObjectClusterizer.cpp
+++ b/AI/Nullkiller2/Analyzers/ObjectClusterizer.cpp
@@ -427,7 +427,7 @@ void ObjectClusterizer::clusterizeObject(
 		logAi->trace("ObjectClusterizer Checking path %s", path.toString());
 #endif

-		if(aiNk->heroManager->getHeroRole(path.targetHero) == HeroRole::SCOUT)
+		if(aiNk->heroManager->getHeroRoleOrDefaultInefficient(path.targetHero) == HeroRole::SCOUT)
 		{
 			// TODO: Mircea: Shouldn't this be linked with scoutHeroTurnDistanceLimit?
 			// TODO: Mircea: Move to constant
--- a/AI/Nullkiller2/Behaviors/BuyArmyBehavior.cpp
+++ b/AI/Nullkiller2/Behaviors/BuyArmyBehavior.cpp
@@ -53,7 +53,7 @@ Goals::TGoalVec BuyArmyBehavior::decompose(const Nullkiller * aiNk) const

 		for(const CGHeroInstance * targetHero : heroes)
 		{
-			if(aiNk->heroManager->getHeroRole(targetHero) == HeroRole::MAIN)
+			if(aiNk->heroManager->getHeroRoleOrDefaultInefficient(targetHero) == HeroRole::MAIN)
 			{
 				auto reinforcement = aiNk->armyManager->howManyReinforcementsCanGet(
 					targetHero,
--- a/AI/Nullkiller2/Behaviors/CaptureObjectsBehavior.cpp
+++ b/AI/Nullkiller2/Behaviors/CaptureObjectsBehavior.cpp
@@ -82,7 +82,7 @@ Goals::TGoalVec CaptureObjectsBehavior::getVisitGoals(
 		if (hero->getOwner() != nullkiller->playerID)
 			continue;

-		if(nullkiller->heroManager->getHeroRole(hero) == HeroRole::SCOUT
+		if(nullkiller->heroManager->getHeroRoleOrDefaultInefficient(hero) == HeroRole::SCOUT
 			&& (path.getTotalDanger() == 0 || path.turn() > 0)
 			&& path.exchangeCount > 1)
 		{
@@ -132,13 +132,10 @@ Goals::TGoalVec CaptureObjectsBehavior::getVisitGoals(
 		{
 			auto newWay = new ExecuteHeroChain(path, objToVisit);
 			TSubgoal sharedPtr;
-
 			sharedPtr.reset(newWay);
-
-			auto heroRole = nullkiller->heroManager->getHeroRole(path.targetHero);
+			auto heroRole = nullkiller->heroManager->getHeroRoleOrDefaultInefficient(path.targetHero);

 			auto & closestWay = closestWaysByRole[heroRole];
-
 			if(!closestWay || closestWay->movementCost() > path.movementCost())
 			{
 				closestWay = &path;
@@ -152,15 +149,14 @@ Goals::TGoalVec CaptureObjectsBehavior::getVisitGoals(
 		}
 	}

-	for(auto way : waysToVisitObj)
+	for(auto * way : waysToVisitObj)
 	{
-		auto heroRole = nullkiller->heroManager->getHeroRole(way->getPath().targetHero);
-		auto closestWay = closestWaysByRole[heroRole];
+		auto heroRole = nullkiller->heroManager->getHeroRoleOrDefaultInefficient(way->getPath().targetHero);
+		const auto * closestWay = closestWaysByRole[heroRole];

 		if(closestWay)
 		{
-			way->closestWayRatio
-				= closestWay->movementCost() / way->getPath().movementCost();
+			way->closestWayRatio = closestWay->movementCost() / way->getPath().movementCost();
 		}
 	}

@@ -173,14 +169,10 @@ void CaptureObjectsBehavior::decomposeObjects(
 	const Nullkiller * nullkiller) const
 {
 	if(objs.empty())
-	{
 		return;
-	}

 	std::mutex sync;
-
 	logAi->debug("Scanning objects, count %d", objs.size());
-
 	tbb::parallel_for(
 		tbb::blocked_range<size_t>(0, objs.size()),
 		[this, &objs, &sync, &result, nullkiller](const tbb::blocked_range<size_t> & r)
@@ -191,8 +183,7 @@ void CaptureObjectsBehavior::decomposeObjects(

 			for(auto i = r.begin(); i != r.end(); i++)
 			{
-				auto objToVisit = objs[i];
-
+				const auto * objToVisit = objs[i];
 				if(!objectMatchesFilter(objToVisit))
 					continue;

@@ -200,6 +191,7 @@ void CaptureObjectsBehavior::decomposeObjects(
 				logAi->trace("Checking object %s, %s", objToVisit->getObjectName(), objToVisit->visitablePos().toString());
 #endif

+				// FIXME: Mircea: This one uses the deleted hero
 				nullkiller->pathfinder->calculatePathInfo(paths, objToVisit->visitablePos(), nullkiller->isObjectGraphAllowed());

 #if NK2AI_TRACE_LEVEL >= 1
--- a/AI/Nullkiller2/Behaviors/DefenceBehavior.cpp
+++ b/AI/Nullkiller2/Behaviors/DefenceBehavior.cpp
@@ -84,7 +84,7 @@ void handleCounterAttack(
 	const Nullkiller * aiNk,
 	Goals::TGoalVec & tasks)
 {
-	if(threat.heroPtr.isValid()
+	if(threat.heroPtr.isVerified()
 		&& threat.turn <= 1
 		&& (threat.danger == maximumDanger.danger || threat.turn < maximumDanger.turn))
 	{
@@ -132,7 +132,7 @@ bool handleGarrisonHeroFromPreviousTurn(const CGTownInstance * town, Goals::TGoa

 			return false;
 		}
-		else if(aiNk->heroManager->getHeroRole(town->getGarrisonHero()) == HeroRole::MAIN)
+		else if(aiNk->heroManager->getHeroRoleOrDefaultInefficient(town->getGarrisonHero()) == HeroRole::MAIN)
 		{
 			auto armyDismissLimit = 1000;
 			auto heroToDismiss = aiNk->heroManager->findWeakHeroToDismiss(armyDismissLimit);
--- a/AI/Nullkiller2/Behaviors/GatherArmyBehavior.cpp
+++ b/AI/Nullkiller2/Behaviors/GatherArmyBehavior.cpp
@@ -43,7 +43,7 @@ Goals::TGoalVec GatherArmyBehavior::decompose(const Nullkiller * aiNk) const

 	for(const CGHeroInstance * hero : heroes)
 	{
-		if(aiNk->heroManager->getHeroRole(hero) == HeroRole::MAIN)
+		if(aiNk->heroManager->getHeroRoleOrDefaultInefficient(hero) == HeroRole::MAIN)
 		{
 			vstd::concatenate(tasks, deliverArmyToHero(aiNk, hero));
 		}
@@ -121,7 +121,7 @@ Goals::TGoalVec GatherArmyBehavior::deliverArmyToHero(const Nullkiller * aiNk, c
 		{
 			if(!node.targetHero) continue;

-			auto heroRole = aiNk->heroManager->getHeroRole(node.targetHero);
+			auto heroRole = aiNk->heroManager->getHeroRoleOrDefaultInefficient(node.targetHero);

 			if(heroRole == HeroRole::MAIN)
 			{
@@ -239,7 +239,7 @@ Goals::TGoalVec GatherArmyBehavior::upgradeArmy(const Nullkiller * aiNk, const C

 	for(const AIPath & path : paths)
 	{
-		auto heroRole = aiNk->heroManager->getHeroRole(path.targetHero);
+		auto heroRole = aiNk->heroManager->getHeroRoleOrDefaultInefficient(path.targetHero);

 		if(heroRole == HeroRole::MAIN && path.turn() < aiNk->settings->getScoutHeroTurnDistanceLimit())
 			hasMainAround = true;
@@ -290,7 +290,7 @@ Goals::TGoalVec GatherArmyBehavior::upgradeArmy(const Nullkiller * aiNk, const C
 		auto upgrade = aiNk->armyManager->calculateCreaturesUpgrade(path.heroArmy, upgrader, availableResources);

 		if(!upgrader->getGarrisonHero()
-		   && (hasMainAround || aiNk->heroManager->getHeroRole(path.targetHero) == HeroRole::MAIN))
+		   && (hasMainAround || aiNk->heroManager->getHeroRoleOrDefaultInefficient(path.targetHero) == HeroRole::MAIN))
 		{
 			ArmyUpgradeInfo armyToGetOrBuy;

--- a/AI/Nullkiller2/Behaviors/StartupBehavior.cpp
+++ b/AI/Nullkiller2/Behaviors/StartupBehavior.cpp
@@ -172,7 +172,7 @@ Goals::TGoalVec StartupBehavior::decompose(const Nullkiller * aiNk) const
 				auto garrisonHeroScore = aiNk->heroManager->evaluateHero(garrisonHero);

 				if(visitingHeroScore > garrisonHeroScore
-					|| (aiNk->heroManager->getHeroRole(garrisonHero) == HeroRole::SCOUT && aiNk->heroManager->getHeroRole(visitingHero) == HeroRole::MAIN))
+					|| (aiNk->heroManager->getHeroRoleOrDefaultInefficient(garrisonHero) == HeroRole::SCOUT && aiNk->heroManager->getHeroRoleOrDefaultInefficient(visitingHero) == HeroRole::MAIN))
 				{
 					if(canRecruitHero || aiNk->armyManager->howManyReinforcementsCanGet(visitingHero, garrisonHero) > 200)
 					{
--- a/AI/Nullkiller2/Engine/Nullkiller.cpp
+++ b/AI/Nullkiller2/Engine/Nullkiller.cpp
@@ -15,7 +15,6 @@
 #include "../Behaviors/CaptureObjectsBehavior.h"
 #include "../Behaviors/RecruitHeroBehavior.h"
 #include "../Behaviors/BuyArmyBehavior.h"
-#include "../Behaviors/StartupBehavior.h"
 #include "../Behaviors/DefenceBehavior.h"
 #include "../Behaviors/BuildingBehavior.h"
 #include "../Behaviors/GatherArmyBehavior.h"
@@ -23,7 +22,6 @@
 #include "../Behaviors/StayAtTownBehavior.h"
 #include "../Behaviors/ExplorationBehavior.h"
 #include "../Goals/Invalid.h"
-#include "../Goals/Composition.h"
 #include "../../../lib/CPlayerState.h"
 #include "../../lib/StartInfo.h"
 #include "../../lib/pathfinder/PathfinderCache.h"
@@ -41,7 +39,6 @@ Nullkiller::Nullkiller()
 	: activeHero(nullptr)
 	, scanDepth(ScanDepth::MAIN_FULL)
 	, useHeroChain(true)
-	, pathfinderInvalidated(false)
 	, memory(std::make_unique<AIMemory>())
 {

@@ -49,7 +46,7 @@ Nullkiller::Nullkiller()

 Nullkiller::~Nullkiller() = default;

-bool canUseOpenMap(std::shared_ptr<CCallback> cb, PlayerColor playerID)
+bool canUseOpenMap(const std::shared_ptr<CCallback>& cb, const PlayerColor playerID)
 {
 	if(!cb->getStartInfo()->extraOptionsInfo.cheatsAllowed)
 	{
@@ -58,31 +55,33 @@ bool canUseOpenMap(std::shared_ptr<CCallback> cb, PlayerColor playerID)

 	const TeamState * team = cb->getPlayerTeam(playerID);

-	auto hasHumanInTeam = vstd::contains_if(team->players, [cb](PlayerColor teamMateID) -> bool
+	const auto hasHumanInTeam = vstd::contains_if(
+		team->players,
+		[cb](const PlayerColor teamMateID) -> bool
 		{
 			return cb->getPlayerState(teamMateID)->isHuman();
-		});
+		}
+	);

 	return !hasHumanInTeam;
 }

-void Nullkiller::init(std::shared_ptr<CCallback> cb, AIGateway * aiGw)
+void Nullkiller::init(const std::shared_ptr<CCallback> & cbInput, AIGateway * aiGwInput)
 {
-	this->cc = cb;
-	this->aiGw = aiGw;
-	this->playerID = aiGw->playerID;
+	cc = cbInput;
+	aiGw = aiGwInput;
+	playerID = aiGwInput->playerID;

-	settings = std::make_unique<Settings>(cb->getStartInfo()->difficulty);
-
-	PathfinderOptions pathfinderOptions(*cb);
+	settings = std::make_unique<Settings>(cc->getStartInfo()->difficulty);

+	PathfinderOptions pathfinderOptions(*cc);
 	pathfinderOptions.useTeleportTwoWay = true;
 	pathfinderOptions.useTeleportOneWay = settings->isOneWayMonolithUsageAllowed();
 	pathfinderOptions.useTeleportOneWayRandom = settings->isOneWayMonolithUsageAllowed();

-	pathfinderCache = std::make_unique<PathfinderCache>(cb.get(), pathfinderOptions);
+	pathfinderCache = std::make_unique<PathfinderCache>(cc.get(), pathfinderOptions);

-	if(canUseOpenMap(cb, playerID))
+	if(canUseOpenMap(cc, playerID))
 	{
 		useObjectGraph = settings->isObjectGraphAllowed();
 		openMap = settings->isOpenMap() || useObjectGraph;
@@ -94,7 +93,6 @@ void Nullkiller::init(std::shared_ptr<CCallback> cb, AIGateway * aiGw)
 	}

 	baseGraph.reset();
-
 	priorityEvaluator.reset(new PriorityEvaluator(this));
 	priorityEvaluators.reset(
 		new SharedPool<PriorityEvaluator>(
@@ -107,15 +105,16 @@ void Nullkiller::init(std::shared_ptr<CCallback> cb, AIGateway * aiGw)
 	buildAnalyzer.reset(new BuildAnalyzer(this));
 	objectClusterizer.reset(new ObjectClusterizer(this));
 	dangerEvaluator.reset(new FuzzyHelper(this));
-	pathfinder.reset(new AIPathfinder(cb.get(), this));
-	armyManager.reset(new ArmyManager(cb.get(), this));
-	heroManager.reset(new HeroManager(cb.get(), this));
+	pathfinder.reset(new AIPathfinder(cc.get(), this));
+	armyManager.reset(new ArmyManager(cc.get(), this));
+	heroManager.reset(new HeroManager(cc.get(), this));
 	decomposer.reset(new DeepDecomposer(this));
-	armyFormation.reset(new ArmyFormation(cb, this));
+	armyFormation.reset(new ArmyFormation(cc, this));
 }

 TaskPlanItem::TaskPlanItem(const TSubgoal & task)
-	:task(task), affectedObjects(task->asTask()->getAffectedObjects())
+	: affectedObjects(task->asTask()->getAffectedObjects())
+	, task(task)
 {
 }

@@ -216,12 +215,12 @@ Goals::TTaskVec Nullkiller::buildPlan(TGoalVec & tasks, int priorityTier) const

 void Nullkiller::decompose(Goals::TGoalVec & results, const Goals::TSubgoal& behavior, int decompositionMaxDepth) const
 {
-	makingTurnInterrupption.interruptionPoint();
+	makingTurnInterruption.interruptionPoint();
 	logAi->debug("Decomposing behavior %s", behavior->toString());
 	const auto start = std::chrono::high_resolution_clock::now();
 	decomposer->decompose(results, behavior, decompositionMaxDepth);

-	makingTurnInterrupption.interruptionPoint();
+	makingTurnInterruption.interruptionPoint();
 	logAi->debug("Decomposing behavior %s done in %ld", behavior->toString(), timeElapsed(start));
 }

@@ -250,9 +249,13 @@ void Nullkiller::invalidatePathfinderData()

 void Nullkiller::updateState()
 {
-	makingTurnInterrupption.interruptionPoint();
+#if NK2AI_TRACE_LEVEL >= 1
+	logAi->info("PERFORMANCE: AI updateState started");
+#endif
+
+	makingTurnInterruption.interruptionPoint();
 	std::unique_lock lockGuard(aiStateMutex);
-	auto start = std::chrono::high_resolution_clock::now();
+	const auto start = std::chrono::high_resolution_clock::now();

 	activeHero = nullptr;
 	setTargetObject(-1);
@@ -260,29 +263,18 @@ void Nullkiller::updateState()

 	buildAnalyzer->update();

-	if (!pathfinderInvalidated)
-		logAi->trace("Skipping paths regeneration - up to date");
+	if(!pathfinderInvalidated && dangerHitMap->isHitMapUpToDate() && dangerHitMap->isTileOwnersUpToDate())
+		logAi->trace("Skipping full state regeneration - up to date");
 	else
 	{
 		memory->removeInvisibleObjects(cc.get());
 		dangerHitMap->updateHitMap();
 		dangerHitMap->calculateTileOwners();

-		makingTurnInterrupption.interruptionPoint();
-
+		makingTurnInterruption.interruptionPoint();
 		heroManager->update();
 		logAi->trace("Updating paths");

-		std::map<const CGHeroInstance *, HeroRole> activeHeroes;
-
-		for(auto hero : cc->getHeroesInfo())
-		{
-			if(getHeroLockedReason(hero) == HeroLockedReason::DEFENCE)
-				continue;
-
-			activeHeroes[hero] = heroManager->getHeroRole(hero);
-		}
-
 		PathfinderSettings cfg;
 		cfg.useHeroChain = useHeroChain;
 		cfg.allowBypassObjects = true;
@@ -297,27 +289,35 @@ void Nullkiller::updateState()
 			cfg.scoutTurnDistanceLimit = settings->getScoutHeroTurnDistanceLimit();
 		}

-		makingTurnInterrupption.interruptionPoint();
-
-		pathfinder->updatePaths(activeHeroes, cfg);
+		makingTurnInterruption.interruptionPoint();
+		const auto heroes = getHeroesForPathfinding();
+		pathfinder->updatePaths(heroes, cfg);

 		if(isObjectGraphAllowed())
 		{
 			pathfinder->updateGraphs(
-				activeHeroes,
+				heroes,
 				scanDepth == ScanDepth::SMALL ? PathfinderSettings::MaxTurnDistanceLimit : 10,
 				scanDepth == ScanDepth::ALL_FULL ? PathfinderSettings::MaxTurnDistanceLimit : 3);
 		}

-		makingTurnInterrupption.interruptionPoint();
-
+		makingTurnInterruption.interruptionPoint();
 		objectClusterizer->clusterize();
 		pathfinderInvalidated = false;
 	}

 	armyManager->update();

-	logAi->debug("AI state updated in %ld ms", timeElapsed(start));
+#if NK2AI_TRACE_LEVEL >= 1
+	if(const auto timeElapsedMs = timeElapsed(start); timeElapsedMs > 499)
+	{
+		logAi->warn("PERFORMANCE: AI updateState took %ld ms", timeElapsedMs);
+	}
+	else
+	{
+		logAi->info("PERFORMANCE: AI updateState took %ld ms", timeElapsedMs);
+	}
+#endif
 }

 bool Nullkiller::isHeroLocked(const CGHeroInstance * hero) const
@@ -335,7 +335,7 @@ bool Nullkiller::arePathHeroesLocked(const AIPath & path) const
 		return true;
 	}

-	for(auto & node : path.nodes)
+	for(const auto & node : path.nodes)
 	{
 		auto lockReason = getHeroLockedReason(node.targetHero);

@@ -368,9 +368,7 @@ void Nullkiller::makeTurn()

 	for(int i = 1; i <= settings->getMaxPass() && cc->getPlayerStatus(playerID) == EPlayerStatus::INGAME; i++)
 	{
-		updateState();
-
-		if (!makeTurnHelperPriorityPass(tasks, i)) return;
+		if (!updateStateAndExecutePriorityPass(tasks, i)) return;

 		tasks.clear();
 		decompose(tasks, sptr(CaptureObjectsBehavior()), 1);
@@ -493,16 +491,15 @@ void Nullkiller::makeTurn()
 	}
 }

-bool Nullkiller::makeTurnHelperPriorityPass(Goals::TGoalVec & tempResults, int passIndex)
+bool Nullkiller::updateStateAndExecutePriorityPass(Goals::TGoalVec & tempResults, const int passIndex)
 {
+	updateState();
+
 	Goals::TTask bestPrioPassTask = taskptr(Goals::Invalid());
 	for(int i = 1; i <= settings->getMaxPriorityPass() && cc->getPlayerStatus(playerID) == EPlayerStatus::INGAME; i++)
 	{
 		tempResults.clear();

-		// Call updateHitMap here instead of inside each of the 3 behaviors
-		dangerHitMap->updateHitMap();
-
 		decompose(tempResults, sptr(RecruitHeroBehavior()), 1);
 		decompose(tempResults, sptr(BuyArmyBehavior()), 1);
 		decompose(tempResults, sptr(BuildingBehavior()), 1);
@@ -510,13 +507,15 @@ bool Nullkiller::makeTurnHelperPriorityPass(Goals::TGoalVec & tempResults, int p
 		bestPrioPassTask = choseBestTask(tempResults);
 		if(bestPrioPassTask->priority > 0)
 		{
-#if NK2AI_TRACE_LEVEL >= 1
 			logAi->info("Pass %d: Performing priorityPass %d task %s with prio: %d", passIndex, i, bestPrioPassTask->toString(), bestPrioPassTask->priority);
-#endif

 			if(!executeTask(bestPrioPassTask))
+			{
+				logAi->warn("Task failed to execute");
 				return false;
+			}

+			// TODO: Mircea: Might want to consider calling it before executeTask just in case
 			updateState();
 		}
 		else
@@ -532,7 +531,7 @@ bool Nullkiller::makeTurnHelperPriorityPass(Goals::TGoalVec & tempResults, int p
 	return true;
 }

-bool Nullkiller::areAffectedObjectsPresent(Goals::TTask task) const
+bool Nullkiller::areAffectedObjectsPresent(const Goals::TTask & task) const
 {
 	auto affectedObjs = task->getAffectedObjects();

@@ -545,23 +544,23 @@ bool Nullkiller::areAffectedObjectsPresent(Goals::TTask task) const
 	return true;
 }

-HeroRole Nullkiller::getTaskRole(Goals::TTask task) const
+HeroRole Nullkiller::getTaskRole(const Goals::TTask & task) const
 {
 	HeroPtr heroPtr(task->getHero(), cc);
 	HeroRole heroRole = HeroRole::MAIN;

-	if(heroPtr.isValid())
-		heroRole = heroManager->getHeroRole(heroPtr);
+	if(heroPtr.isVerified())
+		heroRole = heroManager->getHeroRoleOrDefault(heroPtr);

 	return heroRole;
 }

-bool Nullkiller::executeTask(Goals::TTask task)
+bool Nullkiller::executeTask(const Goals::TTask & task)
 {
 	auto start = std::chrono::high_resolution_clock::now();
 	std::string taskDescr = task->toString();

-	makingTurnInterrupption.interruptionPoint();
+	makingTurnInterruption.interruptionPoint();
 	logAi->debug("Trying to realize %s (value %2.3f)", taskDescr, task->priority);

 	try
@@ -602,7 +601,7 @@ bool Nullkiller::handleTrading()
 	bool haveTraded = false;
 	bool shouldTryToTrade = true;
 	ObjectInstanceID marketId;
-	for (auto town : cc->getTownsInfo())
+	for (const auto town : cc->getTownsInfo())
 	{
 		if (town->hasBuiltSomeTradeBuilding())
 		{
@@ -645,7 +644,7 @@ bool Nullkiller::handleTrading()

 				for (int i = 0; i < required.size(); ++i)
 				{
-					float ratio = available[i];
+					float ratio;
 					if (required[i] > 0)
 						ratio = static_cast<float>(available[i]) / required[i];
 					else
@@ -724,4 +723,17 @@ void Nullkiller::tracePlayerStatus(bool beginning) const
 #endif
 }

+std::map<const CGHeroInstance *, HeroRole> Nullkiller::getHeroesForPathfinding() const
+{
+	std::map<const CGHeroInstance *, HeroRole> activeHeroes;
+	for(auto hero : cc->getHeroesInfo())
+	{
+		if(getHeroLockedReason(hero) == HeroLockedReason::DEFENCE)
+			continue;
+
+		activeHeroes[hero] = heroManager->getHeroRoleOrDefaultInefficient(hero);
+	}
+	return activeHeroes;
+}
+
 }
--- a/AI/Nullkiller2/Engine/Nullkiller.h
+++ b/AI/Nullkiller2/Engine/Nullkiller.h
@@ -110,13 +110,13 @@ public:
 	/// Same value as AIGateway->cc
 	std::shared_ptr<CCallback> cc;
 	std::mutex aiStateMutex;
-	mutable ThreadInterruption makingTurnInterrupption;
+	mutable ThreadInterruption makingTurnInterruption;

 	Nullkiller();
 	virtual ~Nullkiller();
-	void init(std::shared_ptr<CCallback> cb, AIGateway * aiGw);
+	void init(const std::shared_ptr<CCallback> & cbInput, AIGateway * aiGwInput);
 	virtual void makeTurn();
-	bool makeTurnHelperPriorityPass(Goals::TGoalVec& tempResults, int passIndex);
+	bool updateStateAndExecutePriorityPass(Goals::TGoalVec& tempResults, int passIndex);
 	bool isActive(const CGHeroInstance * hero) const { return activeHero == hero; }
 	bool isHeroLocked(const CGHeroInstance * hero) const;
 	HeroPtr getActiveHero() { return HeroPtr(activeHero, cc); }
@@ -140,6 +140,7 @@ public:

 	std::shared_ptr<const CPathsInfo> getPathsInfo(const CGHeroInstance * h) const;
 	void invalidatePaths();
+	std::map<const CGHeroInstance *, HeroRole> getHeroesForPathfinding() const;

 private:
 	void resetState();
@@ -147,9 +148,9 @@ private:
 	void decompose(Goals::TGoalVec & results, const Goals::TSubgoal& behavior, int decompositionMaxDepth) const;
 	Goals::TTask choseBestTask(Goals::TGoalVec & tasks) const;
 	Goals::TTaskVec buildPlan(Goals::TGoalVec & tasks, int priorityTier) const;
-	bool executeTask(Goals::TTask task);
-	bool areAffectedObjectsPresent(Goals::TTask task) const;
-	HeroRole getTaskRole(Goals::TTask task) const;
+	bool executeTask(const Goals::TTask & task);
+	bool areAffectedObjectsPresent(const Goals::TTask & task) const;
+	HeroRole getTaskRole(const Goals::TTask & task) const;
 	void tracePlayerStatus(bool beginning) const;
 };

--- a/AI/Nullkiller2/Engine/PriorityEvaluator.cpp
+++ b/AI/Nullkiller2/Engine/PriorityEvaluator.cpp
@@ -778,7 +778,7 @@ public:

 		evaluationContext.addNonCriticalStrategicalValue(2.0f * armyStrength / (float)heroExchange.hero->getArmyStrength());
 		evaluationContext.conquestValue += 2.0f * armyStrength / (float)heroExchange.hero->getArmyStrength();
-		evaluationContext.heroRole = evaluationContext.evaluator.aiNk->heroManager->getHeroRole(heroExchange.hero);
+		evaluationContext.heroRole = evaluationContext.evaluator.aiNk->heroManager->getHeroRoleOrDefaultInefficient(heroExchange.hero);
 		evaluationContext.isExchange = true;
 	}
 };
@@ -958,7 +958,7 @@ public:
 		float highestCostForSingleHero = 0;
 		for(auto pair : costsPerHero)
 		{
-			auto role = evaluationContext.evaluator.aiNk->heroManager->getHeroRole(pair.first);
+			auto role = evaluationContext.evaluator.aiNk->heroManager->getHeroRoleOrDefaultInefficient(pair.first);
 			evaluationContext.movementCostByRole[role] += pair.second;
 			if (pair.second > highestCostForSingleHero)
 				highestCostForSingleHero = pair.second;
@@ -975,7 +975,7 @@ public:
 		auto army = path.heroArmy;

 		const CGObjectInstance * target = aiNk->cc->getObj((ObjectInstanceID)task->objid, false);
-		auto heroRole = evaluationContext.evaluator.aiNk->heroManager->getHeroRole(hero);
+		auto heroRole = evaluationContext.evaluator.aiNk->heroManager->getHeroRoleOrDefaultInefficient(hero);

 		if(heroRole == HeroRole::MAIN)
 			evaluationContext.heroRole = heroRole;
@@ -1056,7 +1056,7 @@ public:
 		std::shared_ptr<ObjectCluster> cluster = clusterGoal.getCluster();

 		auto hero = clusterGoal.hero;
-		auto role = evaluationContext.evaluator.aiNk->heroManager->getHeroRole(hero);
+		auto role = evaluationContext.evaluator.aiNk->heroManager->getHeroRoleOrDefaultInefficient(hero);

 		std::vector<std::pair<ObjectInstanceID, ClusterObjectInfo>> objects(cluster->objects.begin(), cluster->objects.end());

@@ -1113,7 +1113,7 @@ public:

 		if(garrisonHero && swapCommand.getLockingReason() == HeroLockedReason::DEFENCE)
 		{
-			auto defenderRole = evaluationContext.evaluator.aiNk->heroManager->getHeroRole(garrisonHero);
+			auto defenderRole = evaluationContext.evaluator.aiNk->heroManager->getHeroRoleOrDefaultInefficient(garrisonHero);
 			auto mpLeft = garrisonHero->movementPointsRemaining() / (float)garrisonHero->movementPointsLimit(true);

 			evaluationContext.movementCost += mpLeft;
@@ -1142,7 +1142,7 @@ public:
 		Goals::DismissHero & dismissCommand = dynamic_cast<Goals::DismissHero &>(*task);
 		const CGHeroInstance * dismissedHero = dismissCommand.getHero();

-		auto role = aiNk->heroManager->getHeroRole(dismissedHero);
+		auto role = aiNk->heroManager->getHeroRoleOrDefaultInefficient(dismissedHero);
 		auto mpLeft = dismissedHero->movementPointsRemaining();

 		evaluationContext.movementCost += mpLeft;
--- a/AI/Nullkiller2/Goals/ExecuteHeroChain.cpp
+++ b/AI/Nullkiller2/Goals/ExecuteHeroChain.cpp
@@ -111,7 +111,7 @@ void ExecuteHeroChain::accept(AIGateway * aiGw)
 		const CGHeroInstance * hero = node->targetHero;
 		HeroPtr heroPtr(hero, aiGw->cc);

-		if(!heroPtr.isValid())
+		if(!heroPtr.isVerified())
 		{
 			logAi->error("Hero %s was lost. Exit hero chain.", heroPtr.nameOrDefault());

@@ -148,7 +148,7 @@ void ExecuteHeroChain::accept(AIGateway * aiGw)
 					
 					node->specialAction->execute(aiGw, hero);
 					
-					if(!heroPtr.isValid())
+					if(!heroPtr.isVerified())
 					{
 						logAi->error("Hero %s was lost trying to execute special action. Exit hero chain.", heroPtr.nameOrDefault());

@@ -229,7 +229,7 @@ void ExecuteHeroChain::accept(AIGateway * aiGw)
 					}
 					catch(const cannotFulfillGoalException &)
 					{
-						if(!heroPtr.isValid())
+						if(!heroPtr.isVerified())
 						{
 							logAi->error("Hero %s was lost. Exit hero chain.", heroPtr.nameOrDefault());

@@ -275,7 +275,7 @@ void ExecuteHeroChain::accept(AIGateway * aiGw)
 		}
 		catch(const goalFulfilledException &)
 		{
-			if(!heroPtr.isValid())
+			if(!heroPtr.isVerified())
 			{
 				logAi->debug("Hero %s was killed while attempting to reach %s", heroPtr.nameOrDefault(), node->coord.toString());

--- a/AI/Nullkiller2/Goals/RecruitHero.cpp
+++ b/AI/Nullkiller2/Goals/RecruitHero.cpp
@@ -70,8 +70,8 @@ void RecruitHero::accept(AIGateway * aiGw)
 	ccTl->recruitHero(t, heroToHire);

 	{
+		// TODO: Mircea: Consider same behavior when a hero is lost? Relevant?
 		std::unique_lock lockGuard(aiGw->nullkiller->aiStateMutex);
-
 		aiGw->nullkiller->heroManager->update();
 		aiGw->nullkiller->objectClusterizer->reset();
 	}
--- a/AI/Nullkiller2/Pathfinding/AINodeStorage.cpp
+++ b/AI/Nullkiller2/Pathfinding/AINodeStorage.cpp
@@ -43,22 +43,22 @@ const bool DO_NOT_SAVE_TO_COMMITTED_TILES = false;

 AISharedStorage::AISharedStorage(int3 sizes, int numChains)
 {
-	if(!shared){
-		shared.reset(new boost::multi_array<AIPathNode, 4>(
-			boost::extents[sizes.z][sizes.x][sizes.y][numChains]));
-
+	if(!shared)
+	{
+		shared.reset(new boost::multi_array<AIPathNode, 4>(boost::extents[sizes.z][sizes.x][sizes.y][numChains]));
 		nodes = shared;

-		foreach_tile_pos([&](const int3 & pos)
+		foreach_tile_pos(
+			[&](const int3 & pos)
 			{
 				for(auto i = 0; i < numChains; i++)
 				{
 					auto & node = get(pos)[i];
-						
 					node.version = -1;
 					node.coord = pos;
 				}
-			});
+			}
+		);
 	}
 	else
 		nodes = shared;
@@ -105,7 +105,7 @@ int AINodeStorage::getBucketSize() const
 }

 AINodeStorage::AINodeStorage(const Nullkiller * aiNk, const int3 & Sizes)
-	: sizes(Sizes), aiNk(aiNk), cc(aiNk->cc.get()), nodes(Sizes, aiNk->settings->getPathfinderBucketSize() * aiNk->settings->getPathfinderBucketsCount())
+	: sizes(Sizes), aiNk(aiNk), nodes(Sizes, aiNk->settings->getPathfinderBucketSize() * aiNk->settings->getPathfinderBucketsCount())
 {
 	accessibility = std::make_unique<boost::multi_array<EPathAccessibility, 4>>(
 		boost::extents[sizes.z][sizes.x][sizes.y][EPathfindingLayer::NUM_LAYERS]);
@@ -197,7 +197,6 @@ std::optional<AIPathNode *> AINodeStorage::getOrCreateNode(
 	for(auto i = aiNk->settings->getPathfinderBucketSize() - 1; i >= 0; i--)
 	{
 		AIPathNode & node = chains[i + bucketOffset];
-
 		if(node.version != AISharedStorage::version)
 		{
 			node.reset(layer, getAccessibility(pos, layer));
@@ -1169,11 +1168,11 @@ void AINodeStorage::calculateTownPortal(
 	const std::vector<CGPathNode *> & initialNodes,
 	TVector & output)
 {
-	auto towns = cc->getTownsInfo(false);
+	auto towns = aiNk->cc->getTownsInfo(false);

 	vstd::erase_if(towns, [&](const CGTownInstance * t) -> bool
 		{
-			return cc->getPlayerRelations(actor->hero->tempOwner, t->tempOwner) == PlayerRelations::ENEMIES;
+			return aiNk->cc->getPlayerRelations(actor->hero->tempOwner, t->tempOwner) == PlayerRelations::ENEMIES;
 		});

 	if(!towns.size())
@@ -1399,7 +1398,6 @@ bool AINodeStorage::isOtherChainBetter(
 bool AINodeStorage::isTileAccessible(const HeroPtr & heroPtr, const int3 & pos, const EPathfindingLayer layer) const
 {
 	auto chains = nodes.get(pos);
-
 	for(const AIPathNode & node : chains)
 	{
 		if(node.version == AISharedStorage::version
@@ -1415,7 +1413,7 @@ bool AINodeStorage::isTileAccessible(const HeroPtr & heroPtr, const int3 & pos,
 	return false;
 }

-void AINodeStorage::calculateChainInfo(std::vector<AIPath> & paths, const int3 & pos, bool isOnLand) const
+void AINodeStorage::calculateChainInfo(std::vector<AIPath> & paths, const int3 & pos, const bool isOnLand) const
 {
 	auto layer = isOnLand ? EPathfindingLayer::LAND : EPathfindingLayer::SAIL;
 	auto chains = nodes.get(pos);
@@ -1440,7 +1438,6 @@ void AINodeStorage::calculateChainInfo(std::vector<AIPath> & paths, const int3 &
 		}

 		AIPath & path = paths.emplace_back();
-
 		path.targetHero = node.actor->hero;
 		path.heroArmy = node.actor->creatureSet;
 		path.armyLoss = node.armyLoss;
@@ -1473,7 +1470,7 @@ void AINodeStorage::calculateChainInfo(std::vector<AIPath> & paths, const int3 &
 		}

 		int fortLevel = 0;
-		auto visitableObjects = cc->getVisitableObjs(pos);
+		auto visitableObjects = aiNk->cc->getVisitableObjs(pos);
 		for (auto obj : visitableObjects)
 		{
 			if (objWithID<Obj::TOWN>(obj))
--- a/AI/Nullkiller2/Pathfinding/AINodeStorage.h
+++ b/AI/Nullkiller2/Pathfinding/AINodeStorage.h
@@ -162,11 +162,8 @@ class AINodeStorage : public INodeStorage
 {
 private:
 	int3 sizes;
-
 	std::unique_ptr<boost::multi_array<EPathAccessibility, 4>> accessibility;
-
-	const CPlayerSpecificInfoCallback * cc;
-	const Nullkiller * aiNk;
+	const Nullkiller * aiNk; // TODO: Mircea: Replace with &
 	AISharedStorage nodes;
 	std::vector<std::shared_ptr<ChainActor>> actors;
 	std::vector<CGPathNode *> heroChain;
@@ -265,7 +262,7 @@ public:
 	bool isDistanceLimitReached(const PathNodeInfo & source, CDestinationNodeInfo & destination) const;

 	std::optional<AIPathNode *> getOrCreateNode(const int3 & coord, const EPathfindingLayer layer, const ChainActor * actor);
-	void calculateChainInfo(std::vector<AIPath> & result, const int3 & pos, bool isOnLand) const;
+	void calculateChainInfo(std::vector<AIPath> & paths, const int3 & pos, bool isOnLand) const;
 	bool isTileAccessible(const HeroPtr & heroPtr, const int3 & pos, const EPathfindingLayer layer) const;
 	void setHeroes(std::map<const CGHeroInstance *, HeroRole> heroes);
 	void setScoutTurnDistanceLimit(uint8_t distanceLimit) { turnDistanceLimit[HeroRole::SCOUT] = distanceLimit; }
--- a/AI/Nullkiller2/Pathfinding/AIPathfinder.cpp
+++ b/AI/Nullkiller2/Pathfinding/AIPathfinder.cpp
@@ -49,27 +49,23 @@ void AIPathfinder::calculateQuickPathsWithBlocker(std::vector<AIPath> & result,
 	}
 }

-void AIPathfinder::calculatePathInfo(std::vector<AIPath> & result, const int3 & tile, bool includeGraph) const
+void AIPathfinder::calculatePathInfo(std::vector<AIPath> & paths, const int3 & tile, bool includeGraph) const
 {
 	const TerrainTile * tileInfo = cb->getTile(tile, false);
-
-	result.clear();
-
+	paths.clear();
 	if(!tileInfo)
-	{
 		return;
-	}

-	storage->calculateChainInfo(result, tile, !tileInfo->isWater());
+	storage->calculateChainInfo(paths, tile, !tileInfo->isWater());

 	if(includeGraph)
 	{
-		for(auto hero : cb->getHeroesInfo())
+		for(const auto * hero : cb->getHeroesInfo())
 		{
 			auto graph = heroGraphs.find(hero->id);

 			if(graph != heroGraphs.end())
-				graph->second->addChainInfo(result, tile, hero, aiNk);
+				graph->second->addChainInfo(paths, tile, hero, aiNk);
 		}
 	}
 }
@@ -118,11 +114,11 @@ void AIPathfinder::updatePaths(const std::map<const CGHeroInstance *, HeroRole>

 		do
 		{
-			aiNk->makingTurnInterrupption.interruptionPoint();
+			aiNk->makingTurnInterruption.interruptionPoint();

 			while(storage->calculateHeroChain())
 			{
-				aiNk->makingTurnInterrupption.interruptionPoint();
+				aiNk->makingTurnInterruption.interruptionPoint();

 				logAi->trace("Recalculate paths pass %d", pass++);
 				cb->calculatePaths(config);
@@ -131,11 +127,11 @@ void AIPathfinder::updatePaths(const std::map<const CGHeroInstance *, HeroRole>
 			logAi->trace("Select next actor");
 		} while(storage->selectNextActor());

-		aiNk->makingTurnInterrupption.interruptionPoint();
+		aiNk->makingTurnInterruption.interruptionPoint();

 		if(storage->calculateHeroChainFinal())
 		{
-			aiNk->makingTurnInterrupption.interruptionPoint();
+			aiNk->makingTurnInterruption.interruptionPoint();

 			logAi->trace("Recalculate paths pass final");
 			cb->calculatePaths(config);