NKAI: fix sonar and refactoring

AI/Nullkiller/Analyzers/DangerHitMapAnalyzer.cpp

@@ -157,12 +157,13 @@ void DangerHitMapAnalyzer::calculateTileOwners()
 	if(hitMap.shape()[0] != mapSize.x || hitMap.shape()[1] != mapSize.y || hitMap.shape()[2] != mapSize.z)
 		hitMap.resize(boost::extents[mapSize.x][mapSize.y][mapSize.z]);
 
-	std::map<const CGHeroInstance *, HeroRole> townHeroes;
+	std::vector<std::unique_ptr<CGHeroInstance>> temporaryHeroes;
 	std::map<const CGHeroInstance *, const CGTownInstance *> heroTownMap;
+	std::map<const CGHeroInstance *, HeroRole> townHeroes;
 
 	auto addTownHero = [&](const CGTownInstance * town)
 	{
-			auto townHero = new CGHeroInstance(town->cb);
+			auto townHero = temporaryHeroes.emplace_back(std::make_unique<CGHeroInstance>(town->cb)).get();
 			CRandomGenerator rng;
 			auto visitablePos = town->visitablePos();
 			
@@ -238,11 +239,6 @@ void DangerHitMapAnalyzer::calculateTileOwners()
 				hitMap[pos.x][pos.y][pos.z].closestTown = enemyTown;
 			}
 		});
-
-	for(auto h : townHeroes)
-	{
-		delete h.first;
-	}
 }
 
 const std::vector<HitMapInfo> & DangerHitMapAnalyzer::getTownThreats(const CGTownInstance * town) const

AI/Nullkiller/Analyzers/ObjectClusterizer.cpp

@@ -189,15 +189,7 @@ bool ObjectClusterizer::shouldVisitObject(const CGObjectInstance * obj) const
 		return true; //all of the following is met
 }
 
-void ObjectClusterizer::clusterize()
+Obj ObjectClusterizer::IgnoredObjectTypes[] = {
-{
-	auto start = std::chrono::high_resolution_clock::now();
-
-	nearObjects.reset();
-	farObjects.reset();
-	blockedObjects.clear();
-
-	Obj ignoreObjects[] = {
 		Obj::BOAT,
 		Obj::EYE_OF_MAGI,
 		Obj::MONOLITH_ONE_WAY_ENTRANCE,
@@ -218,6 +210,14 @@ void ObjectClusterizer::clusterize()
 		Obj::PILLAR_OF_FIRE
 };
 
+void ObjectClusterizer::clusterize()
+{
+	auto start = std::chrono::high_resolution_clock::now();
+
+	nearObjects.reset();
+	farObjects.reset();
+	blockedObjects.clear();
+
 	logAi->debug("Begin object clusterization");
 
 	std::vector<const CGObjectInstance *> objs(
@@ -225,23 +225,46 @@ void ObjectClusterizer::clusterize()
 		ai->memory->visitableObjs.end());
 
 #if NKAI_TRACE_LEVEL == 0
-	parallel_for(blocked_range<size_t>(0, objs.size()), [&](const blocked_range<size_t> & r)
+	parallel_for(blocked_range<size_t>(0, objs.size()), [&](const blocked_range<size_t> & r) {
 #else
 	blocked_range<size_t> r(0, objs.size());
 #endif
-	{
 		auto priorityEvaluator = ai->priorityEvaluators->acquire();
 
 		for(int i = r.begin(); i != r.end(); i++)
 		{
-			auto obj = objs[i];
+			clusterizeObject(objs[i], priorityEvaluator.get());
+		}
+#if NKAI_TRACE_LEVEL == 0
+	});
+#endif
 
+	logAi->trace("Near objects count: %i", nearObjects.objects.size());
+	logAi->trace("Far objects count: %i", farObjects.objects.size());
+
+	for(auto pair : blockedObjects)
+	{
+		logAi->trace("Cluster %s %s count: %i", pair.first->getObjectName(), pair.first->visitablePos().toString(), pair.second->objects.size());
+
+#if NKAI_TRACE_LEVEL >= 1
+		for(auto obj : pair.second->getObjects())
+		{
+			logAi->trace("Object %s %s", obj->getObjectName(), obj->visitablePos().toString());
+		}
+#endif
+	}
+
+	logAi->trace("Clusterization complete in %ld", timeElapsed(start));
+}
+
+void ObjectClusterizer::clusterizeObject(const CGObjectInstance * obj, PriorityEvaluator * priorityEvaluator)
+{
 	if(!shouldVisitObject(obj))
 	{
 #if NKAI_TRACE_LEVEL >= 2
 		logAi->trace("Skip object %s%s.", obj->getObjectName(), obj->visitablePos().toString());
 #endif
-				continue;
+		return;
 	}
 
 #if NKAI_TRACE_LEVEL >= 2
@@ -255,7 +278,7 @@ void ObjectClusterizer::clusterize()
 #if NKAI_TRACE_LEVEL >= 2
 		logAi->trace("No paths found.");
 #endif
-				continue;
+		return;
 	}
 
 	std::sort(paths.begin(), paths.end(), [](const AIPath & p1, const AIPath & p2) -> bool
@@ -263,7 +286,7 @@ void ObjectClusterizer::clusterize()
 			return p1.movementCost() < p2.movementCost();
 		});
 
-			if(vstd::contains(ignoreObjects, obj->ID))
+	if(vstd::contains(IgnoredObjectTypes, obj->ID))
 	{
 		farObjects.addObject(obj, paths.front(), 0);
 
@@ -271,7 +294,7 @@ void ObjectClusterizer::clusterize()
 		logAi->trace("Object ignored. Moved to far objects with path %s", paths.front().toString());
 #endif
 
-				continue;
+		return;
 	}
 
 	std::set<const CGHeroInstance *> heroesProcessed;
@@ -352,28 +375,5 @@ void ObjectClusterizer::clusterize()
 #endif
 	}
 }
-#if NKAI_TRACE_LEVEL == 0
-	});
-#else
-	}
-#endif
-
-	logAi->trace("Near objects count: %i", nearObjects.objects.size());
-	logAi->trace("Far objects count: %i", farObjects.objects.size());
-
-	for(auto pair : blockedObjects)
-	{
-		logAi->trace("Cluster %s %s count: %i", pair.first->getObjectName(), pair.first->visitablePos().toString(), pair.second->objects.size());
-
-#if NKAI_TRACE_LEVEL >= 1
-		for(auto obj : pair.second->getObjects())
-		{
-			logAi->trace("Object %s %s", obj->getObjectName(), obj->visitablePos().toString());
-		}
-#endif
-	}
-
-	logAi->trace("Clusterization complete in %ld", timeElapsed(start));
-}
 
 }

AI/Nullkiller/Analyzers/ObjectClusterizer.h

@@ -49,9 +49,13 @@ public:
 
 using ClusterMap = tbb::concurrent_hash_map<const CGObjectInstance *, std::shared_ptr<ObjectCluster>>;
 
+class PriorityEvaluator;
+
 class ObjectClusterizer
 {
 private:
+	static Obj IgnoredObjectTypes[];
+
 	ObjectCluster nearObjects;
 	ObjectCluster farObjects;
 	ClusterMap blockedObjects;
@@ -68,6 +72,7 @@ public:
 
 private:
 	bool shouldVisitObject(const CGObjectInstance * obj) const;
+	void clusterizeObject(const CGObjectInstance * obj, PriorityEvaluator * priorityEvaluator);
 };
 
 }

AI/Nullkiller/Pathfinding/AINodeStorage.h

@@ -11,8 +11,8 @@
 #pragma once
 
 #define NKAI_PATHFINDER_TRACE_LEVEL 0
-#define NKAI_GRAPH_TRACE_LEVEL 0
+constexpr int NKAI_GRAPH_TRACE_LEVEL = 0;
-#define NKAI_TRACE_LEVEL 1
+#define NKAI_TRACE_LEVEL 0
 
 #include "../../../lib/pathfinder/CGPathNode.h"
 #include "../../../lib/pathfinder/INodeStorage.h"

AI/Nullkiller/Pathfinding/AIPathfinder.cpp

@@ -58,7 +58,7 @@ std::vector<AIPath> AIPathfinder::getPathInfo(const int3 & tile, bool includeGra
 	return info;
 }
 
-void AIPathfinder::updatePaths(std::map<const CGHeroInstance *, HeroRole> heroes, PathfinderSettings pathfinderSettings)
+void AIPathfinder::updatePaths(const std::map<const CGHeroInstance *, HeroRole> & heroes, PathfinderSettings pathfinderSettings)
 {
 	if(!storage)
 	{
@@ -125,7 +125,7 @@ void AIPathfinder::updatePaths(std::map<const CGHeroInstance *, HeroRole> heroes
 	logAi->trace("Recalculated paths in %ld", timeElapsed(start));
 }
 
-void AIPathfinder::updateGraphs(std::map<const CGHeroInstance *, HeroRole> heroes)
+void AIPathfinder::updateGraphs(const std::map<const CGHeroInstance *, HeroRole> & heroes)
 {
 	auto start = std::chrono::high_resolution_clock::now();
 	std::vector<const CGHeroInstance *> heroesVector;
@@ -134,22 +134,23 @@ void AIPathfinder::updateGraphs(std::map<const CGHeroInstance *, HeroRole> heroe
 
 	for(auto hero : heroes)
 	{
-		heroGraphs.emplace(hero.first->id, GraphPaths());
+		if(heroGraphs.try_emplace(hero.first->id, GraphPaths()).second)
 			heroesVector.push_back(hero.first);
 	}
 
-	parallel_for(blocked_range<size_t>(0, heroesVector.size()), [&](const blocked_range<size_t> & r)
+	parallel_for(blocked_range<size_t>(0, heroesVector.size()), [this, &heroesVector](const blocked_range<size_t> & r)
 		{
 			for(auto i = r.begin(); i != r.end(); i++)
 				heroGraphs.at(heroesVector[i]->id).calculatePaths(heroesVector[i], ai);
 		});
 
-#if NKAI_GRAPH_TRACE_LEVEL >= 1
+	if(NKAI_GRAPH_TRACE_LEVEL >= 1)
+	{
 		for(auto hero : heroes)
 		{
 			heroGraphs[hero.first->id].dumpToLog();
 		}
-#endif
+	}
 
 	logAi->trace("Graph paths updated in %lld", timeElapsed(start));
 }

AI/Nullkiller/Pathfinding/AIPathfinder.h

@@ -46,8 +46,8 @@ public:
 	AIPathfinder(CPlayerSpecificInfoCallback * cb, Nullkiller * ai);
 	std::vector<AIPath> getPathInfo(const int3 & tile, bool includeGraph = false) const;
 	bool isTileAccessible(const HeroPtr & hero, const int3 & tile) const;
-	void updatePaths(std::map<const CGHeroInstance *, HeroRole> heroes, PathfinderSettings pathfinderSettings);
+	void updatePaths(const std::map<const CGHeroInstance *, HeroRole> & heroes, PathfinderSettings pathfinderSettings);
-	void updateGraphs(std::map<const CGHeroInstance *, HeroRole> heroes);
+	void updateGraphs(const std::map<const CGHeroInstance *, HeroRole> & heroes);
 	void init();
 
 	std::shared_ptr<AINodeStorage>getStorage()

AI/Nullkiller/Pathfinding/ObjectGraph.cpp

@@ -19,36 +19,22 @@
 namespace NKAI
 {
 
-void ObjectGraph::updateGraph(const Nullkiller * ai)
+class ObjectGraphCalculator
 {
-	auto cb = ai->cb;
+private:
+	ObjectGraph * target;
+	const Nullkiller * ai;
 
 	std::map<const CGHeroInstance *, HeroRole> actors;
 	std::map<const CGHeroInstance *, const CGObjectInstance *> actorObjectMap;
-	std::vector<CGBoat *> boats;
 
-	auto addObjectActor = [&](const CGObjectInstance * obj)
+	std::vector<std::unique_ptr<CGBoat>> temporaryBoats;
+	std::vector<std::unique_ptr<CGHeroInstance>> temporaryActorHeroes;
+
+public:
+	ObjectGraphCalculator(ObjectGraph * target, const Nullkiller * ai)
+		:ai(ai), target(target)
 	{
-		auto objectActor = new CGHeroInstance(obj->cb);
-		CRandomGenerator rng;
-		auto visitablePos = obj->visitablePos();
-
-		objectActor->setOwner(ai->playerID); // lets avoid having multiple colors
-		objectActor->initHero(rng, static_cast<HeroTypeID>(0));
-		objectActor->pos = objectActor->convertFromVisitablePos(visitablePos);
-		objectActor->initObj(rng);
-
-		if(cb->getTile(visitablePos)->isWater())
-		{
-			boats.push_back(new CGBoat(objectActor->cb));
-			objectActor->boat = boats.back();
-		}
-
-		actorObjectMap[objectActor] = obj;
-		actors[objectActor] = obj->ID == Obj::TOWN || obj->ID == Obj::SHIPYARD ?  HeroRole::MAIN : HeroRole::SCOUT;
-		addObject(obj);
-	};
-
 		for(auto obj : ai->memory->visitableObjs)
 		{
 			if(obj && obj->isVisitable() && obj->ID != Obj::HERO)
@@ -57,7 +43,7 @@ void ObjectGraph::updateGraph(const Nullkiller * ai)
 			}
 		}
 
-	for(auto town : cb->getTownsInfo())
+		for(auto town : ai->cb->getTownsInfo())
 		{
 			addObjectActor(town);
 		}
@@ -69,12 +55,10 @@ void ObjectGraph::updateGraph(const Nullkiller * ai)
 		ps.allowBypassObjects = false;
 
 		ai->pathfinder->updatePaths(actors, ps);
+	}
 
-	foreach_tile_pos(cb.get(), [&](const CPlayerSpecificInfoCallback * cb, const int3 & pos)
+	void calculateConnections(const int3 & pos)
 	{
-			if(nodes.find(pos) != nodes.end())
-				return;
-
 		auto guarded = ai->cb->getGuardingCreaturePosition(pos).valid();
 
 		if(guarded)
@@ -105,12 +89,13 @@ void ObjectGraph::updateGraph(const Nullkiller * ai)
 
 				auto danger = ai->pathfinder->getStorage()->evaluateDanger(obj2->visitablePos(), path1.targetHero, true);
 
-					auto updated = nodes[obj1->visitablePos()].connections[obj2->visitablePos()].update(
+				auto updated = target->tryAddConnection(
+					obj1->visitablePos(),
+					obj2->visitablePos(), 
 					path1.movementCost() + path2.movementCost(),
 					danger);
 
-#if NKAI_GRAPH_TRACE_LEVEL >= 2
+				if(NKAI_GRAPH_TRACE_LEVEL >= 2 && updated)
-					if(updated)
 				{
 					logAi->trace(
 						"Connected %s[%s] -> %s[%s] through [%s], cost %2f",
@@ -119,26 +104,61 @@ void ObjectGraph::updateGraph(const Nullkiller * ai)
 						pos.toString(),
 						path1.movementCost() + path2.movementCost());
 				}
-#else
-					(void)updated;
-#endif
 			}
 		}
+	}
+
+private:
+	void addObjectActor(const CGObjectInstance * obj)
+	{
+		auto objectActor = temporaryActorHeroes.emplace_back(std::make_unique<CGHeroInstance>(obj->cb)).get();
+
+		CRandomGenerator rng;
+		auto visitablePos = obj->visitablePos();
+
+		objectActor->setOwner(ai->playerID); // lets avoid having multiple colors
+		objectActor->initHero(rng, static_cast<HeroTypeID>(0));
+		objectActor->pos = objectActor->convertFromVisitablePos(visitablePos);
+		objectActor->initObj(rng);
+
+		if(cb->getTile(visitablePos)->isWater())
+		{
+			objectActor->boat = temporaryBoats.emplace_back(std::make_unique<CGBoat>(objectActor->cb)).get();
+		}
+
+		actorObjectMap[objectActor] = obj;
+		actors[objectActor] = obj->ID == Obj::TOWN || obj->ID == Obj::SHIPYARD ? HeroRole::MAIN : HeroRole::SCOUT;
+
+		target->addObject(obj);
+	};
+};
+
+bool ObjectGraph::tryAddConnection(
+	const int3 & from,
+	const int3 & to,
+	float cost,
+	uint64_t danger)
+{
+	return nodes[from].connections[to].update(cost, danger);
+}
+
+void ObjectGraph::updateGraph(const Nullkiller * ai)
+{
+	auto cb = ai->cb;
+
+	ObjectGraphCalculator calculator(this, ai);
+
+	foreach_tile_pos(cb.get(), [this, &calculator](const CPlayerSpecificInfoCallback * cb, const int3 & pos)
+		{
+			if(nodes.find(pos) != nodes.end())
+				return;
+
+			calculator.calculateConnections(pos);
 		});
 
-	for(auto h : actorObjectMap)
+	if(NKAI_GRAPH_TRACE_LEVEL >= 1)
-	{
-		delete h.first;
-	}
-
-	for(auto boat : boats)
-	{
-		delete boat;
-	}
-
-#if NKAI_GRAPH_TRACE_LEVEL >= 1
 		dumpToLog("graph");
-#endif
+
 }
 
 void ObjectGraph::addObject(const CGObjectInstance * obj)
@@ -199,14 +219,15 @@ void ObjectGraph::dumpToLog(std::string visualKey) const
 			{
 				for(auto & node : tile.second.connections)
 				{
-#if NKAI_GRAPH_TRACE_LEVEL >= 2
+					if(NKAI_GRAPH_TRACE_LEVEL >= 2)
+					{
 						logAi->trace(
 							"%s -> %s: %f !%d",
 							node.first.toString(),
 							tile.first.toString(),
 							node.second.cost,
 							node.second.danger);
-#endif
+					}
 
 					logBuilder.addLine(tile.first, node.first);
 				}
@@ -278,14 +299,15 @@ void GraphPaths::dumpToLog() const
 					if(!node.previous.valid())
 						continue;
 
-#if NKAI_GRAPH_TRACE_LEVEL >= 2
+					if(NKAI_GRAPH_TRACE_LEVEL >= 2)
+					{
 						logAi->trace(
 							"%s -> %s: %f !%d",
 							node.previous.coord.toString(),
 							tile.first.toString(),
 							node.cost,
 							node.danger);
-#endif
+					}
 
 					logBuilder.addLine(node.previous.coord, tile.first);
 				}
@@ -343,7 +365,7 @@ void GraphPaths::addChainInfo(std::vector<AIPath> & paths, int3 tile, const CGHe
 
 			auto currentNode = currentTile->second[current.nodeType];
 
-			if(currentNode.cost == 0)
+			if(!currentNode.previous.valid())
 				break;
 
 			allowBattle = allowBattle || currentNode.nodeType == GrapthPathNodeType::BATTLE;
@@ -352,7 +374,7 @@ void GraphPaths::addChainInfo(std::vector<AIPath> & paths, int3 tile, const CGHe
 
 			tilesToPass.push_back(current.coord);
 
-			if(currentNode.cost < 2)
+			if(currentNode.cost < 2.0f)
 				break;
 
 			current = currentNode.previous;
@@ -377,6 +399,7 @@ void GraphPaths::addChainInfo(std::vector<AIPath> & paths, int3 tile, const CGHe
 				n.turns = static_cast<ui8>(cost) + 1; // just in case lets select worst scenario
 				n.danger = danger;
 				n.targetHero = hero;
+				n.parentIndex = 0;
 
 				for(auto & node : path.nodes)
 				{

AI/Nullkiller/Pathfinding/ObjectGraph.h

@@ -76,6 +76,8 @@ public:
 	{
 		return nodes.at(tile);
 	}
+
+	bool tryAddConnection(const int3 & from, const int3 & to, float cost, uint64_t danger);
 };
 
 struct GraphPathNode;

AI/Nullkiller/Pathfinding/Rules/AIMovementToDestinationRule.cpp

@@ -45,7 +45,7 @@ namespace AIPathfinding
 
 			if(!allowBypassObjects)
 			{
-				if (source.node->getCost() == 0)
+				if (source.node->getCost() < 0.0001f)
 					return;
 
 				// when actor represents moster graph node, we need to let him escape monster