Do not allow heroes banned for player as starting heroes

- `disposedHeroes` is now in map header, for use in map selection
- heroes that are marked as unavailable for player will now be hidden
from starting hero selection

client/lobby/OptionsTab.cpp

@@ -432,6 +432,12 @@ OptionsTab::SelectionWindow::SelectionWindow(const PlayerColor & color, SelType
 			unusableHeroes.insert(player.second.hero);
 	}
 
+	for (const auto & disposedHero : SEL->getMapInfo()->mapHeader->disposedHeroes)
+	{
+		if (!disposedHero.players.count(color))
+			allowedHeroes.erase(disposedHero.heroId);
+	}
+
 	allowedBonus.push_back(PlayerStartingBonus::RANDOM);
 
 	if(initialHero != HeroTypeID::NONE|| SEL->getPlayerInfo(color).heroesNames.size() > 0)
@@ -459,8 +465,10 @@ std::tuple<int, int> OptionsTab::SelectionWindow::calcLines(FactionID faction)
 	for(auto & elemh : allowedHeroes)
 	{
 		const CHero * type = elemh.toHeroType();
-		if(type->heroClass->faction == faction)
-			count++;
+		if(type->heroClass->faction != faction)
+			continue;
+
+		count++;
 	}
 
 	return std::make_tuple(

lib/mapping/CMap.cpp

@@ -39,11 +39,6 @@ void Rumor::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeStruct("text", text);
 }
 
-DisposedHero::DisposedHero() : heroId(0), portrait(255)
-{
-
-}
-
 CMapEvent::CMapEvent()
 	: humanAffected(false)
 	, computerAffected(false)

lib/mapping/CMap.h

@@ -56,26 +56,6 @@ struct DLL_LINKAGE Rumor
 	void serializeJson(JsonSerializeFormat & handler);
 };
 
-/// The disposed hero struct describes which hero can be hired from which player.
-struct DLL_LINKAGE DisposedHero
-{
-	DisposedHero();
-
-	HeroTypeID heroId;
-	HeroTypeID portrait; /// The portrait id of the hero, -1 is default.
-	std::string name;
-	std::set<PlayerColor> players; /// Who can hire this hero (bitfield).
-
-	template <typename Handler>
-	void serialize(Handler & h)
-	{
-		h & heroId;
-		h & portrait;
-		h & name;
-		h & players;
-	}
-};
-
 /// The map contains the map header, the tiles of the terrain, objects, heroes, towns, rumors...
 class DLL_LINKAGE CMap : public CMapHeader, public GameCallbackHolder
 {
@@ -138,7 +118,6 @@ public:
 	void reindexObjects();
 
 	std::vector<Rumor> rumors;
-	std::vector<DisposedHero> disposedHeroes;
 	std::vector<ConstTransitivePtr<CGHeroInstance> > predefinedHeroes;
 	std::set<SpellID> allowedSpells;
 	std::set<ArtifactID> allowedArtifact;

lib/mapping/CMapHeader.h

@@ -202,6 +202,24 @@ enum class EMapDifficulty : uint8_t
 	IMPOSSIBLE = 4
 };
 
+/// The disposed hero struct describes which hero can be hired from which player.
+struct DLL_LINKAGE DisposedHero
+{
+	HeroTypeID heroId;
+	HeroTypeID portrait; /// The portrait id of the hero, -1 is default.
+	std::string name;
+	std::set<PlayerColor> players; /// Who can hire this hero (bitfield).
+
+	template <typename Handler>
+	void serialize(Handler & h)
+	{
+		h & heroId;
+		h & portrait;
+		h & name;
+		h & players;
+	}
+};
+
 /// The map header holds information about loss/victory condition,map format, version, players, height, width,...
 class DLL_LINKAGE CMapHeader: public Serializeable
 {
@@ -248,6 +266,8 @@ public:
 	std::set<HeroTypeID> allowedHeroes;
 	std::set<HeroTypeID> reservedCampaignHeroes; /// Heroes that have placeholders in this map and are reserved for campaign
 
+	std::vector<DisposedHero> disposedHeroes;
+
 	bool areAnyPlayers; /// Unused. True if there are any playable players on the map.
 
 	/// "main quests" of the map that describe victory and loss conditions
@@ -298,6 +318,8 @@ public:
 		h & victoryIconIndex;
 		h & defeatMessage;
 		h & defeatIconIndex;
+		if (h.version >= Handler::Version::MAP_HEADER_DISPOSED_HEROES)
+			h & disposedHeroes;
 		h & translations;
 		if(!h.saving)
 			registerMapStrings();

lib/mapping/MapFormatH3M.cpp

@@ -95,7 +95,6 @@ void CMapLoaderH3M::init()
 	inputStream->seek(0);
 
 	readHeader();
-	readDisposedHeroes();
 	readMapOptions();
 	readAllowedArtifacts();
 	readAllowedSpellsAbilities();
@@ -223,6 +222,7 @@ void CMapLoaderH3M::readHeader()
 	readVictoryLossConditions();
 	readTeamInfo();
 	readAllowedHeroes();
+	readDisposedHeroes();
 }
 
 void CMapLoaderH3M::readPlayerInfo()
@@ -703,13 +703,13 @@ void CMapLoaderH3M::readDisposedHeroes()
 	if(features.levelSOD)
 	{
 		size_t disp = reader->readUInt8();
-		map->disposedHeroes.resize(disp);
+		mapHeader->disposedHeroes.resize(disp);
 		for(size_t g = 0; g < disp; ++g)
 		{
-			map->disposedHeroes[g].heroId = reader->readHero();
-			map->disposedHeroes[g].portrait = reader->readHeroPortrait();
-			map->disposedHeroes[g].name = readLocalizedString(TextIdentifier("header", "heroes", map->disposedHeroes[g].heroId.getNum()));
-			reader->readBitmaskPlayers(map->disposedHeroes[g].players, false);
+			mapHeader->disposedHeroes[g].heroId = reader->readHero();
+			mapHeader->disposedHeroes[g].portrait = reader->readHeroPortrait();
+			mapHeader->disposedHeroes[g].name = readLocalizedString(TextIdentifier("header", "heroes", mapHeader->disposedHeroes[g].heroId.getNum()));
+			reader->readBitmaskPlayers(mapHeader->disposedHeroes[g].players, false);
 		}
 	}
 }

lib/mapping/MapFormatJson.cpp

@@ -640,19 +640,19 @@ void CMapFormatJson::readDisposedHeroes(JsonSerializeFormat & handler)
 			hero.players = mask;
 			//name and portrait are not used
 
-			map->disposedHeroes.push_back(hero);
+			mapHeader->disposedHeroes.push_back(hero);
 		}
 	}
 }
 
 void CMapFormatJson::writeDisposedHeroes(JsonSerializeFormat & handler)
 {
-	if(map->disposedHeroes.empty())
+	if(mapHeader->disposedHeroes.empty())
 		return;
 
 	auto definitions = handler.enterStruct("predefinedHeroes");//DisposedHeroes are part of predefinedHeroes in VCMI map format
 
-	for(DisposedHero & hero : map->disposedHeroes)
+	for(DisposedHero & hero : mapHeader->disposedHeroes)
 	{
 		std::string type = HeroTypeID::encode(hero.heroId.getNum());
 

lib/serializer/ESerializationVersion.h

@@ -70,6 +70,10 @@ enum class ESerializationVersion : int32_t
 	REWARDABLE_GUARDS, // 871 - fix missing serialization of guards in rewardable objects
 	MARKET_TRANSLATION_FIX, // 872 - remove serialization of markets translateable strings
 	EVENT_OBJECTS_DELETION, //873 - allow events to remove map objects
+
+	RELEASE_160 = 873,
+
+	MAP_HEADER_DISPOSED_HEROES, // map header contains disposed heroes list
 	
-	CURRENT = EVENT_OBJECTS_DELETION
+	CURRENT = MAP_HEADER_DISPOSED_HEROES
 };

server/CVCMIServer.cpp

@@ -974,10 +974,26 @@ void CVCMIServer::optionSetBonus(PlayerColor player, PlayerStartingBonus id)
 
 bool CVCMIServer::canUseThisHero(PlayerColor player, HeroTypeID ID)
 {
-	return VLC->heroh->size() > ID
-		&& si->playerInfos[player].castle == VLC->heroh->objects[ID]->heroClass->faction
-		&& !vstd::contains(getUsedHeroes(), ID)
-		&& mi->mapHeader->allowedHeroes.count(ID);
+	if (!ID.hasValue())
+		return false;
+
+	if (ID >= VLC->heroh->size())
+		return false;
+
+	if (si->playerInfos[player].castle != VLC->heroh->objects[ID]->heroClass->faction)
+		return false;
+
+	if (vstd::contains(getUsedHeroes(), ID))
+		return false;
+
+	if (!mi->mapHeader->allowedHeroes.count(ID))
+		return false;
+
+	for (const auto & disposedHero : mi->mapHeader->disposedHeroes)
+		if (disposedHero.heroId == ID && !disposedHero.players.count(player))
+			return false;
+
+	return true;
 }
 
 std::vector<HeroTypeID> CVCMIServer::getUsedHeroes()

test/map/MapComparer.cpp

@@ -180,12 +180,12 @@ void MapComparer::compareHeader()
 	boost::sort (expectedEvents, sortByIdentifier);
 
 	checkEqual(actualEvents, expectedEvents);
+	checkEqual(actual->disposedHeroes, expected->disposedHeroes);
 }
 
 void MapComparer::compareOptions()
 {
 	checkEqual(actual->rumors, expected->rumors);
-	checkEqual(actual->disposedHeroes, expected->disposedHeroes);
 	//todo: compareOptions predefinedHeroes
 
 	checkEqual(actual->allowedAbilities, expected->allowedAbilities);