Merge pull request #535 from vcmi/VCAI_Fixes

VCAI performance improvements

AI/VCAI/AIUtility.cpp

@@ -191,15 +191,7 @@ bool CDistanceSorter::operator()(const CGObjectInstance * lhs, const CGObjectIns
 	const CGPathNode * ln = ai->myCb->getPathsInfo(hero)->getPathInfo(lhs->visitablePos());
 	const CGPathNode * rn = ai->myCb->getPathsInfo(hero)->getPathInfo(rhs->visitablePos());
 
-	if(ln->turns != rn->turns)
-		return ln->turns < rn->turns;
-
-	return (ln->moveRemains > rn->moveRemains);
-}
-
-bool compareMovement(HeroPtr lhs, HeroPtr rhs)
-{
-	return lhs->movement > rhs->movement;
+	return ln->cost < rn->cost;
 }
 
 ui64 evaluateDanger(crint3 tile)
@@ -410,6 +402,7 @@ bool isBlockedBorderGate(int3 tileToHit) //TODO: is that function needed? should
 	auto gate = dynamic_cast<const CGKeys *>(cb->getTile(tileToHit)->topVisitableObj());
 	return !gate->passableFor(ai->playerID);
 }
+
 bool isBlockVisitObj(const int3 & pos)
 {
 	if(auto obj = cb->getTopObj(pos))
@@ -439,7 +432,6 @@ creInfo infoFromDC(const dwellingContent & dc)
 	return ci;
 }
 
-
 ui64 howManyReinforcementsCanBuy(const CArmedInstance * h, const CGDwelling * t)
 {
 	ui64 aivalue = 0;
@@ -529,14 +521,3 @@ bool compareArtifacts(const CArtifactInstance * a1, const CArtifactInstance * a2
 	else
 		return false;
 }
-
-uint32_t distanceToTile(const CGHeroInstance * hero, int3 pos)
-{
-	auto pathInfo = cb->getPathsInfo(hero)->getPathInfo(pos);
-	uint32_t totalMovementPoints = pathInfo->turns * hero->maxMovePoints(true) + hero->movement;
-
-	if(totalMovementPoints < pathInfo->moveRemains) // should not be but who knows
-		return 0;
-
-	return totalMovementPoints - pathInfo->moveRemains;
-}

AI/VCAI/AIUtility.h

@@ -172,13 +172,11 @@ bool isObjectRemovable(const CGObjectInstance * obj); //FIXME FIXME: move logic
 bool isSafeToVisit(HeroPtr h, uint64_t dangerStrength);
 bool isSafeToVisit(HeroPtr h, crint3 tile);
 
-bool compareMovement(HeroPtr lhs, HeroPtr rhs);
 bool compareHeroStrength(HeroPtr h1, HeroPtr h2);
 bool compareArmyStrength(const CArmedInstance * a1, const CArmedInstance * a2);
 bool compareArtifacts(const CArtifactInstance * a1, const CArtifactInstance * a2);
 ui64 howManyReinforcementsCanBuy(const CArmedInstance * h, const CGDwelling * t);
 ui64 howManyReinforcementsCanGet(const CArmedInstance * h, const CGTownInstance * t);
-uint32_t distanceToTile(const CGHeroInstance * hero, int3 pos);
 
 class CDistanceSorter
 {

AI/VCAI/FuzzyEngines.cpp

@@ -50,15 +50,25 @@ armyStructure evaluateArmyStructure(const CArmedInstance * army)
 	double shootersStrenght = 0;
 	ui32 maxSpeed = 0;
 
+	static const CSelector selectorSHOOTER = Selector::type(Bonus::SHOOTER);
+	static const std::string keySHOOTER = "type_"+std::to_string((int32_t)Bonus::SHOOTER);
+
+	static const CSelector selectorFLYING = Selector::type(Bonus::FLYING);
+	static const std::string keyFLYING = "type_"+std::to_string((int32_t)Bonus::FLYING);
+
+	static const CSelector selectorSTACKS_SPEED = Selector::type(Bonus::STACKS_SPEED);
+	static const std::string keySTACKS_SPEED = "type_"+std::to_string((int32_t)Bonus::STACKS_SPEED);
+
 	for(auto s : army->Slots())
 	{
 		bool walker = true;
-		if(s.second->type->hasBonusOfType(Bonus::SHOOTER))
+		const CCreature * creature = s.second->type;
+		if(creature->hasBonus(selectorSHOOTER, keySHOOTER))
 		{
 			shootersStrenght += s.second->getPower();
 			walker = false;
 		}
-		if(s.second->type->hasBonusOfType(Bonus::FLYING))
+		if(creature->hasBonus(selectorFLYING, keyFLYING))
 		{
 			flyersStrenght += s.second->getPower();
 			walker = false;
@@ -66,7 +76,7 @@ armyStructure evaluateArmyStructure(const CArmedInstance * army)
 		if(walker)
 			walkersStrenght += s.second->getPower();
 
-		vstd::amax(maxSpeed, s.second->type->valOfBonuses(Bonus::STACKS_SPEED));
+		vstd::amax(maxSpeed, creature->valOfBonuses(selectorSTACKS_SPEED, keySTACKS_SPEED));
 	}
 	armyStructure as;
 	as.walkers = walkersStrenght / totalStrenght;
@@ -79,12 +89,15 @@ armyStructure evaluateArmyStructure(const CArmedInstance * army)
 
 float HeroMovementGoalEngineBase::calculateTurnDistanceInputValue(const Goals::AbstractGoal & goal) const
 {
-	if(goal.evaluationContext.movementCost != 0)
+	if(goal.evaluationContext.movementCost > 0)
 	{
-		return goal.evaluationContext.movementCost / (float)goal.hero->maxMovePoints(true);
+		return goal.evaluationContext.movementCost;
+	}
+	else
+	{
+		auto pathInfo = ai->myCb->getPathsInfo(goal.hero.h)->getPathInfo(goal.tile);
+		return pathInfo->cost;
 	}
-
-	return distanceToTile(goal.hero.h, goal.tile) / (float)goal.hero->maxMovePoints(true);
 }
 
 TacticalAdvantageEngine::TacticalAdvantageEngine()
@@ -437,4 +450,4 @@ float VisitTileEngine::evaluate(Goals::VisitTile & goal)
 	}
 	assert(goal.priority >= 0);
 	return goal.priority;
-}
+}

AI/VCAI/Goals/AbstractGoal.h

@@ -91,12 +91,14 @@ namespace Goals
 
 	struct DLL_EXPORT EvaluationContext
 	{
-		uint64_t movementCost;
+		float movementCost;
 		int manaCost;
 		uint64_t danger;
 
 		EvaluationContext()
-			:movementCost(0), danger(0), manaCost(0)
+			: movementCost(0.0),
+			manaCost(0),
+			danger(0)
 		{
 		}
 	};

AI/VCAI/Goals/Explore.cpp

@@ -244,7 +244,8 @@ TSubgoal Explore::explorationBestNeighbour(int3 hpos, int radius, HeroPtr h) con
 				auto distance = hpos.dist2d(tile); // diagonal movement opens more tiles but spends more mp
 				int tilesDiscovered = howManyTilesWillBeDiscovered(tile, radius, cbp, ts, aip, h);
 
-				dstToRevealedTiles[tile] = tilesDiscovered / distance;
+				if(tilesDiscovered > 0)
+					dstToRevealedTiles[tile] = tilesDiscovered / distance;
 			}
 		}
 	}
@@ -252,11 +253,12 @@ TSubgoal Explore::explorationBestNeighbour(int3 hpos, int radius, HeroPtr h) con
 	if(dstToRevealedTiles.empty()) //yes, it DID happen!
 		return sptr(Invalid());
 
+	auto paths = cb->getPathsInfo(h.get());
+
 	auto best = dstToRevealedTiles.begin();
 	for(auto i = dstToRevealedTiles.begin(); i != dstToRevealedTiles.end(); i++)
 	{
-		const CGPathNode * pn = cb->getPathsInfo(h.get())->getPathInfo(i->first);
-		//const TerrainTile *t = cb->getTile(i->first);
+		const CGPathNode * pn = paths->getPathInfo(i->first);
 		if(best->second < i->second && pn->reachable() && pn->accessible == CGPathNode::ACCESSIBLE)
 			best = i;
 	}
@@ -335,7 +337,7 @@ TSubgoal Explore::explorationScanRange(HeroPtr h, std::vector<int3> & range) con
 		auto waysToVisit = aip->ah->howToVisitTile(h, tile, allowGatherArmy);
 		for(auto goal : waysToVisit)
 		{
-			if(goal->evaluationContext.movementCost == 0) // should not happen
+			if(goal->evaluationContext.movementCost <= 0.0) // should not happen
 				continue;
 
 			float ourValue = (float)tilesDiscovered * tilesDiscovered / goal->evaluationContext.movementCost;
@@ -371,9 +373,9 @@ TSubgoal Explore::exploreNearestNeighbour(HeroPtr h) const
 	int radius = h->getSightRadius();
 	int3 hpos = h->visitablePos();
 
-	//look for nearby objs -> visit them if they're close enouh
+	//look for nearby objs -> visit them if they're close enough
 	const int DIST_LIMIT = 3;
-	const int MP_LIMIT = DIST_LIMIT * 150; // aproximate cost of diagonal movement
+	const float COST_LIMIT = .2; //todo: fine tune
 
 	std::vector<const CGObjectInstance *> nearbyVisitableObjs;
 	for(int x = hpos.x - DIST_LIMIT; x <= hpos.x + DIST_LIMIT; ++x) //get only local objects instead of all possible objects on the map
@@ -382,7 +384,7 @@ TSubgoal Explore::exploreNearestNeighbour(HeroPtr h) const
 		{
 			for(auto obj : cb->getVisitableObjs(int3(x, y, hpos.z), false))
 			{
-				if(ai->isGoodForVisit(obj, h, MP_LIMIT))
+				if(ai->isGoodForVisit(obj, h, COST_LIMIT))
 				{
 					nearbyVisitableObjs.push_back(obj);
 				}

AI/VCAI/Goals/Explore.h

@@ -54,9 +54,9 @@ namespace Goals
 		bool hasReachableNeighbor(const int3 &pos, HeroPtr hero, CCallback * cbp, VCAI * vcai) const;
 
 		void getVisibleNeighbours(
-			const std::vector<int3> & tiles, 
-			std::vector<int3> & out, 
-			CCallback * cbp, 
+			const std::vector<int3> & tiles,
+			std::vector<int3> & out,
+			CCallback * cbp,
 			const TeamState * ts) const;
 
 		int howManyTilesWillBeDiscovered(

AI/VCAI/Pathfinding/AINodeStorage.cpp

@@ -14,16 +14,61 @@
 #include "../../../lib/mapping/CMap.h"
 #include "../../../lib/mapObjects/MapObjects.h"
 
+#include "../../../lib/PathfinderUtil.h"
+#include "../../../lib/CPlayerState.h"
 extern boost::thread_specific_ptr<CCallback> cb;
 
+
 AINodeStorage::AINodeStorage(const int3 & Sizes)
 	: sizes(Sizes)
 {
 	nodes.resize(boost::extents[sizes.x][sizes.y][sizes.z][EPathfindingLayer::NUM_LAYERS][NUM_CHAINS]);
 }
 
-AINodeStorage::~AINodeStorage()
+AINodeStorage::~AINodeStorage() = default;
+
+void AINodeStorage::initialize(const PathfinderOptions & options, const CGameState * gs, const CGHeroInstance * hero)
 {
+	//TODO: fix this code duplication with NodeStorage::initialize, problem is to keep `resetTile` inline
+
+	int3 pos;
+	const int3 sizes = gs->getMapSize();
+	const auto & fow = static_cast<const CGameInfoCallback *>(gs)->getPlayerTeam(hero->tempOwner)->fogOfWarMap;
+	const PlayerColor player = hero->tempOwner;
+
+	//make 200% sure that these are loop invariants (also a bit shorter code), let compiler do the rest(loop unswitching)
+	const bool useFlying = options.useFlying;
+	const bool useWaterWalking = options.useWaterWalking;
+
+	for(pos.x=0; pos.x < sizes.x; ++pos.x)
+	{
+		for(pos.y=0; pos.y < sizes.y; ++pos.y)
+		{
+			for(pos.z=0; pos.z < sizes.z; ++pos.z)
+			{
+				const TerrainTile * tile = &gs->map->getTile(pos);
+				switch(tile->terType)
+				{
+				case ETerrainType::ROCK:
+					break;
+
+				case ETerrainType::WATER:
+					resetTile(pos, ELayer::SAIL, PathfinderUtil::evaluateAccessibility<ELayer::SAIL>(pos, tile, fow, player, gs));
+					if(useFlying)
+						resetTile(pos, ELayer::AIR, PathfinderUtil::evaluateAccessibility<ELayer::AIR>(pos, tile, fow, player, gs));
+					if(useWaterWalking)
+						resetTile(pos, ELayer::WATER, PathfinderUtil::evaluateAccessibility<ELayer::WATER>(pos, tile, fow, player, gs));
+					break;
+
+				default:
+					resetTile(pos, ELayer::LAND, PathfinderUtil::evaluateAccessibility<ELayer::LAND>(pos, tile, fow, player, gs));
+					if(useFlying)
+						resetTile(pos, ELayer::AIR, PathfinderUtil::evaluateAccessibility<ELayer::AIR>(pos, tile, fow, player, gs));
+					break;
+				}
+			}
+		}
+	}
 }
 
 const AIPathNode * AINodeStorage::getAINode(const CGPathNode * node) const
@@ -75,6 +120,7 @@ CGPathNode * AINodeStorage::getInitialNode()
 	initialNode->turns = 0;
 	initialNode->moveRemains = hero->movement;
 	initialNode->danger = 0;
+	initialNode->cost = 0.0;
 
 	return initialNode;
 }
@@ -97,9 +143,11 @@ void AINodeStorage::commit(CDestinationNodeInfo & destination, const PathNodeInf
 {
 	const AIPathNode * srcNode = getAINode(source.node);
 
-	updateAINode(destination.node, [&](AIPathNode * dstNode) {
+	updateAINode(destination.node, [&](AIPathNode * dstNode)
+	{
 		dstNode->moveRemains = destination.movementLeft;
 		dstNode->turns = destination.turn;
+		dstNode->cost = destination.cost;
 		dstNode->danger = srcNode->danger;
 		dstNode->action = destination.action;
 		dstNode->theNodeBefore = srcNode->theNodeBefore;
@@ -172,18 +220,18 @@ std::vector<CGPathNode *> AINodeStorage::calculateTeleportations(
 
 	if(source.isNodeObjectVisitable())
 	{
-		auto accessibleExits = pathfinderHelper->getTeleportExits(source);
-		auto srcNode = getAINode(source.node);
+	auto accessibleExits = pathfinderHelper->getTeleportExits(source);
+	auto srcNode = getAINode(source.node);
 
-		for(auto & neighbour : accessibleExits)
-		{
-			auto node = getOrCreateNode(neighbour, source.node->layer, srcNode->chainMask);
+	for(auto & neighbour : accessibleExits)
+	{
+		auto node = getOrCreateNode(neighbour, source.node->layer, srcNode->chainMask);
 
-			if(!node)
-				continue;
+		if(!node)
+			continue;
 
-			neighbours.push_back(node.get());
-		}
+		neighbours.push_back(node.get());
+	}
 	}
 
 	if(hero->getPosition(false) == source.coord)
@@ -276,8 +324,7 @@ bool AINodeStorage::hasBetterChain(const PathNodeInfo & source, CDestinationNode
 
 		if(node.danger <= destinationNode->danger && destinationNode->chainMask == 1 && node.chainMask == 0)
 		{
-			if(node.turns < destinationNode->turns
-				|| (node.turns == destinationNode->turns && node.moveRemains >= destinationNode->moveRemains))
+			if(node.cost < destinationNode->cost)
 			{
 #ifdef VCMI_TRACE_PATHFINDER
 				logAi->trace(
@@ -287,7 +334,6 @@ bool AINodeStorage::hasBetterChain(const PathNodeInfo & source, CDestinationNode
 					destinationNode->chainMask,
 					node.moveRemains - destinationNode->moveRemains);
 #endif
-
 				return true;
 			}
 		}
@@ -298,7 +344,6 @@ bool AINodeStorage::hasBetterChain(const PathNodeInfo & source, CDestinationNode
 
 bool AINodeStorage::isTileAccessible(int3 pos, const EPathfindingLayer layer) const
 {
-	std::vector<AIPath> paths;
 	auto chains = nodes[pos.x][pos.y][pos.z][layer];
 
 	for(const AIPathNode & node : chains)
@@ -331,9 +376,7 @@ std::vector<AIPath> AINodeStorage::getChainInfo(int3 pos, bool isOnLand) const
 		while(current != nullptr && current->coord != initialPos)
 		{
 			AIPathNodeInfo pathNode;
-
-			pathNode.movementPointsLeft = current->moveRemains;
-			pathNode.movementPointsUsed = (int)(current->turns * hero->maxMovePoints(true) + hero->movement) - (int)current->moveRemains;
+			pathNode.cost = current->cost;
 			pathNode.turns = current->turns;
 			pathNode.danger = current->danger;
 			pathNode.coord = current->coord;
@@ -375,15 +418,15 @@ uint64_t AIPath::getPathDanger() const
 	return 0;
 }
 
-uint32_t AIPath::movementCost() const
+float AIPath::movementCost() const
 {
 	if(nodes.size())
 	{
-		return nodes.front().movementPointsUsed;
+		return nodes.front().cost;
 	}
 
 	// TODO: boost:optional?
-	return 0;
+	return 0.0;
 }
 
 uint64_t AIPath::getTotalDanger(HeroPtr hero) const

AI/VCAI/Pathfinding/AINodeStorage.h

@@ -24,7 +24,7 @@ public:
 
 	virtual void applyOnDestination(
 		HeroPtr hero,
-		CDestinationNodeInfo & destination, 
+		CDestinationNodeInfo & destination,
 		const PathNodeInfo & source,
 		AIPathNode * dstMode,
 		const AIPathNode * srcNode) const
@@ -42,8 +42,7 @@ struct AIPathNode : public CGPathNode
 
 struct AIPathNodeInfo
 {
-	uint32_t movementPointsLeft;
-	uint32_t movementPointsUsed;
+	float cost;
 	int turns;
 	int3 coord;
 	uint64_t danger;
@@ -64,7 +63,7 @@ struct AIPath
 
 	int3 firstTileToGet() const;
 
-	uint32_t movementCost() const;
+	float movementCost() const;
 };
 
 class AINodeStorage : public INodeStorage
@@ -76,6 +75,9 @@ private:
 	boost::multi_array<AIPathNode, 5> nodes;
 	const CGHeroInstance * hero;
 
+	STRONG_INLINE
+	void resetTile(const int3 & tile, EPathfindingLayer layer, CGPathNode::EAccessibility accessibility);
+
 public:
 	/// more than 1 chain layer allows us to have more than 1 path to each tile so we can chose more optimal one.
 	static const int NUM_CHAINS = 3;
@@ -89,8 +91,9 @@ public:
 	AINodeStorage(const int3 & sizes);
 	~AINodeStorage();
 
+	void initialize(const PathfinderOptions & options, const CGameState * gs, const CGHeroInstance * hero) override;
+
 	virtual CGPathNode * getInitialNode() override;
-	virtual void resetTile(const int3 & tile, EPathfindingLayer layer, CGPathNode::EAccessibility accessibility) override;
 
 	virtual std::vector<CGPathNode *> calculateNeighbours(
 		const PathNodeInfo & source,

AI/VCAI/Pathfinding/AIPathfinderConfig.cpp

@@ -76,12 +76,14 @@ namespace AIPathfinding
 
 		bool isAffordableBy(HeroPtr hero, const AIPathNode * source) const
 		{
+#ifdef VCMI_TRACE_PATHFINDER
 			logAi->trace(
 				"Hero %s has %d mana and needed %d and already spent %d",
 				hero->name,
 				hero->mana,
 				getManaCost(hero),
 				source->manaCost);
+#endif
 
 			return hero->mana >= source->manaCost + getManaCost(hero);
 		}
@@ -148,7 +150,9 @@ namespace AIPathfinding
 
 				if(virtualBoat && tryEmbarkVirtualBoat(destination, source, virtualBoat))
 				{
+#ifdef VCMI_TRACE_PATHFINDER
 					logAi->trace("Embarking to virtual boat while moving %s -> %s!", source.coord.toString(), destination.coord.toString());
+#endif
 				}
 			}
 		}
@@ -242,15 +246,17 @@ namespace AIPathfinding
 					}
 					else
 					{
+#ifdef VCMI_TRACE_PATHFINDER
 						logAi->trace(
 							"Special transition node already allocated. Blocked moving %s -> %s",
 							source.coord.toString(),
 							destination.coord.toString());
+#endif
 					}
 				}
 				else
 				{
-					logAi->trace(
+					logAi->debug(
 						"Can not allocate special transition node while moving %s -> %s",
 						source.coord.toString(),
 						destination.coord.toString());
@@ -330,10 +336,12 @@ namespace AIPathfinding
 				auto guardsAlreadyBypassed = destGuardians.empty() && srcGuardians.size();
 				if(guardsAlreadyBypassed && nodeStorage->isBattleNode(source.node))
 				{
-					//logAi->trace(
-					//	"Bypass guard at destination while moving %s -> %s",
-					//	source.coord.toString(),
-					//	destination.coord.toString());
+#ifdef VCMI_TRACE_PATHFINDER
+					logAi->trace(
+						"Bypass guard at destination while moving %s -> %s",
+						source.coord.toString(),
+						destination.coord.toString());
+#endif
 
 					return;
 				}
@@ -346,10 +354,12 @@ namespace AIPathfinding
 
 				if(!battleNodeOptional)
 				{
-					//logAi->trace(
-					//	"Can not allocate battle node while moving %s -> %s",
-					//	source.coord.toString(),
-					//	destination.coord.toString());
+#ifdef VCMI_TRACE_PATHFINDER
+					logAi->trace(
+						"Can not allocate battle node while moving %s -> %s",
+						source.coord.toString(),
+						destination.coord.toString());
+#endif
 
 					destination.blocked = true;
 
@@ -360,11 +370,12 @@ namespace AIPathfinding
 
 				if(battleNode->locked)
 				{
-					//logAi->trace(
-					//	"Block bypass guard at destination while moving %s -> %s",
-					//	source.coord.toString(),
-					//	destination.coord.toString());
-
+#ifdef VCMI_TRACE_PATHFINDER
+					logAi->trace(
+						"Block bypass guard at destination while moving %s -> %s",
+						source.coord.toString(),
+						destination.coord.toString());
+#endif
 					destination.blocked = true;
 
 					return;
@@ -382,13 +393,13 @@ namespace AIPathfinding
 				}
 
 				battleNode->specialAction = std::make_shared<BattleAction>(destination.coord);
-
-				//logAi->trace(
-				//	"Begin bypass guard at destination with danger %s while moving %s -> %s",
-				//	std::to_string(danger),
-				//	source.coord.toString(),
-				//	destination.coord.toString());
-
+#ifdef VCMI_TRACE_PATHFINDER
+				logAi->trace(
+					"Begin bypass guard at destination with danger %s while moving %s -> %s",
+					std::to_string(danger),
+					source.coord.toString(),
+					destination.coord.toString());
+#endif
 				return;
 			}
 
@@ -428,11 +439,12 @@ namespace AIPathfinding
 
 			if(blocker == BlockingReason::SOURCE_GUARDED && nodeStorage->isBattleNode(source.node))
 			{
-				//logAi->trace(
-				//	"Bypass src guard while moving from %s to %s",
-				//	source.coord.toString(),
-				//	destination.coord.toString());
-
+#ifdef VCMI_TRACE_PATHFINDER
+				logAi->trace(
+					"Bypass src guard while moving from %s to %s",
+					source.coord.toString(),
+					destination.coord.toString());
+#endif
 				return;
 			}
 
@@ -462,12 +474,12 @@ namespace AIPathfinding
 			{
 				// we can not directly bypass objects, we need to interact with them first
 				destination.node->theNodeBefore = source.node;
-
-				//logAi->trace(
-				//	"Link src node %s to destination node %s while bypassing visitable obj",
-				//	source.coord.toString(),
-				//	destination.coord.toString());
-
+#ifdef VCMI_TRACE_PATHFINDER
+				logAi->trace(
+					"Link src node %s to destination node %s while bypassing visitable obj",
+					source.coord.toString(),
+					destination.coord.toString());
+#endif
 				return;
 			}
 

AI/VCAI/Pathfinding/PathfindingManager.cpp

@@ -124,14 +124,16 @@ Goals::TGoalVec PathfindingManager::findPath(
 
 	std::vector<AIPath> chainInfo = pathfinder->getPathInfo(hero, dest);
 
+#ifdef VCMI_TRACE_PATHFINDER
 	logAi->trace("Trying to find a way for %s to visit tile %s", hero->name, dest.toString());
+#endif
 
 	for(auto path : chainInfo)
 	{
 		int3 firstTileToGet = path.firstTileToGet();
-
+#ifdef VCMI_TRACE_PATHFINDER
 		logAi->trace("Path found size=%i, first tile=%s", path.nodes.size(), firstTileToGet.toString());
-
+#endif
 		if(firstTileToGet.valid() && ai->isTileNotReserved(hero.get(), firstTileToGet))
 		{
 			danger = path.getTotalDanger(hero);
@@ -158,9 +160,9 @@ Goals::TGoalVec PathfindingManager::findPath(
 					solution->evaluationContext.danger = danger;
 
 				solution->evaluationContext.movementCost += path.movementCost();
-
+#ifdef VCMI_TRACE_PATHFINDER
 				logAi->trace("It's safe for %s to visit tile %s with danger %s, goal %s", hero->name, dest.toString(), std::to_string(danger), solution->name());
-
+#endif
 				result.push_back(solution);
 
 				continue;
@@ -178,7 +180,9 @@ Goals::TGoalVec PathfindingManager::findPath(
 	if(allowGatherArmy && danger > 0)
 	{
 		//we need to get army in order to conquer that place
+#ifdef VCMI_TRACE_PATHFINDER
 		logAi->trace("Gather army for %s, value=%s", hero->name, std::to_string(danger));
+#endif
 		result.push_back(sptr(Goals::GatherArmy(danger * SAFE_ATTACK_CONSTANT).sethero(hero).setisAbstract(true)));
 	}
 

AI/VCAI/VCAI.cpp

@@ -65,31 +65,6 @@ struct SetGlobalState
 #define NET_EVENT_HANDLER SET_GLOBAL_STATE(this)
 #define MAKING_TURN SET_GLOBAL_STATE(this)
 
-void foreach_tile(std::vector<std::vector<std::vector<unsigned char>>> & vectors, std::function<void(unsigned char & in)> foo)
-{
-	for(auto & vector : vectors)
-	{
-		for(auto j = vector.begin(); j != vector.end(); j++)
-		{
-			for(auto & elem : *j)
-				foo(elem);
-		}
-	}
-}
-
-struct ObjInfo
-{
-	int3 pos;
-	std::string name;
-	ObjInfo(){}
-	ObjInfo(const CGObjectInstance * obj)
-		: pos(obj->pos), name(obj->getObjectName())
-	{
-	}
-};
-
-std::map<const CGObjectInstance *, ObjInfo> helperObjInfo;
-
 VCAI::VCAI()
 {
 	LOG_TRACE(logAi);
@@ -1014,13 +989,13 @@ void VCAI::mainLoop()
 			//remove goals we couldn't decompose
 			for (auto goal : goalsToRemove)
 				vstd::erase_if_present(basicGoals, goal);
-			
+
 			//add abstract goals
 			boost::sort(goalsToAdd, [](const Goals::TSubgoal & lhs, const Goals::TSubgoal & rhs) -> bool
 			{
 				return lhs->priority > rhs->priority; //highest priority at the beginning
 			});
-			
+
 			//max number of goals = 10
 			int i = 0;
 			while (basicGoals.size() < 10 && goalsToAdd.size() > i)
@@ -1303,17 +1278,17 @@ void VCAI::recruitCreatures(const CGDwelling * d, const CArmedInstance * recruit
 	}
 }
 
-bool VCAI::isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, boost::optional<uint32_t> movementCostLimit)
+bool VCAI::isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, boost::optional<float> movementCostLimit)
 {
 	int3 op = obj->visitablePos();
 	auto paths = ah->getPathsToTile(h, op);
 
-	for(auto path : paths)
+	for(const auto & path : paths)
 	{
 		if(movementCostLimit && movementCostLimit.get() < path.movementCost())
 			return false;
 
-		if(ai->isGoodForVisit(obj, h, path))
+		if(isGoodForVisit(obj, h, path))
 			return true;
 	}
 
@@ -1330,7 +1305,7 @@ bool VCAI::isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, const AIPath
 		return false;
 	if (obj->wasVisited(playerID))
 		return false;
-	if (cb->getPlayerRelations(ai->playerID, obj->tempOwner) != PlayerRelations::ENEMIES && !isWeeklyRevisitable(obj))
+	if (cb->getPlayerRelations(playerID, obj->tempOwner) != PlayerRelations::ENEMIES && !isWeeklyRevisitable(obj))
 		return false; // Otherwise we flag or get weekly resources / creatures
 	if (!isSafeToVisit(h, pos))
 		return false;
@@ -1424,17 +1399,18 @@ void VCAI::wander(HeroPtr h)
 		});
 
 		int pass = 0;
-		std::vector<boost::optional<ui32>> distanceLimits = {
-			h->movement,
-			h->movement + h->maxMovePoints(true),
+		std::vector<boost::optional<float>> distanceLimits =
+		{
+			1.0,
+			2.0,
 			boost::none
 		};
 
 		while(!dests.size() && pass < distanceLimits.size())
 		{
-			boost::optional<ui32> distanceLimit = distanceLimits[pass];
+			auto & distanceLimit = distanceLimits[pass];
 
-			logAi->debug("Looking for wander destination pass=%i, distance limit=%i", pass, distanceLimit.get_value_or(-1));
+			logAi->debug("Looking for wander destination pass=%i, cost limit=%f", pass, distanceLimit.get_value_or(-1.0));
 
 			vstd::copy_if(visitableObjs, std::back_inserter(dests), [&](ObjectIdRef obj) -> bool
 			{
@@ -1773,7 +1749,6 @@ std::vector<const CGObjectInstance *> VCAI::getFlaggedObjects() const
 void VCAI::addVisitableObj(const CGObjectInstance * obj)
 {
 	visitableObjs.insert(obj);
-	helperObjInfo[obj] = ObjInfo(obj);
 
 	// All teleport objects seen automatically assigned to appropriate channels
 	auto teleportObj = dynamic_cast<const CGTeleport *>(obj);
@@ -2218,8 +2193,8 @@ void VCAI::tryRealize(Goals::BuyArmy & g)
 		{
 			auto ci = infoFromDC(t->creatures[i]);
 
-			if(!ci.count 
-				|| ci.creID == -1 
+			if(!ci.count
+				|| ci.creID == -1
 				|| (g.objid != -1 && ci.creID != g.objid)
 				|| t->getUpperArmy()->getSlotFor(ci.creID) == SlotID())
 				continue;

AI/VCAI/VCAI.h

@@ -210,7 +210,7 @@ public:
 	void completeGoal(Goals::TSubgoal goal); //safely removes goal from reserved hero
 
 	void recruitHero(const CGTownInstance * t, bool throwing = false);
-	bool isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, boost::optional<uint32_t> movementCostLimit = boost::none);
+	bool isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, boost::optional<float> movementCostLimit = boost::none);
 	bool isGoodForVisit(const CGObjectInstance * obj, HeroPtr h, const AIPath & path) const;
 	//void recruitCreatures(const CGTownInstance * t);
 	void recruitCreatures(const CGDwelling * d, const CArmedInstance * recruiter);

CCallback.cpp

@@ -286,7 +286,7 @@ bool CCallback::canMoveBetween(const int3 &a, const int3 &b)
 	return gs->map->canMoveBetween(a, b);
 }
 
-const CPathsInfo * CCallback::getPathsInfo(const CGHeroInstance *h)
+std::shared_ptr<const CPathsInfo> CCallback::getPathsInfo(const CGHeroInstance * h)
 {
 	return cl->getPathsInfo(h);
 }

CCallback.h

@@ -106,7 +106,7 @@ public:
 	//client-specific functionalities (pathfinding)
 	virtual bool canMoveBetween(const int3 &a, const int3 &b);
 	virtual int3 getGuardingCreaturePosition(int3 tile);
-	virtual const CPathsInfo * getPathsInfo(const CGHeroInstance *h);
+	virtual std::shared_ptr<const CPathsInfo> getPathsInfo(const CGHeroInstance * h);
 
 	virtual void calculatePaths(const CGHeroInstance *hero, CPathsInfo &out);
 

client/Client.cpp

@@ -109,7 +109,6 @@ public:
 CClient::CClient()
 {
 	waitingRequest.clear();
-	pathInfo = nullptr;
 	applier = std::make_shared<CApplier<CBaseForCLApply>>();
 	registerTypesClientPacks1(*applier);
 	registerTypesClientPacks2(*applier);
@@ -316,7 +315,8 @@ void CClient::initMapHandler()
 		CGI->mh->init();
 		logNetwork->trace("Initializing mapHandler (together): %d ms", CSH->th->getDiff());
 	}
-	pathInfo = make_unique<CPathsInfo>(getMapSize());
+
+	pathCache.clear();
 }
 
 void CClient::initPlayerInterfaces()
@@ -612,20 +612,30 @@ void CClient::waitForMoveAndSend(PlayerColor color)
 
 void CClient::invalidatePaths()
 {
-	// turn pathfinding info into invalid. It will be regenerated later
-	boost::unique_lock<boost::mutex> pathLock(pathInfo->pathMx);
-	pathInfo->hero = nullptr;
+	boost::unique_lock<boost::mutex> pathLock(pathCacheMutex);
+	pathCache.clear();
 }
 
-const CPathsInfo * CClient::getPathsInfo(const CGHeroInstance * h)
+std::shared_ptr<const CPathsInfo> CClient::getPathsInfo(const CGHeroInstance * h)
 {
 	assert(h);
-	boost::unique_lock<boost::mutex> pathLock(pathInfo->pathMx);
-	if(pathInfo->hero != h)
+	boost::unique_lock<boost::mutex> pathLock(pathCacheMutex);
+
+	auto iter = pathCache.find(h);
+
+	if(iter == std::end(pathCache))
+	{
+		std::shared_ptr<CPathsInfo> paths = std::make_shared<CPathsInfo>(getMapSize(), h);
+
+		gs->calculatePaths(h, *paths.get());
+
+		pathCache[h] = paths;
+		return paths;
+	}
+	else
 	{
-		gs->calculatePaths(h, *pathInfo.get());
+		return iter->second;
 	}
-	return pathInfo.get();
 }
 
 PlayerColor CClient::getLocalPlayer() const

client/Client.h

@@ -99,7 +99,9 @@ public:
 class CClient : public IGameCallback
 {
 	std::shared_ptr<CApplier<CBaseForCLApply>> applier;
-	std::unique_ptr<CPathsInfo> pathInfo;
+
+	mutable boost::mutex pathCacheMutex;
+	std::map<const CGHeroInstance *, std::shared_ptr<CPathsInfo>> pathCache;
 
 	std::map<PlayerColor, std::shared_ptr<boost::thread>> playerActionThreads;
 	void waitForMoveAndSend(PlayerColor color);
@@ -150,7 +152,7 @@ public:
 	void stopAllBattleActions();
 
 	void invalidatePaths();
-	const CPathsInfo * getPathsInfo(const CGHeroInstance * h);
+	std::shared_ptr<const CPathsInfo> getPathsInfo(const CGHeroInstance * h);
 	virtual PlayerColor getLocalPlayer() const override;
 
 	friend class CCallback; //handling players actions

client/VCMI_client.cbp

@@ -13,6 +13,7 @@
 				<Option object_output="../obj/Client/Debug/x86" />
 				<Option type="1" />
 				<Option compiler="gcc" />
+				<Option parameters="--testmap=Maps/Reclamation --spectate --spectate-skip-battle --spectate-ignore-hero" />
 				<Compiler>
 					<Add option="-Og" />
 					<Add option="-g" />

client/battle/CBattleInterface.cpp

@@ -3644,7 +3644,7 @@ std::shared_ptr<IImage> CBattleInterface::getObstacleImage(const CObstacleInstan
 
 		if(cacheIter == animationsCache.end())
 		{
-			logAi->trace("Creating obstacle animation %s", animationName);
+			logAnim->trace("Creating obstacle animation %s", animationName);
 
 			animation = std::make_shared<CAnimation>(animationName);
 			animation->preload();

config/schemas/settings.json

@@ -331,7 +331,7 @@
 					"properties" : {
 						"format" : {
 							"type" : "string",
-							"default" : "%d %l %n [%t] - %m"
+							"default" : "%l %n [%t] - %m"
 						}
 					}
 				},

lib/CMakeLists.txt

@@ -341,6 +341,7 @@ set(lib_HEADERS
 		NetPacksBase.h
 		NetPacks.h
 		NetPacksLobby.h
+		PathfinderUtil.h
 		ResourceSet.h
 		ScopeGuard.h
 		StartInfo.h

lib/CPathfinder.cpp

@@ -17,7 +17,8 @@
 #include "GameConstants.h"
 #include "CStopWatch.h"
 #include "CConfigHandler.h"
-#include "../lib/CPlayerState.h"
+#include "CPlayerState.h"
+#include "PathfinderUtil.h"
 
 bool canSeeObj(const CGObjectInstance * obj)
 {
@@ -25,6 +26,50 @@ bool canSeeObj(const CGObjectInstance * obj)
 	return obj != nullptr && obj->ID != Obj::EVENT;
 }
 
+void NodeStorage::initialize(const PathfinderOptions & options, const CGameState * gs, const CGHeroInstance * hero)
+{
+	//TODO: fix this code duplication with AINodeStorage::initialize, problem is to keep `resetTile` inline
+
+	int3 pos;
+	const int3 sizes = gs->getMapSize();
+	const auto & fow = static_cast<const CGameInfoCallback *>(gs)->getPlayerTeam(hero->tempOwner)->fogOfWarMap;
+	const PlayerColor player = hero->tempOwner;
+
+	//make 200% sure that these are loop invariants (also a bit shorter code), let compiler do the rest(loop unswitching)
+	const bool useFlying = options.useFlying;
+	const bool useWaterWalking = options.useWaterWalking;
+
+	for(pos.x=0; pos.x < sizes.x; ++pos.x)
+	{
+		for(pos.y=0; pos.y < sizes.y; ++pos.y)
+		{
+			for(pos.z=0; pos.z < sizes.z; ++pos.z)
+			{
+				const TerrainTile * tile = &gs->map->getTile(pos);
+				switch(tile->terType)
+				{
+				case ETerrainType::ROCK:
+					break;
+
+				case ETerrainType::WATER:
+					resetTile(pos, ELayer::SAIL, PathfinderUtil::evaluateAccessibility<ELayer::SAIL>(pos, tile, fow, player, gs));
+					if(useFlying)
+						resetTile(pos, ELayer::AIR, PathfinderUtil::evaluateAccessibility<ELayer::AIR>(pos, tile, fow, player, gs));
+					if(useWaterWalking)
+						resetTile(pos, ELayer::WATER, PathfinderUtil::evaluateAccessibility<ELayer::WATER>(pos, tile, fow, player, gs));
+					break;
+
+				default:
+					resetTile(pos, ELayer::LAND, PathfinderUtil::evaluateAccessibility<ELayer::LAND>(pos, tile, fow, player, gs));
+					if(useFlying)
+						resetTile(pos, ELayer::AIR, PathfinderUtil::evaluateAccessibility<ELayer::AIR>(pos, tile, fow, player, gs));
+					break;
+				}
+			}
+		}
+	}
+}
+
 std::vector<CGPathNode *> NodeStorage::calculateNeighbours(
 	const PathNodeInfo & source,
 	const PathfinderConfig * pathfinderConfig,
@@ -91,7 +136,7 @@ std::vector<int3> CPathfinderHelper::getNeighbourTiles(const PathNodeInfo & sour
 			return !canMoveBetween(tile, source.nodeObject->visitablePos());
 		});
 	}
-	
+
 	return neighbourTiles;
 }
 
@@ -102,11 +147,6 @@ NodeStorage::NodeStorage(CPathsInfo & pathsInfo, const CGHeroInstance * hero)
 	out.hpos = hero->getPosition(false);
 }
 
-CGPathNode * NodeStorage::getNode(const int3 & coord, const EPathfindingLayer layer)
-{
-	return out.getNode(coord, layer);
-}
-
 void NodeStorage::resetTile(
 	const int3 & tile,
 	EPathfindingLayer layer,
@@ -121,6 +161,7 @@ CGPathNode * NodeStorage::getInitialNode()
 
 	initialNode->turns = 0;
 	initialNode->moveRemains = out.hero->movement;
+	initialNode->cost = 0.0;
 
 	return initialNode;
 }
@@ -128,6 +169,7 @@ CGPathNode * NodeStorage::getInitialNode()
 void NodeStorage::commit(CDestinationNodeInfo & destination, const PathNodeInfo & source)
 {
 	assert(destination.node != source.node->theNodeBefore); //two tiles can't point to each other
+	destination.node->cost = destination.cost;
 	destination.node->moveRemains = destination.movementLeft;
 	destination.node->turns = destination.turn;
 	destination.node->theNodeBefore = source.node;
@@ -157,24 +199,36 @@ void MovementCostRule::process(
 	const PathfinderConfig * pathfinderConfig,
 	CPathfinderHelper * pathfinderHelper) const
 {
-	int turnAtNextTile = destination.turn, moveAtNextTile = destination.movementLeft;
+	float costAtNextTile = destination.cost;
+	int turnAtNextTile = destination.turn;
+	int moveAtNextTile = destination.movementLeft;
 	int cost = pathfinderHelper->getMovementCost(source, destination, moveAtNextTile);
 	int remains = moveAtNextTile - cost;
+	int maxMovePoints = pathfinderHelper->getMaxMovePoints(destination.node->layer);
 	if(remains < 0)
 	{
 		//occurs rarely, when hero with low movepoints tries to leave the road
+		costAtNextTile += static_cast<float>(moveAtNextTile) / maxMovePoints;//we spent all points of current turn
 		pathfinderHelper->updateTurnInfo(++turnAtNextTile);
-		moveAtNextTile = pathfinderHelper->getMaxMovePoints(destination.node->layer);
+
+		maxMovePoints = pathfinderHelper->getMaxMovePoints(destination.node->layer);
+		moveAtNextTile = maxMovePoints;
+
 		cost = pathfinderHelper->getMovementCost(source, destination, moveAtNextTile); //cost must be updated, movement points changed :(
 		remains = moveAtNextTile - cost;
 	}
+
 	if(destination.action == CGPathNode::EMBARK || destination.action == CGPathNode::DISEMBARK)
 	{
 		/// FREE_SHIP_BOARDING bonus only remove additional penalty
 		/// land <-> sail transition still cost movement points as normal movement
-		remains = pathfinderHelper->movementPointsAfterEmbark(moveAtNextTile, cost, destination.action - 1);
+		remains = pathfinderHelper->movementPointsAfterEmbark(moveAtNextTile, cost, (destination.action == CGPathNode::DISEMBARK));
+		cost = moveAtNextTile - remains;
 	}
 
+	costAtNextTile += static_cast<float>(cost) / maxMovePoints;
+
+	destination.cost = costAtNextTile;
 	destination.turn = turnAtNextTile;
 	destination.movementLeft = remains;
 
@@ -221,7 +275,7 @@ CPathfinder::CPathfinder(
 	std::shared_ptr<PathfinderConfig> config)
 	: CGameInfoCallback(_gs, boost::optional<PlayerColor>())
 	, hero(_hero)
-	, FoW(getPlayerTeam(hero->tempOwner)->fogOfWarMap), patrolTiles({})
+	, patrolTiles({})
 	, config(config)
 	, source()
 	, destination()
@@ -261,7 +315,10 @@ void CPathfinder::calculatePaths()
 		pq.pop();
 		source.node->locked = true;
 
-		int movement = source.node->moveRemains, turn = source.node->turns;
+		int movement = source.node->moveRemains;
+		uint8_t turn = source.node->turns;
+		float cost = source.node->cost;
+
 		hlp->updateTurnInfo(turn);
 		if(!movement)
 		{
@@ -296,12 +353,12 @@ void CPathfinder::calculatePaths()
 
 			destination.turn = turn;
 			destination.movementLeft = movement;
-
+			destination.cost = cost;
 			destination.guarded = isDestinationGuarded();
 			destination.isGuardianTile = destination.guarded && isDestinationGuardian();
 			if(destination.nodeObject)
 				destination.objectRelations = gs->getPlayerRelations(hero->tempOwner, destination.nodeObject->tempOwner);
-				
+
 			for(auto rule : config->rules)
 			{
 				rule->process(source, destination, config.get(), hlp.get());
@@ -312,7 +369,7 @@ void CPathfinder::calculatePaths()
 
 			if(!destination.blocked)
 				pq.push(destination.node);
-			
+
 		} //neighbours loop
 
 		//just add all passable teleport exits
@@ -338,6 +395,7 @@ void CPathfinder::calculatePaths()
 			destination.setNode(gs, teleportNode);
 			destination.turn = turn;
 			destination.movementLeft = movement;
+			destination.cost = cost;
 
 			if(destination.isBetterWay())
 			{
@@ -632,7 +690,7 @@ void MovementAfterDestinationRule::process(
 }
 
 PathfinderBlockingRule::BlockingReason MovementAfterDestinationRule::getBlockingReason(
-	const PathNodeInfo & source, 
+	const PathNodeInfo & source,
 	const CDestinationNodeInfo & destination,
 	const PathfinderConfig * config,
 	const CPathfinderHelper * pathfinderHelper) const
@@ -702,7 +760,9 @@ void DestinationActionRule::process(
 {
 	if(destination.action != CGPathNode::ENodeAction::UNKNOWN)
 	{
+#ifdef VCMI_TRACE_PATHFINDER
 		logAi->trace("Accepted precalculated action at %s", destination.coord.toString());
+#endif
 		return;
 	}
 
@@ -851,106 +911,8 @@ void CPathfinder::initializePatrol()
 
 void CPathfinder::initializeGraph()
 {
-	auto updateNode = [&](int3 pos, ELayer layer, const TerrainTile * tinfo)
-	{
-		auto accessibility = evaluateAccessibility(pos, tinfo, layer);
-		
-		config->nodeStorage->resetTile(pos, layer, accessibility);
-	};
-
-	int3 pos;
-	int3 sizes = gs->getMapSize();
-	for(pos.x=0; pos.x < sizes.x; ++pos.x)
-	{
-		for(pos.y=0; pos.y < sizes.y; ++pos.y)
-		{
-			for(pos.z=0; pos.z < sizes.z; ++pos.z)
-			{
-				const TerrainTile * tinfo = &gs->map->getTile(pos);
-				switch(tinfo->terType)
-				{
-				case ETerrainType::ROCK:
-					break;
-
-				case ETerrainType::WATER:
-					updateNode(pos, ELayer::SAIL, tinfo);
-					if(config->options.useFlying)
-						updateNode(pos, ELayer::AIR, tinfo);
-					if(config->options.useWaterWalking)
-						updateNode(pos, ELayer::WATER, tinfo);
-					break;
-
-				default:
-					updateNode(pos, ELayer::LAND, tinfo);
-					if(config->options.useFlying)
-						updateNode(pos, ELayer::AIR, tinfo);
-					break;
-				}
-			}
-		}
-	}
-}
-
-CGPathNode::EAccessibility CPathfinder::evaluateAccessibility(const int3 & pos, const TerrainTile * tinfo, const ELayer layer) const
-{
-	if(tinfo->terType == ETerrainType::ROCK || !FoW[pos.x][pos.y][pos.z])
-		return CGPathNode::BLOCKED;
-
-	switch(layer)
-	{
-	case ELayer::LAND:
-	case ELayer::SAIL:
-		if(tinfo->visitable)
-		{
-			if(tinfo->visitableObjects.front()->ID == Obj::SANCTUARY && tinfo->visitableObjects.back()->ID == Obj::HERO && tinfo->visitableObjects.back()->tempOwner != hero->tempOwner) //non-owned hero stands on Sanctuary
-			{
-				return CGPathNode::BLOCKED;
-			}
-			else
-			{
-				for(const CGObjectInstance * obj : tinfo->visitableObjects)
-				{
-					if(obj->blockVisit)
-					{
-						return CGPathNode::BLOCKVIS;
-					}
-					else if(obj->passableFor(hero->tempOwner))
-					{
-						return CGPathNode::ACCESSIBLE;
-					}
-					else if(canSeeObj(obj))
-					{
-						return CGPathNode::VISITABLE;
-					}
-				}
-			}
-		}
-		else if(tinfo->blocked)
-		{
-			return CGPathNode::BLOCKED;
-		}
-		else if(gs->guardingCreaturePosition(pos).valid())
-		{
-			// Monster close by; blocked visit for battle
-			return CGPathNode::BLOCKVIS;
-		}
-
-		break;
-
-	case ELayer::WATER:
-		if(tinfo->blocked || tinfo->terType != ETerrainType::WATER)
-			return CGPathNode::BLOCKED;
-
-		break;
-
-	case ELayer::AIR:
-		if(tinfo->blocked || tinfo->terType == ETerrainType::WATER)
-			return CGPathNode::FLYABLE;
-
-		break;
-	}
-
-	return CGPathNode::ACCESSIBLE;
+	INodeStorage * nodeStorage = config->nodeStorage.get();
+	nodeStorage->initialize(config->options, gs, hero);
 }
 
 bool CPathfinderHelper::canMoveBetween(const int3 & a, const int3 & b) const
@@ -1007,14 +969,9 @@ bool CPathfinderHelper::addTeleportWhirlpool(const CGWhirlpool * obj) const
 	return options.useTeleportWhirlpool && hasBonusOfType(Bonus::WHIRLPOOL_PROTECTION) && obj;
 }
 
-int CPathfinderHelper::getHeroMaxMovementPoints(EPathfindingLayer layer) const
-{
-	return hero->maxMovePoints(layer);
-}
-
-int CPathfinderHelper::movementPointsAfterEmbark(int movement, int turn, int action) const
+int CPathfinderHelper::movementPointsAfterEmbark(int movement, int basicCost, bool disembark) const
 {
-	return hero->movementPointsAfterEmbark(movement, turn, action, getTurnInfo());
+	return hero->movementPointsAfterEmbark(movement, basicCost, disembark, getTurnInfo());
 }
 
 bool CPathfinderHelper::passOneTurnLimitCheck(const PathNodeInfo & source) const
@@ -1055,10 +1012,7 @@ TurnInfo::BonusCache::BonusCache(TBonusListPtr bl)
 TurnInfo::TurnInfo(const CGHeroInstance * Hero, const int turn)
 	: hero(Hero), maxMovePointsLand(-1), maxMovePointsWater(-1)
 {
-	std::stringstream cachingStr;
-	cachingStr << "days_" << turn;
-
-	bonuses = hero->getAllBonuses(Selector::days(turn), nullptr, nullptr, cachingStr.str());
+	bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, nullptr, "");
 	bonusCache = make_unique<BonusCache>(bonuses);
 	nativeTerrain = hero->getNativeTerrain();
 }
@@ -1117,9 +1071,9 @@ int TurnInfo::valOfBonuses(Bonus::BonusType type, int subtype) const
 int TurnInfo::getMaxMovePoints(const EPathfindingLayer layer) const
 {
 	if(maxMovePointsLand == -1)
-		maxMovePointsLand = hero->maxMovePoints(true, this);
+		maxMovePointsLand = hero->maxMovePointsCached(true, this);
 	if(maxMovePointsWater == -1)
-		maxMovePointsWater = hero->maxMovePoints(false, this);
+		maxMovePointsWater = hero->maxMovePointsCached(false, this);
 
 	return layer == EPathfindingLayer::SAIL ? maxMovePointsWater : maxMovePointsLand;
 }
@@ -1186,10 +1140,10 @@ int CPathfinderHelper::getMaxMovePoints(const EPathfindingLayer layer) const
 }
 
 void CPathfinderHelper::getNeighbours(
-	const TerrainTile & srct, 
+	const TerrainTile & srct,
 	const int3 & tile,
 	std::vector<int3> & vec,
-	const boost::logic::tribool & onLand, 
+	const boost::logic::tribool & onLand,
 	const bool limitCoastSailing) const
 {
 	CMap * map = gs->map;
@@ -1237,10 +1191,10 @@ void CPathfinderHelper::getNeighbours(
 
 int CPathfinderHelper::getMovementCost(
 	const int3 & src,
-	const int3 & dst, 
+	const int3 & dst,
 	const TerrainTile * ct,
 	const TerrainTile * dt,
-	const int remainingMovePoints, 
+	const int remainingMovePoints,
 	const bool checkLast) const
 {
 	if(src == dst) //same tile
@@ -1309,40 +1263,6 @@ int CPathfinderHelper::getMovementCost(
 	return ret;
 }
 
-CGPathNode::CGPathNode()
-	: coord(int3(-1, -1, -1)), layer(ELayer::WRONG)
-{
-	reset();
-}
-
-void CGPathNode::reset()
-{
-	locked = false;
-	accessible = NOT_SET;
-	moveRemains = 0;
-	turns = 255;
-	theNodeBefore = nullptr;
-	action = UNKNOWN;
-}
-
-void CGPathNode::update(const int3 & Coord, const ELayer Layer, const EAccessibility Accessible)
-{
-	if(layer == ELayer::WRONG)
-	{
-		coord = Coord;
-		layer = Layer;
-	}
-	else
-		reset();
-
-	accessible = Accessible;
-}
-
-bool CGPathNode::reachable() const
-{
-	return turns < 255;
-}
-
 int3 CGPath::startPos() const
 {
 	return nodes[nodes.size()-1].coord;
@@ -1364,16 +1284,13 @@ void CGPath::convert(ui8 mode)
 	}
 }
 
-CPathsInfo::CPathsInfo(const int3 & Sizes)
-	: sizes(Sizes)
+CPathsInfo::CPathsInfo(const int3 & Sizes, const CGHeroInstance * hero_)
+	: sizes(Sizes), hero(hero_)
 {
-	hero = nullptr;
 	nodes.resize(boost::extents[sizes.x][sizes.y][sizes.z][ELayer::NUM_LAYERS]);
 }
 
-CPathsInfo::~CPathsInfo()
-{
-}
+CPathsInfo::~CPathsInfo() = default;
 
 const CGPathNode * CPathsInfo::getPathInfo(const int3 & tile) const
 {
@@ -1381,14 +1298,11 @@ const CGPathNode * CPathsInfo::getPathInfo(const int3 & tile) const
 	assert(vstd::iswithin(tile.y, 0, sizes.y));
 	assert(vstd::iswithin(tile.z, 0, sizes.z));
 
-	boost::unique_lock<boost::mutex> pathLock(pathMx);
 	return getNode(tile);
 }
 
 bool CPathsInfo::getPath(CGPath & out, const int3 & dst) const
 {
-	boost::unique_lock<boost::mutex> pathLock(pathMx);
-
 	out.nodes.clear();
 	const CGPathNode * curnode = getNode(dst);
 	if(!curnode->theNodeBefore)
@@ -1403,17 +1317,6 @@ bool CPathsInfo::getPath(CGPath & out, const int3 & dst) const
 	return true;
 }
 
-int CPathsInfo::getDistance(const int3 & tile) const
-{
-	boost::unique_lock<boost::mutex> pathLock(pathMx);
-
-	CGPath ret;
-	if(getPath(ret, tile))
-		return ret.nodes.size();
-	else
-		return 255;
-}
-
 const CGPathNode * CPathsInfo::getNode(const int3 & coord) const
 {
 	auto landNode = &nodes[coord.x][coord.y][coord.z][ELayer::LAND];
@@ -1423,11 +1326,6 @@ const CGPathNode * CPathsInfo::getNode(const int3 & coord) const
 		return &nodes[coord.x][coord.y][coord.z][ELayer::SAIL];
 }
 
-CGPathNode * CPathsInfo::getNode(const int3 & coord, const ELayer layer)
-{
-	return &nodes[coord.x][coord.y][coord.z][layer];
-}
-
 PathNodeInfo::PathNodeInfo()
 	: node(nullptr), nodeObject(nullptr), tile(nullptr), coord(-1, -1, -1), guarded(false)
 {
@@ -1450,7 +1348,9 @@ void PathNodeInfo::setNode(CGameState * gs, CGPathNode * n, bool excludeTopObjec
 }
 
 CDestinationNodeInfo::CDestinationNodeInfo()
-	: PathNodeInfo(), blocked(false), action(CGPathNode::ENodeAction::UNKNOWN)
+	: PathNodeInfo(),
+	blocked(false),
+	action(CGPathNode::ENodeAction::UNKNOWN)
 {
 }
 
@@ -1466,13 +1366,8 @@ bool CDestinationNodeInfo::isBetterWay() const
 {
 	if(node->turns == 0xff) //we haven't been here before
 		return true;
-	else if(node->turns > turn)
-		return true;
-	else if(node->turns >= turn && node->moveRemains < movementLeft) //this route is faster
-		return true;
-
-	return false;
-
+	else
+		return cost < node->cost; //this route is faster
 }
 
 bool PathNodeInfo::isNodeObjectVisitable() const

lib/CPathfinder.h

@@ -49,24 +49,62 @@ struct DLL_LINKAGE CGPathNode
 		NOT_SET = 0,
 		ACCESSIBLE = 1, //tile can be entered and passed
 		VISITABLE, //tile can be entered as the last tile in path
-		BLOCKVIS,  //visitable from neighbouring tile but not passable
+		BLOCKVIS,  //visitable from neighboring tile but not passable
 		FLYABLE, //can only be accessed in air layer
 		BLOCKED //tile can't be entered nor visited
 	};
 
 	CGPathNode * theNodeBefore;
 	int3 coord; //coordinates
-	ui32 moveRemains; //remaining tiles after hero reaches the tile
-	ui8 turns; //how many turns we have to wait before reachng the tile - 0 means current turn
 	ELayer layer;
+	ui32 moveRemains; //remaining movement points after hero reaches the tile
+	float cost; //total cost of the path to this tile measured in turns with fractions
+	ui8 turns; //how many turns we have to wait before reaching the tile - 0 means current turn
+
 	EAccessibility accessible;
 	ENodeAction action;
 	bool locked;
 
-	CGPathNode();
-	void reset();
-	void update(const int3 & Coord, const ELayer Layer, const EAccessibility Accessible);
-	bool reachable() const;
+	CGPathNode()
+		: coord(-1),
+		layer(ELayer::WRONG)
+	{
+		reset();
+	}
+
+	STRONG_INLINE
+	void reset()
+	{
+		locked = false;
+		accessible = NOT_SET;
+		moveRemains = 0;
+		cost = std::numeric_limits<float>::max();
+		turns = 255;
+		theNodeBefore = nullptr;
+		action = UNKNOWN;
+	}
+
+	STRONG_INLINE
+	void update(const int3 & Coord, const ELayer Layer, const EAccessibility Accessible)
+	{
+		if(layer == ELayer::WRONG)
+		{
+			coord = Coord;
+			layer = Layer;
+		}
+		else
+		{
+			reset();
+		}
+
+		accessible = Accessible;
+	}
+
+	STRONG_INLINE
+	bool reachable() const
+	{
+		return turns < 255;
+	}
 };
 
 struct DLL_LINKAGE CGPath
@@ -82,21 +120,22 @@ struct DLL_LINKAGE CPathsInfo
 {
 	typedef EPathfindingLayer ELayer;
 
-	mutable boost::mutex pathMx;
-
 	const CGHeroInstance * hero;
 	int3 hpos;
 	int3 sizes;
 	boost::multi_array<CGPathNode, 4> nodes; //[w][h][level][layer]
 
-	CPathsInfo(const int3 & Sizes);
+	CPathsInfo(const int3 & Sizes, const CGHeroInstance * hero_);
 	~CPathsInfo();
 	const CGPathNode * getPathInfo(const int3 & tile) const;
 	bool getPath(CGPath & out, const int3 & dst) const;
-	int getDistance(const int3 & tile) const;
 	const CGPathNode * getNode(const int3 & coord) const;
 
-	CGPathNode * getNode(const int3 & coord, const ELayer layer);
+	STRONG_INLINE
+	CGPathNode * getNode(const int3 & coord, const ELayer layer)
+	{
+		return &nodes[coord.x][coord.y][coord.z][layer];
+	}
 };
 
 struct DLL_LINKAGE PathNodeInfo
@@ -120,6 +159,7 @@ struct DLL_LINKAGE CDestinationNodeInfo : public PathNodeInfo
 	CGPathNode::ENodeAction action;
 	int turn;
 	int movementLeft;
+	float cost; //same as CGPathNode::cost
 	bool blocked;
 	bool isGuardianTile;
 
@@ -230,55 +270,6 @@ protected:
 		const CPathfinderHelper * pathfinderHelper) const override;
 };
 
-
-class INodeStorage
-{
-public:
-	virtual CGPathNode * getInitialNode() = 0;
-	virtual void resetTile(const int3 & tile, EPathfindingLayer layer, CGPathNode::EAccessibility accessibility) = 0;
-
-	virtual std::vector<CGPathNode *> calculateNeighbours(
-		const PathNodeInfo & source,
-		const PathfinderConfig * pathfinderConfig,
-		const CPathfinderHelper * pathfinderHelper) = 0;
-
-	virtual std::vector<CGPathNode *> calculateTeleportations(
-		const PathNodeInfo & source,
-		const PathfinderConfig * pathfinderConfig,
-		const CPathfinderHelper * pathfinderHelper) = 0;
-
-	virtual void commit(CDestinationNodeInfo & destination, const PathNodeInfo & source) = 0;
-};
-
-class DLL_LINKAGE NodeStorage : public INodeStorage
-{
-private:
-	CPathsInfo & out;
-
-public:
-	NodeStorage(CPathsInfo & pathsInfo, const CGHeroInstance * hero);
-
-	CGPathNode * getNode(const int3 & coord, const EPathfindingLayer layer);
-	virtual CGPathNode * getInitialNode() override;
-
-	virtual std::vector<CGPathNode *> calculateNeighbours(
-		const PathNodeInfo & source,
-		const PathfinderConfig * pathfinderConfig,
-		const CPathfinderHelper * pathfinderHelper) override;
-
-	virtual std::vector<CGPathNode *> calculateTeleportations(
-		const PathNodeInfo & source,
-		const PathfinderConfig * pathfinderConfig,
-		const CPathfinderHelper * pathfinderHelper) override;
-
-	virtual void resetTile(
-		const int3 & tile, 
-		EPathfindingLayer layer, 
-		CGPathNode::EAccessibility accessibility) override;
-
-	virtual void commit(CDestinationNodeInfo & destination, const PathNodeInfo & source) override;
-};
-
 struct DLL_LINKAGE PathfinderOptions
 {
 	bool useFlying;
@@ -330,6 +321,61 @@ struct DLL_LINKAGE PathfinderOptions
 	PathfinderOptions();
 };
 
+class DLL_LINKAGE INodeStorage
+{
+public:
+	using ELayer = EPathfindingLayer;
+	virtual CGPathNode * getInitialNode() = 0;
+
+	virtual std::vector<CGPathNode *> calculateNeighbours(
+		const PathNodeInfo & source,
+		const PathfinderConfig * pathfinderConfig,
+		const CPathfinderHelper * pathfinderHelper) = 0;
+
+	virtual std::vector<CGPathNode *> calculateTeleportations(
+		const PathNodeInfo & source,
+		const PathfinderConfig * pathfinderConfig,
+		const CPathfinderHelper * pathfinderHelper) = 0;
+
+	virtual void commit(CDestinationNodeInfo & destination, const PathNodeInfo & source) = 0;
+
+	virtual void initialize(const PathfinderOptions & options, const CGameState * gs, const CGHeroInstance * hero) = 0;
+};
+
+class DLL_LINKAGE NodeStorage : public INodeStorage
+{
+private:
+	CPathsInfo & out;
+
+	STRONG_INLINE
+	void resetTile(const int3 & tile, EPathfindingLayer layer, CGPathNode::EAccessibility accessibility);
+
+public:
+	NodeStorage(CPathsInfo & pathsInfo, const CGHeroInstance * hero);
+
+	STRONG_INLINE
+	CGPathNode * getNode(const int3 & coord, const EPathfindingLayer layer)
+	{
+		return out.getNode(coord, layer);
+	}
+
+	void initialize(const PathfinderOptions & options, const CGameState * gs, const CGHeroInstance * hero) override;
+
+	virtual CGPathNode * getInitialNode() override;
+
+	virtual std::vector<CGPathNode *> calculateNeighbours(
+		const PathNodeInfo & source,
+		const PathfinderConfig * pathfinderConfig,
+		const CPathfinderHelper * pathfinderHelper) override;
+
+	virtual std::vector<CGPathNode *> calculateTeleportations(
+		const PathNodeInfo & source,
+		const PathfinderConfig * pathfinderConfig,
+		const CPathfinderHelper * pathfinderHelper) override;
+
+	virtual void commit(CDestinationNodeInfo & destination, const PathNodeInfo & source) override;
+};
+
 class DLL_LINKAGE PathfinderConfig
 {
 public:
@@ -349,17 +395,16 @@ public:
 
 	CPathfinder(CPathsInfo & _out, CGameState * _gs, const CGHeroInstance * _hero);
 	CPathfinder(
-		CGameState * _gs, 
-		const CGHeroInstance * _hero, 
+		CGameState * _gs,
+		const CGHeroInstance * _hero,
 		std::shared_ptr<PathfinderConfig> config);
 
 	void calculatePaths(); //calculates possible paths for hero, uses current hero position and movement left; returns pointer to newly allocated CPath or nullptr if path does not exists
 
 private:
 	typedef EPathfindingLayer ELayer;
-	
+
 	const CGHeroInstance * hero;
-	const std::vector<std::vector<std::vector<ui8> > > &FoW;
 	std::unique_ptr<CPathfinderHelper> hlp;
 	std::shared_ptr<PathfinderConfig> config;
 
@@ -372,14 +417,10 @@ private:
 
 	struct NodeComparer
 	{
+		STRONG_INLINE
 		bool operator()(const CGPathNode * lhs, const CGPathNode * rhs) const
 		{
-			if(rhs->turns > lhs->turns)
-				return false;
-			else if(rhs->turns == lhs->turns && rhs->moveRemains <= lhs->moveRemains)
-				return false;
-
-			return true;
+			return lhs->cost > rhs->cost;
 		}
 	};
 	boost::heap::priority_queue<CGPathNode *, boost::heap::compare<NodeComparer> > pq;
@@ -400,8 +441,6 @@ private:
 
 	void initializePatrol();
 	void initializeGraph();
-
-	CGPathNode::EAccessibility evaluateAccessibility(const int3 & pos, const TerrainTile * tinfo, const ELayer layer) const;
 };
 
 struct DLL_LINKAGE TurnInfo
@@ -463,17 +502,17 @@ public:
 
 	void getNeighbours(
 		const TerrainTile & srct,
-		const int3 & tile, 
-		std::vector<int3> & vec, 
-		const boost::logic::tribool & onLand, 
+		const int3 & tile,
+		std::vector<int3> & vec,
+		const boost::logic::tribool & onLand,
 		const bool limitCoastSailing) const;
-	
+
 	int getMovementCost(
-		const int3 & src, 
-		const int3 & dst, 
+		const int3 & src,
+		const int3 & dst,
 		const TerrainTile * ct,
 		const TerrainTile * dt,
-		const int remainingMovePoints =- 1, 
+		const int remainingMovePoints =- 1,
 		const bool checkLast = true) const;
 
 	int getMovementCost(
@@ -492,7 +531,6 @@ public:
 		);
 	}
 
-	int getHeroMaxMovementPoints(EPathfindingLayer layer) const;
-	int movementPointsAfterEmbark(int movement, int cost, int action) const;
+	int movementPointsAfterEmbark(int movement, int basicCost, bool disembark) const;
 	bool passOneTurnLimitCheck(const PathNodeInfo & source) const;
 };

lib/HeroBonus.cpp

@@ -474,14 +474,14 @@ int IBonusBearer::valOfBonuses(Bonus::BonusType type, const CSelector &selector)
 
 int IBonusBearer::valOfBonuses(Bonus::BonusType type, int subtype) const
 {
-	std::stringstream cachingStr;
-	cachingStr << "type_" << type << "s_" << subtype;
+	boost::format fmt("type_%ds_%d");
+	fmt % (int)type % subtype;
 
 	CSelector s = Selector::type(type);
 	if(subtype != -1)
 		s = s.And(Selector::subtype(subtype));
 
-	return valOfBonuses(s, cachingStr.str());
+	return valOfBonuses(s, fmt.str());
 }
 
 int IBonusBearer::valOfBonuses(const CSelector &selector, const std::string &cachingStr) const
@@ -502,14 +502,14 @@ bool IBonusBearer::hasBonus(const CSelector &selector, const CSelector &limit, c
 
 bool IBonusBearer::hasBonusOfType(Bonus::BonusType type, int subtype) const
 {
-	std::stringstream cachingStr;
-	cachingStr << "type_" << type << "s_" << subtype;
+	boost::format fmt("type_%ds_%d");
+	fmt % (int)type % subtype;
 
 	CSelector s = Selector::type(type);
 	if(subtype != -1)
 		s = s.And(Selector::subtype(subtype));
 
-	return hasBonus(s, cachingStr.str());
+	return hasBonus(s, fmt.str());
 }
 
 const TBonusListPtr IBonusBearer::getBonuses(const CSelector &selector, const std::string &cachingStr) const
@@ -524,9 +524,10 @@ const TBonusListPtr IBonusBearer::getBonuses(const CSelector &selector, const CS
 
 bool IBonusBearer::hasBonusFrom(Bonus::BonusSource source, ui32 sourceID) const
 {
-	std::stringstream cachingStr;
-	cachingStr << "source_" << source << "id_" << sourceID;
-	return hasBonus(Selector::source(source,sourceID), cachingStr.str());
+	boost::format fmt("source_%did_%d");
+	fmt % (int)source % sourceID;
+
+	return hasBonus(Selector::source(source,sourceID), fmt.str());
 }
 
 int IBonusBearer::MoraleVal() const
@@ -605,7 +606,12 @@ si32 IBonusBearer::manaLimit() const
 
 int IBonusBearer::getPrimSkillLevel(PrimarySkill::PrimarySkill id) const
 {
-	int ret = valOfBonuses(Bonus::PRIMARY_SKILL, id);
+	static const CSelector selectorAllSkills = Selector::type(Bonus::PRIMARY_SKILL);
+	static const std::string keyAllSkills = "type_PRIMARY_SKILL";
+
+	auto allSkills = getBonuses(selectorAllSkills, keyAllSkills);
+
+	int ret = allSkills->valOfBonuses(Selector::subtype(id));
 
 	vstd::amax(ret, id/2); //minimal value is 0 for attack and defense and 1 for spell power and knowledge
 	return ret;
@@ -634,11 +640,12 @@ ui32 IBonusBearer::Speed(int turn, bool useBind) const
 
 bool IBonusBearer::isLiving() const //TODO: theoreticaly there exists "LIVING" bonus in stack experience documentation
 {
-	std::stringstream cachingStr;
-	cachingStr << "type_" << Bonus::UNDEAD << "s_-1Otype_" << Bonus::NON_LIVING << "s_-11type_" << Bonus::SIEGE_WEAPON; //I don't really get what string labels mean?
-	return !hasBonus(Selector::type(Bonus::UNDEAD)
-					.Or(Selector::type(Bonus::NON_LIVING))
-					.Or(Selector::type(Bonus::SIEGE_WEAPON)), cachingStr.str());
+	static const std::string cachingStr = "IBonusBearer::isLiving";
+	static const CSelector selector = Selector::type(Bonus::UNDEAD)
+		.Or(Selector::type(Bonus::NON_LIVING))
+		.Or(Selector::type(Bonus::SIEGE_WEAPON));
+
+	return !hasBonus(selector, cachingStr);
 }
 
 const std::shared_ptr<Bonus> IBonusBearer::getBonus(const CSelector &selector) const

lib/PathfinderUtil.h

@@ -0,0 +1,76 @@
+/*
+ * PathfinderUtil.h, 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
+ *
+ */
+#pragma once
+
+#include "mapping/CMapDefines.h"
+#include "CGameState.h"
+
+namespace PathfinderUtil
+{
+	using FoW = std::vector<std::vector<std::vector<ui8> > >;
+	using ELayer = EPathfindingLayer;
+
+	template<EPathfindingLayer::EEPathfindingLayer layer>
+	CGPathNode::EAccessibility evaluateAccessibility(const int3 & pos, const TerrainTile * tinfo, const FoW & fow, const PlayerColor player, const CGameState * gs)
+	{
+		if(!fow[pos.x][pos.y][pos.z])
+			return CGPathNode::BLOCKED;
+
+		switch(layer)
+		{
+		case ELayer::LAND:
+		case ELayer::SAIL:
+			if(tinfo->visitable)
+			{
+				if(tinfo->visitableObjects.front()->ID == Obj::SANCTUARY && tinfo->visitableObjects.back()->ID == Obj::HERO && tinfo->visitableObjects.back()->tempOwner != player) //non-owned hero stands on Sanctuary
+				{
+					return CGPathNode::BLOCKED;
+				}
+				else
+				{
+					for(const CGObjectInstance * obj : tinfo->visitableObjects)
+					{
+						if(obj->blockVisit)
+							return CGPathNode::BLOCKVIS;
+						else if(obj->passableFor(player))
+							return CGPathNode::ACCESSIBLE;
+						else if(obj->ID != Obj::EVENT)
+							return CGPathNode::VISITABLE;
+					}
+				}
+			}
+			else if(tinfo->blocked)
+			{
+				return CGPathNode::BLOCKED;
+			}
+			else if(gs->guardingCreaturePosition(pos).valid())
+			{
+				// Monster close by; blocked visit for battle
+				return CGPathNode::BLOCKVIS;
+			}
+
+			break;
+
+		case ELayer::WATER:
+			if(tinfo->blocked || tinfo->terType != ETerrainType::WATER)
+				return CGPathNode::BLOCKED;
+
+			break;
+
+		case ELayer::AIR:
+			if(tinfo->blocked || tinfo->terType == ETerrainType::WATER)
+				return CGPathNode::FLYABLE;
+
+			break;
+		}
+
+		return CGPathNode::ACCESSIBLE;
+	}
+}

lib/VCMI_lib.cbp

@@ -202,6 +202,7 @@
 		<Unit filename="NetPacksBase.h" />
 		<Unit filename="NetPacksLib.cpp" />
 		<Unit filename="NetPacksLobby.h" />
+		<Unit filename="PathfinderUtil.h" />
 		<Unit filename="ResourceSet.cpp" />
 		<Unit filename="ResourceSet.h" />
 		<Unit filename="ScopeGuard.h" />
@@ -331,6 +332,7 @@
 		<Unit filename="mapping/CDrawRoadsOperation.h" />
 		<Unit filename="mapping/CMap.cpp" />
 		<Unit filename="mapping/CMap.h" />
+		<Unit filename="mapping/CMapDefines.h" />
 		<Unit filename="mapping/CMapEditManager.cpp" />
 		<Unit filename="mapping/CMapEditManager.h" />
 		<Unit filename="mapping/CMapInfo.cpp" />

lib/logging/CLogger.cpp

@@ -231,12 +231,14 @@ std::vector<std::string> CLogManager::getRegisteredDomains() const
 	return std::move(domains);
 }
 
-CLogFormatter::CLogFormatter() : CLogFormatter("%m") { }
+CLogFormatter::CLogFormatter()
+	: CLogFormatter("%m")
+{
+}
 
-CLogFormatter::CLogFormatter(const std::string & pattern) : pattern(pattern)
+CLogFormatter::CLogFormatter(const std::string & pattern)
+	: pattern(pattern)
 {
-	boost::posix_time::time_facet * facet = new boost::posix_time::time_facet("%H:%M:%S.%f");
-	dateStream.imbue(std::locale(dateStream.getloc(), facet));
 }
 
 CLogFormatter::CLogFormatter(const CLogFormatter & c) : pattern(c.pattern) { }
@@ -258,7 +260,7 @@ std::string CLogFormatter::format(const LogRecord & record) const
 	std::string message = pattern;
 
 	//Format date
-	boost::algorithm::replace_first(message, "%d", boost::posix_time::to_simple_string (record.timeStamp));
+//	boost::algorithm::replace_first(message, "%d", boost::posix_time::to_simple_string (record.timeStamp));
 
 	//Format log level
 	std::string level;
@@ -387,7 +389,8 @@ void CLogConsoleTarget::setColorMapping(const CColorMapping & colorMapping) { th
 CLogFileTarget::CLogFileTarget(boost::filesystem::path filePath, bool append)
 	: file(std::move(filePath), append ? std::ios_base::app : std::ios_base::out)
 {
-	formatter.setPattern("%d %l %n [%t] - %m");
+//	formatter.setPattern("%d %l %n [%t] - %m");
+	formatter.setPattern("%l %n [%t] - %m");
 }
 
 void CLogFileTarget::write(const LogRecord & record)

lib/logging/CLogger.h

@@ -149,7 +149,6 @@ public:
 
 private:
 	std::string pattern;
-	mutable std::stringstream dateStream;
 };
 
 /// The interface ILogTarget is used by all log target implementations. It holds

lib/mapObjects/CGHeroInstance.cpp

@@ -55,13 +55,17 @@ static int lowestSpeed(const CGHeroInstance * chi)
 	}
 	auto i = chi->Slots().begin();
 	//TODO? should speed modifiers (eg from artifacts) affect hero movement?
-	int ret = (i++)->second->valOfBonuses(Bonus::STACKS_SPEED);
+
+	static const CSelector selectorSTACKS_SPEED = Selector::type(Bonus::STACKS_SPEED);
+	static const std::string keySTACKS_SPEED = "type_"+std::to_string((si32)Bonus::STACKS_SPEED);
+
+	int ret = (i++)->second->valOfBonuses(selectorSTACKS_SPEED, keySTACKS_SPEED);
 	for(; i != chi->Slots().end(); i++)
-		ret = std::min(ret, i->second->valOfBonuses(Bonus::STACKS_SPEED));
+		ret = std::min(ret, i->second->valOfBonuses(selectorSTACKS_SPEED, keySTACKS_SPEED));
 	return ret;
 }
 
-ui32 CGHeroInstance::getTileCost(const TerrainTile &dest, const TerrainTile &from, const TurnInfo * ti) const
+ui32 CGHeroInstance::getTileCost(const TerrainTile & dest, const TerrainTile & from, const TurnInfo * ti) const
 {
 	unsigned ret = GameConstants::BASE_MOVEMENT_COST;
 
@@ -87,8 +91,11 @@ ui32 CGHeroInstance::getTileCost(const TerrainTile &dest, const TerrainTile &fro
 	}
 	else if(ti->nativeTerrain != from.terType && !ti->hasBonusOfType(Bonus::NO_TERRAIN_PENALTY, from.terType))
 	{
+		static const CSelector selectorPATHFINDING = Selector::typeSubtype(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::PATHFINDING);
+		static const std::string keyPATHFINDING = "type_"+std::to_string((si32)Bonus::SECONDARY_SKILL_PREMY)+"s_"+std::to_string((si32)SecondarySkill::PATHFINDING);
+
 		ret = VLC->heroh->terrCosts[from.terType];
-		ret -= valOfBonuses(Selector::typeSubtype(Bonus::SECONDARY_SKILL_PREMY, SecondarySkill::PATHFINDING));
+		ret -= valOfBonuses(selectorPATHFINDING, keyPATHFINDING);
 		if(ret < GameConstants::BASE_MOVEMENT_COST)
 			ret = GameConstants::BASE_MOVEMENT_COST;
 	}
@@ -186,15 +193,14 @@ bool CGHeroInstance::canLearnSkill() const
 	return secSkills.size() < GameConstants::SKILL_PER_HERO;
 }
 
-int CGHeroInstance::maxMovePoints(bool onLand, const TurnInfo * ti) const
+int CGHeroInstance::maxMovePoints(bool onLand) const
 {
-	bool localTi = false;
-	if(!ti)
-	{
-		localTi = true;
-		ti = new TurnInfo(this);
-	}
+	TurnInfo ti(this);
+	return maxMovePointsCached(onLand, &ti);
+}
 
+int CGHeroInstance::maxMovePointsCached(bool onLand, const TurnInfo * ti) const
+{
 	int base;
 
 	if(onLand)
@@ -218,10 +224,7 @@ int CGHeroInstance::maxMovePoints(bool onLand, const TurnInfo * ti) const
 	const int subtype = onLand ? SecondarySkill::LOGISTICS : SecondarySkill::NAVIGATION;
 	const double modifier = ti->valOfBonuses(Bonus::SECONDARY_SKILL_PREMY, subtype) / 100.0;
 
-	if(localTi)
-		delete ti;
-
-	return int(base* (1+modifier)) + bonus;
+	return int(base * (1 + modifier)) + bonus;
 }
 
 CGHeroInstance::CGHeroInstance()

lib/mapObjects/CGHeroInstance.h

@@ -195,7 +195,10 @@ public:
 	void setSecSkillLevel(SecondarySkill which, int val, bool abs);// abs == 0 - changes by value; 1 - sets to value
 	void levelUp(std::vector<SecondarySkill> skills);
 
-	int maxMovePoints(bool onLand, const TurnInfo * ti = nullptr) const;
+	int maxMovePoints(bool onLand) const;
+	//cached version is much faster, TurnInfo construction is costly
+	int maxMovePointsCached(bool onLand, const TurnInfo * ti) const;
+
 	int movementPointsAfterEmbark(int MPsBefore, int basicCost, bool disembark = false, const TurnInfo * ti = nullptr) const;
 
 	static int3 convertPosition(int3 src, bool toh3m); //toh3m=true: manifest->h3m; toh3m=false: h3m->manifest

lib/spells/effects/Obstacle.cpp

@@ -280,7 +280,12 @@ void Obstacle::placeObstacles(BattleStateProxy * battleState, const Mechanics *
 
 	BattleObstaclesChanged pack;
 
-	auto all = m->cb->battleGetAllObstacles(BattlePerspective::ALL_KNOWING);
+	boost::optional<BattlePerspective::BattlePerspective> perspective;
+
+	if(!m->cb->getPlayerID())
+		perspective = boost::make_optional(BattlePerspective::ALL_KNOWING);
+
+	auto all = m->cb->battleGetAllObstacles(perspective);
 
 	int obstacleIdToGive = 1;
 	for(auto & one : all)

server/CGameHandler.cpp

@@ -1750,7 +1750,7 @@ void CGameHandler::newTurn()
 			hth.id = h->id;
 			auto ti = make_unique<TurnInfo>(h, 1);
 			// TODO: this code executed when bonuses of previous day not yet updated (this happen in NewTurn::applyGs). See issue 2356
-			hth.move = h->maxMovePoints(gs->map->getTile(h->getPosition(false)).terType != ETerrainType::WATER, ti.get());
+			hth.move = h->maxMovePointsCached(gs->map->getTile(h->getPosition(false)).terType != ETerrainType::WATER, ti.get());
 			hth.mana = h->getManaNewTurn();
 
 			n.heroes.insert(hth);