Merge branch 'mutexRelax_fix' into develop

AI/VCAI/Fuzzy.cpp

@@ -302,6 +302,8 @@ Goals::TSubgoal FuzzyHelper::chooseSolution (Goals::TGoalVec vec)
 	if (vec.empty()) //no possibilities found
 		return sptr(Goals::Invalid());
 
+	cachedSectorMaps.clear();
+
 	//a trick to switch between heroes less often - calculatePaths is costly
 	auto sortByHeroes = [](const Goals::TSubgoal & lhs, const Goals::TSubgoal & rhs) -> bool
 	{
@@ -480,8 +482,7 @@ float FuzzyHelper::evaluate (Goals::ClearWayTo & g)
 	if (!g.hero.h)
 		throw cannotFulfillGoalException("ClearWayTo called without hero!");
 
-	SectorMap sm(g.hero);
-	int3 t = sm.firstTileToGet(g.hero, g.tile);
+	int3 t = getCachedSectorMap(g.hero).firstTileToGet(g.hero, g.tile);
 
 	if (t.valid())
 	{
@@ -530,3 +531,15 @@ void FuzzyHelper::setPriority (Goals::TSubgoal & g)
 {
 	g->setpriority(g->accept(this)); //this enforces returned value is set
 }
+
+SectorMap& FuzzyHelper::getCachedSectorMap(HeroPtr h)
+{
+	auto it = cachedSectorMaps.find(h);
+	if (it != cachedSectorMaps.end())
+		return it->second;
+	else
+	{
+		cachedSectorMaps.insert (std::make_pair(h, SectorMap(h)));
+		return cachedSectorMaps[h];
+	}
+}

AI/VCAI/Fuzzy.h

@@ -15,6 +15,7 @@
 class VCAI;
 class CArmedInstance;
 class CBank;
+class SectorMap;
 
 class engineBase
 {
@@ -54,6 +55,8 @@ class FuzzyHelper
 		~EvalVisitTile();
 	} vt;
 
+	std::map <HeroPtr, SectorMap> cachedSectorMaps;
+
 public:
 	enum RuleBlocks {BANK_DANGER, TACTICAL_ADVANTAGE, VISIT_TILE};
 	//blocks should be initialized in this order, which may be confusing :/
@@ -81,4 +84,7 @@ public:
 
 	Goals::TSubgoal chooseSolution (Goals::TGoalVec vec);
 	//shared_ptr<AbstractGoal> chooseSolution (std::vector<shared_ptr<AbstractGoal>> & vec);
+
+	//optimization - use one SM for every hero call
+	SectorMap& getCachedSectorMap (HeroPtr h);
 };

AI/VCAI/VCAI.cpp

@@ -701,12 +701,12 @@ void makePossibleUpgrades(const CArmedInstance *obj)
 
 void VCAI::makeTurn()
 {
+	logGlobal->infoStream() << boost::format("Player %d starting turn") % static_cast<int>(playerID.getNum());
+
 	MAKING_TURN;
 	boost::shared_lock<boost::shared_mutex> gsLock(cb->getGsMutex());
 	setThreadName("VCAI::makeTurn");
 
-    logGlobal->infoStream() << boost::format("Player %d starting turn") % static_cast<int>(playerID.getNum());
-
 	switch(cb->getDate(Date::DAY_OF_WEEK))
 	{
 		case 1:
@@ -2779,18 +2779,20 @@ BattleState AIStatus::getBattle()
 }
 
 void AIStatus::addQuery(QueryID ID, std::string description)
-{
-	boost::unique_lock<boost::mutex> lock(mx);
+{	
 	if(ID == QueryID(-1))
 	{
         logAi->debugStream() << boost::format("The \"query\" has an id %d, it'll be ignored as non-query. Description: %s") % ID % description;
 		return;
 	}
 
-	assert(!vstd::contains(remainingQueries, ID));
 	assert(ID.getNum() >= 0);
+	boost::unique_lock<boost::mutex> lock(mx);
+
+	assert(!vstd::contains(remainingQueries, ID));
 
 	remainingQueries[ID] = description;
+
 	cv.notify_all();
     logAi->debugStream() << boost::format("Adding query %d - %s. Total queries count: %d") % ID % description % remainingQueries.size();
 }
@@ -2802,6 +2804,7 @@ void AIStatus::removeQuery(QueryID ID)
 
 	std::string description = remainingQueries[ID];
 	remainingQueries.erase(ID);
+	
 	cv.notify_all();
     logAi->debugStream() << boost::format("Removing query %d - %s. Total queries count: %d") % ID % description % remainingQueries.size();
 }
@@ -2912,7 +2915,7 @@ SectorMap::SectorMap(HeroPtr h)
 	makeParentBFS(h->visitablePos());
 }
 
-bool markIfBlocked(ui8 &sec, crint3 pos, const TerrainTile *t)
+bool SectorMap::markIfBlocked(ui8 &sec, crint3 pos, const TerrainTile *t)
 {
 	if(t->blocked && !t->visitable)
 	{
@@ -2923,13 +2926,15 @@ bool markIfBlocked(ui8 &sec, crint3 pos, const TerrainTile *t)
 	return false;
 }
 
-bool markIfBlocked(ui8 &sec, crint3 pos)
+bool SectorMap::markIfBlocked(ui8 &sec, crint3 pos)
 {
-	return markIfBlocked(sec, pos, cb->getTile(pos));
+	return markIfBlocked(sec, pos, getTile(pos));
 }
 
 void SectorMap::update()
 {
+	visibleTiles = cb->getAllVisibleTiles();
+
 	clear();
 	int curSector = 3; //0 is invisible, 1 is not explored
 
@@ -2955,7 +2960,7 @@ void SectorMap::exploreNewSector(crint3 pos, int num, CCallback * cbp)
 {
 	Sector &s = infoOnSectors[num];
 	s.id = num;
-	s.water = cbp->getTile(pos)->isWater();
+	s.water = getTile(pos)->isWater();
 
 	std::queue<int3> toVisit;
 	toVisit.push(pos);
@@ -2966,7 +2971,7 @@ void SectorMap::exploreNewSector(crint3 pos, int num, CCallback * cbp)
 		ui8 &sec = retreiveTile(curPos);
 		if(sec == NOT_CHECKED)
 		{
-			const TerrainTile *t = cbp->getTile(curPos);
+			const TerrainTile *t = getTile(curPos);
 			if(!markIfBlocked(sec, curPos, t))
 			{
 				if(t->isWater() == s.water) //sector is only-water or only-land
@@ -2980,7 +2985,7 @@ void SectorMap::exploreNewSector(crint3 pos, int num, CCallback * cbp)
 							toVisit.push(neighPos);
 							//parent[neighPos] = curPos;
 						}
-						const TerrainTile *nt = cbp->getTile(neighPos, false);
+						const TerrainTile *nt = getTile(neighPos);
 						if(nt && nt->isWater() != s.water && canBeEmbarkmentPoint(nt, s.water))
 						{
 							s.embarkmentPoints.push_back(neighPos);
@@ -3221,7 +3226,7 @@ For ship construction etc, another function (goal?) is needed
 				//embark on ship -> look for an EP with a boat
 				auto firstEP = boost::find_if(src->embarkmentPoints, [=](crint3 pos) -> bool
 				{
-					const TerrainTile *t = cb->getTile(pos);
+					const TerrainTile *t = getTile(pos);
                     return t && t->visitableObjects.size() == 1 && t->topVisitableId() == Obj::BOAT
 						&& retreiveTile(pos) == sectorToReach->id;
 				});
@@ -3311,7 +3316,7 @@ For ship construction etc, another function (goal?) is needed
 				{
 					//make sure no hero block the way
 					auto pos = ai->knownSubterraneanGates[gate]->visitablePos();
-					const TerrainTile *t = cb->getTile(pos);
+					const TerrainTile *t = getTile(pos);
 					return t && t->visitableObjects.size() == 1 && t->topVisitableId() == Obj::SUBTERRANEAN_GATE
 						&& retreiveTile(pos) == sectorToReach->id;
 				});
@@ -3412,3 +3417,9 @@ unsigned char & SectorMap::retreiveTile(crint3 pos)
 	return retreiveTileN(sector, pos);
 }
 
+TerrainTile* SectorMap::getTile(crint3 pos) const
+{
+	//out of bounds access should be handled by boost::multi_array
+	//still we cached this array to avoid any checks
+	return visibleTiles->operator[](pos.x)[pos.y][pos.z];
+}

AI/VCAI/VCAI.h

@@ -95,6 +95,7 @@ struct SectorMap
 	//std::vector<std::vector<std::vector<unsigned char>>> pathfinderSector;
 
 	std::map<int, Sector> infoOnSectors;
+	shared_ptr<boost::multi_array<TerrainTile*, 3>> visibleTiles;
 
 	SectorMap();
 	SectorMap(HeroPtr h);
@@ -103,7 +104,10 @@ struct SectorMap
 	void exploreNewSector(crint3 pos, int num, CCallback * cbp);
 	void write(crstring fname);
 
+	bool markIfBlocked(ui8 &sec, crint3 pos, const TerrainTile *t);
+	bool markIfBlocked(ui8 &sec, crint3 pos);
 	unsigned char &retreiveTile(crint3 pos);
+	TerrainTile* getTile(crint3 pos) const;
 
 	void makeParentBFS(crint3 source);
 

Global.h

@@ -158,6 +158,7 @@ static_assert(sizeof(bool) == 1, "Bool needs to be 1 byte in size.");
 #include <boost/thread.hpp>
 #include <boost/variant.hpp>
 #include <boost/math/special_functions/round.hpp>
+#include <boost/multi_array.hpp>
 
 #ifndef M_PI
 #  define M_PI 3.14159265358979323846

client/Graphics.cpp

@@ -373,11 +373,21 @@ void Graphics::loadFonts()
 
 CDefEssential * Graphics::getDef( const CGObjectInstance * obj )
 {
+	if (obj->appearance.animationFile.empty())
+	{
+		logGlobal->warnStream() << boost::format("Def name for obj %d (%d,%d) is empty!") % obj->id % obj->ID % obj->subID;
+		return nullptr;
+	}
 	return advmapobjGraphics[obj->appearance.animationFile];
 }
 
 CDefEssential * Graphics::getDef( const ObjectTemplate & info )
 {
+	if (info.animationFile.empty())
+	{
+		logGlobal->warnStream() << boost::format("Def name for obj (%d,%d) is empty!") % info.id % info.subid;
+		return nullptr;
+	}
 	return advmapobjGraphics[info.animationFile];
 }
 

client/mapHandler.cpp

@@ -860,8 +860,15 @@ void CMapHandler::CMapBlitter::drawObjects(SDL_Surface * targetSurf, const Terra
 		
 		if (!graphics->getDef(obj))
 			processDef(obj->appearance);
-		if (!graphics->getDef(obj) && !obj->appearance.animationFile.empty())
-			logGlobal->errorStream() << "Failed to load image " << obj->appearance.animationFile;
+		if (!graphics->getDef(obj))
+		{
+			if (!obj->appearance.animationFile.empty())
+				logGlobal->errorStream() << "Failed to load image " << obj->appearance.animationFile;
+			else
+				logGlobal->warnStream() << boost::format("Def name for obj %d (%d,%d) is empty!") % obj->id % obj->ID % obj->subID;
+
+			continue;
+		}
 
 		if (!canDrawObject(obj))
 			continue;
@@ -1322,22 +1329,54 @@ bool CMapHandler::printObject(const CGObjectInstance *obj, bool fadein /* = fals
 					curt.objects.insert(i, toAdd);
 			}
 
-		} // for(int fy=0; fy<tilesH; ++fy)
-	} //for(int fx=0; fx<tilesW; ++fx)
+		}
+	}
 	return true;
 }
 
 bool CMapHandler::hideObject(const CGObjectInstance *obj, bool fadeout /* = false */)
 {
-	// do we actually need to search through the whole map for this?
-	for (size_t i=0; i<map->width; i++)
-	{
-		for (size_t j=0; j<map->height; j++)
+	//optimized version which reveals weird bugs with missing def name
+	//auto pos = obj->pos;
+
+	//for (size_t i = pos.x; i > pos.x - obj->getWidth(); i--)
+	//{
+	//	for (size_t j = pos.y; j > pos.y - obj->getHeight(); j--)
+	//	{
+	//		int3 t(i, j, pos.z);
+	//		if (!map->isInTheMap(t))
+	//			continue;
+
+	//		auto &objs = ttiles[i][j][pos.z].objects;
+	//		for (size_t x = 0; x < objs.size(); x++)
+	//		{
+	//			auto ourObj = objs[x].obj;
+	//			if (ourObj && ourObj->id == obj->id)
+	//			{
+	//				if (fadeout && ADVOPT.objectFading) // object should be faded == erase is delayed until the end of fadeout
+	//				{
+	//					if (startObjectFade(objs[x], false, t))
+	//						objs[x].obj = nullptr; //set original pointer to null
+	//					else
+	//						objs.erase(objs.begin() + x);
+	//				}
+	//				else
+	//					objs.erase(objs.begin() + x);
+	//				break;
+	//			}
+	//		}
+	//	}
+
+	//}
+
+	for (size_t i = 0; i<map->width; i++)
+	{
+		for (size_t j = 0; j<map->height; j++)
 		{
-			for (size_t k=0; k<(map->twoLevel ? 2 : 1); k++)
+			for (size_t k = 0; k<(map->twoLevel ? 2 : 1); k++)
 			{
 				auto &objs = ttiles[i][j][k].objects;
-				for(size_t x=0; x < objs.size(); x++)
+				for (size_t x = 0; x < objs.size(); x++)
 				{
 					if (objs[x].obj && objs[x].obj->id == obj->id)
 					{
@@ -1356,6 +1395,7 @@ bool CMapHandler::hideObject(const CGObjectInstance *obj, bool fadeout /* = fals
 			}
 		}
 	}
+
 	return true;
 }
 bool CMapHandler::removeObject(CGObjectInstance *obj, bool fadeout /* = false */)

lib/CGameInfoCallback.cpp

@@ -61,17 +61,26 @@ bool CGameInfoCallback::isAllowed( int type, int id )
 
 const PlayerState * CGameInfoCallback::getPlayer(PlayerColor color, bool verbose) const
 {
-	ERROR_VERBOSE_OR_NOT_RET_VAL_IF(!hasAccess(color), verbose, "Cannot access player " << color << "info!", nullptr);
-	//if (!vstd::contains(gs->players, color))
-	//{
-	//	logGlobal->errorStream() << "Cannot access player " << color << "info!";
-	//	return nullptr; //macros are not really useful when debugging :?
-	//}
-	//else
-	//{
-	ERROR_VERBOSE_OR_NOT_RET_VAL_IF(!vstd::contains(gs->players,color), verbose, "Cannot find player " << color << "info!", nullptr);
-	return &gs->players[color];
-	//}
+	//funtion written from scratch since it's accessed A LOT by AI
+
+	auto player = gs->players.find(color);
+	if (player != gs->players.end())
+	{
+		if (hasAccess(color))
+			return &player->second;
+		else
+		{
+			if (verbose)
+				logGlobal->errorStream() << boost::format("Cannot access player %d info!") % color;
+			return nullptr;
+		}
+	}
+	else
+	{
+		if (verbose)
+			logGlobal->errorStream() << boost::format("Cannot find player %d info!") % color;
+		return nullptr;
+	}
 }
 
 const CTown * CGameInfoCallback::getNativeTown(PlayerColor color) const
@@ -455,6 +464,31 @@ const TerrainTile * CGameInfoCallback::getTile( int3 tile, bool verbose) const
 	return &gs->map->getTile(tile);
 }
 
+//TODO: typedef?
+shared_ptr<boost::multi_array<TerrainTile*, 3>> CGameInfoCallback::getAllVisibleTiles() const
+{
+	assert(player.is_initialized());
+	auto team = getPlayerTeam(player.get());
+
+	size_t width = gs->map->width;
+	size_t height = gs->map->height;
+	size_t levels = (gs->map->twoLevel ? 2 : 1);
+
+
+	boost::multi_array<TerrainTile*, 3> tileArray(boost::extents[width][height][levels]);
+	
+	for (size_t x = 0; x < width; x++)
+		for (size_t y = 0; y < height; y++)
+			for (size_t z = 0; z < levels; z++)
+			{
+				if (team->fogOfWarMap[x][y][z])
+					tileArray[x][y][z] = &gs->map->getTile(int3(x, y, z));
+				else
+					tileArray[x][y][z] = nullptr;
+			}
+	return make_shared<boost::multi_array<TerrainTile*, 3>>(tileArray);
+}
+
 EBuildingState::EBuildingState CGameInfoCallback::canBuildStructure( const CGTownInstance *t, BuildingID ID )
 {
 	ERROR_RET_VAL_IF(!canGetFullInfo(t), "Town is not owned!", EBuildingState::TOWN_NOT_OWNED);
@@ -747,18 +781,43 @@ TResources CPlayerSpecificInfoCallback::getResourceAmount() const
 
 const TeamState * CGameInfoCallback::getTeam( TeamID teamID ) const
 {
-	ERROR_RET_VAL_IF(!vstd::contains(gs->teams, teamID), "Cannot find info for team " << teamID, nullptr);
-	const TeamState *ret = &gs->teams[teamID];
-	ERROR_RET_VAL_IF(!!player && !vstd::contains(ret->players, *player), "Illegal attempt to access team data!", nullptr);
-	return ret;
+	//rewritten by hand, AI calls this function a lot
+
+	auto team = gs->teams.find(teamID);
+	if (team != gs->teams.end())
+	{
+		const TeamState *ret = &team->second;
+		if (!player.is_initialized()) //neutral (or invalid) player
+			return ret;
+		else
+		{
+			if (vstd::contains(ret->players, *player)) //specific player
+				return ret;
+			else
+			{
+				logGlobal->errorStream() << boost::format("Illegal attempt to access team data!");
+				return nullptr;
+			}
+		}
+	}
+	else
+	{
+		logGlobal->errorStream() << boost::format("Cannot find info for team %d") % teamID;
+		return nullptr;
+	}
 }
 
 const TeamState * CGameInfoCallback::getPlayerTeam( PlayerColor color ) const
 {
-	const PlayerState * ps = getPlayer(color);
-	if (ps)
-		return getTeam(ps->team);
-	return nullptr;
+	auto player = gs->players.find(color);
+	if (player != gs->players.end())
+	{
+		return getTeam (player->second.team);
+	}
+	else
+	{
+		return nullptr;
+	}
 }
 
 const CGHeroInstance* CGameInfoCallback::getHeroWithSubid( int subid ) const

lib/CGameInfoCallback.h

@@ -91,6 +91,7 @@ public:
 	const CMapHeader * getMapHeader()const;
 	int3 getMapSize() const; //returns size of map - z is 1 for one - level map and 2 for two level map
 	const TerrainTile * getTile(int3 tile, bool verbose = true) const;
+	shared_ptr<boost::multi_array<TerrainTile*, 3>> getAllVisibleTiles() const;
 	bool isInTheMap(const int3 &pos) const;
 
 	//town