- fixed crash on start of some campaigns

- allowed on map factions is now set instead of bit field

client/CPreGame.cpp

@@ -1914,41 +1914,30 @@ void OptionsTab::nextCastle( int player, int dir )
 
 	PlayerSettings &s = SEL->sInfo.playerInfos[player];
 	si32 &cur = s.castle;
-	ui32 allowed = SEL->current->mapHeader->players[s.color].allowedFactions;
+	auto & allowed = SEL->current->mapHeader->players[s.color].allowedFactions;
 
 	if (cur == -2) //no castle - no change
 		return;
 
 	if (cur == -1) //random => first/last available
 	{
-		int pom = (dir>0) ? (0) : (GameConstants::F_NUMBER-1); // last or first
-		for (;pom >= 0  &&  pom < GameConstants::F_NUMBER;  pom+=dir)
-		{
-			if((1 << pom) & allowed)
-			{
-				cur=pom;
-				break;
-			}
-		}
+		if (dir > 0)
+			cur = *allowed.begin(); //id of first town
+		else
+			cur = *allowed.rbegin(); //id of last town
+
 	}
 	else // next/previous available
 	{
-		for (;;)
+		if ( (cur == *allowed.begin() && dir < 0 )
+		  || (cur == *allowed.rbegin() && dir > 0) )
+			cur = -1;
+		else
 		{
-			cur+=dir;
-			if ((1 << cur) & allowed)
-				break;
-
-			if (cur >= GameConstants::F_NUMBER  ||  cur<0)
-			{
-				double p1 = log((double)allowed) / log(2.0)+0.000001;
-				double check = p1 - ((int)p1);
-				if (check < 0.001)
-					cur = (int)p1;
-				else
-					cur = -1;
-				break;
-			}
+			assert(dir >= -1 && dir <= 1); //othervice std::advance may go out of range
+			auto iter = allowed.find(cur);
+			std::advance(iter, dir);
+			cur = *iter;
 		}
 	}
 

client/GUIClasses.cpp

@@ -448,7 +448,7 @@ CGarrisonSlot::CGarrisonSlot(CGarrisonInt *Owner, int x, int y, int IID, int Upg
 	if (creature)
 	{
 		std::string imgName = owner->smallIcons ? "cprsmall" : "TWCRPORT";
-		creatureImage = new CAnimImage(imgName, creature->idNumber + 2);
+		creatureImage = new CAnimImage(imgName, creature->iconIndex);
 	}
 	else
 		creatureImage = nullptr;

lib/CCampaignHandler.cpp

@@ -52,23 +52,18 @@ unique_ptr<CCampaign> CCampaignHandler::getCampaign( const std::string & name )
 		ret->scenarios.push_back(sc);
 	}
 
-
 	int scenarioID = 0;
 
 	//first entry is campaign header. start loop from 1
-	for (int g=1; g<file.size() && g<howManyScenarios; ++g)
+	for (int g=1; g<file.size() && scenarioID<howManyScenarios; ++g)
 	{
 		while(!ret->scenarios[scenarioID].isNotVoid()) //skip void scenarios
 		{
 			scenarioID++;
 		}
-		//set map piece appropriately
 
-
-		ret->mapPieces[scenarioID].resize(file[g].size());
-		for(int i = 0; i < file[g].size(); i++)
-			ret->mapPieces[scenarioID][i] = file[g][i];
-		
+		//set map piece appropriately, convert vector to string
+		ret->mapPieces[scenarioID].assign(reinterpret_cast< const char* >(file[g].data()), file[g].size());
 		scenarioID++;
 	}
 

lib/CGameState.cpp

@@ -931,12 +931,11 @@ void CGameState::init(StartInfo * si)
 	{
 		if(it->second.castle==-1)
 		{
-			int f;
-			do
-			{
-				f = ran()%GameConstants::F_NUMBER;
-			}while(!(map->players[it->first].allowedFactions  &  1<<f));
-			it->second.castle = f;
+			int randomID = ran() % map->players[it->first].allowedFactions.size();
+			auto iter = map->players[it->first].allowedFactions.begin();
+			std::advance(iter, randomID);
+
+			it->second.castle = *iter;
 		}
 	}
 

lib/CModHandler.cpp

@@ -81,6 +81,8 @@ void CModHandler::loadConfigFromFile (std::string name)
 		tlog3 << "\t\tFound mod file: " << entry.getResourceName() << "\n";
 		const JsonNode config ((char*)textData.get(), stream->getSize());
 
+		VLC->townh->loadFactions(config["factions"]);
+
 		const JsonNode *value = &config["creatures"];
 		BOOST_FOREACH (auto creature, value->Vector())
 		{

lib/CModHandler.h

@@ -5,6 +5,7 @@
 #include "VCMI_Lib.h"
 #include "CCreatureHandler.h"
 #include "CArtHandler.h"
+#include "CTownHandler.h"
 
 /*
  * CModHandler.h, part of VCMI engine

lib/CTownHandler.h

@@ -189,10 +189,6 @@ class DLL_LINKAGE CTownHandler
 
 	void loadPuzzle(CFaction & faction, const JsonNode & source);
 
-	/// main loading function, accepts merged JSON source and add all entries from it into game
-	/// all entries in JSON should be checked for validness before using this function
-	void loadFactions(const JsonNode & source);
-
 	/// load all available data from h3 txt(s) into json structure using format similar to vcmi configs
 	/// returns 2d array [townID] [buildID] of buildings
 	void loadLegacyData(JsonNode & dest);
@@ -203,7 +199,11 @@ public:
 
 	CTownHandler(); //c-tor, set pointer in VLC to this
 
-	/// "entry point" for towns loading.
+	/// main loading function for mods, accepts merged JSON source and add all entries from it into game
+	/// all entries in JSON should be checked for validness before using this function
+	void loadFactions(const JsonNode & source);
+
+	/// "entry point" for loading of OH3 town.
 	/// reads legacy txt's from H3 + vcmi json, merges them
 	/// and loads resulting structure to game using loadTowns method
 	/// in future may require loaded Creature Handler

lib/IGameCallback.cpp

@@ -543,31 +543,38 @@ int CGameInfoCallback::canBuildStructure( const CGTownInstance *t, int ID )
 {
 	ERROR_RET_VAL_IF(!canGetFullInfo(t), "Town is not owned!", -1);
 
-	int ret = EBuildingState::ALLOWED;
-	if(t->builded >= VLC->modh->settings.MAX_BUILDING_PER_TURN)
-		ret = EBuildingState::CANT_BUILD_TODAY; //building limit
-
 	CBuilding * pom = t->town->buildings[ID];
 
 	if(!pom)
 		return EBuildingState::BUILDING_ERROR;
 
-	//checking resources
-	if(!pom->resources.canBeAfforded(getPlayer(t->tempOwner)->resources))
-		ret = EBuildingState::NO_RESOURCES; //lack of res
+	if(t->hasBuilt(ID))	//already built
+		return EBuildingState::ALREADY_PRESENT;
+
+	//can we build it?
+	if(t->forbiddenBuildings.find(ID)!=t->forbiddenBuildings.end())
+		return EBuildingState::FORBIDDEN; //forbidden
 
 	//checking for requirements
 	std::set<int> reqs = getBuildingRequiments(t, ID);//getting all requirements
 
+	bool notAllBuilt = false;
 	for( std::set<int>::iterator ri  =  reqs.begin(); ri != reqs.end(); ri++ )
 	{
-		if(!t->hasBuilt(*ri))
-			ret = EBuildingState::PREREQUIRES; //lack of requirements - cannot build
+		if(!t->hasBuilt(*ri)) //lack of requirements - cannot build
+		{
+			if(vstd::contains(t->forbiddenBuildings, *ri)) // not built requirement forbidden - same goes to this build
+				return EBuildingState::FORBIDDEN;
+			else
+				notAllBuilt = true; // no return here - we need to check if any required builds are forbidden
+		}
 	}
 
-	//can we build it?
-	if(t->forbiddenBuildings.find(ID)!=t->forbiddenBuildings.end())
-		ret = EBuildingState::FORBIDDEN; //forbidden
+	if(t->builded >= VLC->modh->settings.MAX_BUILDING_PER_TURN)
+		return EBuildingState::CANT_BUILD_TODAY; //building limit
+
+	if (notAllBuilt)
+		return EBuildingState::PREREQUIRES;
 
 	if(ID == 13) //capitol
 	{
@@ -578,8 +585,7 @@ int CGameInfoCallback::canBuildStructure( const CGTownInstance *t, int ID )
 			{
 				if(t->hasBuilt(EBuilding::CAPITOL))
 				{
-					ret = EBuildingState::HAVE_CAPITAL; //no more than one capitol
-					break;
+					return EBuildingState::HAVE_CAPITAL; //no more than one capitol
 				}
 			}
 		}
@@ -589,12 +595,14 @@ int CGameInfoCallback::canBuildStructure( const CGTownInstance *t, int ID )
 		const TerrainTile *tile = getTile(t->bestLocation(), false);
 		
 		if(!tile || tile->tertype != TerrainTile::water )
-			ret = EBuildingState::NO_WATER; //lack of water
+			return EBuildingState::NO_WATER; //lack of water
 	}
 
-	if(t->hasBuilt(ID))	//already built
-		ret = EBuildingState::ALREADY_PRESENT;
-	return ret;
+	//checking resources
+	if(!pom->resources.canBeAfforded(getPlayer(t->tempOwner)->resources))
+		return EBuildingState::NO_RESOURCES; //lack of res
+
+	return EBuildingState::ALLOWED;
 }
 
 std::set<int> CGameInfoCallback::getBuildingRequiments( const CGTownInstance *t, int ID )

lib/map.cpp

@@ -228,10 +228,13 @@ void CMapHeader::loadPlayerInfo( int &pom, const ui8 * bufor, int &i )
 			players[pom].p7= -1;
 
 		//factions this player can choose
-		players[pom].allowedFactions = 0;
-		players[pom].allowedFactions += bufor[i++];
+		ui16 allowedFactions = bufor[i++];
 		if(version != RoE)
-			players[pom].allowedFactions += (bufor[i++])*256;
+			allowedFactions += (bufor[i++])*256;
+
+		for (size_t fact=0; fact<16; fact++)
+			if (allowedFactions & (1 << fact))
+				players[pom].allowedFactions.insert(fact);
 
 		players[pom].isFactionRandom = bufor[i++];
 		players[pom].hasMainTown = bufor[i++];

lib/map.h

@@ -96,7 +96,7 @@ struct DLL_LINKAGE PlayerInfo
 	ui8 canHumanPlay;
 	ui8 canComputerPlay;
 	ui32 AITactic; //(00 - random, 01 -  warrior, 02 - builder, 03 - explorer)
-	ui32 allowedFactions; //(01 - castle; 02 - rampart; 04 - tower; 08 - inferno; 16 - necropolis; 32 - dungeon; 64 - stronghold; 128 - fortress; 256 - conflux);
+	std::set<ui32> allowedFactions; //set with factions player can play with
 	ui8 isFactionRandom;
 	ui32 mainHeroPortrait; //it's ID of hero with chosen portrait; 255 if standard
 	std::string mainHeroName;
@@ -108,7 +108,7 @@ struct DLL_LINKAGE PlayerInfo
 	ui8 generateHero;
 
 	PlayerInfo(): p7(0), p8(0), p9(0), canHumanPlay(0), canComputerPlay(0),
-		AITactic(0), allowedFactions(0), isFactionRandom(0),
+		AITactic(0), isFactionRandom(0),
 		mainHeroPortrait(0), hasMainTown(0), generateHeroAtMainTown(0),
 		team(255), generateHero(0) {};
 
@@ -117,7 +117,7 @@ struct DLL_LINKAGE PlayerInfo
 		si8 ret = -2;
 		for (int j = 0; j < GameConstants::F_NUMBER  &&  ret != -1; j++) //we start with none and find matching faction. if more than one, then set to random
 		{
-			if((1 << j) & allowedFactions)
+			if(vstd::contains(allowedFactions, j))
 			{
 				if (ret >= 0) //we've already assigned a castle and another one is possible -> set random and let player choose
 					ret = -1; //breaks