- some tweaks towards new towns

- it is possible for faction to have 0 available heroes

client/BattleInterface/CBattleInterface.h

@@ -180,7 +180,6 @@ private:
 	class SiegeHelper
 	{
 	private:
-		static std::string townTypeInfixes[GameConstants::F_NUMBER]; //for internal use only - to build filenames
 		SDL_Surface* walls[18];
 		const CBattleInterface * owner;
 	public:

client/CPreGame.cpp

@@ -2012,7 +2012,9 @@ bool OptionsTab::canUseThisHero( int ID )
 	//	if(CPG->ret.playerInfos[i].hero==ID) //hero is already taken
 	//		return false;
 
-	return !vstd::contains(usedHeroes, ID) && SEL->current->mapHeader->allowedHeroes[ID];
+	return CGI->heroh->heroes.size() > ID
+	    && !vstd::contains(usedHeroes, ID)
+	    && SEL->current->mapHeader->allowedHeroes[ID];
 }
 
 void OptionsTab::nextBonus( int player, int dir )
@@ -2334,12 +2336,12 @@ SDL_Surface * OptionsTab::SelectedBox::getImg() const
 	switch(which)
 	{
 	case TOWN:
-		if (s.castle < GameConstants::F_NUMBER  &&  s.castle >= 0)
-			return graphics->getPic(s.castle, true, false);
-		else if (s.castle == -1)
+		if (s.castle == -1)
 			return CGP->rTown;
-		else if (s.castle == -2)
+		if (s.castle == -2)
 			return CGP->nTown;
+		else
+			return graphics->getPic(s.castle, true, false);
 	case HERO:
 		if (s.hero == -1)
 		{
@@ -2370,12 +2372,12 @@ const std::string * OptionsTab::SelectedBox::getText() const
 	switch(which)
 	{
 	case TOWN:
-		if (s.castle < GameConstants::F_NUMBER  &&  s.castle >= 0)
-			return &CGI->townh->factions[s.castle].name;
-		else if (s.castle == -1)
+		if (s.castle == -1)
 			return &CGI->generaltexth->allTexts[522];
 		else if (s.castle == -2)
 			return &CGI->generaltexth->allTexts[523];
+		else
+			return &CGI->townh->factions[s.castle].name;
 	case HERO:
 		if (s.hero == -1)
 			return &CGI->generaltexth->allTexts[522];

lib/CGameState.cpp

@@ -407,7 +407,7 @@ CGHeroInstance * CGameState::HeroesPool::pickHeroFor(bool native, TPlayerColor p
 		}
 		if(!pool.size())
 		{
-			tlog1 << "Cannot pick native hero for " << player << ". Picking any...\n";
+			tlog1 << "Cannot pick native hero for " << int(player) << ". Picking any...\n";
 			return pickHeroFor(false, player, town, available);
 		}
 		else
@@ -428,10 +428,10 @@ CGHeroInstance * CGameState::HeroesPool::pickHeroFor(bool native, TPlayerColor p
 				sum += i->second->type->heroClass->selectionProbability[town->typeID]; //total weight
 			}
 		}
-		if(!pool.size())
+		if(!pool.size() || sum == 0)
 		{
 			tlog1 << "There are no heroes available for player " << player<<"!\n";
-			return NULL;
+			return nullptr;
 		}
 
 		r = rand()%sum;
@@ -480,21 +480,32 @@ int CGameState::pickHero(int owner)
 		}
 	}
 
-	int i=0;
+	//list of heroes for this faction
+	std::vector<si32> factionHeroes;
+	factionHeroes.reserve(GameConstants::HEROES_PER_TYPE*2);
 
-	do //try to find free hero of our faction
+	size_t firstHero = ps.castle*GameConstants::HEROES_PER_TYPE*2;
+	size_t lastHero  = std::min(firstHero + GameConstants::HEROES_PER_TYPE*2, VLC->heroh->heroes.size());
+
+	//generate list of heroes
+	for (si32 i=firstHero; i<lastHero; i++)
+		factionHeroes.push_back(i);
+	// we need random order to select hero
+	std::random_shuffle(factionHeroes.begin(), factionHeroes.end());
+
+	for (size_t i=0; i<factionHeroes.size(); i++)
 	{
-		i++;
-		h = ps.castle*GameConstants::HEROES_PER_TYPE*2+(ran()%(GameConstants::HEROES_PER_TYPE*2));//->scenarioOps->playerInfos[pru].hero = VLC->
-	} while( map->getHero(h)  &&  i<(GameConstants::HEROES_QUANTITY+18+1));
-	if(i>GameConstants::HEROES_QUANTITY+18) //probably no free heroes - there's no point in further search, we'll take first free
-	{
-		tlog3 << "Warning: cannot find free hero - trying to get first available..."<<std::endl;
-		for(int j=0; j<GameConstants::HEROES_QUANTITY; j++)
-			if(!map->getHero(j))
-				h=j;
+		if (!map->getHero(factionHeroes[i]))
+			return factionHeroes[i];
 	}
-	return h;
+
+	tlog3 << "Warning: cannot find free hero - trying to get first available..."<<std::endl;
+	for(int j=0; j<VLC->heroh->heroes.size(); j++)
+		if(!map->getHero(j))
+			return j;
+
+	assert(0); //currrent code can't handle this situation
+	return -1; // no available heroes at all
 }
 
 

lib/CHeroHandler.cpp

@@ -261,7 +261,7 @@ void CHeroHandler::loadHeroClasses()
 			hc->proSec[dd] = parser.readNumber();
 		}
 
-		for(int dd=0; dd<ARRAY_COUNT(hc->selectionProbability); ++dd)
+		for(int dd=0; dd<GameConstants::F_NUMBER; ++dd)
 		{
 			hc->selectionProbability[dd] = parser.readNumber();
 		}

lib/CHeroHandler.h

@@ -68,7 +68,7 @@ public:
 	int initialPrimSkills[GameConstants::PRIMARY_SKILLS]; //initial values of primary skills, uses PrimarySkill enum
 	std::vector<std::pair<int,int> > primChance;//primChance[PRIMARY_SKILL_ID] - first is for levels 2 - 9, second for 10+;;; probability (%) of getting point of primary skill when getting new level
 	std::vector<int> proSec; //probabilities of gaining secondary skills (out of 112), in id order
-	int selectionProbability[GameConstants::F_NUMBER]; //probability of selection in towns
+	std::map<TFaction, int> selectionProbability; //probability of selection in towns
 
 	int chooseSecSkill(const std::set<int> & possibles) const; //picks secondary skill out from given possibilities
 	CHeroClass(); //c-tor

lib/map.h

@@ -117,19 +117,11 @@ struct DLL_LINKAGE PlayerInfo
 
 	si8 defaultCastle() const
 	{
-		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(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
+		assert(!allowedFactions.empty()); // impossible?
 
-				if (ret == -2) //first available castle - pick
-					ret = j;
-			}
-		}
-		return ret;
+		if (allowedFactions.size() == 1)
+			return *allowedFactions.begin(); //only one faction s available - pick it
+		return -1; // set to random
 	}
 	si8 defaultHero() const
 	{