Implemented serialization of MapObjectSubID, refactoring of related code

lib/CPlayerState.h

@@ -27,12 +27,33 @@ struct QuestInfo;
 
 struct DLL_LINKAGE PlayerState : public CBonusSystemNode, public Player
 {
+	struct VisitedObjectGlobal
+	{
+		MapObjectID id;
+		MapObjectSubID subID;
+
+		bool operator < (const VisitedObjectGlobal & other) const
+		{
+			if (id != other.id)
+				return id < other.id;
+			else
+				return subID < other.subID;
+		}
+
+		template <typename Handler> void serialize(Handler &h, const int version)
+		{
+			h & id;
+			subID.serializeIdentifier(h, id, version);
+		}
+	};
+
 public:
 	PlayerColor color;
 	bool human; //true if human controlled player, false for AI
 	TeamID team;
 	TResources resources;
 	std::set<ObjectInstanceID> visitedObjects; // as a std::set, since most accesses here will be from visited status checks
+	std::set<VisitedObjectGlobal> visitedObjectsGlobal;
 	std::vector<ConstTransitivePtr<CGHeroInstance> > heroes;
 	std::vector<ConstTransitivePtr<CGTownInstance> > towns;
 	std::vector<ConstTransitivePtr<CGDwelling> > dwellings; //used for town growth
@@ -82,6 +103,7 @@ public:
 		h & dwellings;
 		h & quests;
 		h & visitedObjects;
+		h & visitedObjectsGlobal;
 		h & status;
 		h & daysWithoutCastle;
 		h & cheated;

lib/constants/EntityIdentifiers.cpp

@@ -177,6 +177,35 @@ si32 MapObjectID::decode(const std::string & identifier)
 	return rawId.value();
 }
 
+std::string MapObjectSubID::encode(MapObjectID primaryID, int32_t index)
+{
+	if (index == -1)
+		return "";
+
+	if(primaryID == Obj::PRISON || primaryID == Obj::HERO)
+		return HeroTypeID::encode(index);
+
+	if (primaryID == Obj::SPELL_SCROLL)
+		return SpellID::encode(index);
+
+	return VLC->objtypeh->getHandlerFor(primaryID, index)->getJsonKey();
+}
+
+si32 MapObjectSubID::decode(MapObjectID primaryID, const std::string & identifier)
+{
+	if (identifier.empty())
+		return -1;
+
+	if(primaryID == Obj::PRISON || primaryID == Obj::HERO)
+		return HeroTypeID::decode(identifier);
+
+	if (primaryID == Obj::SPELL_SCROLL)
+		return SpellID::decode(identifier);
+
+	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), VLC->objtypeh->getJsonKey(primaryID), identifier);
+	return rawId.value();
+}
+
 std::string BoatId::encode(int32_t index)
 {
 	if (index == -1)

lib/constants/EntityIdentifiers.h

@@ -490,16 +490,14 @@ public:
 	}
 };
 
-class MapObjectSubID : public StaticIdentifier<MapObjectSubID>
+class DLL_LINKAGE MapObjectSubID : public Identifier<MapObjectSubID>
 {
 public:
-	MapObjectID primaryIdentifier;
-
 	constexpr MapObjectSubID(const IdentifierBase & value):
-		StaticIdentifier<MapObjectSubID>(value.getNum())
+		Identifier<MapObjectSubID>(value.getNum())
 	{}
 	constexpr MapObjectSubID(int32_t value = -1):
-		StaticIdentifier<MapObjectSubID>(value)
+		Identifier<MapObjectSubID>(value)
 	{}
 
 	MapObjectSubID & operator =(int32_t value)
@@ -514,14 +512,28 @@ public:
 		return *this;
 	}
 
-	static si32 decode(const std::string & identifier);
-	static std::string encode(const si32 index);
+	static si32 decode(MapObjectID primaryID, const std::string & identifier);
+	static std::string encode(MapObjectID primaryID, si32 index);
 
 	// TODO: Remove
 	constexpr operator int32_t () const
 	{
 		return num;
 	}
+
+	template <typename Handler>
+	void serializeIdentifier(Handler &h, const MapObjectID & primaryID, const int version)
+	{
+		std::string secondaryStringID;
+
+		if (h.saving)
+			secondaryStringID = encode(primaryID, num);
+
+		h & secondaryStringID;
+
+		if (!h.saving)
+			num = decode(primaryID, secondaryStringID);
+	}
 };
 
 class DLL_LINKAGE RoadId : public EntityIdentifier<RoadId>

lib/gameState/CGameState.cpp

@@ -223,11 +223,6 @@ void CGameState::init(const IMapService * mapService, StartInfo * si, Load::Prog
 	initVisitingAndGarrisonedHeroes();
 	initFogOfWar();
 
-	// Explicitly initialize static variables
-	for(auto & elem : players)
-	{
-		CGKeys::playerKeyMap[elem.first] = std::set<MapObjectSubID>();
-	}
 	for(auto & elem : teams)
 	{
 		CGObelisk::visited[elem.first] = 0;

lib/mapObjects/CGObjectInstance.h

@@ -140,7 +140,7 @@ public:
 		h & subTypeName;
 		h & pos;
 		h & ID;
-		h & subID;
+		subID.serializeIdentifier(h, ID, version);
 		h & id;
 		h & tempOwner;
 		h & blockVisit;

lib/mapObjects/CQuest.cpp

@@ -23,6 +23,7 @@
 #include "../serializer/JsonSerializeFormat.h"
 #include "../GameConstants.h"
 #include "../constants/StringConstants.h"
+#include "../CPlayerState.h"
 #include "../CSkillHandler.h"
 #include "../mapping/CMap.h"
 #include "../mapObjects/CGHeroInstance.h"
@@ -34,8 +35,6 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 
-std::map <PlayerColor, std::set <MapObjectSubID> > CGKeys::playerKeyMap;
-
 //TODO: Remove constructor
 CQuest::CQuest():
 	qid(-1),
@@ -777,24 +776,9 @@ void CGQuestGuard::serializeJsonOptions(JsonSerializeFormat & handler)
 	quest->serializeJson(handler, "quest");
 }
 
-void CGKeys::reset()
-{
-	playerKeyMap.clear();
-}
-
-void CGKeys::setPropertyDer (ObjProperty what, ObjPropertyID identifier)
-{
-	if (what == ObjProperty::KEYMASTER_VISITED)
-	{
-		playerKeyMap[identifier.as<PlayerColor>()].insert(subID);
-	}
-	else
-		logGlobal->error("Unexpected properties requested to set: what=%d, val=%d", static_cast<int>(what), identifier.getNum());
-}
-
 bool CGKeys::wasMyColorVisited(const PlayerColor & player) const
 {
-	return playerKeyMap.count(player) && vstd::contains(playerKeyMap[player], subID);
+	return cb->getPlayerState(player)->visitedObjectsGlobal.count({ID, subID}) != 0;
 }
 
 std::string CGKeys::getHoverText(PlayerColor player) const
@@ -817,7 +801,11 @@ void CGKeymasterTent::onHeroVisit( const CGHeroInstance * h ) const
 	int txt_id;
 	if (!wasMyColorVisited (h->getOwner()) )
 	{
-		cb->setObjPropertyID(id, ObjProperty::KEYMASTER_VISITED, h->tempOwner);
+		ChangeObjectVisitors cow;
+		cow.mode = ChangeObjectVisitors::VISITOR_GLOBAL;
+		cow.hero = h->id;
+		cow.object = id;
+		cb->sendAndApply(&cow);
 		txt_id=19;
 	}
 	else

lib/mapObjects/CQuest.h

@@ -168,11 +168,6 @@ protected:
 class DLL_LINKAGE CGKeys : public CGObjectInstance //Base class for Keymaster and guards
 {
 public:
-	static std::map <PlayerColor, std::set <MapObjectSubID> > playerKeyMap; //[players][keysowned]
-	//SubID 0 - lightblue, 1 - green, 2 - red, 3 - darkblue, 4 - brown, 5 - purple, 6 - white, 7 - black
-
-	static void reset();
-
 	bool wasMyColorVisited(const PlayerColor & player) const;
 
 	std::string getObjectName() const override; //depending on color
@@ -182,8 +177,6 @@ public:
 	{
 		h & static_cast<CGObjectInstance&>(*this);
 	}
-protected:
-	void setPropertyDer(ObjProperty what, ObjPropertyID identifier) override;
 };
 
 class DLL_LINKAGE CGKeymasterTent : public CGKeys

lib/mapObjects/MiscObjects.cpp

@@ -33,7 +33,6 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-std::map <MapObjectSubID, std::vector<ObjectInstanceID> > CGMagi::eyelist;
 ui8 CGObelisk::obeliskCount = 0; //how many obelisks are on map
 std::map<TeamID, ui8> CGObelisk::visited; //map: team_id => how many obelisks has been visited
 
@@ -1003,26 +1002,27 @@ void CGGarrison::serializeJsonOptions(JsonSerializeFormat& handler)
 	CArmedInstance::serializeJsonOptions(handler);
 }
 
-void CGMagi::reset()
-{
-	eyelist.clear();
-}
-
 void CGMagi::initObj(CRandomGenerator & rand)
 {
 	if (ID == Obj::EYE_OF_MAGI)
-	{
 		blockVisit = true;
-		eyelist[subID].push_back(id);
-	}
 }
+
 void CGMagi::onHeroVisit(const CGHeroInstance * h) const
 {
 	if (ID == Obj::HUT_OF_MAGI)
 	{
 		h->showInfoDialog(61);
 
-		if (!eyelist[subID].empty())
+		std::vector<const CGObjectInstance *> eyes;
+
+		for (auto object : cb->gameState()->map->objects)
+		{
+			if (object && object->ID == Obj::EYE_OF_MAGI && object->subID == this->subID)
+				eyes.push_back(object);
+		}
+
+		if (!eyes.empty())
 		{
 			CenterView cv;
 			cv.player = h->tempOwner;
@@ -1033,10 +1033,8 @@ void CGMagi::onHeroVisit(const CGHeroInstance * h) const
 			fw.mode = ETileVisibility::REVEALED;
 			fw.waitForDialogs = true;
 
-			for(const auto & it : eyelist[subID])
+			for(const auto & eye : eyes)
 			{
-				const CGObjectInstance *eye = cb->getObj(it);
-
 				cb->getTilesInRange (fw.tiles, eye->pos, 10, ETileVisibility::HIDDEN, h->tempOwner);
 				cb->sendAndApply(&fw);
 				cv.pos = eye->pos;

lib/mapObjects/MiscObjects.h

@@ -338,10 +338,6 @@ protected:
 class DLL_LINKAGE CGMagi : public CGObjectInstance
 {
 public:
-	static std::map <MapObjectSubID, std::vector<ObjectInstanceID> > eyelist; //[subID][id], supports multiple sets as in H5
-
-	static void reset();
-
 	void initObj(CRandomGenerator & rand) override;
 	void onHeroVisit(const CGHeroInstance * h) const override;
 

lib/mapObjects/ObjectTemplate.h

@@ -164,7 +164,7 @@ public:
 		h & animationFile;
 		h & stringID;
 		h & id;
-		h & subid;
+		subid.serializeIdentifier(h, id, version);
 		h & printPriority;
 		h & visitDir;
 		h & editorAnimationFile;

lib/mapping/CMap.cpp

@@ -671,8 +671,6 @@ CMapEditManager * CMap::getEditManager()
 
 void CMap::resetStaticData()
 {
-	CGKeys::reset();
-	CGMagi::reset();
 	CGObelisk::reset();
 	CGTownInstance::reset();
 }

lib/mapping/CMap.h

@@ -191,8 +191,6 @@ public:
 		h & artInstances;
 
 		// static members
-		h & CGKeys::playerKeyMap;
-		h & CGMagi::eyelist;
 		h & CGObelisk::obeliskCount;
 		h & CGObelisk::visited;
 		h & CGTownInstance::merchantArtifacts;

lib/networkPacks/NetPacksLib.cpp

@@ -1061,6 +1061,12 @@ void ChangeObjectVisitors::applyGs(CGameState * gs) const
 			}
 
 			break;
+		case VISITOR_GLOBAL:
+			{
+				CGObjectInstance * objectPtr = gs->getObjInstance(object);
+				gs->getPlayerState(gs->getHero(hero)->tempOwner)->visitedObjectsGlobal.insert({objectPtr->ID, objectPtr->subID});
+				break;
+			}
 		case VISITOR_REMOVE:
 			gs->getHero(hero)->visitedObjects.erase(object);
 			break;
@@ -1488,7 +1494,7 @@ void NewObject::applyGs(CGameState *gs)
 	const TerrainTile & t = gs->map->getTile(targetPos);
 	terrainType = t.terType->getId();
 
-	auto handler = VLC->objtypeh->getHandlerFor(ID, subID.getNum());
+	auto handler = VLC->objtypeh->getHandlerFor(ID, subID);
 
 	CGObjectInstance * o = handler->create();
 	handler->configureObject(o, gs->getRandomGenerator());
@@ -1504,7 +1510,7 @@ void NewObject::applyGs(CGameState *gs)
 		cre->character = 2;
 		cre->gainedArtifact = ArtifactID::NONE;
 		cre->identifier = -1;
-		cre->addToSlot(SlotID(0), new CStackInstance(subID.as<CreatureID>(), -1)); //add placeholder stack
+		cre->addToSlot(SlotID(0), new CStackInstance(subID.getNum(), -1)); //add placeholder stack
 	}
 
 	assert(!handler->getTemplates(terrainType).empty());

lib/networkPacks/ObjProperty.h

@@ -39,7 +39,6 @@ enum class ObjProperty : int8_t
 
 	SEERHUT_VISITED,
 	SEERHUT_COMPLETE,
-	KEYMASTER_VISITED,
 	OBELISK_VISITED,
 
 	//creature-bank specific

lib/networkPacks/PacksForClient.h

@@ -784,7 +784,7 @@ struct DLL_LINKAGE NewObject : public CPackForClient
 	/// Object ID to create
 	MapObjectID ID;
 	/// Object secondary ID to create
-	VariantIdentifier<MapObjectSubID, HeroTypeID, CreatureID, BoatId> subID;
+	MapObjectSubID subID;
 	/// Position of visitable tile of created object
 	int3 targetPos;
 	/// Which player initiated creation of this object
@@ -797,7 +797,7 @@ struct DLL_LINKAGE NewObject : public CPackForClient
 	template <typename Handler> void serialize(Handler & h, const int version)
 	{
 		h & ID;
-		h & subID;
+		subID.serializeIdentifier(h, ID, version);
 		h & targetPos;
 		h & initiator;
 	}
@@ -1247,10 +1247,11 @@ struct DLL_LINKAGE ChangeObjectVisitors : public CPackForClient
 	{
 		VISITOR_ADD,      // mark hero as one that have visited this object
 		VISITOR_ADD_TEAM, // mark team as one that have visited this object
+		VISITOR_GLOBAL,   // mark player as one that have visited object of this type
 		VISITOR_REMOVE,   // unmark visitor, reversed to ADD
 		VISITOR_CLEAR     // clear all visitors from this object (object reset)
 	};
-	ui32 mode = VISITOR_CLEAR; // uses VisitMode enum
+	VisitMode mode = VISITOR_CLEAR; // uses VisitMode enum
 	ObjectInstanceID object;
 	ObjectInstanceID hero; // note: hero owner will be also marked as "visited" this object
 
@@ -1260,7 +1261,7 @@ struct DLL_LINKAGE ChangeObjectVisitors : public CPackForClient
 
 	ChangeObjectVisitors() = default;
 
-	ChangeObjectVisitors(ui32 mode, const ObjectInstanceID & object, const ObjectInstanceID & heroID = ObjectInstanceID(-1))
+	ChangeObjectVisitors(VisitMode mode, const ObjectInstanceID & object, const ObjectInstanceID & heroID = ObjectInstanceID(-1))
 		: mode(mode)
 		, object(object)
 		, hero(heroID)