Fix regressions

lib/NetPacksLib.cpp

@@ -2012,6 +2012,9 @@ void NewTurn::applyGs(CGameState *gs)
 			logGlobal->error("Hero %d not found in NewTurn::applyGs", h.id.getNum());
 			continue;
 		}
+
+		hero->setMovementPoints(h.move);
+		hero->mana = h.mana;
 	}
 
 	gs->hpool->onNewDay();

lib/gameState/CGameState.cpp

@@ -385,7 +385,7 @@ int CGameState::getDate(Date::EDateType mode) const
 CGameState::CGameState()
 {
 	gs = this;
-	hpool = std::make_unique<TavernHeroesPool>(this);
+	hpool = std::make_unique<TavernHeroesPool>();
 	applier = std::make_shared<CApplier<CBaseForGSApply>>();
 	registerTypesClientPacks1(*applier);
 	registerTypesClientPacks2(*applier);

lib/gameState/TavernHeroesPool.cpp

@@ -10,18 +10,9 @@
 #include "StdInc.h"
 #include "TavernHeroesPool.h"
 
-#include "CGameState.h"
-#include "CPlayerState.h"
-
 #include "../mapObjects/CGHeroInstance.h"
-#include "../CHeroHandler.h"
-
-TavernHeroesPool::TavernHeroesPool() = default;
 
-TavernHeroesPool::TavernHeroesPool(CGameState * gameState)
-	: gameState(gameState)
-{
-}
+VCMI_LIB_NAMESPACE_BEGIN
 
 TavernHeroesPool::~TavernHeroesPool()
 {
@@ -29,7 +20,7 @@ TavernHeroesPool::~TavernHeroesPool()
 		delete ptr.second;
 }
 
-std::map<HeroTypeID, CGHeroInstance*> TavernHeroesPool::unusedHeroesFromPool()
+std::map<HeroTypeID, CGHeroInstance*> TavernHeroesPool::unusedHeroesFromPool() const
 {
 	std::map<HeroTypeID, CGHeroInstance*> pool = heroesPool;
 	for(const auto & player : currentTavern)
@@ -63,71 +54,6 @@ bool TavernHeroesPool::isHeroAvailableFor(HeroTypeID hero, PlayerColor color) co
 	return true;
 }
 
-CGHeroInstance * TavernHeroesPool::pickHeroFor(TavernHeroSlot slot,
-													 const PlayerColor & player,
-													 const FactionID & factionID,
-													 CRandomGenerator & rand,
-													 const CHeroClass * bannedClass) const
-{
-	if(player>=PlayerColor::PLAYER_LIMIT)
-	{
-		logGlobal->error("Cannot pick hero for player %d. Wrong owner!", player.getStr());
-		return nullptr;
-	}
-
-	if(slot == TavernHeroSlot::NATIVE)
-	{
-		std::vector<CGHeroInstance *> pool;
-
-		for(auto & elem : heroesPool)
-		{
-			//get all available heroes
-			bool heroAvailable = isHeroAvailableFor(elem.first, player);
-			bool heroClassNative = elem.second->type->heroClass->faction == factionID;
-
-			if(heroAvailable && heroClassNative)
-				pool.push_back(elem.second);
-		}
-
-		if(!pool.empty())
-			return *RandomGeneratorUtil::nextItem(pool, rand);
-
-		logGlobal->error("Cannot pick native hero for %s. Picking any...", player.getStr());
-	}
-
-	std::vector<CGHeroInstance *> pool;
-	int totalWeight = 0;
-
-	for(auto & elem : heroesPool)
-	{
-		bool heroAvailable = isHeroAvailableFor(elem.first, player);
-		bool heroClassBanned = bannedClass && elem.second->type->heroClass == bannedClass;
-
-		if ( heroAvailable && !heroClassBanned)
-		{
-			pool.push_back(elem.second);
-			totalWeight += elem.second->type->heroClass->selectionProbability[factionID]; //total weight
-		}
-	}
-	if(pool.empty() || totalWeight == 0)
-	{
-		logGlobal->error("There are no heroes available for player %s!", player.getStr());
-		return nullptr;
-	}
-
-	int roll = rand.nextInt(totalWeight - 1);
-	for (auto & elem : pool)
-	{
-		roll -= elem->type->heroClass->selectionProbability[factionID];
-		if(roll < 0)
-		{
-			return elem;
-		}
-	}
-
-	return pool.back();
-}
-
 std::vector<const CGHeroInstance *> TavernHeroesPool::getHeroesFor(PlayerColor color) const
 {
 	std::vector<const CGHeroInstance *> result;
@@ -174,3 +100,5 @@ void TavernHeroesPool::setAvailability(HeroTypeID hero, PlayerColor::Mask mask)
 {
 	pavailable[hero] = mask;
 }
+
+VCMI_LIB_NAMESPACE_END

lib/gameState/TavernHeroesPool.h

@@ -23,38 +23,34 @@ class CSimpleArmy;
 
 enum class TavernHeroSlot
 {
-	NATIVE,
-	RANDOM
+	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
 };
 
 class DLL_LINKAGE TavernHeroesPool
 {
-	CGameState * gameState;
-
-	//[subID] - heroes available to buy; nullptr if not available
+	/// list of all heroes in pool, including those currently present in taverns
 	std::map<HeroTypeID, CGHeroInstance* > heroesPool;
 
-	// [subid] -> which players can recruit hero (binary flags)
+	/// 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> pavailable;
 
-	std::map<HeroTypeID, CGHeroInstance* > unusedHeroesFromPool(); //heroes pool without heroes that are available in taverns
-
+	/// list of heroes currently available in a tavern of a specific player
 	std::map<PlayerColor, std::map<TavernHeroSlot, CGHeroInstance*> > currentTavern;
 
-	bool isHeroAvailableFor(HeroTypeID hero, PlayerColor color) const;
 public:
-	TavernHeroesPool();
-	TavernHeroesPool(CGameState * gameState);
 	~TavernHeroesPool();
 
-	CGHeroInstance * pickHeroFor(TavernHeroSlot slot,
-								 const PlayerColor & player,
-								 const FactionID & faction,
-								 CRandomGenerator & rand,
-								 const CHeroClass * bannedClass = nullptr) const;
-
+	/// 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;
+
 	CGHeroInstance * takeHero(HeroTypeID hero);
 
 	/// reset mana and movement points for all heroes in pool
@@ -66,7 +62,6 @@ public:
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
-		h & gameState;
 		h & heroesPool;
 		h & pavailable;
 	}

server/HeroPoolProcessor.cpp

@@ -58,19 +58,28 @@ void HeroPoolProcessor::clearHeroFromSlot(const PlayerColor & color, TavernHeroS
 	gameHandler->sendAndApply(&sah);
 }
 
-void HeroPoolProcessor::selectNewHeroForSlot(const PlayerColor & color, TavernHeroSlot slot)
+void HeroPoolProcessor::selectNewHeroForSlot(const PlayerColor & color, TavernHeroSlot slot, bool needNativeHero, bool giveArmy)
 {
 	SetAvailableHero sah;
 	sah.player = color;
 	sah.slotID = static_cast<int>(slot);
 
 	//first hero - native if possible, second hero -> any other class
-	CGHeroInstance *h = gameHandler->gameState()->hpool->pickHeroFor(slot, color, gameHandler->getPlayerSettings(color)->castle, gameHandler->getRandomGenerator());
+	CGHeroInstance *h = pickHeroFor(needNativeHero, color, gameHandler->getPlayerSettings(color)->castle, gameHandler->getRandomGenerator(), nullptr);
 
 	if (h)
 	{
 		sah.hid = h->subID;
-		h->initArmy(gameHandler->getRandomGenerator(), &sah.army);
+
+		if (giveArmy)
+		{
+			h->initArmy(gameHandler->getRandomGenerator(), &sah.army);
+		}
+		else
+		{
+			sah.army.clear();
+			sah.army.setCreature(SlotID(0), h->type->initialArmy[0].creature, 1);
+		}
 	}
 	else
 	{
@@ -83,8 +92,8 @@ void HeroPoolProcessor::onNewWeek(const PlayerColor & color)
 {
 	clearHeroFromSlot(color, TavernHeroSlot::NATIVE);
 	clearHeroFromSlot(color, TavernHeroSlot::RANDOM);
-	selectNewHeroForSlot(color, TavernHeroSlot::NATIVE);
-	selectNewHeroForSlot(color, TavernHeroSlot::RANDOM);
+	selectNewHeroForSlot(color, TavernHeroSlot::NATIVE, true, true);
+	selectNewHeroForSlot(color, TavernHeroSlot::RANDOM, false, true);
 }
 
 bool HeroPoolProcessor::hireHero(const CGObjectInstance *obj, const HeroTypeID & heroToRecruit, const PlayerColor & player)
@@ -101,34 +110,34 @@ bool HeroPoolProcessor::hireHero(const CGObjectInstance *obj, const HeroTypeID &
 	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(town) //tavern in town
 	{
-		if (!town->hasBuilt(BuildingID::TAVERN) && gameHandler->complain("No tavern!"))
+		if(!town->hasBuilt(BuildingID::TAVERN) && gameHandler->complain("No tavern!"))
 			return false;
 
-		if (town->visitingHero  && gameHandler->complain("There is visiting hero - no place!"))
+		if(town->visitingHero && gameHandler->complain("There is visiting hero - no place!"))
 			return false;
 	}
 
-	if (obj->ID == Obj::TAVERN)
+	if(obj->ID == Obj::TAVERN)
 	{
-		if (gameHandler->getTile(obj->visitablePos())->visitableObjects.back() != obj && gameHandler->complain("Tavern entry must be unoccupied!"))
+		if(gameHandler->getTile(obj->visitablePos())->visitableObjects.back() != obj && gameHandler->complain("Tavern entry must be unoccupied!"))
 			return false;
 	}
 
 	auto recruitableHeroes = gameHandler->gameState()->hpool->getHeroesFor(player);
 
-	const CGHeroInstance *recruitedHero = nullptr;;
+	const CGHeroInstance * recruitedHero = nullptr;
 
 	for(const auto & hero : recruitableHeroes)
 	{
-		if (hero->subID == heroToRecruit)
+		if(hero->subID == heroToRecruit)
 			recruitedHero = hero;
 	}
 
-	if (!recruitedHero)
+	if(!recruitedHero)
 	{
-		gameHandler->complain ("Hero is not available for hiring!");
+		gameHandler->complain("Hero is not available for hiring!");
 		return false;
 	}
 
@@ -137,19 +146,21 @@ bool HeroPoolProcessor::hireHero(const CGObjectInstance *obj, const HeroTypeID &
 	hr.hid = recruitedHero->subID;
 	hr.player = player;
 	hr.tile = recruitedHero->convertFromVisitablePos(obj->visitablePos());
-	if (gameHandler->getTile(hr.tile)->isWater())
+	if(gameHandler->getTile(hr.tile)->isWater())
 	{
 		//Create a new boat for hero
 		gameHandler->createObject(obj->visitablePos(), Obj::BOAT, recruitedHero->getBoatType().getNum());
 
 		hr.boatId = gameHandler->getTopObj(hr.tile)->id;
 	}
+
+	// apply netpack -> this will remove hired hero from tavern slot
 	gameHandler->sendAndApply(&hr);
 
-	if (recruitableHeroes[0] == recruitedHero)
-		selectNewHeroForSlot(player, TavernHeroSlot::NATIVE);
+	if(recruitableHeroes[0] == recruitedHero)
+		selectNewHeroForSlot(player, TavernHeroSlot::NATIVE, false, false);
 	else
-		selectNewHeroForSlot(player, TavernHeroSlot::RANDOM);
+		selectNewHeroForSlot(player, TavernHeroSlot::RANDOM, false, false);
 
 	gameHandler->giveResource(player, EGameResID::GOLD, -GameConstants::HERO_GOLD_COST);
 
@@ -160,3 +171,70 @@ bool HeroPoolProcessor::hireHero(const CGObjectInstance *obj, const HeroTypeID &
 	}
 	return true;
 }
+
+CGHeroInstance * HeroPoolProcessor::pickHeroFor(bool isNative,
+													 const PlayerColor & player,
+													 const FactionID & factionID,
+													 CRandomGenerator & rand,
+													 const CHeroClass * bannedClass) const
+{
+	if(player >= PlayerColor::PLAYER_LIMIT)
+	{
+		logGlobal->error("Cannot pick hero for player %d. Wrong owner!", player.getStr());
+		return nullptr;
+	}
+
+	const auto & hpool = gameHandler->gameState()->hpool;
+
+	if(isNative)
+	{
+		std::vector<CGHeroInstance *> pool;
+
+		for(auto & elem : hpool->unusedHeroesFromPool())
+		{
+			//get all available heroes
+			bool heroAvailable = hpool->isHeroAvailableFor(elem.first, player);
+			bool heroClassNative = elem.second->type->heroClass->faction == factionID;
+
+			if(heroAvailable && heroClassNative)
+				pool.push_back(elem.second);
+		}
+
+		if(!pool.empty())
+			return *RandomGeneratorUtil::nextItem(pool, rand);
+
+		logGlobal->error("Cannot pick native hero for %s. Picking any...", player.getStr());
+	}
+
+	std::vector<CGHeroInstance *> pool;
+	int totalWeight = 0;
+
+	for(auto & elem : hpool->unusedHeroesFromPool())
+	{
+		bool heroAvailable = hpool->isHeroAvailableFor(elem.first, player);
+		bool heroClassBanned = bannedClass && elem.second->type->heroClass == bannedClass;
+
+		if(heroAvailable && !heroClassBanned)
+		{
+			pool.push_back(elem.second);
+			totalWeight += elem.second->type->heroClass->selectionProbability[factionID]; //total weight
+		}
+	}
+	if(pool.empty() || totalWeight == 0)
+	{
+		logGlobal->error("There are no heroes available for player %s!", player.getStr());
+		return nullptr;
+	}
+
+	int roll = rand.nextInt(totalWeight - 1);
+	for(auto & elem : pool)
+	{
+		roll -= elem->type->heroClass->selectionProbability[factionID];
+		if(roll < 0)
+		{
+			return elem;
+		}
+	}
+
+	return pool.back();
+}

server/HeroPoolProcessor.h

@@ -16,6 +16,9 @@ class PlayerColor;
 class CGHeroInstance;
 class HeroTypeID;
 class CGObjectInstance;
+class FactionID;
+class CRandomGenerator;
+class CHeroClass;
 
 VCMI_LIB_NAMESPACE_END
 
@@ -26,7 +29,9 @@ class HeroPoolProcessor : boost::noncopyable
 	CGameHandler * gameHandler;
 
 	void clearHeroFromSlot(const PlayerColor & color, TavernHeroSlot slot);
-	void selectNewHeroForSlot(const PlayerColor & color, TavernHeroSlot slot);
+	void selectNewHeroForSlot(const PlayerColor & color, TavernHeroSlot slot, bool needNativeHero, bool giveStartingArmy);
+
+	CGHeroInstance * pickHeroFor(bool isNative, const PlayerColor & player, const FactionID & faction, CRandomGenerator & rand, const CHeroClass * bannedClass) const;
 public:
 	HeroPoolProcessor();
 	HeroPoolProcessor(CGameHandler * gameHandler);