AI: hero chain stabilisation

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -76,6 +76,7 @@ void AINodeStorage::clear()
 {
 	actors.clear();
 	heroChainPass = false;
+	heroChainTurn = 0;
 }
 
 const AIPathNode * AINodeStorage::getAINode(const CGPathNode * node) const
@@ -185,12 +186,13 @@ void AINodeStorage::commit(CDestinationNodeInfo & destination, const PathNodeInf
 
 #ifdef VCMI_TRACE_PATHFINDER_EX
 		logAi->trace(
-			"Commited %s -> %s, cost: %f, hero: %s, mask: %i", 
+			"Commited %s -> %s, cost: %f, hero: %s, mask: %x, army: %i", 
 			source.coord.toString(),
 			destination.coord.toString(),
 			destination.cost,
 			dstNode->actor->toString(),
-			dstNode->actor->chainMask);
+			dstNode->actor->chainMask,
+			dstNode->actor->armyValue);
 #endif
 	});
 }
@@ -263,13 +265,13 @@ bool AINodeStorage::calculateHeroChain()
 			if(node.coord.x == 60 && node.coord.y == 56 && node.actor)
 				logAi->trace(node.actor->toString());
 
-			if(node.turns <= heroChainMaxTurns && node.action != CGPathNode::ENodeAction::UNKNOWN)
+			if(node.turns <= heroChainTurn && node.action != CGPathNode::ENodeAction::UNKNOWN)
 				existingChains.push_back(&node);
 		}
 
 		for(AIPathNode * node : existingChains)
 		{
-			if(node->actor->hero)
+			if(node->actor->isMovable)
 			{
 				calculateHeroChain(node, existingChains, newChains);
 			}
@@ -301,8 +303,9 @@ void AINodeStorage::calculateHeroChain(
 {
 	for(AIPathNode * node : variants)
 	{
-		if(node == srcNode || !node->actor || node->turns > heroChainMaxTurns 
-			|| node->action == CGPathNode::ENodeAction::UNKNOWN && node->actor->hero)
+		if(node == srcNode || !node->actor || node->turns > heroChainTurn 
+			|| node->action == CGPathNode::ENodeAction::UNKNOWN && node->actor->hero
+			|| (node->actor->chainMask & srcNode->actor->chainMask) != 0)
 		{
 			continue;
 		}
@@ -325,10 +328,7 @@ void AINodeStorage::calculateHeroChain(
 	AIPathNode * carrier, 
 	AIPathNode * other, 
 	std::vector<ExchangeCandidate> & result) const
-{
-	if(!carrier->actor->isMovable)
-		return;
-	
+{	
 	if(carrier->actor->canExchange(other->actor))
 	{
 #ifdef VCMI_TRACE_PATHFINDER_EX
@@ -404,12 +404,13 @@ void AINodeStorage::addHeroChain(const std::vector<ExchangeCandidate> & result)
 
 #ifdef VCMI_TRACE_PATHFINDER_EX
 		logAi->trace(
-			"Chain accepted at %s %s -> %s, mask %i, cost %f", 
+			"Chain accepted at %s %s -> %s, mask %x, cost %f, army %i", 
 			exchangeNode->coord.toString(), 
 			other->actor->toString(), 
 			exchangeNode->actor->toString(),
 			exchangeNode->actor->chainMask,
-			exchangeNode->cost);
+			exchangeNode->cost,
+			exchangeNode->actor->armyValue);
 #endif
 		heroChain.push_back(exchangeNode);
 	}
@@ -496,7 +497,7 @@ void AINodeStorage::setTownsAndDwellings(
 		}
 	}
 
-	auto dayOfWeek = cb->getDate(Date::DAY_OF_WEEK);
+	/*auto dayOfWeek = cb->getDate(Date::DAY_OF_WEEK);
 	auto waitForGrowth = dayOfWeek > 4;
 
 	for(auto obj: visitableObjs)
@@ -524,7 +525,7 @@ void AINodeStorage::setTownsAndDwellings(
 				}
 			}
 		}
-	}
+	}*/
 }
 
 std::vector<CGPathNode *> AINodeStorage::calculateTeleportations(
@@ -636,44 +637,53 @@ bool AINodeStorage::hasBetterChain(const PathNodeInfo & source, CDestinationNode
 template<class NodeRange>
 bool AINodeStorage::hasBetterChain(
 	const CGPathNode * source, 
-	const AIPathNode * destinationNode,
+	const AIPathNode * candidateNode,
 	const NodeRange & chains) const
 {
-	auto dstActor = destinationNode->actor;
+	auto candidateActor = candidateNode->actor;
 
 	for(const AIPathNode & node : chains)
 	{
-		auto sameNode = node.actor == destinationNode->actor;
+		auto sameNode = node.actor == candidateNode->actor;
 
 		if(sameNode	|| node.action == CGPathNode::ENodeAction::UNKNOWN || !node.actor->hero)
 		{
 			continue;
 		}
 
-		if(node.danger <= destinationNode->danger && destinationNode->actor == node.actor->battleActor)
+		if(node.danger <= candidateNode->danger && candidateNode->actor == node.actor->battleActor)
 		{
-			if(node.cost < destinationNode->cost)
+			if(node.cost < candidateNode->cost)
 			{
 #ifdef VCMI_TRACE_PATHFINDER
 				logAi->trace(
 					"Block ineficient move %s:->%s, mask=%i, mp diff: %i",
 					source->coord.toString(),
-					destinationNode->coord.toString(),
-					destinationNode->actor->chainMask,
-					node.moveRemains - destinationNode->moveRemains);
+					candidateNode->coord.toString(),
+					candidateNode->actor->chainMask,
+					node.moveRemains - candidateNode->moveRemains);
 #endif
 				return true;
 			}
 		}
 
-		if(dstActor->actorExchangeCount == 1)
+		if(candidateActor->actorExchangeCount == 1
+			&& (candidateActor->chainMask & node.actor->chainMask) == 0)
 			continue;
 
 		auto nodeActor = node.actor;
+		auto nodeArmyValue = nodeActor->armyValue - node.armyLoss;
+		auto candidateArmyValue = candidateActor->armyValue - candidateNode->armyLoss;
+
+		if(nodeArmyValue > candidateArmyValue
+			&& node.cost <= candidateNode->cost)
+		{
+			return true;
+		}
 
-		if(nodeActor->armyValue - node.armyLoss >= dstActor->armyValue - destinationNode->armyLoss
-			&& nodeActor->heroFightingStrength >= dstActor->heroFightingStrength
-			&& node.cost >= destinationNode->cost)
+		if(nodeArmyValue == candidateArmyValue
+			&& nodeActor->heroFightingStrength >= candidateActor->heroFightingStrength
+			&& node.cost <= candidateNode->cost)
 		{
 			return true;
 		}

AI/Nullkiller/Pathfinding/AINodeStorage.h

@@ -11,7 +11,7 @@
 #pragma once
 
 #define VCMI_TRACE_PATHFINDER
-#define VCMI_TRACE_PATHFINDER_EX
+#define NVCMI_TRACE_PATHFINDER_EX
 
 #include "../../../lib/CPathfinder.h"
 #include "../../../lib/mapObjects/CGHeroInstance.h"
@@ -87,7 +87,7 @@ private:
 	std::vector<std::shared_ptr<ChainActor>> actors;
 	std::vector<CGPathNode *> heroChain;
 	bool heroChainPass; // true if we need to calculate hero chain
-	int heroChainMaxTurns = 0;
+	int heroChainTurn;
 
 public:
 	/// more than 1 chain layer for each hero allows us to have more than 1 path to each tile so we can chose more optimal one.

AI/Nullkiller/Pathfinding/Actors.cpp

@@ -16,6 +16,11 @@
 #include "../../../lib/mapping/CMap.h"
 #include "../../../lib/mapObjects/MapObjects.h"
 
+bool HeroExchangeArmy::needsLastStack() const
+{
+	return true;
+}
+
 ChainActor::ChainActor(const CGHeroInstance * hero, uint64_t chainMask)
 	:hero(hero), isMovable(true), chainMask(chainMask), creatureSet(hero),
 	baseActor(this), carrierParent(nullptr), otherParent(nullptr), actorExchangeCount(1), armyCost()
@@ -180,6 +185,17 @@ ChainActor * HeroActor::exchange(const ChainActor * specialActor, const ChainAct
 	return &result->specialActors[index];
 }
 
+HeroExchangeMap::~HeroExchangeMap()
+{
+	for(auto & exchange : exchangeMap)
+	{
+		delete exchange.second->creatureSet;
+		delete exchange.second;
+	}
+
+	exchangeMap.clear();
+}
+
 HeroActor * HeroExchangeMap::exchange(const ChainActor * other)
 {
 	HeroActor * result;
@@ -188,7 +204,6 @@ HeroActor * HeroExchangeMap::exchange(const ChainActor * other)
 		result = exchangeMap.at(other);
 	else 
 	{
-		// TODO: decide where to release this CCreatureSet and HeroActor. Probably custom ~ctor?
 		CCreatureSet * newArmy = pickBestCreatures(actor->creatureSet, other->creatureSet);
 		result = new HeroActor(actor, other, newArmy, ai);
 		exchangeMap[other] = result;
@@ -199,7 +214,7 @@ HeroActor * HeroExchangeMap::exchange(const ChainActor * other)
 
 CCreatureSet * HeroExchangeMap::pickBestCreatures(const CCreatureSet * army1, const CCreatureSet * army2) const
 {
-	CCreatureSet * target = new CCreatureSet();
+	CCreatureSet * target = new HeroExchangeArmy();
 	auto bestArmy = ai->ah->getBestArmy(army1, army2);
 
 	for(auto & slotInfo : bestArmy)

AI/Nullkiller/Pathfinding/Actors.h

@@ -18,6 +18,12 @@
 class HeroActor;
 class VCAI;
 
+class HeroExchangeArmy : public CCreatureSet
+{
+public:
+	virtual bool needsLastStack() const override;
+};
+
 class ChainActor
 {
 protected:
@@ -73,6 +79,8 @@ public:
 	{
 	}
 
+	~HeroExchangeMap();
+
 	HeroActor * exchange(const ChainActor * other);
 	bool canExchange(const ChainActor * other);
 

AI/Nullkiller/VCAI.cpp

@@ -603,7 +603,7 @@ void VCAI::init(std::shared_ptr<CCallback> CB)
 	if(!fh)
 		fh = new FuzzyHelper();
 
-	if(playerID.getStr(false) == "blue")
+	if(playerID.getStr(false) == "green")
 	{
 		nullkiller.reset(new Nullkiller());
 	}
@@ -815,18 +815,16 @@ void VCAI::makeTurn()
 		{
 			nullkiller->makeTurn();
 		}
-		else
-		{
-			//it looks messy here, but it's better to have armed heroes before attempting realizing goals
-			for(const CGTownInstance * t : cb->getTownsInfo())
-				moveCreaturesToHero(t);
 
-			mainLoop();
+		//it looks messy here, but it's better to have armed heroes before attempting realizing goals
+		for(const CGTownInstance * t : cb->getTownsInfo())
+			moveCreaturesToHero(t);
 
-			/*Below function is also responsible for hero movement via internal wander function. By design it is separate logic for heroes that have nothing to do.
-			Heroes that were not picked by striveToGoal(sptr(Goals::Win())); recently (so they do not have new goals and cannot continue/reevaluate previously locked goals) will do logic in wander().*/
-			performTypicalActions();
-		}
+		mainLoop();
+
+		/*Below function is also responsible for hero movement via internal wander function. By design it is separate logic for heroes that have nothing to do.
+		Heroes that were not picked by striveToGoal(sptr(Goals::Win())); recently (so they do not have new goals and cannot continue/reevaluate previously locked goals) will do logic in wander().*/
+		performTypicalActions();
 
 		//for debug purpose
 		for (auto h : cb->getHeroesInfo())
@@ -2465,6 +2463,10 @@ void VCAI::recruitHero(const CGTownInstance * t, bool throwing)
 				hero = heroes[1];
 		}
 		cb->recruitHero(t, hero);
+
+		if(t->visitingHero)
+			moveHeroToTile(t->visitablePos(), t->visitingHero.get());
+
 		throw goalFulfilledException(sptr(Goals::RecruitHero().settown(t)));
 	}
 	else if(throwing)