Merge pull request #2321 from IvanSavenko/tavern_fixes

Fixes & improvements to tavern/hero pool handling

CCallback.cpp

@@ -270,17 +270,10 @@ void CCallback::recruitHero(const CGObjectInstance *townOrTavern, const CGHeroIn
 {
 	assert(townOrTavern);
 	assert(hero);
-	ui8 i=0;
-	for(; i<gs->players[*player].availableHeroes.size(); i++)
-	{
-		if(gs->players[*player].availableHeroes[i] == hero)
-		{
-			HireHero pack(i, townOrTavern->id);
-			pack.player = *player;
-			sendRequest(&pack);
-			return;
-		}
-	}
+
+	HireHero pack(HeroTypeID(hero->subID), townOrTavern->id);
+	pack.player = *player;
+	sendRequest(&pack);
 }
 
 void CCallback::save( const std::string &fname )

cmake_modules/VCMI_lib.cmake

@@ -68,6 +68,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/gameState/CGameState.cpp
 		${MAIN_LIB_DIR}/gameState/CGameStateCampaign.cpp
 		${MAIN_LIB_DIR}/gameState/InfoAboutArmy.cpp
+		${MAIN_LIB_DIR}/gameState/TavernHeroesPool.cpp
 
 		${MAIN_LIB_DIR}/logging/CBasicLogConfigurator.cpp
 		${MAIN_LIB_DIR}/logging/CLogger.cpp
@@ -394,6 +395,8 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/gameState/EVictoryLossCheckResult.h
 		${MAIN_LIB_DIR}/gameState/InfoAboutArmy.h
 		${MAIN_LIB_DIR}/gameState/SThievesGuildInfo.h
+		${MAIN_LIB_DIR}/gameState/TavernHeroesPool.h
+		${MAIN_LIB_DIR}/gameState/TavernSlot.h
 		${MAIN_LIB_DIR}/gameState/QuestInfo.h
 
 		${MAIN_LIB_DIR}/logging/CBasicLogConfigurator.h

lib/CGameInfoCallback.cpp

@@ -13,6 +13,7 @@
 #include "gameState/CGameState.h"
 #include "gameState/InfoAboutArmy.h"
 #include "gameState/SThievesGuildInfo.h"
+#include "gameState/TavernHeroesPool.h"
 #include "CGeneralTextHandler.h"
 #include "StartInfo.h" // for StartInfo
 #include "battle/BattleInfo.h" // for BattleInfo
@@ -99,13 +100,6 @@ const PlayerState * CGameInfoCallback::getPlayerState(PlayerColor color, bool ve
 	}
 }
 
-const CTown * CGameInfoCallback::getNativeTown(PlayerColor color) const
-{
-	const PlayerSettings *ps = getPlayerSettings(color);
-	ERROR_RET_VAL_IF(!ps, "There is no such player!", nullptr);
-	return (*VLC->townh)[ps->castle]->town;
-}
-
 const CGObjectInstance * CGameInfoCallback::getObjByQuestIdentifier(int identifier) const
 {
 	if(gs->map->questIdentifierToId.empty())
@@ -486,13 +480,10 @@ std::vector<const CGHeroInstance *> CGameInfoCallback::getAvailableHeroes(const
 	//ERROR_RET_VAL_IF(!isOwnedOrVisited(townOrTavern), "Town or tavern must be owned or visited!", ret);
 	//TODO: town needs to be owned, advmap tavern needs to be visited; to be reimplemented when visit tracking is done
 	const CGTownInstance * town = getTown(townOrTavern->id);
+
 	if(townOrTavern->ID == Obj::TAVERN || (town && town->hasBuilt(BuildingID::TAVERN)))
-	{
-		range::copy(gs->players[*player].availableHeroes, std::back_inserter(ret));
-		vstd::erase_if(ret, [](const CGHeroInstance * h) {
-			return h == nullptr;
-		});
-	}
+		return gs->heroesPool->getHeroesFor(*player);
+
 	return ret;
 }
 

lib/CGameInfoCallback.h

@@ -108,7 +108,6 @@ public:
 //	std::string getTavernRumor(const CGObjectInstance * townOrTavern) const;
 //	EBuildingState::EBuildingState canBuildStructure(const CGTownInstance *t, BuildingID ID);//// 0 - no more than one capitol, 1 - lack of water, 2 - forbidden, 3 - Add another level to Mage Guild, 4 - already built, 5 - cannot build, 6 - cannot afford, 7 - build, 8 - lack of requirements
 //	virtual bool getTownInfo(const CGObjectInstance * town, InfoAboutTown & dest, const CGObjectInstance * selectedObject = nullptr) const;
-//	const CTown *getNativeTown(PlayerColor color) const;
 
 	//from gs
 //	const TeamState *getTeam(TeamID teamID) const;
@@ -206,7 +205,6 @@ public:
 	virtual std::string getTavernRumor(const CGObjectInstance * townOrTavern) const;
 	virtual EBuildingState::EBuildingState canBuildStructure(const CGTownInstance *t, BuildingID ID);//// 0 - no more than one capitol, 1 - lack of water, 2 - forbidden, 3 - Add another level to Mage Guild, 4 - already built, 5 - cannot build, 6 - cannot afford, 7 - build, 8 - lack of requirements
 	virtual bool getTownInfo(const CGObjectInstance * town, InfoAboutTown & dest, const CGObjectInstance * selectedObject = nullptr) const;
-	virtual const CTown *getNativeTown(PlayerColor color) const;
 
 	//from gs
 	virtual const TeamState *getTeam(TeamID teamID) const;

lib/CPlayerState.cpp

@@ -35,7 +35,6 @@ PlayerState::PlayerState(PlayerState && other) noexcept:
 	std::swap(visitedObjects, other.visitedObjects);
 	std::swap(heroes, other.heroes);
 	std::swap(towns, other.towns);
-	std::swap(availableHeroes, other.availableHeroes);
 	std::swap(dwellings, other.dwellings);
 	std::swap(quests, other.quests);
 }

lib/CPlayerState.h

@@ -33,7 +33,6 @@ public:
 	std::set<ObjectInstanceID> visitedObjects; // as a std::set, since most accesses here will be from visited status checks
 	std::vector<ConstTransitivePtr<CGHeroInstance> > heroes;
 	std::vector<ConstTransitivePtr<CGTownInstance> > towns;
-	std::vector<ConstTransitivePtr<CGHeroInstance> > availableHeroes; //heroes available in taverns
 	std::vector<ConstTransitivePtr<CGDwelling> > dwellings; //used for town growth
 	std::vector<QuestInfo> quests; //store info about all received quests
 
@@ -74,7 +73,6 @@ public:
 		h & status;
 		h & heroes;
 		h & towns;
-		h & availableHeroes;
 		h & dwellings;
 		h & quests;
 		h & visitedObjects;

lib/CRandomGenerator.cpp

@@ -20,6 +20,11 @@ CRandomGenerator::CRandomGenerator()
 	resetSeed();
 }
 
+CRandomGenerator::CRandomGenerator(int seed)
+{
+	setSeed(seed);
+}
+
 void CRandomGenerator::setSeed(int seed)
 {
 	rand.seed(seed);

lib/CRandomGenerator.h

@@ -30,6 +30,9 @@ public:
 	/// current thread ID.
 	CRandomGenerator();
 
+	/// Seeds the generator with provided initial seed
+	explicit CRandomGenerator(int seed);
+
 	void setSeed(int seed);
 
 	/// Resets the seed to the product of the current time in milliseconds and the

lib/GameConstants.h

@@ -357,9 +357,11 @@ class PlayerColor : public BaseForID<PlayerColor, ui8>
 
 	enum EPlayerColor
 	{
-		PLAYER_LIMIT_I = 8
+		PLAYER_LIMIT_I = 8,
 	};
 
+	using Mask = uint8_t;
+
 	DLL_LINKAGE static const PlayerColor SPECTATOR; //252
 	DLL_LINKAGE static const PlayerColor CANNOT_DETERMINE; //253
 	DLL_LINKAGE static const PlayerColor UNFLAGGABLE; //254 - neutral objects (pandora, banks)

lib/IGameCallback.cpp

@@ -36,6 +36,7 @@
 #include "StartInfo.h"
 #include "gameState/CGameState.h"
 #include "gameState/CGameStateCampaign.h"
+#include "gameState/TavernHeroesPool.h"
 #include "mapping/CMap.h"
 #include "CPlayerState.h"
 #include "GameSettings.h"

lib/NetPackVisitor.h

@@ -36,7 +36,7 @@ public:
 	virtual void visitSetMana(SetMana & pack) {}
 	virtual void visitSetMovePoints(SetMovePoints & pack) {}
 	virtual void visitFoWChange(FoWChange & pack) {}
-	virtual void visitSetAvailableHeroes(SetAvailableHeroes & pack) {}
+	virtual void visitSetAvailableHeroes(SetAvailableHero & pack) {}
 	virtual void visitGiveBonus(GiveBonus & pack) {}
 	virtual void visitChangeObjPos(ChangeObjPos & pack) {}
 	virtual void visitPlayerEndsGame(PlayerEndsGame & pack) {}
@@ -162,4 +162,4 @@ public:
 	virtual void visitLobbyShowMessage(LobbyShowMessage & pack) {}
 };
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

lib/NetPacks.h

@@ -19,6 +19,7 @@
 #include "battle/BattleAction.h"
 #include "battle/CObstacleInstance.h"
 #include "gameState/EVictoryLossCheckResult.h"
+#include "gameState/TavernSlot.h"
 #include "gameState/QuestInfo.h"
 #include "mapObjects/CGHeroInstance.h"
 #include "mapping/CMapDefines.h"
@@ -330,23 +331,26 @@ struct DLL_LINKAGE FoWChange : public CPackForClient
 	}
 };
 
-struct DLL_LINKAGE SetAvailableHeroes : public CPackForClient
+struct DLL_LINKAGE SetAvailableHero : public CPackForClient
 {
-	SetAvailableHeroes()
+	SetAvailableHero()
 	{
-		for(auto & i : army)
-			i.clear();
+		army.clear();
 	}
 	void applyGs(CGameState * gs);
 
+	TavernHeroSlot slotID;
+	TavernSlotRole roleID;
 	PlayerColor player;
-	si32 hid[GameConstants::AVAILABLE_HEROES_PER_PLAYER]; //-1 if no hero
-	CSimpleArmy army[GameConstants::AVAILABLE_HEROES_PER_PLAYER];
+	HeroTypeID hid; //HeroTypeID::NONE if no hero
+	CSimpleArmy army;
 
 	virtual void visitTyped(ICPackVisitor & visitor) override;
 
 	template <typename Handler> void serialize(Handler & h, const int version)
 	{
+		h & slotID;
+		h & roleID;
 		h & player;
 		h & hid;
 		h & army;
@@ -692,7 +696,7 @@ struct DLL_LINKAGE HeroRecruited : public CPackForClient
 {
 	void applyGs(CGameState * gs) const;
 
-	si32 hid = -1; //subID of hero
+	HeroTypeID hid; //subID of hero
 	ObjectInstanceID tid;
 	ObjectInstanceID boatId;
 	int3 tile;
@@ -2437,12 +2441,12 @@ struct DLL_LINKAGE SetFormation : public CPackForServer
 struct DLL_LINKAGE HireHero : public CPackForServer
 {
 	HireHero() = default;
-	HireHero(si32 HID, const ObjectInstanceID & TID)
+	HireHero(HeroTypeID HID, const ObjectInstanceID & TID)
 		: hid(HID)
 		, tid(TID)
 	{
 	}
-	si32 hid = 0; //available hero serial
+	HeroTypeID hid; //available hero serial
 	ObjectInstanceID tid; //town (tavern) id
 	PlayerColor player;
 

lib/NetPacksLib.cpp

@@ -20,6 +20,7 @@
 #include "spells/CSpellHandler.h"
 #include "CCreatureHandler.h"
 #include "gameState/CGameState.h"
+#include "gameState/TavernHeroesPool.h"
 #include "CStack.h"
 #include "battle/BattleInfo.h"
 #include "CTownHandler.h"
@@ -151,7 +152,7 @@ void FoWChange::visitTyped(ICPackVisitor & visitor)
 	visitor.visitFoWChange(*this);
 }
 
-void SetAvailableHeroes::visitTyped(ICPackVisitor & visitor)
+void SetAvailableHero::visitTyped(ICPackVisitor & visitor)
 {
 	visitor.visitSetAvailableHeroes(*this);
 }
@@ -939,18 +940,9 @@ void FoWChange::applyGs(CGameState *gs)
 	}
 }
 
-void SetAvailableHeroes::applyGs(CGameState *gs)
+void SetAvailableHero::applyGs(CGameState *gs)
 {
-	PlayerState *p = gs->getPlayerState(player);
-	p->availableHeroes.clear();
-
-	for (int i = 0; i < GameConstants::AVAILABLE_HEROES_PER_PLAYER; i++)
-	{
-		CGHeroInstance *h = (hid[i]>=0 ?  gs->hpool.heroesPool[hid[i]].get() : nullptr);
-		if(h && army[i])
-			h->setToArmy(army[i]);
-		p->availableHeroes.emplace_back(h);
-	}
+	gs->heroesPool->setHeroForPlayer(player, slotID, hid, army, roleID);
 }
 
 void GiveBonus::applyGs(CGameState *gs)
@@ -1150,11 +1142,8 @@ void RemoveObject::applyGs(CGameState *gs)
 			beatenHero->inTownGarrison = false;
 		}
 		//return hero to the pool, so he may reappear in tavern
-		gs->hpool.heroesPool[beatenHero->subID] = beatenHero;
-
-		if(!vstd::contains(gs->hpool.pavailable, beatenHero->subID))
-			gs->hpool.pavailable[beatenHero->subID] = 0xff;
 
+		gs->heroesPool->addHeroToPool(beatenHero);
 		gs->map->objects[id.getNum()] = nullptr;
 
 		//If hero on Boat is removed, the Boat disappears
@@ -1379,8 +1368,7 @@ void SetHeroesInTown::applyGs(CGameState * gs) const
 
 void HeroRecruited::applyGs(CGameState * gs) const
 {
-	assert(vstd::contains(gs->hpool.heroesPool, hid));
-	CGHeroInstance *h = gs->hpool.heroesPool[hid];
+	CGHeroInstance *h = gs->heroesPool->takeHeroFromPool(hid);
 	CGTownInstance *t = gs->getTown(tid);
 	PlayerState *p = gs->getPlayerState(player);
 
@@ -1411,7 +1399,6 @@ void HeroRecruited::applyGs(CGameState * gs) const
 		}
 	}
 
-	gs->hpool.heroesPool.erase(hid);
 	if(h->id == ObjectInstanceID())
 	{
 		h->id = ObjectInstanceID(static_cast<si32>(gs->map->objects.size()));
@@ -2021,26 +2008,17 @@ void NewTurn::applyGs(CGameState *gs)
 	{
 		CGHeroInstance *hero = gs->getHero(h.id);
 		if(!hero)
-		{
-			// retreated or surrendered hero who has not been reset yet
-			for(auto& hp : gs->hpool.heroesPool)
-			{
-				if(hp.second->id == h.id)
-				{
-					hero = hp.second;
-					break;
-				}
-			}
-		}
-		if(!hero)
 		{
 			logGlobal->error("Hero %d not found in NewTurn::applyGs", h.id.getNum());
 			continue;
 		}
+
 		hero->setMovementPoints(h.move);
 		hero->mana = h.mana;
 	}
 
+	gs->heroesPool->onNewDay();
+
 	for(const auto & re : res)
 	{
 		assert(re.first < PlayerColor::PLAYER_LIMIT);

lib/StartInfo.h

@@ -35,9 +35,9 @@ struct DLL_LINKAGE PlayerSettings
 	};
 
 	Ebonus bonus;
-	si16 castle;
-	si32 hero,
-		 heroPortrait; //-1 if default, else ID
+	FactionID castle;
+	HeroTypeID hero;
+	HeroTypeID heroPortrait; //-1 if default, else ID
 
 	std::string heroName;
 	PlayerColor color; //from 0 -

lib/gameState/CGameState.cpp

@@ -12,6 +12,7 @@
 
 #include "EVictoryLossCheckResult.h"
 #include "InfoAboutArmy.h"
+#include "TavernHeroesPool.h"
 #include "CGameStateCampaign.h"
 #include "SThievesGuildInfo.h"
 
@@ -102,81 +103,6 @@ static CGObjectInstance * createObject(const Obj & id, int subid, const int3 & p
 	return nobj;
 }
 
-CGHeroInstance * CGameState::HeroesPool::pickHeroFor(bool native,
-													 const PlayerColor & player,
-													 const CTown * town,
-													 std::map<ui32, ConstTransitivePtr<CGHeroInstance>> & available,
-													 CRandomGenerator & rand,
-													 const CHeroClass * bannedClass) const
-{
-	CGHeroInstance *ret = nullptr;
-
-	if(player>=PlayerColor::PLAYER_LIMIT)
-	{
-		logGlobal->error("Cannot pick hero for faction %s. Wrong owner!", town->faction->getJsonKey());
-		return nullptr;
-	}
-
-	std::vector<CGHeroInstance *> pool;
-
-	if(native)
-	{
-		for(auto & elem : available)
-		{
-			if(pavailable.find(elem.first)->second & 1<<player.getNum()
-				&& elem.second->type->heroClass->faction == town->faction->getIndex())
-			{
-				pool.push_back(elem.second); //get all available heroes
-			}
-		}
-		if(pool.empty())
-		{
-			logGlobal->error("Cannot pick native hero for %s. Picking any...", player.getStr());
-			return pickHeroFor(false, player, town, available, rand);
-		}
-		else
-		{
-			ret = *RandomGeneratorUtil::nextItem(pool, rand);
-		}
-	}
-	else
-	{
-		int sum = 0;
-		int r;
-
-		for(auto & elem : available)
-		{
-			if (pavailable.find(elem.first)->second & (1<<player.getNum()) &&    // hero is available
-			    ( !bannedClass || elem.second->type->heroClass != bannedClass) ) // and his class is not same as other hero
-			{
-				pool.push_back(elem.second);
-				sum += elem.second->type->heroClass->selectionProbability[town->faction->getId()]; //total weight
-			}
-		}
-		if(pool.empty() || sum == 0)
-		{
-			logGlobal->error("There are no heroes available for player %s!", player.getStr());
-			return nullptr;
-		}
-
-		r = rand.nextInt(sum - 1);
-		for (auto & elem : pool)
-		{
-			r -= elem->type->heroClass->selectionProbability[town->faction->getId()];
-			if(r < 0)
-			{
-				ret = elem;
-				break;
-			}
-		}
-		if(!ret)
-			ret = pool.back();
-	}
-
-	available.erase(ret->subID);
-	return ret;
-}
-
 HeroTypeID CGameState::pickNextHeroType(const PlayerColor & owner)
 {
 	const PlayerSettings &ps = scenarioOps->getIthPlayersSettings(owner);
@@ -459,6 +385,7 @@ int CGameState::getDate(Date::EDateType mode) const
 CGameState::CGameState()
 {
 	gs = this;
+	heroesPool = std::make_unique<TavernHeroesPool>();
 	applier = std::make_shared<CApplier<CBaseForGSApply>>();
 	registerTypesClientPacks1(*applier);
 	registerTypesClientPacks2(*applier);
@@ -469,9 +396,6 @@ CGameState::~CGameState()
 {
 	map.dellNull();
 	curB.dellNull();
-
-	for(auto ptr : hpool.heroesPool) // clean hero pool
-		ptr.second.dellNull();
 }
 
 void CGameState::preInit(Services * services)
@@ -951,8 +875,7 @@ void CGameState::initHeroes()
 		if(!vstd::contains(heroesToCreate, HeroTypeID(ph->subID)))
 			continue;
 		ph->initHero(getRandomGenerator());
-		hpool.heroesPool[ph->subID] = ph;
-		hpool.pavailable[ph->subID] = 0xff;
+		heroesPool->addHeroToPool(ph);
 		heroesToCreate.erase(ph->type->getId());
 
 		map->allHeroes[ph->subID] = ph;
@@ -965,14 +888,11 @@ void CGameState::initHeroes()
 
 		int typeID = htype.getNum();
 		map->allHeroes[typeID] = vhi;
-		hpool.heroesPool[typeID] = vhi;
-		hpool.pavailable[typeID] = 0xff;
+		heroesPool->addHeroToPool(vhi);
 	}
 
 	for(auto & elem : map->disposedHeroes)
-	{
-		hpool.pavailable[elem.heroId] = elem.players;
-	}
+		heroesPool->setAvailability(elem.heroId, elem.players);
 
 	if (campaign)
 		campaign->initHeroes();
@@ -2067,17 +1987,6 @@ void CGameState::obtainPlayersStats(SThievesGuildInfo & tgi, int level)
 #undef FILL_FIELD
 }
 
-std::map<ui32, ConstTransitivePtr<CGHeroInstance> > CGameState::unusedHeroesFromPool()
-{
-	std::map<ui32, ConstTransitivePtr<CGHeroInstance> > pool = hpool.heroesPool;
-	for(const auto & player : players)
-		for(auto availableHero : player.second.availableHeroes)
-			if(availableHero)
-				pool.erase((*availableHero).subID);
-
-	return pool;
-}
-
 void CGameState::buildBonusSystemTree()
 {
 	buildGlobalTeamPlayerTree();

lib/gameState/CGameState.h

@@ -29,6 +29,7 @@ struct EventCondition;
 struct CampaignTravel;
 class CStackInstance;
 class CGameStateCampaign;
+class TavernHeroesPool;
 struct SThievesGuildInfo;
 
 template<typename T> class CApplier;
@@ -78,25 +79,10 @@ DLL_LINKAGE std::ostream & operator<<(std::ostream & os, const EVictoryLossCheck
 class DLL_LINKAGE CGameState : public CNonConstInfoCallback
 {
 	friend class CGameStateCampaign;
+
 public:
-	struct DLL_LINKAGE HeroesPool
-	{
-		std::map<ui32, ConstTransitivePtr<CGHeroInstance> > heroesPool; //[subID] - heroes available to buy; nullptr if not available
-		std::map<ui32,ui8> pavailable; // [subid] -> which players can recruit hero (binary flags)
-
-		CGHeroInstance * pickHeroFor(bool native,
-									 const PlayerColor & player,
-									 const CTown * town,
-									 std::map<ui32, ConstTransitivePtr<CGHeroInstance>> & available,
-									 CRandomGenerator & rand,
-									 const CHeroClass * bannedClass = nullptr) const;
-
-		template <typename Handler> void serialize(Handler &h, const int version)
-		{
-			h & heroesPool;
-			h & pavailable;
-		}
-	} hpool; //we have here all heroes available on this map that are not hired
+	//we have here all heroes available on this map that are not hired
+	std::unique_ptr<TavernHeroesPool> heroesPool;
 
 	CGameState();
 	virtual ~CGameState();
@@ -142,7 +128,6 @@ public:
 	bool checkForStandardLoss(const PlayerColor & player) const; //checks if given player lost the game
 
 	void obtainPlayersStats(SThievesGuildInfo & tgi, int level); //fills tgi with info about other players that is available at given level of thieves' guild
-	std::map<ui32, ConstTransitivePtr<CGHeroInstance> > unusedHeroesFromPool(); //heroes pool without heroes that are available in taverns
 
 	bool isVisible(int3 pos, const std::optional<PlayerColor> & player) const override;
 	bool isVisible(const CGObjectInstance * obj, const std::optional<PlayerColor> & player) const override;
@@ -169,7 +154,7 @@ public:
 		h & map;
 		h & players;
 		h & teams;
-		h & hpool;
+		h & heroesPool;
 		h & globalEffects;
 		h & rand;
 		h & rumor;

lib/gameState/TavernHeroesPool.cpp

@@ -0,0 +1,142 @@
+/*
+ * TavernHeroesPool.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#include "StdInc.h"
+#include "TavernHeroesPool.h"
+
+#include "../mapObjects/CGHeroInstance.h"
+#include "../CHeroHandler.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+TavernHeroesPool::~TavernHeroesPool()
+{
+	for(const auto & ptr : heroesPool) // clean hero pool
+		delete ptr.second;
+}
+
+std::map<HeroTypeID, CGHeroInstance*> TavernHeroesPool::unusedHeroesFromPool() const
+{
+	std::map<HeroTypeID, CGHeroInstance*> pool = heroesPool;
+	for(const auto & slot : currentTavern)
+		pool.erase(HeroTypeID(slot.hero->subID));
+
+	return pool;
+}
+
+TavernSlotRole TavernHeroesPool::getSlotRole(HeroTypeID hero) const
+{
+	for (auto const & slot : currentTavern)
+	{
+		if (HeroTypeID(slot.hero->subID) == hero)
+			return slot.role;
+	}
+	return TavernSlotRole::NONE;
+}
+
+void TavernHeroesPool::setHeroForPlayer(PlayerColor player, TavernHeroSlot slot, HeroTypeID hero, CSimpleArmy & army, TavernSlotRole role)
+{
+	vstd::erase_if(currentTavern, [&](const TavernSlot & entry){
+		return entry.player == player && entry.slot == slot;
+	});
+
+	if (hero == HeroTypeID::NONE)
+		return;
+
+	CGHeroInstance * h = heroesPool[hero];
+
+	if (h && army)
+		h->setToArmy(army);
+
+	TavernSlot newSlot;
+	newSlot.hero = h;
+	newSlot.player = player;
+	newSlot.role = role;
+	newSlot.slot = slot;
+
+	currentTavern.push_back(newSlot);
+
+	boost::range::sort(currentTavern, [](const TavernSlot & left, const TavernSlot & right)
+	{
+		if (left.slot == right.slot)
+			return left.player < right.player;
+		else
+			return left.slot < right.slot;
+	});
+}
+
+bool TavernHeroesPool::isHeroAvailableFor(HeroTypeID hero, PlayerColor color) const
+{
+	if (perPlayerAvailability.count(hero))
+		return perPlayerAvailability.at(hero) & (1 << color.getNum());
+
+	return true;
+}
+
+std::vector<const CGHeroInstance *> TavernHeroesPool::getHeroesFor(PlayerColor color) const
+{
+	std::vector<const CGHeroInstance *> result;
+
+	for(const auto & slot : currentTavern)
+	{
+		if (slot.player == color)
+			result.push_back(slot.hero);
+	}
+
+	return result;
+}
+
+CGHeroInstance * TavernHeroesPool::takeHeroFromPool(HeroTypeID hero)
+{
+	assert(heroesPool.count(hero));
+
+	CGHeroInstance * result = heroesPool[hero];
+	heroesPool.erase(hero);
+
+	vstd::erase_if(currentTavern, [&](const TavernSlot & entry){
+		return entry.hero->type->getId() == hero;
+	});
+
+	assert(result);
+	return result;
+}
+
+void TavernHeroesPool::onNewDay()
+{
+	for(auto & hero : heroesPool)
+	{
+		assert(hero.second);
+		if(!hero.second)
+			continue;
+
+		hero.second->setMovementPoints(hero.second->movementPointsLimit(true));
+		hero.second->mana = hero.second->manaLimit();
+	}
+
+	for (auto & slot : currentTavern)
+	{
+		if (slot.role == TavernSlotRole::RETREATED_TODAY)
+			slot.role = TavernSlotRole::RETREATED;
+
+		if (slot.role == TavernSlotRole::SURRENDERED_TODAY)
+			slot.role = TavernSlotRole::SURRENDERED;
+	}
+}
+
+void TavernHeroesPool::addHeroToPool(CGHeroInstance * hero)
+{
+	heroesPool[HeroTypeID(hero->subID)] = hero;
+}
+
+void TavernHeroesPool::setAvailability(HeroTypeID hero, PlayerColor::Mask mask)
+{
+	perPlayerAvailability[hero] = mask;
+}
+
+VCMI_LIB_NAMESPACE_END

lib/gameState/TavernHeroesPool.h

@@ -0,0 +1,87 @@
+/*
+ * TavernHeroesPool.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+#include "../GameConstants.h"
+#include "TavernSlot.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+class CGHeroInstance;
+class CTown;
+class CRandomGenerator;
+class CHeroClass;
+class CGameState;
+class CSimpleArmy;
+
+class DLL_LINKAGE TavernHeroesPool
+{
+	struct TavernSlot
+	{
+		CGHeroInstance * hero;
+		TavernHeroSlot slot;
+		TavernSlotRole role;
+		PlayerColor player;
+
+		template <typename Handler> void serialize(Handler &h, const int version)
+		{
+			h & hero;
+			h & slot;
+			h & role;
+			h & player;
+		}
+	};
+
+	/// list of all heroes in pool, including those currently present in taverns
+	std::map<HeroTypeID, CGHeroInstance* > heroesPool;
+
+	/// list of which players are able to purchase specific hero
+	/// if hero is not present in list, he is available for everyone
+	std::map<HeroTypeID, PlayerColor::Mask> perPlayerAvailability;
+
+	/// list of heroes currently available in taverns
+	std::vector<TavernSlot> currentTavern;
+
+public:
+	~TavernHeroesPool();
+
+	/// Returns heroes currently availabe in tavern of a specific player
+	std::vector<const CGHeroInstance *> getHeroesFor(PlayerColor color) const;
+
+	/// returns heroes in pool without heroes that are available in taverns
+	std::map<HeroTypeID, CGHeroInstance* > unusedHeroesFromPool() const;
+
+	/// Returns true if hero is available to a specific player
+	bool isHeroAvailableFor(HeroTypeID hero, PlayerColor color) const;
+
+	TavernSlotRole getSlotRole(HeroTypeID hero) const;
+
+	CGHeroInstance * takeHeroFromPool(HeroTypeID hero);
+
+	/// reset mana and movement points for all heroes in pool
+	void onNewDay();
+
+	void addHeroToPool(CGHeroInstance * hero);
+
+	/// Marks hero as available to only specific set of players
+	void setAvailability(HeroTypeID hero, PlayerColor::Mask mask);
+
+	/// Makes hero available in tavern of specified player
+	void setHeroForPlayer(PlayerColor player, TavernHeroSlot slot, HeroTypeID hero, CSimpleArmy & army, TavernSlotRole role);
+
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & heroesPool;
+		h & perPlayerAvailability;
+		h & currentTavern;
+	}
+};
+
+VCMI_LIB_NAMESPACE_END

lib/gameState/TavernSlot.h

@@ -0,0 +1,35 @@
+/*
+ * TavernSlot.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+enum class TavernHeroSlot : int8_t
+{
+	NONE = -1,
+	NATIVE, // 1st / left slot in tavern, contains hero native to player's faction on new week
+	RANDOM  // 2nd / right slot in tavern, contains hero of random class
+};
+
+enum class TavernSlotRole : int8_t
+{
+	NONE = -1,
+
+	SINGLE_UNIT, // hero was added after buying hero from this slot, and only has 1 creature in army
+	FULL_ARMY, // hero was added to tavern on new week and still has full army
+
+	RETREATED, // hero was owned by player before, but have retreated from battle and only has 1 creature in army
+	RETREATED_TODAY,
+
+	SURRENDERED, // hero was owned by player before, but have surrendered in battle and kept some troops
+	SURRENDERED_TODAY,
+};
+
+VCMI_LIB_NAMESPACE_END

lib/mapping/CMap.h

@@ -56,10 +56,10 @@ struct DLL_LINKAGE DisposedHero
 {
 	DisposedHero();
 
-	ui32 heroId;
-	ui32 portrait; /// The portrait id of the hero, -1 is default.
+	HeroTypeID heroId;
+	HeroTypeID portrait; /// The portrait id of the hero, -1 is default.
 	std::string name;
-	ui8 players; /// Who can hire this hero (bitfield).
+	PlayerColor::Mask players; /// Who can hire this hero (bitfield).
 
 	template <typename Handler>
 	void serialize(Handler & h, const int version)

lib/registerTypes/RegisterTypes.h

@@ -239,7 +239,7 @@ void registerTypesClientPacks1(Serializer &s)
 	s.template registerType<CPackForClient, SetMana>();
 	s.template registerType<CPackForClient, SetMovePoints>();
 	s.template registerType<CPackForClient, FoWChange>();
-	s.template registerType<CPackForClient, SetAvailableHeroes>();
+	s.template registerType<CPackForClient, SetAvailableHero>();
 	s.template registerType<CPackForClient, GiveBonus>();
 	s.template registerType<CPackForClient, ChangeObjPos>();
 	s.template registerType<CPackForClient, PlayerEndsGame>();

lib/registerTypes/TypesLobbyPacks.cpp

@@ -14,6 +14,7 @@
 #include "../StartInfo.h"
 #include "../gameState/CGameState.h"
 #include "../gameState/CGameStateCampaign.h"
+#include "../gameState/TavernHeroesPool.h"
 #include "../mapping/CMap.h"
 #include "../CModHandler.h"
 #include "../mapObjects/CObjectHandler.h"

server/CGameHandler.cpp

@@ -8,6 +8,12 @@
  *
  */
 #include "StdInc.h"
+#include "CGameHandler.h"
+
+#include "HeroPoolProcessor.h"
+#include "ServerNetPackVisitors.h"
+#include "ServerSpellCastEnvironment.h"
+#include "CVCMIServer.h"
 
 #include "../lib/filesystem/Filesystem.h"
 #include "../lib/filesystem/FileInfo.h"
@@ -35,7 +41,6 @@
 #include "../lib/GameSettings.h"
 #include "../lib/battle/BattleInfo.h"
 #include "../lib/CondSh.h"
-#include "ServerNetPackVisitors.h"
 #include "../lib/VCMI_Lib.h"
 #include "../lib/mapping/CMap.h"
 #include "../lib/mapping/CMapService.h"
@@ -44,9 +49,6 @@
 #include "../lib/ScopeGuard.h"
 #include "../lib/CSoundBase.h"
 #include "../lib/TerrainHandler.h"
-#include "CGameHandler.h"
-#include "ServerSpellCastEnvironment.h"
-#include "CVCMIServer.h"
 #include "../lib/CCreatureSet.h"
 #include "../lib/CThreadHelper.h"
 #include "../lib/GameConstants.h"
@@ -868,24 +870,12 @@ void CGameHandler::battleAfterLevelUp(const BattleResult &result)
 	std::set<PlayerColor> playerColors = {finishingBattle->loser, finishingBattle->victor};
 	checkVictoryLossConditions(playerColors);
 
-	if (result.result == BattleResult::SURRENDER || result.result == BattleResult::ESCAPE) //loser has escaped or surrendered
-	{
-		SetAvailableHeroes sah;
-		sah.player = finishingBattle->loser;
-		sah.hid[0] = finishingBattle->loserHero->subID;
-		if (result.result == BattleResult::ESCAPE) //retreat
-		{
-			sah.army[0].clear();
-			sah.army[0].setCreature(SlotID(0), finishingBattle->loserHero->type->initialArmy.at(0).creature, 1);
-		}
+	if (result.result == BattleResult::SURRENDER)
+		heroPool->onHeroSurrendered(finishingBattle->loser, finishingBattle->loserHero);
 
-		if (const CGHeroInstance *another = getPlayerState(finishingBattle->loser)->availableHeroes.at(0))
-			sah.hid[1] = another->subID;
-		else
-			sah.hid[1] = -1;
+	if (result.result == BattleResult::ESCAPE)
+		heroPool->onHeroEscaped(finishingBattle->loser, finishingBattle->loserHero);
 
-		sendAndApply(&sah);
-	}
 	if (result.winner != 2 && finishingBattle->winnerHero && finishingBattle->winnerHero->stacks.empty()
 		&& (!finishingBattle->winnerHero->commander || !finishingBattle->winnerHero->commander->alive))
 	{
@@ -893,20 +883,7 @@ void CGameHandler::battleAfterLevelUp(const BattleResult &result)
 		sendAndApply(&ro);
 
 		if (VLC->settings()->getBoolean(EGameSettings::HEROES_RETREAT_ON_WIN_WITHOUT_TROOPS))
-		{
-			SetAvailableHeroes sah;
-			sah.player = finishingBattle->victor;
-			sah.hid[0] = finishingBattle->winnerHero->subID;
-			sah.army[0].clear();
-			sah.army[0].setCreature(SlotID(0), finishingBattle->winnerHero->type->initialArmy.at(0).creature, 1);
-
-			if (const CGHeroInstance *another = getPlayerState(finishingBattle->victor)->availableHeroes.at(0))
-				sah.hid[1] = another->subID;
-			else
-				sah.hid[1] = -1;
-
-			sendAndApply(&sah);
-		}
+			heroPool->onHeroEscaped(finishingBattle->victor, finishingBattle->winnerHero);
 	}
 	
 	finishingBattle.reset();
@@ -1576,6 +1553,7 @@ int CGameHandler::moveStack(int stack, BattleHex dest)
 
 CGameHandler::CGameHandler(CVCMIServer * lobby)
 	: lobby(lobby)
+	, heroPool(std::make_unique<HeroPoolProcessor>(this))
 	, complainNoCreatures("No creatures to split")
 	, complainNotEnoughCreatures("Cannot split that stack, not enough creatures!")
 	, complainInvalidSlot("Invalid slot accessed!")
@@ -1765,27 +1743,6 @@ void CGameHandler::newTurn()
 		}
 	}
 
-	std::map<ui32, ConstTransitivePtr<CGHeroInstance> > pool = gs->hpool.heroesPool;
-
-	for (auto& hp : pool)
-	{
-		auto hero = hp.second;
-		if (hero->isInitialized() && hero->stacks.size())
-		{
-			// reset retreated or surrendered heroes
-			auto maxmove = hero->movementPointsLimit(true);
-			// if movement is greater than maxmove, we should decrease it
-			if (hero->movementPointsRemaining() != maxmove || hero->mana < hero->manaLimit())
-			{
-				NewTurn::Hero hth;
-				hth.id = hero->id;
-				hth.move = maxmove;
-				hth.mana = hero->getManaNewTurn();
-				n.heroes.insert(hth);
-			}
-		}
-	}
-
 	for (auto & elem : gs->players)
 	{
 		if (elem.first == PlayerColor::NEUTRAL)
@@ -1797,29 +1754,7 @@ void CGameHandler::newTurn()
 		hadGold.insert(playerGold);
 
 		if (newWeek) //new heroes in tavern
-		{
-			SetAvailableHeroes sah;
-			sah.player = elem.first;
-
-			//pick heroes and their armies
-			CHeroClass *banned = nullptr;
-			for (int j = 0; j < GameConstants::AVAILABLE_HEROES_PER_PLAYER; j++)
-			{
-				//first hero - native if possible, second hero -> any other class
-				if (CGHeroInstance *h = gs->hpool.pickHeroFor(j == 0, elem.first, getNativeTown(elem.first), pool, getRandomGenerator(), banned))
-				{
-					sah.hid[j] = h->subID;
-					h->initArmy(getRandomGenerator(), &sah.army[j]);
-					banned = h->type->heroClass;
-				}
-				else
-				{
-					sah.hid[j] = -1;
-				}
-			}
-
-			sendAndApply(&sah);
-		}
+			heroPool->onNewWeek(elem.first);
 
 		n.res[elem.first] = elem.second.resources;
 
@@ -4383,94 +4318,6 @@ bool CGameHandler::setFormation(ObjectInstanceID hid, ui8 formation)
 	return true;
 }
 
-bool CGameHandler::hireHero(const CGObjectInstance *obj, ui8 hid, PlayerColor player)
-{
-	const PlayerState * p = getPlayerState(player);
-	const CGTownInstance * t = getTown(obj->id);
-
-	//common preconditions
-//	if ((p->resources.at(EGameResID::GOLD)<GOLD_NEEDED  && complain("Not enough gold for buying hero!"))
-//		|| (getHeroCount(player, false) >= GameConstants::MAX_HEROES_PER_PLAYER && complain("Cannot hire hero, only 8 wandering heroes are allowed!")))
-	if ((p->resources[EGameResID::GOLD] < GameConstants::HERO_GOLD_COST && complain("Not enough gold for buying hero!"))
-		|| ((getHeroCount(player, false) >= VLC->settings()->getInteger(EGameSettings::HEROES_PER_PLAYER_ON_MAP_CAP) && complain("Cannot hire hero, too many wandering heroes already!")))
-		|| ((getHeroCount(player, true) >= VLC->settings()->getInteger(EGameSettings::HEROES_PER_PLAYER_TOTAL_CAP) && complain("Cannot hire hero, too many heroes garrizoned and wandering already!"))))
-	{
-		return false;
-	}
-
-	if (t) //tavern in town
-	{
-		if ((!t->hasBuilt(BuildingID::TAVERN) && complain("No tavern!"))
-			 || (t->visitingHero  && complain("There is visiting hero - no place!")))
-		{
-			return false;
-		}
-	}
-	else if (obj->ID == Obj::TAVERN)
-	{
-		if (getTile(obj->visitablePos())->visitableObjects.back() != obj && complain("Tavern entry must be unoccupied!"))
-		{
-			return false;
-		}
-	}
-
-	const CGHeroInstance *nh = p->availableHeroes.at(hid);
-	if (!nh)
-	{
-		complain ("Hero is not available for hiring!");
-		return false;
-	}
-
-	HeroRecruited hr;
-	hr.tid = obj->id;
-	hr.hid = nh->subID;
-	hr.player = player;
-	hr.tile = nh->convertFromVisitablePos(obj->visitablePos());
-	if (getTile(hr.tile)->isWater())
-	{
-		//Create a new boat for hero
-		createObject(obj->visitablePos(), Obj::BOAT, nh->getBoatType().getNum());
-
-		hr.boatId = getTopObj(hr.tile)->id;
-	}
-	sendAndApply(&hr);
-
-	std::map<ui32, ConstTransitivePtr<CGHeroInstance> > pool = gs->unusedHeroesFromPool();
-
-	const CGHeroInstance *theOtherHero = p->availableHeroes.at(!hid);
-	const CGHeroInstance *newHero = nullptr;
-	if (theOtherHero) //on XXL maps all heroes can be imprisoned :(
-	{
-		newHero = gs->hpool.pickHeroFor(false, player, getNativeTown(player), pool, getRandomGenerator(), theOtherHero->type->heroClass);
-	}
-
-	SetAvailableHeroes sah;
-	sah.player = player;
-
-	if (newHero)
-	{
-		sah.hid[hid] = newHero->subID;
-		sah.army[hid].clear();
-		sah.army[hid].setCreature(SlotID(0), newHero->type->initialArmy[0].creature, 1);
-	}
-	else
-	{
-		sah.hid[hid] = -1;
-	}
-
-	sah.hid[!hid] = theOtherHero ? theOtherHero->subID : -1;
-	sendAndApply(&sah);
-
-	giveResource(player, EGameResID::GOLD, -GameConstants::HERO_GOLD_COST);
-
-	if(t)
-	{
-		visitCastleObjects(t, nh);
-		giveSpells (t,nh);
-	}
-	return true;
-}
-
 bool CGameHandler::queryReply(QueryID qid, const JsonNode & answer, PlayerColor player)
 {
 	boost::unique_lock<boost::recursive_mutex> lock(gsm);
@@ -7395,3 +7242,10 @@ void CGameHandler::createObject(const int3 & visitablePosition, Obj type, int32_
 	no.targetPos = visitablePosition;
 	sendAndApply(&no);
 }
+
+void CGameHandler::deserializationFix()
+{
+	//FIXME: pointer to GameHandler itself can't be deserialized at the moment since GameHandler is top-level entity in serialization
+	// restore any places that requires such pointer manually
+	heroPool->gameHandler = this;
+}

server/CGameHandler.h

@@ -46,6 +46,7 @@ template<typename T> class CApplier;
 
 VCMI_LIB_NAMESPACE_END
 
+class HeroPoolProcessor;
 class CGameHandler;
 class CVCMIServer;
 class CBaseForGHApply;
@@ -97,7 +98,10 @@ class CGameHandler : public IGameCallback, public CBattleInfoCallback, public En
 	CVCMIServer * lobby;
 	std::shared_ptr<CApplier<CBaseForGHApply>> applier;
 	std::unique_ptr<boost::thread> battleThread;
+
 public:
+	std::unique_ptr<HeroPoolProcessor> heroPool;
+
 	using FireShieldInfo = std::vector<std::pair<const CStack *, int64_t>>;
 	//use enums as parameters, because doMove(sth, true, false, true) is not readable
 	enum EGuardLook {CHECK_FOR_GUARDS, IGNORE_GUARDS};
@@ -145,6 +149,7 @@ public:
 	void setupBattle(int3 tile, const CArmedInstance *armies[2], const CGHeroInstance *heroes[2], bool creatureBank, const CGTownInstance *town);
 	void setBattleResult(BattleResult::EResult resultType, int victoriusSide);
 
+	CGameHandler() = default;
 	CGameHandler(CVCMIServer * lobby);
 	~CGameHandler();
 
@@ -240,7 +245,6 @@ public:
 
 	void removeObstacle(const CObstacleInstance &obstacle);
 	bool queryReply( QueryID qid, const JsonNode & answer, PlayerColor player );
-	bool hireHero( const CGObjectInstance *obj, ui8 hid, PlayerColor player );
 	bool buildBoat( ObjectInstanceID objid, PlayerColor player );
 	bool setFormation( ObjectInstanceID hid, ui8 formation );
 	bool tradeResources(const IMarket *market, ui32 val, PlayerColor player, ui32 id1, ui32 id2);
@@ -283,8 +287,12 @@ public:
 		h & QID;
 		h & states;
 		h & finishingBattle;
+		h & heroPool;
 		h & getRandomGenerator();
 
+		if (!h.saving)
+			deserializationFix();
+
 #if SCRIPTING_ENABLED
 		JsonNode scriptsState;
 		if(h.saving)
@@ -355,6 +363,8 @@ public:
 	scripting::Pool * getContextPool() const override;
 #endif
 
+	std::list<PlayerColor> generatePlayerTurnOrder() const;
+
 	friend class CVCMIServer;
 private:
 	std::unique_ptr<events::EventBus> serverEventBus;
@@ -363,8 +373,9 @@ private:
 #endif
 
 	void reinitScripting();
+	void deserializationFix();
+
 
-	std::list<PlayerColor> generatePlayerTurnOrder() const;
 	void makeStackDoNothing(const CStack * next);
 	void getVictoryLossMessage(PlayerColor player, const EVictoryLossCheckResult & victoryLossCheckResult, InfoWindow & out) const;
 

server/CMakeLists.txt

@@ -2,6 +2,7 @@ set(server_SRCS
 		StdInc.cpp
 
 		CGameHandler.cpp
+		HeroPoolProcessor.cpp
 		ServerSpellCastEnvironment.cpp
 		CQuery.cpp
 		CVCMIServer.cpp
@@ -13,6 +14,7 @@ set(server_HEADERS
 		StdInc.h
 
 		CGameHandler.h
+		HeroPoolProcessor.h
 		ServerSpellCastEnvironment.h
 		CQuery.h
 		CVCMIServer.h

server/CVCMIServer.cpp

@@ -822,7 +822,7 @@ void CVCMIServer::setPlayer(PlayerColor clickedColor)
 void CVCMIServer::optionNextCastle(PlayerColor player, int dir)
 {
 	PlayerSettings & s = si->playerInfos[player];
-	si16 & cur = s.castle;
+	FactionID & cur = s.castle;
 	auto & allowed = getPlayerInfo(player.getNum()).allowedFactions;
 	const bool allowRandomTown = getPlayerInfo(player.getNum()).isFactionRandom;
 
@@ -856,7 +856,7 @@ void CVCMIServer::optionNextCastle(PlayerColor player, int dir)
 		else
 		{
 			assert(dir >= -1 && dir <= 1); //othervice std::advance may go out of range
-			auto iter = allowed.find(FactionID(cur));
+			auto iter = allowed.find(cur);
 			std::advance(iter, dir);
 			cur = *iter;
 		}

server/HeroPoolProcessor.cpp

@@ -0,0 +1,397 @@
+/*
+ * HeroPoolProcessor.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#include "StdInc.h"
+#include "HeroPoolProcessor.h"
+
+#include "CGameHandler.h"
+
+#include "../lib/CHeroHandler.h"
+#include "../lib/CPlayerState.h"
+#include "../lib/GameSettings.h"
+#include "../lib/NetPacks.h"
+#include "../lib/StartInfo.h"
+#include "../lib/mapObjects/CGTownInstance.h"
+#include "../lib/gameState/CGameState.h"
+#include "../lib/gameState/TavernHeroesPool.h"
+#include "../lib/gameState/TavernSlot.h"
+
+HeroPoolProcessor::HeroPoolProcessor()
+	: gameHandler(nullptr)
+{
+}
+
+HeroPoolProcessor::HeroPoolProcessor(CGameHandler * gameHandler)
+	: gameHandler(gameHandler)
+{
+}
+
+bool HeroPoolProcessor::playerEndedTurn(const PlayerColor & player)
+{
+	// our player is acting right now and have not ended turn
+	if (player == gameHandler->gameState()->currentPlayer)
+		return false;
+
+	auto turnOrder = gameHandler->generatePlayerTurnOrder();
+
+	for (auto const & entry : turnOrder)
+	{
+		// our player is yet to start turn
+		if (entry == gameHandler->gameState()->currentPlayer)
+			return false;
+
+		// our player have finished turn
+		if (entry == player)
+			return true;
+	}
+
+	assert(false);
+	return false;
+}
+
+TavernHeroSlot HeroPoolProcessor::selectSlotForRole(const PlayerColor & player, TavernSlotRole roleID)
+{
+	const auto & heroesPool = gameHandler->gameState()->heroesPool;
+
+	const auto & heroes = heroesPool->getHeroesFor(player);
+
+	// if tavern has empty slot - use it
+	if (heroes.size() == 0)
+		return TavernHeroSlot::NATIVE;
+
+	if (heroes.size() == 1)
+		return TavernHeroSlot::RANDOM;
+
+	// try to find "better" slot to overwrite
+	// we want to avoid overwriting retreated heroes when tavern still has slot with random hero
+	// as well as avoid overwriting surrendered heroes if we can overwrite retreated hero
+	auto roleLeft = heroesPool->getSlotRole(HeroTypeID(heroes[0]->subID));
+	auto roleRight = heroesPool->getSlotRole(HeroTypeID(heroes[1]->subID));
+
+	if (roleLeft > roleRight)
+		return TavernHeroSlot::RANDOM;
+
+	if (roleLeft < roleRight)
+		return TavernHeroSlot::NATIVE;
+
+	// both slots are equal in "value", so select randomly
+	if (getRandomGenerator(player).nextInt(100) > 50)
+		return TavernHeroSlot::RANDOM;
+	else
+		return TavernHeroSlot::NATIVE;
+}
+
+void HeroPoolProcessor::onHeroSurrendered(const PlayerColor & color, const CGHeroInstance * hero)
+{
+	SetAvailableHero sah;
+	if (playerEndedTurn(color))
+		sah.roleID = TavernSlotRole::SURRENDERED_TODAY;
+	else
+		sah.roleID = TavernSlotRole::SURRENDERED;
+
+	sah.slotID = selectSlotForRole(color, sah.roleID);
+	sah.player = color;
+	sah.hid = hero->subID;
+	gameHandler->sendAndApply(&sah);
+}
+
+void HeroPoolProcessor::onHeroEscaped(const PlayerColor & color, const CGHeroInstance * hero)
+{
+	SetAvailableHero sah;
+	if (playerEndedTurn(color))
+		sah.roleID = TavernSlotRole::RETREATED_TODAY;
+	else
+		sah.roleID = TavernSlotRole::RETREATED;
+
+	sah.slotID = selectSlotForRole(color, sah.roleID);
+	sah.player = color;
+	sah.hid = hero->subID;
+	sah.army.clear();
+	sah.army.setCreature(SlotID(0), hero->type->initialArmy.at(0).creature, 1);
+
+	gameHandler->sendAndApply(&sah);
+}
+
+void HeroPoolProcessor::clearHeroFromSlot(const PlayerColor & color, TavernHeroSlot slot)
+{
+	SetAvailableHero sah;
+	sah.player = color;
+	sah.roleID = TavernSlotRole::NONE;
+	sah.slotID = slot;
+	sah.hid = HeroTypeID::NONE;
+	gameHandler->sendAndApply(&sah);
+}
+
+void HeroPoolProcessor::selectNewHeroForSlot(const PlayerColor & color, TavernHeroSlot slot, bool needNativeHero, bool giveArmy)
+{
+	SetAvailableHero sah;
+	sah.player = color;
+	sah.slotID = slot;
+
+	CGHeroInstance *newHero = pickHeroFor(needNativeHero, color);
+
+	if (newHero)
+	{
+		sah.hid = newHero->subID;
+
+		if (giveArmy)
+		{
+			sah.roleID = TavernSlotRole::FULL_ARMY;
+			newHero->initArmy(getRandomGenerator(color), &sah.army);
+		}
+		else
+		{
+			sah.roleID = TavernSlotRole::SINGLE_UNIT;
+			sah.army.clear();
+			sah.army.setCreature(SlotID(0), newHero->type->initialArmy[0].creature, 1);
+		}
+	}
+	else
+	{
+		sah.hid = -1;
+	}
+	gameHandler->sendAndApply(&sah);
+}
+
+void HeroPoolProcessor::onNewWeek(const PlayerColor & color)
+{
+	const auto & heroesPool = gameHandler->gameState()->heroesPool;
+	const auto & heroes = heroesPool->getHeroesFor(color);
+
+	const auto nativeSlotRole = heroes.size() < 1 ? TavernSlotRole::NONE : heroesPool->getSlotRole(heroes[0]->type->getId());
+	const auto randomSlotRole = heroes.size() < 2 ? TavernSlotRole::NONE : heroesPool->getSlotRole(heroes[1]->type->getId());
+
+	bool resetNativeSlot = nativeSlotRole != TavernSlotRole::RETREATED_TODAY && nativeSlotRole != TavernSlotRole::SURRENDERED_TODAY;
+	bool resetRandomSlot = randomSlotRole != TavernSlotRole::RETREATED_TODAY && randomSlotRole != TavernSlotRole::SURRENDERED_TODAY;
+
+	if (resetNativeSlot)
+		clearHeroFromSlot(color, TavernHeroSlot::NATIVE);
+
+	if (resetRandomSlot)
+		clearHeroFromSlot(color, TavernHeroSlot::RANDOM);
+
+	if (resetNativeSlot)
+		selectNewHeroForSlot(color, TavernHeroSlot::NATIVE, true, true);
+
+	if (resetRandomSlot)
+		selectNewHeroForSlot(color, TavernHeroSlot::RANDOM, false, true);
+}
+
+bool HeroPoolProcessor::hireHero(const ObjectInstanceID & objectID, const HeroTypeID & heroToRecruit, const PlayerColor & player)
+{
+	const PlayerState * playerState = gameHandler->getPlayerState(player);
+	const CGObjectInstance * mapObject = gameHandler->getObj(objectID);
+	const CGTownInstance * town = gameHandler->getTown(objectID);
+
+	if (!mapObject && gameHandler->complain("Invalid map object!"))
+		return false;
+
+	if (!playerState && gameHandler->complain("Invalid player!"))
+		return false;
+
+	if (playerState->resources[EGameResID::GOLD] < GameConstants::HERO_GOLD_COST && gameHandler->complain("Not enough gold for buying hero!"))
+		return false;
+
+	if (gameHandler->getHeroCount(player, false) >= VLC->settings()->getInteger(EGameSettings::HEROES_PER_PLAYER_ON_MAP_CAP) && gameHandler->complain("Cannot hire hero, too many wandering heroes already!"))
+		return false;
+
+	if (gameHandler->getHeroCount(player, true) >= VLC->settings()->getInteger(EGameSettings::HEROES_PER_PLAYER_TOTAL_CAP) && gameHandler->complain("Cannot hire hero, too many heroes garrizoned and wandering already!"))
+		return false;
+
+	if(town) //tavern in town
+	{
+		if(gameHandler->getPlayerRelations(mapObject->tempOwner, player) == PlayerRelations::ENEMIES && gameHandler->complain("Can't buy hero in enemy town!"))
+			return false;
+
+		if(!town->hasBuilt(BuildingID::TAVERN) && gameHandler->complain("No tavern!"))
+			return false;
+
+		if(town->visitingHero && gameHandler->complain("There is visiting hero - no place!"))
+			return false;
+	}
+
+	if(mapObject->ID == Obj::TAVERN)
+	{
+		if(gameHandler->getTile(mapObject->visitablePos())->visitableObjects.back() != mapObject && gameHandler->complain("Tavern entry must be unoccupied!"))
+			return false;
+	}
+
+	auto recruitableHeroes = gameHandler->gameState()->heroesPool->getHeroesFor(player);
+
+	const CGHeroInstance * recruitedHero = nullptr;
+
+	for(const auto & hero : recruitableHeroes)
+	{
+		if(hero->subID == heroToRecruit)
+			recruitedHero = hero;
+	}
+
+	if(!recruitedHero)
+	{
+		gameHandler->complain("Hero is not available for hiring!");
+		return false;
+	}
+
+	HeroRecruited hr;
+	hr.tid = mapObject->id;
+	hr.hid = recruitedHero->subID;
+	hr.player = player;
+	hr.tile = recruitedHero->convertFromVisitablePos(mapObject->visitablePos());
+	if(gameHandler->getTile(hr.tile)->isWater())
+	{
+		//Create a new boat for hero
+		gameHandler->createObject(mapObject->visitablePos(), Obj::BOAT, recruitedHero->getBoatType().getNum());
+
+		hr.boatId = gameHandler->getTopObj(hr.tile)->id;
+	}
+
+	// apply netpack -> this will remove hired hero from pool
+	gameHandler->sendAndApply(&hr);
+
+	if(recruitableHeroes[0] == recruitedHero)
+		selectNewHeroForSlot(player, TavernHeroSlot::NATIVE, false, false);
+	else
+		selectNewHeroForSlot(player, TavernHeroSlot::RANDOM, false, false);
+
+	gameHandler->giveResource(player, EGameResID::GOLD, -GameConstants::HERO_GOLD_COST);
+
+	if(town)
+	{
+		gameHandler->visitCastleObjects(town, recruitedHero);
+		gameHandler->giveSpells(town, recruitedHero);
+	}
+	return true;
+}
+
+std::vector<const CHeroClass *> HeroPoolProcessor::findAvailableClassesFor(const PlayerColor & player) const
+{
+	std::vector<const CHeroClass *> result;
+
+	const auto & heroesPool = gameHandler->gameState()->heroesPool;
+	FactionID factionID = gameHandler->getPlayerSettings(player)->castle;
+
+	for(auto & elem : heroesPool->unusedHeroesFromPool())
+	{
+		if (vstd::contains(result, elem.second->type->heroClass))
+			continue;
+
+		bool heroAvailable = heroesPool->isHeroAvailableFor(elem.first, player);
+		bool heroClassBanned = elem.second->type->heroClass->selectionProbability[factionID] == 0;
+
+		if(heroAvailable && !heroClassBanned)
+			result.push_back(elem.second->type->heroClass);
+	}
+
+	return result;
+}
+
+std::vector<CGHeroInstance *> HeroPoolProcessor::findAvailableHeroesFor(const PlayerColor & player, const CHeroClass * heroClass) const
+{
+	std::vector<CGHeroInstance *> result;
+
+	const auto & heroesPool = gameHandler->gameState()->heroesPool;
+
+	for(auto & elem : heroesPool->unusedHeroesFromPool())
+	{
+		assert(!vstd::contains(result, elem.second));
+
+		bool heroAvailable = heroesPool->isHeroAvailableFor(elem.first, player);
+		bool heroClassMatches = elem.second->type->heroClass == heroClass;
+
+		if(heroAvailable && heroClassMatches)
+			result.push_back(elem.second);
+	}
+
+	return result;
+}
+
+const CHeroClass * HeroPoolProcessor::pickClassFor(bool isNative, const PlayerColor & player)
+{
+	if(player >= PlayerColor::PLAYER_LIMIT)
+	{
+		logGlobal->error("Cannot pick hero for player %d. Wrong owner!", player.getStr());
+		return nullptr;
+	}
+
+	FactionID factionID = gameHandler->getPlayerSettings(player)->castle;
+	const auto & heroesPool = gameHandler->gameState()->heroesPool;
+	const auto & currentTavern = heroesPool->getHeroesFor(player);
+
+	std::vector<const CHeroClass *> potentialClasses = findAvailableClassesFor(player);
+	std::vector<const CHeroClass *> possibleClasses;
+
+	if(potentialClasses.empty())
+	{
+		logGlobal->error("There are no heroes available for player %s!", player.getStr());
+		return nullptr;
+	}
+
+	for(const auto & heroClass : potentialClasses)
+	{
+		if (isNative && heroClass->faction != factionID)
+			continue;
+
+		bool hasSameClass = vstd::contains_if(currentTavern, [&](const CGHeroInstance * hero){
+			return hero->type->heroClass == heroClass;
+		});
+
+		if (hasSameClass)
+			continue;
+
+		possibleClasses.push_back(heroClass);
+	}
+
+	if (possibleClasses.empty())
+	{
+		logGlobal->error("Cannot pick native hero for %s. Picking any...", player.getStr());
+		possibleClasses = potentialClasses;
+	}
+
+	int totalWeight = 0;
+	for(const auto & heroClass : possibleClasses)
+		totalWeight += heroClass->selectionProbability.at(factionID);
+
+	int roll = getRandomGenerator(player).nextInt(totalWeight - 1);
+
+	for(const auto & heroClass : possibleClasses)
+	{
+		roll -= heroClass->selectionProbability.at(factionID);
+		if(roll < 0)
+			return heroClass;
+	}
+
+	return *possibleClasses.rbegin();
+}
+
+CGHeroInstance * HeroPoolProcessor::pickHeroFor(bool isNative, const PlayerColor & player)
+{
+	const CHeroClass * heroClass = pickClassFor(isNative, player);
+
+	if(!heroClass)
+		return nullptr;
+
+	std::vector<CGHeroInstance *> possibleHeroes = findAvailableHeroesFor(player, heroClass);
+
+	assert(!possibleHeroes.empty());
+	if(possibleHeroes.empty())
+		return nullptr;
+
+	return *RandomGeneratorUtil::nextItem(possibleHeroes, getRandomGenerator(player));
+}
+
+CRandomGenerator & HeroPoolProcessor::getRandomGenerator(const PlayerColor & player)
+{
+	if (playerSeed.count(player) == 0)
+	{
+		int seed = gameHandler->getRandomGenerator().nextInt();
+		playerSeed.emplace(player, std::make_unique<CRandomGenerator>(seed));
+	}
+
+	return *playerSeed.at(player);
+}

server/HeroPoolProcessor.h

@@ -0,0 +1,66 @@
+/*
+ * HeroPoolProcessor.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+enum class TavernHeroSlot : int8_t;
+enum class TavernSlotRole : int8_t;
+class PlayerColor;
+class CGHeroInstance;
+class HeroTypeID;
+class ObjectInstanceID;
+class CRandomGenerator;
+class CHeroClass;
+
+VCMI_LIB_NAMESPACE_END
+
+class CGameHandler;
+
+class HeroPoolProcessor : boost::noncopyable
+{
+	/// per-player random generators
+	std::map<PlayerColor, std::unique_ptr<CRandomGenerator>> playerSeed;
+
+	void clearHeroFromSlot(const PlayerColor & color, TavernHeroSlot slot);
+	void selectNewHeroForSlot(const PlayerColor & color, TavernHeroSlot slot, bool needNativeHero, bool giveStartingArmy);
+
+	std::vector<const CHeroClass *> findAvailableClassesFor(const PlayerColor & player) const;
+	std::vector<CGHeroInstance *> findAvailableHeroesFor(const PlayerColor & player, const CHeroClass * heroClass) const;
+
+	const CHeroClass * pickClassFor(bool isNative, const PlayerColor & player);
+
+	CGHeroInstance * pickHeroFor(bool isNative, const PlayerColor & player);
+
+	CRandomGenerator & getRandomGenerator(const PlayerColor & player);
+
+	TavernHeroSlot selectSlotForRole(const PlayerColor & player, TavernSlotRole roleID);
+
+	bool playerEndedTurn(const PlayerColor & player);
+public:
+	CGameHandler * gameHandler;
+
+	HeroPoolProcessor();
+	HeroPoolProcessor(CGameHandler * gameHandler);
+
+	void onHeroSurrendered(const PlayerColor & color, const CGHeroInstance * hero);
+	void onHeroEscaped(const PlayerColor & color, const CGHeroInstance * hero);
+
+	void onNewWeek(const PlayerColor & color);
+
+	/// Incoming net pack handling
+	bool hireHero(const ObjectInstanceID & objectID, const HeroTypeID & hid, const PlayerColor & player);
+
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		// h & gameHandler; // FIXME: make this work instead of using deserializationFix in gameHandler
+		h & playerSeed;
+	}
+};

server/NetPacksServer.cpp

@@ -11,6 +11,8 @@
 #include "ServerNetPackVisitors.h"
 
 #include "CGameHandler.h"
+#include "HeroPoolProcessor.h"
+
 #include "../lib/IGameCallback.h"
 #include "../lib/mapObjects/CGTownInstance.h"
 #include "../lib/gameState/CGameState.h"
@@ -246,12 +248,10 @@ void ApplyGhNetPackVisitor::visitSetFormation(SetFormation & pack)
 
 void ApplyGhNetPackVisitor::visitHireHero(HireHero & pack)
 {
-	const CGObjectInstance * obj = gh.getObj(pack.tid);
-	const CGTownInstance * town = dynamic_ptr_cast<CGTownInstance>(obj);
-	if(town && PlayerRelations::ENEMIES == gh.getPlayerRelations(obj->tempOwner, gh.getPlayerAt(pack.c)))
-		gh.throwAndComplain(&pack, "Can't buy hero in enemy town!");
+	if (!gh.hasPlayerAt(pack.player, pack.c))
+		gh.throwAndComplain(&pack, "No such pack.player!");
 
-	result = gh.hireHero(obj, pack.hid, pack.player);
+	result = gh.heroPool->hireHero(pack.tid, pack.hid, pack.player);
 }
 
 void ApplyGhNetPackVisitor::visitBuildBoat(BuildBoat & pack)