Moved mana and movement points refresh to NewTurnProcessor

lib/networkPacks/NetPacksLib.cpp

@@ -1901,18 +1901,11 @@ void NewTurn::applyGs(CGameState *gs)
 	gs->globalEffects.reduceBonusDurations(Bonus::OneWeek);
 	//TODO not really a single root hierarchy, what about bonuses placed elsewhere? [not an issue with H3 mechanics but in the future...]
 
-	for(const NewTurn::Hero & h : heroes) //give mana/movement point
-	{
-		CGHeroInstance *hero = gs->getHero(h.id);
-		if(!hero)
-		{
-			logGlobal->error("Hero %d not found in NewTurn::applyGs", h.id.getNum());
-			continue;
-		}
+	for(const auto & manaPack : heroesMana)
+		manaPack.applyGs(gs);
 
-		hero->setMovementPoints(h.move);
-		hero->mana = h.mana;
-	}
+	for(const auto & movePack : heroesMovement)
+		movePack.applyGs(gs);
 
 	gs->heroesPool->onNewDay();
 

lib/networkPacks/PacksForClient.h

@@ -294,6 +294,13 @@ struct DLL_LINKAGE SetMana : public CPackForClient
 
 	void visitTyped(ICPackVisitor & visitor) override;
 
+	SetMana() = default;
+	SetMana(ObjectInstanceID hid, si32 val, bool absolute)
+		: hid(hid)
+		, val(val)
+		, absolute(absolute)
+	{}
+
 	ObjectInstanceID hid;
 	si32 val = 0;
 	bool absolute = true;
@@ -310,6 +317,13 @@ struct DLL_LINKAGE SetMovePoints : public CPackForClient
 {
 	void applyGs(CGameState * gs) override;
 
+	SetMovePoints() = default;
+	SetMovePoints(ObjectInstanceID hid, si32 val, bool absolute)
+		: hid(hid)
+		, val(val)
+		, absolute(absolute)
+	{}
+
 	ObjectInstanceID hid;
 	si32 val = 0;
 	bool absolute = true;
@@ -1119,38 +1133,27 @@ struct DLL_LINKAGE NewTurn : public CPackForClient
 
 	void visitTyped(ICPackVisitor & visitor) override;
 
-	struct Hero
-	{
-		ObjectInstanceID id; //id is a general serial id
-		ui32 move;
-		ui32 mana;
-		template <typename Handler> void serialize(Handler & h)
-		{
-			h & id;
-			h & move;
-			h & mana;
-		}
-		bool operator<(const Hero & h)const { return id < h.id; }
-	};
-
-	std::set<Hero> heroes; //updates movement and mana points
-	std::vector<SetAvailableCreatures> availableCreatures;//creatures to be placed in towns
-	std::map<PlayerColor, ResourceSet> playerIncome; //player ID => resource value[res_id]
 	ui32 day = 0;
-	EWeekType specialWeek = EWeekType::NORMAL;
 	CreatureID creatureid; //for creature weeks
+	EWeekType specialWeek = EWeekType::NORMAL;
+
+	std::vector<SetMovePoints> heroesMovement;
+	std::vector<SetMana> heroesMana;
+	std::vector<SetAvailableCreatures> availableCreatures;
+	std::map<PlayerColor, ResourceSet> playerIncome;
 	std::optional<RumorState> newRumor; // only on new weeks
 
 	NewTurn() = default;
 
 	template <typename Handler> void serialize(Handler & h)
 	{
-		h & heroes;
-		h & availableCreatures;
-		h & playerIncome;
 		h & day;
-		h & specialWeek;
 		h & creatureid;
+		h & specialWeek;
+		h & heroesMovement;
+		h & heroesMana;
+		h & availableCreatures;
+		h & playerIncome;
 		h & newRumor;
 	}
 };

server/CGameHandler.cpp

@@ -617,8 +617,6 @@ void CGameHandler::onNewTurn()
 	bool newWeek = getDate(Date::DAY_OF_WEEK) == 7; //day numbers are confusing, as day was not yet switched
 	bool newMonth = getDate(Date::DAY_OF_MONTH) == 28;
 
-	std::map<PlayerColor, si32> hadGold;//starting gold - for buildings like dwarven treasury
-
 	if (firstTurn)
 	{
 		for (auto obj : gs->map->objects)
@@ -656,37 +654,15 @@ void CGameHandler::onNewTurn()
 		n.creatureid = creatureID;
 	}
 
-	for (auto & elem : gs->players)
+	if (firstTurn)
 	{
-		if (elem.first == PlayerColor::NEUTRAL)
-			continue;
-
-		assert(elem.first.isValidPlayer());//illegal player number!
-			
-		auto playerSettings = gameState()->scenarioOps->getIthPlayersSettings(elem.first);
-
-		std::pair<PlayerColor, si32> playerGold(elem.first, elem.second.resources[EGameResID::GOLD]);
-		hadGold.insert(playerGold);
-
-		if (firstTurn)
+		for (auto & elem : gs->players)
 			heroPool->onNewWeek(elem.first);
-
-		for (CGHeroInstance *h : (elem).second.getHeroes())
-		{
-			if (h->visitedTown)
-				giveSpells(h->visitedTown, h);
-
-			NewTurn::Hero hth;
-			hth.id = h->id;
-			auto ti = std::make_unique<TurnInfo>(h, 1);
-			// TODO: this code executed when bonuses of previous day not yet updated (this happen in NewTurn::applyGs). See issue 2356
-			hth.move = h->movementPointsLimitCached(gs->map->getTile(h->visitablePos()).terType->isLand(), ti.get());
-			hth.mana = h->getManaNewTurn();
-
-			n.heroes.insert(hth);
-		}
 	}
 
+	n.heroesMana = newTurnProcessor->updateHeroesManaPoints();
+	n.heroesMovement = newTurnProcessor->updateHeroesMovementPoints();
+
 	if (newWeek)
 	{
 		for (CGTownInstance *t : gs->map->towns)

server/processors/NewTurnProcessor.cpp

@@ -337,3 +337,41 @@ std::tuple<EWeekType, CreatureID> NewTurnProcessor::pickWeekType(bool newMonth)
 		return { EWeekType::NORMAL, CreatureID::NONE};
 	}
 }
+
+std::vector<SetMana> NewTurnProcessor::updateHeroesManaPoints()
+{
+	std::vector<SetMana> result;
+
+	for (auto & elem : gameHandler->gameState()->players)
+	{
+		for (CGHeroInstance *h : elem.second.getHeroes())
+		{
+			int32_t newMana = h->getManaNewTurn();
+
+			if (newMana != h->mana)
+				result.emplace_back(h->id, newMana, true);
+		}
+	}
+
+	return result;
+}
+
+std::vector<SetMovePoints> NewTurnProcessor::updateHeroesMovementPoints()
+{
+	std::vector<SetMovePoints> result;
+
+	for (auto & elem : gameHandler->gameState()->players)
+	{
+		for (CGHeroInstance *h : elem.second.getHeroes())
+		{
+			auto ti = std::make_unique<TurnInfo>(h, 1);
+			// NOTE: this code executed when bonuses of previous day not yet updated (this happen in NewTurn::applyGs). See issue 2356
+			int32_t newMovementPoints = h->movementPointsLimitCached(gameHandler->gameState()->map->getTile(h->visitablePos()).terType->isLand(), ti.get());
+
+			if (newMovementPoints != h->movementPointsRemaining())
+				result.emplace_back(h->id, newMovementPoints, true);
+		}
+	}
+
+	return result;
+}

server/processors/NewTurnProcessor.h

@@ -17,6 +17,8 @@ VCMI_LIB_NAMESPACE_BEGIN
 class CGTownInstance;
 class ResourceSet;
 struct SetAvailableCreatures;
+struct SetMovePoints;
+struct SetMana;
 VCMI_LIB_NAMESPACE_END
 
 class CGameHandler;
@@ -27,6 +29,9 @@ class NewTurnProcessor : boost::noncopyable
 public:
 	NewTurnProcessor(CGameHandler * gameHandler);
 
+	std::vector<SetMana> updateHeroesManaPoints();
+	std::vector<SetMovePoints> updateHeroesMovementPoints();
+
 	ResourceSet generatePlayerIncome(PlayerColor playerID, bool newWeek);
 	SetAvailableCreatures generateTownGrowth(const CGTownInstance * town, EWeekType weekType, CreatureID creatureWeek, bool firstDay);
 	RumorState pickNewRumor();