Moved stateful artifact randomization logic to gamestate from handler

lib/CArtHandler.cpp

@@ -607,75 +607,6 @@ void CArtHandler::loadComponents(CArtifact * art, const JsonNode & node)
 	}
 }
 
-ArtifactID CArtHandler::pickRandomArtifact(CRandomGenerator & rand, int flags, std::function<bool(ArtifactID)> accepts)
-{
-	std::set<ArtifactID> potentialPicks;
-
-	// Select artifacts that satisfy provided criterias
-	for (auto const * artifact : allowedArtifacts)
-	{
-		assert(artifact->aClass != CArtifact::ART_SPECIAL); // should be filtered out when allowedArtifacts is initialized
-
-		if ((flags & CArtifact::ART_TREASURE) == 0 && artifact->aClass == CArtifact::ART_TREASURE)
-			continue;
-
-		if ((flags & CArtifact::ART_MINOR) == 0 && artifact->aClass == CArtifact::ART_MINOR)
-			continue;
-
-		if ((flags & CArtifact::ART_MAJOR) == 0 && artifact->aClass == CArtifact::ART_MAJOR)
-			continue;
-
-		if ((flags & CArtifact::ART_RELIC) == 0 && artifact->aClass == CArtifact::ART_RELIC)
-			continue;
-
-		if (!accepts(artifact->id))
-			continue;
-
-		potentialPicks.insert(artifact->id);
-	}
-
-	return pickRandomArtifact(rand, potentialPicks);
-}
-
-ArtifactID CArtHandler::pickRandomArtifact(CRandomGenerator & rand, std::set<ArtifactID> potentialPicks)
-{
-	// No allowed artifacts at all - give Grail - this can't be banned (hopefully)
-	// FIXME: investigate how such cases are handled by H3 - some heavily customized user-made maps likely rely on H3 behavior
-	if (potentialPicks.empty())
-	{
-		logGlobal->warn("Failed to find artifact that matches requested parameters!");
-		return ArtifactID::GRAIL;
-	}
-
-	// Find how many times least used artifacts were picked by randomizer
-	int leastUsedTimes = std::numeric_limits<int>::max();
-	for (auto const & artifact : potentialPicks)
-		if (allocatedArtifacts[artifact] < leastUsedTimes)
-			leastUsedTimes = allocatedArtifacts[artifact];
-
-	// Pick all artifacts that were used least number of times
-	std::set<ArtifactID> preferredPicks;
-	for (auto const & artifact : potentialPicks)
-		if (allocatedArtifacts[artifact] == leastUsedTimes)
-			preferredPicks.insert(artifact);
-
-	assert(!preferredPicks.empty());
-
-	ArtifactID artID = *RandomGeneratorUtil::nextItem(preferredPicks, rand);
-	allocatedArtifacts[artID] += 1; // record +1 more usage
-	return artID;
-}
-
-ArtifactID CArtHandler::pickRandomArtifact(CRandomGenerator & rand, std::function<bool(ArtifactID)> accepts)
-{
-	return pickRandomArtifact(rand, 0xff, std::move(accepts));
-}
-
-ArtifactID CArtHandler::pickRandomArtifact(CRandomGenerator & rand, int flags)
-{
-	return pickRandomArtifact(rand, flags, [](const ArtifactID &) { return true; });
-}
-
 void CArtHandler::makeItCreatureArt(CArtifact * a, bool onlyCreature)
 {
 	if (onlyCreature)
@@ -723,7 +654,6 @@ bool CArtHandler::legalArtifact(const ArtifactID & id)
 void CArtHandler::initAllowedArtifactsList(const std::vector<bool> &allowed)
 {
 	allowedArtifacts.clear();
-	allocatedArtifacts.clear();
 
 	for (ArtifactID i=ArtifactID::SPELLBOOK; i < ArtifactID(static_cast<si32>(objects.size())); i.advance(1))
 	{

lib/CArtHandler.h

@@ -141,9 +141,6 @@ public:
 class DLL_LINKAGE CArtHandler : public CHandlerBase<ArtifactID, Artifact, CArtifact, ArtifactService>
 {
 public:
-	/// Stores number of times each artifact was placed on map via randomization
-	std::map<ArtifactID, int> allocatedArtifacts;
-
 	/// List of artifacts allowed on the map
 	std::vector<CArtifact *> allowedArtifacts;
 
@@ -151,12 +148,6 @@ public:
 
 	static CArtifact::EartClass stringToClass(const std::string & className); //TODO: rework EartClass to make this a constructor
 
-	/// Gets a artifact ID randomly and removes the selected artifact from this handler.
-	ArtifactID pickRandomArtifact(CRandomGenerator & rand, int flags);
-	ArtifactID pickRandomArtifact(CRandomGenerator & rand, std::function<bool(ArtifactID)> accepts);
-	ArtifactID pickRandomArtifact(CRandomGenerator & rand, int flags, std::function<bool(ArtifactID)> accepts);
-	ArtifactID pickRandomArtifact(CRandomGenerator & rand, std::set<ArtifactID> filtered);
-
 	bool legalArtifact(const ArtifactID & id);
 	void initAllowedArtifactsList(const std::vector<bool> &allowed); //allowed[art_id] -> 0 if not allowed, 1 if allowed
 	static void makeItCreatureArt(CArtifact * a, bool onlyCreature = true);

lib/IGameCallback.cpp

@@ -144,14 +144,14 @@ void CPrivilegedInfoCallback::getAllTiles(std::unordered_set<int3> & tiles, std:
 	}
 }
 
-void CPrivilegedInfoCallback::pickAllowedArtsSet(std::vector<const CArtifact *> & out, CRandomGenerator & rand) const
+void CPrivilegedInfoCallback::pickAllowedArtsSet(std::vector<const CArtifact *> & out, CRandomGenerator & rand)
 {
 	for (int j = 0; j < 3 ; j++)
-		out.push_back(VLC->arth->pickRandomArtifact(rand, CArtifact::ART_TREASURE).toArtifact());
+		out.push_back(gameState()->pickRandomArtifact(rand, CArtifact::ART_TREASURE).toArtifact());
 	for (int j = 0; j < 3 ; j++)
-		out.push_back(VLC->arth->pickRandomArtifact(rand, CArtifact::ART_MINOR).toArtifact());
+		out.push_back(gameState()->pickRandomArtifact(rand, CArtifact::ART_MINOR).toArtifact());
 
-	out.push_back(VLC->arth->pickRandomArtifact(rand, CArtifact::ART_MAJOR).toArtifact());
+	out.push_back(gameState()->pickRandomArtifact(rand, CArtifact::ART_MAJOR).toArtifact());
 }
 
 void CPrivilegedInfoCallback::getAllowedSpells(std::vector<SpellID> & out, std::optional<ui16> level)

lib/IGameCallback.h

@@ -61,7 +61,7 @@ public:
 	void getAllTiles(std::unordered_set<int3> &tiles, std::optional<PlayerColor> player, int level, std::function<bool(const TerrainTile *)> filter) const;
 
 	//gives 3 treasures, 3 minors, 1 major -> used by Black Market and Artifact Merchant
-	void pickAllowedArtsSet(std::vector<const CArtifact *> & out, CRandomGenerator & rand) const; 
+	void pickAllowedArtsSet(std::vector<const CArtifact *> & out, CRandomGenerator & rand);
 	void getAllowedSpells(std::vector<SpellID> &out, std::optional<ui16> level = std::nullopt);
 
 	template<typename Saver>

lib/JsonRandom.cpp

@@ -24,6 +24,7 @@
 #include "CSkillHandler.h"
 #include "CHeroHandler.h"
 #include "IGameCallback.h"
+#include "gameState/CGameState.h"
 #include "mapObjects/IObjectInterface.h"
 #include "modding/IdentifierStorage.h"
 #include "modding/ModScope.h"
@@ -388,7 +389,7 @@ namespace JsonRandom
 
 		std::set<ArtifactID> potentialPicks = filterKeys(value, allowedArts, variables);
 
-		return VLC->arth->pickRandomArtifact(rng, potentialPicks);
+		return IObjectInterface::cb->gameState()->pickRandomArtifact(rng, potentialPicks);
 	}
 
 	std::vector<ArtifactID> loadArtifacts(const JsonNode & value, CRandomGenerator & rng, const Variables & variables)

lib/gameState/CGameState.cpp

@@ -760,7 +760,7 @@ void CGameState::initStartingBonus()
 					logGlobal->error("Cannot give starting artifact - no heroes!");
 					break;
 				}
-				const Artifact * toGive = VLC->arth->pickRandomArtifact(getRandomGenerator(), CArtifact::ART_TREASURE).toEntity(VLC);
+				const Artifact * toGive = pickRandomArtifact(getRandomGenerator(), CArtifact::ART_TREASURE).toEntity(VLC);
 
 				CGHeroInstance *hero = elem.second.heroes[0];
 				if(!giveHeroArtifact(hero, toGive->getId()))
@@ -1971,4 +1971,73 @@ CRandomGenerator & CGameState::getRandomGenerator()
 	return rand;
 }
 
+ArtifactID CGameState::pickRandomArtifact(CRandomGenerator & rand, int flags, std::function<bool(ArtifactID)> accepts)
+{
+	std::set<ArtifactID> potentialPicks;
+
+	// Select artifacts that satisfy provided criterias
+	for (auto const * artifact : VLC->arth->allowedArtifacts)
+	{
+		assert(artifact->aClass != CArtifact::ART_SPECIAL); // should be filtered out when allowedArtifacts is initialized
+
+		if ((flags & CArtifact::ART_TREASURE) == 0 && artifact->aClass == CArtifact::ART_TREASURE)
+			continue;
+
+		if ((flags & CArtifact::ART_MINOR) == 0 && artifact->aClass == CArtifact::ART_MINOR)
+			continue;
+
+		if ((flags & CArtifact::ART_MAJOR) == 0 && artifact->aClass == CArtifact::ART_MAJOR)
+			continue;
+
+		if ((flags & CArtifact::ART_RELIC) == 0 && artifact->aClass == CArtifact::ART_RELIC)
+			continue;
+
+		if (!accepts(artifact->getId()))
+			continue;
+
+		potentialPicks.insert(artifact->getId());
+	}
+
+	return pickRandomArtifact(rand, potentialPicks);
+}
+
+ArtifactID CGameState::pickRandomArtifact(CRandomGenerator & rand, std::set<ArtifactID> potentialPicks)
+{
+	// No allowed artifacts at all - give Grail - this can't be banned (hopefully)
+	// FIXME: investigate how such cases are handled by H3 - some heavily customized user-made maps likely rely on H3 behavior
+	if (potentialPicks.empty())
+	{
+		logGlobal->warn("Failed to find artifact that matches requested parameters!");
+		return ArtifactID::GRAIL;
+	}
+
+	// Find how many times least used artifacts were picked by randomizer
+	int leastUsedTimes = std::numeric_limits<int>::max();
+	for (auto const & artifact : potentialPicks)
+		if (allocatedArtifacts[artifact] < leastUsedTimes)
+			leastUsedTimes = allocatedArtifacts[artifact];
+
+	// Pick all artifacts that were used least number of times
+	std::set<ArtifactID> preferredPicks;
+	for (auto const & artifact : potentialPicks)
+		if (allocatedArtifacts[artifact] == leastUsedTimes)
+			preferredPicks.insert(artifact);
+
+	assert(!preferredPicks.empty());
+
+	ArtifactID artID = *RandomGeneratorUtil::nextItem(preferredPicks, rand);
+	allocatedArtifacts[artID] += 1; // record +1 more usage
+	return artID;
+}
+
+ArtifactID CGameState::pickRandomArtifact(CRandomGenerator & rand, std::function<bool(ArtifactID)> accepts)
+{
+	return pickRandomArtifact(rand, 0xff, std::move(accepts));
+}
+
+ArtifactID CGameState::pickRandomArtifact(CRandomGenerator & rand, int flags)
+{
+	return pickRandomArtifact(rand, flags, [](const ArtifactID &) { return true; });
+}
+
 VCMI_LIB_NAMESPACE_END

lib/gameState/CGameState.h

@@ -83,6 +83,9 @@ class DLL_LINKAGE CGameState : public CNonConstInfoCallback
 	friend class CGameStateCampaign;
 
 public:
+	/// Stores number of times each artifact was placed on map via randomization
+	std::map<ArtifactID, int> allocatedArtifacts;
+
 	/// List of currently ongoing battles
 	std::vector<std::unique_ptr<BattleInfo>> currentBattles;
 	/// ID that can be allocated to next battle
@@ -130,6 +133,12 @@ public:
 	std::vector<CGObjectInstance*> guardingCreatures (int3 pos) const;
 	void updateRumor();
 
+	/// Gets a artifact ID randomly and removes the selected artifact from this handler.
+	ArtifactID pickRandomArtifact(CRandomGenerator & rand, int flags);
+	ArtifactID pickRandomArtifact(CRandomGenerator & rand, std::function<bool(ArtifactID)> accepts);
+	ArtifactID pickRandomArtifact(CRandomGenerator & rand, int flags, std::function<bool(ArtifactID)> accepts);
+	ArtifactID pickRandomArtifact(CRandomGenerator & rand, std::set<ArtifactID> filtered);
+
 	/// Returns battle in which selected player is engaged, or nullptr if none.
 	/// Can NOT be used with neutral player, use battle by ID instead
 	const BattleInfo * getBattle(const PlayerColor & player) const;
@@ -176,6 +185,7 @@ public:
 		h & rand;
 		h & rumor;
 		h & campaign;
+		h & allocatedArtifacts;
 
 		BONUS_TREE_DESERIALIZATION_FIX
 	}

lib/mapObjects/MiscObjects.cpp

@@ -724,19 +724,19 @@ void CGArtifact::pickRandomObject(CRandomGenerator & rand)
 	switch(ID.toEnum())
 	{
 		case MapObjectID::RANDOM_ART:
-			subID = VLC->arth->pickRandomArtifact(rand, CArtifact::ART_TREASURE | CArtifact::ART_MINOR | CArtifact::ART_MAJOR | CArtifact::ART_RELIC);
+			subID = cb->gameState()->pickRandomArtifact(rand, CArtifact::ART_TREASURE | CArtifact::ART_MINOR | CArtifact::ART_MAJOR | CArtifact::ART_RELIC);
 			break;
 		case MapObjectID::RANDOM_TREASURE_ART:
-			subID = VLC->arth->pickRandomArtifact(rand, CArtifact::ART_TREASURE);
+			subID = cb->gameState()->pickRandomArtifact(rand, CArtifact::ART_TREASURE);
 			break;
 		case MapObjectID::RANDOM_MINOR_ART:
-			subID = VLC->arth->pickRandomArtifact(rand, CArtifact::ART_MINOR);
+			subID = cb->gameState()->pickRandomArtifact(rand, CArtifact::ART_MINOR);
 			break;
 		case MapObjectID::RANDOM_MAJOR_ART:
-			subID = VLC->arth->pickRandomArtifact(rand, CArtifact::ART_MAJOR);
+			subID = cb->gameState()->pickRandomArtifact(rand, CArtifact::ART_MAJOR);
 			break;
 		case MapObjectID::RANDOM_RELIC_ART:
-			subID = VLC->arth->pickRandomArtifact(rand, CArtifact::ART_RELIC);
+			subID = cb->gameState()->pickRandomArtifact(rand, CArtifact::ART_RELIC);
 			break;
 	}
 

lib/networkPacks/NetPacksLib.cpp

@@ -858,7 +858,7 @@ void AddQuest::applyGs(CGameState * gs) const
 
 void UpdateArtHandlerLists::applyGs(CGameState * gs) const
 {
-	VLC->arth->allocatedArtifacts = allocatedArtifacts;
+	gs->allocatedArtifacts = allocatedArtifacts;
 }
 
 void UpdateMapEvents::applyGs(CGameState * gs) const

server/CGameHandler.cpp

@@ -4057,7 +4057,7 @@ void CGameHandler::spawnWanderingMonsters(CreatureID creatureID)
 void CGameHandler::synchronizeArtifactHandlerLists()
 {
 	UpdateArtHandlerLists uahl;
-	uahl.allocatedArtifacts = VLC->arth->allocatedArtifacts;
+	uahl.allocatedArtifacts = gs->allocatedArtifacts;
 	sendAndApply(&uahl);
 }