Nullkiller: better tracing and hero locking for hero chain

AI/Nullkiller/Behaviors/CaptureObjectsBehavior.cpp

@@ -22,19 +22,27 @@ extern FuzzyHelper * fh;
 
 using namespace Goals;
 
-std::string CaptureObjectsBehavior::toString() const {
+std::string CaptureObjectsBehavior::toString() const
+{
 	return "Capture objects";
 }
 
-Goals::TGoalVec CaptureObjectsBehavior::getTasks() {
+Goals::TGoalVec CaptureObjectsBehavior::getTasks()
+{
 	Goals::TGoalVec tasks;
-	
-	auto captureObjects = [&](std::vector<const CGObjectInstance*> objs) -> void {
-		if (objs.empty()) {
+
+	auto captureObjects = [&](const std::vector<const CGObjectInstance*> & objs) -> void{
+		if(objs.empty())
+		{
 			return;
 		}
 
-		for (auto objToVisit : objs) {
+		for(auto objToVisit : objs)
+		{			
+#ifdef VCMI_TRACE_PATHFINDER
+			logAi->trace("Checking object %s, %s", objToVisit->getObjectName(), objToVisit->visitablePos().toString());
+#endif
+
 			if(!shouldVisitObject(objToVisit))
 				continue;
 
@@ -42,27 +50,36 @@ Goals::TGoalVec CaptureObjectsBehavior::getTasks() {
 			auto paths = ai->ah->getPathsToTile(pos);
 			std::vector<std::shared_ptr<ExecuteHeroChain>> waysToVisitObj;
 			std::shared_ptr<ExecuteHeroChain> closestWay;
+					
+#ifdef VCMI_TRACE_PATHFINDER
+			logAi->trace("Found %" PRId32 "paths", paths.size());
+#endif
 
 			for(auto & path : paths)
 			{
 #ifdef VCMI_TRACE_PATHFINDER
-				std::stringstream str;
-
-				str << "Path found ";
-
-				for(auto node : path.nodes)
-					str << node.targetHero->name << "->" << node.coord.toString() << "; ";
-
-				logAi->trace(str.str());
+				logAi->trace("Path found %s", path.toString());
 #endif
 
 				if(!shouldVisit(path.targetHero, objToVisit))
-					continue; 
-				
+					continue;
+
 				auto hero = path.targetHero;
 				auto danger = path.getTotalDanger(hero);
+				auto isSafe = isSafeToVisit(hero, path.heroArmy, danger);
+				
+#ifdef VCMI_TRACE_PATHFINDER
+				logAi->trace(
+					"It is %s to visit %s by %s with army %" PRId64 ", danger %" PRId64 " and army loss %" PRId64, 
+					isSafe ? "safe" : "not safe",
+					objToVisit->instanceName, 
+					hero->name,
+					path.getHeroStrength(),
+					danger,
+					path.armyLoss);
+#endif
 
-				if(isSafeToVisit(hero, path.heroArmy, danger))
+				if(isSafe)
 				{
 					auto newWay = std::make_shared<ExecuteHeroChain>(path, objToVisit);
 
@@ -72,26 +89,26 @@ Goals::TGoalVec CaptureObjectsBehavior::getTasks() {
 						closestWay = newWay;
 				}
 			}
-			
+
 			if(waysToVisitObj.empty())
 				continue;
-			
+
 			for(auto way : waysToVisitObj)
 			{
-				way->evaluationContext.closestWayRatio 
+				way->evaluationContext.closestWayRatio
 					= way->evaluationContext.movementCost / closestWay->evaluationContext.movementCost;
 
-				logAi->trace("Behavior %s found %s(%s), danger %d", toString(), way->name(), way->tile.toString(), way->evaluationContext.danger);
-		
 				tasks.push_back(sptr(*way));
 			}
 		}
 	};
 
-	if (specificObjects) {
+	if(specificObjects)
+	{
 		captureObjects(objectsToCapture);
 	}
-	else {
+	else
+	{
 		captureObjects(std::vector<const CGObjectInstance*>(ai->visitableObjs.begin(), ai->visitableObjs.end()));
 	}
 
@@ -102,22 +119,26 @@ bool CaptureObjectsBehavior::shouldVisitObject(ObjectIdRef obj) const
 {
 	const CGObjectInstance* objInstance = obj;
 
-	if (!objInstance || objectTypes.size() && !vstd::contains(objectTypes, objInstance->ID.num)) {
+	if(!objInstance || objectTypes.size() && !vstd::contains(objectTypes, objInstance->ID.num))
+	{
 		return false;
 	}
 
-	if (!objInstance || objectSubTypes.size() && !vstd::contains(objectSubTypes, objInstance->subID)) {
+	if(!objInstance || objectSubTypes.size() && !vstd::contains(objectSubTypes, objInstance->subID))
+	{
 		return false;
 	}
 
 	const int3 pos = objInstance->visitablePos();
 
-	if (vstd::contains(ai->alreadyVisited, objInstance)) {
+	if(vstd::contains(ai->alreadyVisited, objInstance))
+	{
 		return false;
 	}
 
 	auto playerRelations = cb->getPlayerRelations(ai->playerID, objInstance->tempOwner);
-	if (playerRelations != PlayerRelations::ENEMIES && !isWeeklyRevisitable(objInstance)) {
+	if(playerRelations != PlayerRelations::ENEMIES && !isWeeklyRevisitable(objInstance))
+	{
 		return false;
 	}
 
@@ -126,7 +147,7 @@ bool CaptureObjectsBehavior::shouldVisitObject(ObjectIdRef obj) const
 	// -> it will just trigger exchange windows and AI will be confused that obj behind doesn't get visited
 	const CGObjectInstance * topObj = cb->getTopObj(obj->visitablePos());
 
-	if (topObj->ID == Obj::HERO && cb->getPlayerRelations(ai->playerID, topObj->tempOwner) != PlayerRelations::ENEMIES)
+	if(topObj->ID == Obj::HERO && cb->getPlayerRelations(ai->playerID, topObj->tempOwner) != PlayerRelations::ENEMIES)
 		return false;
 	else
 		return true; //all of the following is met

AI/Nullkiller/Engine/Nullkiller.cpp

@@ -49,11 +49,29 @@ Goals::TSubgoal Nullkiller::choseBestTask(Behavior & behavior)
 	return task;
 }
 
+void Nullkiller::resetAiState()
+{
+	lockedHeroes.clear();
+}
+
+void Nullkiller::updateAiState()
+{
+	auto activeHeroes = ai->getMyHeroes();
+
+	vstd::erase_if(activeHeroes, [&](const HeroPtr & hero) -> bool{
+		return vstd::contains(lockedHeroes, hero);
+	});
+
+	ai->ah->updatePaths(activeHeroes, true);
+}
+
 void Nullkiller::makeTurn()
 {
+	resetAiState();
+
 	while(true)
 	{
-		ai->ah->updatePaths(ai->getMyHeroes(), true);
+		updateAiState();
 
 		Goals::TGoalVec bestTasks = {
 			choseBestTask(CaptureObjectsBehavior()),

AI/Nullkiller/Engine/Nullkiller.h

@@ -9,14 +9,18 @@ class Nullkiller
 private:
 	std::unique_ptr<PriorityEvaluator> priorityEvaluator;
 	HeroPtr activeHero;
+	std::set<HeroPtr> lockedHeroes;
 
 public:
 	Nullkiller();
 	void makeTurn();
 	bool isActive(const CGHeroInstance * hero) const { return activeHero.h == hero; }
 	void setActive(const HeroPtr & hero) { activeHero = hero; }
+	void lockHero(const HeroPtr & hero) { lockedHeroes.insert(hero); }
 
 private:
+	void resetAiState();
+	void updateAiState();
 	Goals::TSubgoal choseBestTask(Behavior & behavior);
 	Goals::TSubgoal choseBestTask(Goals::TGoalVec tasks);
 };

AI/Nullkiller/Goals/ExecuteHeroChain.cpp

@@ -26,15 +26,22 @@ using namespace Goals;
 ExecuteHeroChain::ExecuteHeroChain(const AIPath & path, const CGObjectInstance * obj)
 	:CGoal(Goals::EXECUTE_HERO_CHAIN), chainPath(path)
 {
-	if(obj)
-		objid = obj->id.getNum();
-
 	evaluationContext.danger = path.getTotalDanger(hero);
 	evaluationContext.movementCost = path.movementCost();
 	evaluationContext.armyLoss = path.armyLoss;
 	evaluationContext.heroStrength = path.getHeroStrength();
 	hero = path.targetHero;
-	tile = path.firstTileToGet();
+	tile = path.targetTile();
+
+	if(obj)
+	{
+		objid = obj->id.getNum();
+		targetName = obj->getObjectName() + tile.toString();
+	}
+	else
+	{
+		targetName = "tile" + tile.toString();
+	}
 }
 
 bool ExecuteHeroChain::operator==(const ExecuteHeroChain & other) const
@@ -49,18 +56,7 @@ TSubgoal ExecuteHeroChain::whatToDoToAchieve()
 
 void ExecuteHeroChain::accept(VCAI * ai)
 {
-	logAi->debug("Executing hero chain towards %s", tile.toString());
-
-#ifdef VCMI_TRACE_PATHFINDER
-	std::stringstream str;
-
-	str << "Path ";
-
-	for(auto node : chainPath.nodes)
-		str << node.targetHero->name << "->" << node.coord.toString() << "; ";
-
-	logAi->trace(str.str());
-#endif
+	logAi->debug("Executing hero chain towards %s. Path %s", targetName, chainPath.toString());
 
 	std::set<int> blockedIndexes;
 
@@ -69,23 +65,25 @@ void ExecuteHeroChain::accept(VCAI * ai)
 		auto & node = chainPath.nodes[i];
 
 		HeroPtr hero = node.targetHero;
-		auto vt = Goals::sptr(Goals::VisitTile(node.coord).sethero(hero));
 
 		if(vstd::contains(blockedIndexes, i))
 		{
 			blockedIndexes.insert(node.parentIndex);
-			ai->setGoal(hero, vt);
+			ai->nullkiller->lockHero(hero);
 
 			continue;
 		}
 
-		logAi->debug("Moving hero %s to %s", hero.name, node.coord.toString());
+		logAi->debug("Executing chain node %" PRId32 ". Moving hero %s to %s", i, hero.name, node.coord.toString());
 
 		try
 		{
 			ai->nullkiller->setActive(hero);
-			vt->accept(ai);
 
+			Goals::VisitTile(node.coord).sethero(hero).accept(ai);
+
+			// no exception means we were not able to rich the tile
+			ai->nullkiller->lockHero(hero);
 			blockedIndexes.insert(node.parentIndex);
 		}
 		catch(goalFulfilledException)
@@ -102,7 +100,7 @@ void ExecuteHeroChain::accept(VCAI * ai)
 
 std::string ExecuteHeroChain::name() const
 {
-	return "ExecuteHeroChain";
+	return "ExecuteHeroChain " + targetName;
 }
 
 std::string ExecuteHeroChain::completeMessage() const

AI/Nullkiller/Goals/ExecuteHeroChain.h

@@ -17,6 +17,7 @@ namespace Goals
 	{
 	private:
 		AIPath chainPath;
+		std::string targetName;
 
 	public:
 		ExecuteHeroChain(const AIPath & path, const CGObjectInstance * obj = nullptr);

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -76,7 +76,7 @@ void AINodeStorage::clear()
 {
 	actors.clear();
 	heroChainPass = false;
-	heroChainTurn = 0;
+	heroChainTurn = 1;
 }
 
 const AIPathNode * AINodeStorage::getAINode(const CGPathNode * node) const
@@ -184,7 +184,7 @@ void AINodeStorage::commit(CDestinationNodeInfo & destination, const PathNodeInf
 			dstNode->specialAction->applyOnDestination(dstNode->actor->hero, destination, source, dstNode, srcNode);
 		}
 
-#ifdef VCMI_TRACE_PATHFINDER_EX
+#if VCMI_TRACE_PATHFINDER >= 2
 		logAi->trace(
 			"Commited %s -> %s, cost: %f, hero: %s, mask: %x, army: %i", 
 			source.coord.toString(),
@@ -310,7 +310,7 @@ void AINodeStorage::calculateHeroChain(
 			continue;
 		}
 
-#ifdef VCMI_TRACE_PATHFINDER_EX
+#if VCMI_TRACE_PATHFINDER >= 2
 		logAi->trace(
 			"Thy exchange %s[%i] -> %s[%i] at %s",
 			node->actor->toString(),
@@ -331,7 +331,7 @@ void AINodeStorage::calculateHeroChain(
 {	
 	if(carrier->actor->canExchange(other->actor))
 	{
-#ifdef VCMI_TRACE_PATHFINDER_EX
+#if VCMI_TRACE_PATHFINDER >= 2
 		logAi->trace(
 			"Exchange allowed %s[%i] -> %s[%i] at %s",
 			other->actor->toString(),
@@ -341,15 +341,18 @@ void AINodeStorage::calculateHeroChain(
 			carrier->coord.toString());
 #endif
 
-		bool hasLessMp = carrier->turns > other->turns || carrier->moveRemains < other->moveRemains;
-		bool hasLessExperience = carrier->actor->hero->exp < other->actor->hero->exp;
-
-		if(hasLessMp && hasLessExperience)
+		if(other->actor->isMovable)
 		{
-#ifdef VCMI_TRACE_PATHFINDER_EX
-			logAi->trace("Exchange at %s is ineficient. Blocked.", carrier->coord.toString());
+			bool hasLessMp = carrier->turns > other->turns || carrier->moveRemains < other->moveRemains;
+			bool hasLessExperience = carrier->actor->hero->exp < other->actor->hero->exp;
+
+			if(hasLessMp && hasLessExperience)
+			{
+#if VCMI_TRACE_PATHFINDER >= 2
+				logAi->trace("Exchange at %s is ineficient. Blocked.", carrier->coord.toString());
 #endif
-			return;
+				return;
+			}
 		}
 
 		auto newActor = carrier->actor->exchange(other->actor);
@@ -369,7 +372,7 @@ void AINodeStorage::addHeroChain(const std::vector<ExchangeCandidate> & result)
 
 		if(!chainNodeOptional)
 		{
-#ifdef VCMI_TRACE_PATHFINDER_EX
+#if VCMI_TRACE_PATHFINDER >= 2
 			logAi->trace("Exchange at %s can not allocate node. Blocked.", carrier->coord.toString());
 #endif
 			continue;
@@ -379,7 +382,7 @@ void AINodeStorage::addHeroChain(const std::vector<ExchangeCandidate> & result)
 
 		if(exchangeNode->action != CGPathNode::ENodeAction::UNKNOWN)
 		{
-#ifdef VCMI_TRACE_PATHFINDER_EX
+#if VCMI_TRACE_PATHFINDER >= 2
 			logAi->trace("Exchange at %s node is already in use. Blocked.", carrier->coord.toString());
 #endif
 			continue;
@@ -387,7 +390,7 @@ void AINodeStorage::addHeroChain(const std::vector<ExchangeCandidate> & result)
 		
 		if(exchangeNode->turns != 0xFF && exchangeNode->cost < chainInfo.cost)
 		{
-#ifdef VCMI_TRACE_PATHFINDER_EX
+#if VCMI_TRACE_PATHFINDER >= 2
 			logAi->trace(
 				"Exchange at %s is is not effective enough. %f < %f", 
 				exchangeNode->coord.toString(), 
@@ -402,7 +405,7 @@ void AINodeStorage::addHeroChain(const std::vector<ExchangeCandidate> & result)
 		exchangeNode->chainOther = other;
 		exchangeNode->armyLoss = chainInfo.armyLoss;
 
-#ifdef VCMI_TRACE_PATHFINDER_EX
+#if VCMI_TRACE_PATHFINDER >= 2
 		logAi->trace(
 			"Chain accepted at %s %s -> %s, mask %x, cost %f, army %i", 
 			exchangeNode->coord.toString(), 
@@ -785,6 +788,16 @@ int3 AIPath::firstTileToGet() const
 	return int3(-1, -1, -1);
 }
 
+int3 AIPath::targetTile() const
+{
+	if(nodes.size())
+	{
+		return nodes.front().coord;
+	}
+
+	return int3(-1, -1, -1);
+}
+
 const AIPathNodeInfo & AIPath::firstNode() const
 {
 	return nodes.back();
@@ -822,4 +835,14 @@ uint64_t AIPath::getTotalDanger(HeroPtr hero) const
 	uint64_t danger = pathDanger > targetObjectDanger ? pathDanger : targetObjectDanger;
 
 	return danger;
+}
+
+std::string AIPath::toString()
+{
+	std::stringstream str;
+	
+	for(auto node : nodes)
+		str << node.targetHero->name << "->" << node.coord.toString() << "; ";
+
+	return str.str();
 }

AI/Nullkiller/Pathfinding/AINodeStorage.h

@@ -10,8 +10,7 @@
 
 #pragma once
 
-#define VCMI_TRACE_PATHFINDER
-#define NVCMI_TRACE_PATHFINDER_EX
+#define VCMI_TRACE_PATHFINDER 1
 
 #include "../../../lib/CPathfinder.h"
 #include "../../../lib/mapObjects/CGHeroInstance.h"
@@ -61,12 +60,15 @@ struct AIPath
 	uint64_t getTotalDanger(HeroPtr hero) const;
 
 	int3 firstTileToGet() const;
+	int3 targetTile() const;
 
 	const AIPathNodeInfo & firstNode() const;
 
 	float movementCost() const;
 
 	uint64_t getHeroStrength() const;
+
+	std::string toString();
 };
 
 struct ExchangeCandidate : public AIPathNode