Merge pull request #3195 from IvanSavenko/identifier_string_serialization

Serialize Identifiers that can be added by mods as strings

AI/VCAI/Goals/Win.cpp

@@ -54,15 +54,15 @@ TSubgoal Win::whatToDoToAchieve()
 			return sptr(GetArtOfType(goal.objectType.as<ArtifactID>()));
 		case EventCondition::DESTROY:
 		{
-			if(goal.object)
+			if(goal.objectID != ObjectInstanceID::NONE)
 			{
-				auto obj = cb->getObj(goal.object->id);
+				auto obj = cb->getObj(goal.objectID);
 				if(obj)
 					if(obj->getOwner() == ai->playerID) //we can't capture our own object
 						return sptr(Conquer());
 
 
-				return sptr(VisitObj(goal.object->id.getNum()));
+				return sptr(VisitObj(goal.objectID.getNum()));
 			}
 			else
 			{
@@ -124,13 +124,13 @@ TSubgoal Win::whatToDoToAchieve()
 		}
 		case EventCondition::CONTROL:
 		{
-			if(goal.object)
+			if(goal.objectID != ObjectInstanceID::NONE)
 			{
-				auto objRelations = cb->getPlayerRelations(ai->playerID, goal.object->tempOwner);
+				auto obj = cb->getObj(goal.objectID);
 				
-				if(objRelations == PlayerRelations::ENEMIES)
+				if(obj && cb->getPlayerRelations(ai->playerID, obj->tempOwner) == PlayerRelations::ENEMIES)
 				{
-					return sptr(VisitObj(goal.object->id.getNum()));
+					return sptr(VisitObj(goal.objectID.getNum()));
 				}
 				else
 				{

Mods/vcmi/config/vcmi/english.json

@@ -75,6 +75,7 @@
 	"vcmi.server.confirmReconnect"       : "Do you want to reconnect to the last session?",
 	"vcmi.server.errors.modNoDependency" : "Failed to load mod {'%s'}!\n It depends on mod {'%s'} which is not active!\n",
 	"vcmi.server.errors.modConflict" : "Failed to load mod {'%s'}!\n Conflicts with active mod {'%s'}!\n",
+	"vcmi.server.errors.unknownEntity" : "Failed to load save! Unknown entity '%s' found in saved game! Save may not be compatible with currently installed version of mods!",
 
 	"vcmi.settingsMainWindow.generalTab.hover" : "General",
 	"vcmi.settingsMainWindow.generalTab.help"     : "Switches to General Options tab, which contains settings related to general game client behavior.",

Mods/vcmi/config/vcmi/ukrainian.json

@@ -75,6 +75,7 @@
 	"vcmi.server.confirmReconnect"       : "Підключитися до минулої сесії?",
 	"vcmi.server.errors.modNoDependency" : "Не вдалося увімкнути мод {'%s'}!\n Модифікація потребує мод {'%s'} який зараз не активний!\n",
 	"vcmi.server.errors.modConflict" : "Не вдалося увімкнути мод {'%s'}!\n Конфліктує з активним модом {'%s'}!\n",
+	"vcmi.server.errors.unknownEntity" : "Не вдалося завантажити гру! У збереженій грі знайдено невідомий об'єкт '%s'! Це збереження може бути несумісним зі встановленою версією модифікацій!",
 
 	"vcmi.settingsMainWindow.generalTab.hover" : "Загальні",
 	"vcmi.settingsMainWindow.generalTab.help"     : "Перемикає на вкладку загальних параметрів, яка містить налаштування, пов'язані із загальною поведінкою ігрового клієнта",

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/CTownHandler.cpp

@@ -1272,7 +1272,7 @@ std::set<FactionID> CTownHandler::getDefaultAllowed() const
 
 std::set<FactionID> CTownHandler::getAllowedFactions(bool withTown) const
 {
-	if (!withTown)
+	if (withTown)
 		return getDefaultAllowed();
 
 	std::set<FactionID> result;

lib/StartInfo.cpp

@@ -32,7 +32,7 @@ FactionID PlayerSettings::getCastleValidated() const
 {
 	if (!castle.isValid())
 		return FactionID(0);
-	if (castle.getNum() < VLC->townh->size())
+	if (castle.getNum() < VLC->townh->size() && VLC->townh->objects[castle.getNum()]->town != nullptr)
 		return castle;
 
 	return FactionID(0);

lib/bonuses/BonusCustomTypes.h

@@ -13,10 +13,10 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-class DLL_LINKAGE BonusCustomSource : public Identifier<BonusCustomSource>
+class DLL_LINKAGE BonusCustomSource : public StaticIdentifier<BonusCustomSource>
 {
 public:
-	using Identifier<BonusCustomSource>::Identifier;
+	using StaticIdentifier<BonusCustomSource>::StaticIdentifier;
 
 	static std::string encode(int32_t index);
 	static si32 decode(const std::string & identifier);
@@ -24,10 +24,10 @@ public:
 	static const BonusCustomSource undeadMoraleDebuff; // -2
 };
 
-class DLL_LINKAGE BonusCustomSubtype : public Identifier<BonusCustomSubtype>
+class DLL_LINKAGE BonusCustomSubtype : public StaticIdentifier<BonusCustomSubtype>
 {
 public:
-	using Identifier<BonusCustomSubtype>::Identifier;
+	using StaticIdentifier<BonusCustomSubtype>::StaticIdentifier;
 
 	static std::string encode(int32_t index);
 	static si32 decode(const std::string & identifier);

lib/constants/EntityIdentifiers.cpp

@@ -34,6 +34,7 @@
 #include "CCreatureHandler.h"//todo: remove
 #include "spells/CSpellHandler.h" //todo: remove
 #include "CSkillHandler.h"//todo: remove
+#include "mapObjectConstructors/AObjectTypeHandler.h"
 #include "constants/StringConstants.h"
 #include "CGeneralTextHandler.h"
 #include "TerrainHandler.h" //TODO: remove
@@ -121,17 +122,27 @@ namespace GameConstants
 #endif
 }
 
-si32 HeroClassID::decode(const std::string & identifier)
+int32_t IdentifierBase::resolveIdentifier(const std::string & entityType, const std::string identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeMap(), "heroClass", identifier);
+	if (identifier.empty())
+		return -1;
+
+	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), entityType, identifier);
+
 	if (rawId)
 		return rawId.value();
-	else
+	throw IdentifierResolutionException(identifier);
-		return -1;
+}
+
+si32 HeroClassID::decode(const std::string & identifier)
+{
+	return resolveIdentifier("heroClass", identifier);
 }
 
 std::string HeroClassID::encode(const si32 index)
 {
+	if (index == -1)
+		return "";
 	return VLC->heroClasses()->getByIndex(index)->getJsonKey();
 }
 
@@ -162,29 +173,66 @@ std::string CampaignScenarioID::encode(const si32 index)
 
 std::string MapObjectID::encode(int32_t index)
 {
-	return VLC->objtypeh->getObjectHandlerName(MapObjectID(index));
+	if (index == -1)
+		return "";
+	return VLC->objtypeh->getJsonKey(MapObjectID(index));
 }
 
 si32 MapObjectID::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), "objects", identifier);
+	return resolveIdentifier("object", identifier);
-	if(rawId)
+}
-		return rawId.value();
+
-	else
+std::string MapObjectSubID::encode(MapObjectID primaryID, int32_t index)
-		return -1;
+{
+	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(primaryID == Obj::PRISON || primaryID == Obj::HERO)
+		return HeroTypeID::decode(identifier);
+
+	if (primaryID == Obj::SPELL_SCROLL)
+		return SpellID::decode(identifier);
+
+	return resolveIdentifier(VLC->objtypeh->getJsonKey(primaryID), identifier);
+}
+
+std::string BoatId::encode(int32_t index)
+{
+	if (index == -1)
+		return "";
+	return VLC->objtypeh->getHandlerFor(MapObjectID::BOAT, index)->getJsonKey();
+}
+
+si32 BoatId::decode(const std::string & identifier)
+{
+	return resolveIdentifier("core:boat", identifier);
 }
 
 si32 HeroTypeID::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeMap(), "hero", identifier);
+	if (identifier == "random")
-	if(rawId)
+		return -2;
-		return rawId.value();
+	return resolveIdentifier("hero", identifier);
-	else
-		return -1;
 }
 
 std::string HeroTypeID::encode(const si32 index)
 {
+	if (index == -1)
+		return "";
+	if (index == -2)
+		return "random";
 	return VLC->heroTypes()->getByIndex(index)->getJsonKey();
 }
 
@@ -205,15 +253,13 @@ const Artifact * ArtifactIDBase::toEntity(const Services * services) const
 
 si32 ArtifactID::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), "artifact", identifier);
+	return resolveIdentifier("artifact", identifier);
-	if(rawId)
-		return rawId.value();
-	else
-		return -1;
 }
 
 std::string ArtifactID::encode(const si32 index)
 {
+	if (index == -1)
+		return "";
 	return VLC->artifacts()->getByIndex(index)->getJsonKey();
 }
 
@@ -224,15 +270,13 @@ std::string ArtifactID::entityType()
 
 si32 SecondarySkill::decode(const std::string& identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), "secondarySkill", identifier);
+	return resolveIdentifier("secondarySkill", identifier);
-	if(rawId)
-		return rawId.value();
-	else
-		return -1;
 }
 
 std::string SecondarySkill::encode(const si32 index)
 {
+	if (index == -1)
+		return "";
 	return VLC->skills()->getById(SecondarySkill(index))->getJsonKey();
 }
 
@@ -263,15 +307,13 @@ const Creature * CreatureIDBase::toEntity(const CreatureService * creatures) con
 
 si32 CreatureID::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), "creature", identifier);
+	return resolveIdentifier("creature", identifier);
-	if(rawId)
-		return rawId.value();
-	else
-		return -1;
 }
 
 std::string CreatureID::encode(const si32 index)
 {
+	if (index == -1)
+		return "";
 	return VLC->creatures()->getById(CreatureID(index))->getJsonKey();
 }
 
@@ -312,25 +354,19 @@ const HeroType * HeroTypeID::toEntity(const Services * services) const
 
 si32 SpellID::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), "spell", identifier);
+	return resolveIdentifier("spell", identifier);
-	if(rawId)
-		return rawId.value();
-	else
-		return -1;
 }
 
 std::string SpellID::encode(const si32 index)
 {
+	if (index == -1)
+		return "";
 	return VLC->spells()->getByIndex(index)->getJsonKey();
 }
 
 si32 BattleField::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), "spell", identifier);
+	return resolveIdentifier("battlefield", identifier);
-	if(rawId)
-		return rawId.value();
-	else
-		return -1;
 }
 
 std::string BattleField::encode(const si32 index)
@@ -384,7 +420,7 @@ std::string PlayerColor::entityType()
 
 si32 PrimarySkill::decode(const std::string& identifier)
 {
-	return *VLC->identifiers()->getIdentifier(ModScope::scopeGame(), entityType(), identifier);
+	return resolveIdentifier(entityType(), identifier);
 }
 
 std::string PrimarySkill::encode(const si32 index)
@@ -399,15 +435,13 @@ std::string PrimarySkill::entityType()
 
 si32 FactionID::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), entityType(), identifier);
+	return resolveIdentifier(entityType(), identifier);
-	if(rawId)
-		return rawId.value();
-	else
-		return FactionID::DEFAULT.getNum();
 }
 
 std::string FactionID::encode(const si32 index)
 {
+	if (index == -1)
+		return "";
 	return VLC->factions()->getByIndex(index)->getJsonKey();
 }
 
@@ -423,15 +457,18 @@ const Faction * FactionID::toEntity(const Services * service) const
 
 si32 TerrainId::decode(const std::string & identifier)
 {
-	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), entityType(), identifier);
+	if (identifier == "native")
-	if(rawId)
+		return TerrainId::NATIVE_TERRAIN;
-		return rawId.value();
+
-	else
+	return resolveIdentifier(entityType(), identifier);
-		return static_cast<si32>(TerrainId::NONE);
 }
 
 std::string TerrainId::encode(const si32 index)
 {
+	if (index == TerrainId::NONE)
+		return "";
+	if (index == TerrainId::NATIVE_TERRAIN)
+		return "native";
 	return VLC->terrainTypeHandler->getByIndex(index)->getJsonKey();
 }
 
@@ -440,6 +477,46 @@ std::string TerrainId::entityType()
 	return "terrain";
 }
 
+si32 RoadId::decode(const std::string & identifier)
+{
+	if (identifier.empty())
+		return RoadId::NO_ROAD.getNum();
+
+	return resolveIdentifier(entityType(), identifier);
+}
+
+std::string RoadId::encode(const si32 index)
+{
+	if (index == RoadId::NO_ROAD.getNum())
+		return "";
+	return VLC->roadTypeHandler->getByIndex(index)->getJsonKey();
+}
+
+std::string RoadId::entityType()
+{
+	return "road";
+}
+
+si32 RiverId::decode(const std::string & identifier)
+{
+	if (identifier.empty())
+		return RiverId::NO_RIVER.getNum();
+
+	return resolveIdentifier(entityType(), identifier);
+}
+
+std::string RiverId::encode(const si32 index)
+{
+	if (index == RiverId::NO_RIVER.getNum())
+		return "";
+	return VLC->riverTypeHandler->getByIndex(index)->getJsonKey();
+}
+
+std::string RiverId::entityType()
+{
+	return "river";
+}
+
 const TerrainType * TerrainId::toEntity(const Services * service) const
 {
 	return VLC->terrainTypeHandler->getByIndex(num);
@@ -469,7 +546,7 @@ const ObstacleInfo * Obstacle::getInfo() const
 
 si32 SpellSchool::decode(const std::string & identifier)
 {
-	return *VLC->identifiers()->getIdentifier(ModScope::scopeGame(), entityType(), identifier);
+	return resolveIdentifier(entityType(), identifier);
 }
 
 std::string SpellSchool::encode(const si32 index)
@@ -487,7 +564,7 @@ std::string SpellSchool::entityType()
 
 si32 GameResID::decode(const std::string & identifier)
 {
-	return *VLC->identifiers()->getIdentifier(ModScope::scopeGame(), entityType(), identifier);
+	return resolveIdentifier(entityType(), identifier);
 }
 
 std::string GameResID::encode(const si32 index)

lib/constants/EntityIdentifiers.h

@@ -43,153 +43,50 @@ class CSkill;
 class CGameInfoCallback;
 class CNonConstInfoCallback;
 
-///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+class ArtifactInstanceID : public StaticIdentifier<ArtifactInstanceID>
-
-// Note: use template to force different type, blocking any Identifier<A> <=> Identifier<B> comparisons
-template<typename FinalClass>
-class Identifier : public IdentifierBase
 {
-	using BaseClass = IdentifierBase;
 public:
-	constexpr Identifier()
+	using StaticIdentifier<ArtifactInstanceID>::StaticIdentifier;
-	{}
-
-	explicit constexpr Identifier(int32_t value):
-		IdentifierBase(value)
-	{}
-
-	constexpr bool operator == (const Identifier & b) const { return BaseClass::num == b.num; }
-	constexpr bool operator <= (const Identifier & b) const { return BaseClass::num <= b.num; }
-	constexpr bool operator >= (const Identifier & b) const { return BaseClass::num >= b.num; }
-	constexpr bool operator != (const Identifier & b) const { return BaseClass::num != b.num; }
-	constexpr bool operator <  (const Identifier & b) const { return BaseClass::num <  b.num; }
-	constexpr bool operator >  (const Identifier & b) const { return BaseClass::num >  b.num; }
-
-	constexpr FinalClass & operator++()
-	{
-		++BaseClass::num;
-		return static_cast<FinalClass&>(*this);
-	}
-
-	constexpr FinalClass & operator--()
-	{
-		--BaseClass::num;
-		return static_cast<FinalClass&>(*this);
-	}
-
-	constexpr FinalClass operator--(int)
-	{
-		FinalClass ret(num);
-		--BaseClass::num;
-		return ret;
-	}
-
-	constexpr FinalClass operator++(int)
-	{
-		FinalClass ret(num);
-		++BaseClass::num;
-		return ret;
-	}
 };
 
-///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+class QueryID : public StaticIdentifier<QueryID>
-
-template<typename FinalClass, typename BaseClass>
-class IdentifierWithEnum : public BaseClass
-{
-	using EnumType = typename BaseClass::Type;
-
-	static_assert(std::is_same_v<std::underlying_type_t<EnumType>, int32_t>, "Entity Identifier must use int32_t");
-public:
-	constexpr EnumType toEnum() const
-	{
-		return static_cast<EnumType>(BaseClass::num);
-	}
-
-	constexpr IdentifierWithEnum(const EnumType & enumValue)
-	{
-		BaseClass::num = static_cast<int32_t>(enumValue);
-	}
-
-	constexpr IdentifierWithEnum(int32_t _num = -1)
-	{
-		BaseClass::num = _num;
-	}
-
-	constexpr bool operator == (const EnumType & b) const { return BaseClass::num == static_cast<int32_t>(b); }
-	constexpr bool operator <= (const EnumType & b) const { return BaseClass::num <= static_cast<int32_t>(b); }
-	constexpr bool operator >= (const EnumType & b) const { return BaseClass::num >= static_cast<int32_t>(b); }
-	constexpr bool operator != (const EnumType & b) const { return BaseClass::num != static_cast<int32_t>(b); }
-	constexpr bool operator <  (const EnumType & b) const { return BaseClass::num <  static_cast<int32_t>(b); }
-	constexpr bool operator >  (const EnumType & b) const { return BaseClass::num >  static_cast<int32_t>(b); }
-
-	constexpr bool operator == (const IdentifierWithEnum & b) const { return BaseClass::num == b.num; }
-	constexpr bool operator <= (const IdentifierWithEnum & b) const { return BaseClass::num <= b.num; }
-	constexpr bool operator >= (const IdentifierWithEnum & b) const { return BaseClass::num >= b.num; }
-	constexpr bool operator != (const IdentifierWithEnum & b) const { return BaseClass::num != b.num; }
-	constexpr bool operator <  (const IdentifierWithEnum & b) const { return BaseClass::num <  b.num; }
-	constexpr bool operator >  (const IdentifierWithEnum & b) const { return BaseClass::num >  b.num; }
-
-	constexpr FinalClass & operator++()
-	{
-		++BaseClass::num;
-		return static_cast<FinalClass&>(*this);
-	}
-
-	constexpr FinalClass operator++(int)
-	{
-		FinalClass ret(BaseClass::num);
-		++BaseClass::num;
-		return ret;
-	}
-};
-
-///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
-
-class ArtifactInstanceID : public Identifier<ArtifactInstanceID>
 {
 public:
-	using Identifier<ArtifactInstanceID>::Identifier;
+	using StaticIdentifier<QueryID>::StaticIdentifier;
-};
-
-class QueryID : public Identifier<QueryID>
-{
-public:
-	using Identifier<QueryID>::Identifier;
 	DLL_LINKAGE static const QueryID NONE;
 	DLL_LINKAGE static const QueryID CLIENT;
 };
 
-class BattleID : public Identifier<BattleID>
+class BattleID : public StaticIdentifier<BattleID>
 {
 public:
-	using Identifier<BattleID>::Identifier;
+	using StaticIdentifier<BattleID>::StaticIdentifier;
 	DLL_LINKAGE static const BattleID NONE;
 };
-class DLL_LINKAGE ObjectInstanceID : public Identifier<ObjectInstanceID>
+class DLL_LINKAGE ObjectInstanceID : public StaticIdentifier<ObjectInstanceID>
 {
 public:
-	using Identifier<ObjectInstanceID>::Identifier;
+	using StaticIdentifier<ObjectInstanceID>::StaticIdentifier;
 	static const ObjectInstanceID NONE;
 
 	static si32 decode(const std::string & identifier);
 	static std::string encode(const si32 index);
 };
 
-class HeroClassID : public Identifier<HeroClassID>
+class HeroClassID : public EntityIdentifier<HeroClassID>
 {
 public:
-	using Identifier<HeroClassID>::Identifier;
+	using EntityIdentifier<HeroClassID>::EntityIdentifier;
 	///json serialization helpers
 	DLL_LINKAGE static si32 decode(const std::string & identifier);
 	DLL_LINKAGE static std::string encode(const si32 index);
 	static std::string entityType();
 };
 
-class DLL_LINKAGE HeroTypeID : public Identifier<HeroTypeID>
+class DLL_LINKAGE HeroTypeID : public EntityIdentifier<HeroTypeID>
 {
 public:
-	using Identifier<HeroTypeID>::Identifier;
+	using EntityIdentifier<HeroTypeID>::EntityIdentifier;
 	///json serialization helpers
 	static si32 decode(const std::string & identifier);
 	static std::string encode(const si32 index);
@@ -207,10 +104,10 @@ public:
 	}
 };
 
-class SlotID : public Identifier<SlotID>
+class SlotID : public StaticIdentifier<SlotID>
 {
 public:
-	using Identifier<SlotID>::Identifier;
+	using StaticIdentifier<SlotID>::StaticIdentifier;
 
 	DLL_LINKAGE static const SlotID COMMANDER_SLOT_PLACEHOLDER;
 	DLL_LINKAGE static const SlotID SUMMONED_SLOT_PLACEHOLDER; ///<for all summoned creatures, only during battle
@@ -223,10 +120,10 @@ public:
 	}
 };
 
-class DLL_LINKAGE PlayerColor : public Identifier<PlayerColor>
+class DLL_LINKAGE PlayerColor : public StaticIdentifier<PlayerColor>
 {
 public:
-	using Identifier<PlayerColor>::Identifier;
+	using StaticIdentifier<PlayerColor>::StaticIdentifier;
 
 	enum EPlayerColor
 	{
@@ -249,18 +146,18 @@ public:
 	static std::string entityType();
 };
 
-class TeamID : public Identifier<TeamID>
+class TeamID : public StaticIdentifier<TeamID>
 {
 public:
-	using Identifier<TeamID>::Identifier;
+	using StaticIdentifier<TeamID>::StaticIdentifier;
 
 	DLL_LINKAGE static const TeamID NO_TEAM;
 };
 
-class TeleportChannelID : public Identifier<TeleportChannelID>
+class TeleportChannelID : public StaticIdentifier<TeleportChannelID>
 {
 public:
-	using Identifier<TeleportChannelID>::Identifier;
+	using StaticIdentifier<TeleportChannelID>::StaticIdentifier;
 };
 
 class SecondarySkillBase : public IdentifierBase
@@ -302,10 +199,10 @@ public:
 	static_assert(GameConstants::SKILL_QUANTITY == SKILL_SIZE, "Incorrect number of skills");
 };
 
-class SecondarySkill : public IdentifierWithEnum<SecondarySkill, SecondarySkillBase>
+class DLL_LINKAGE SecondarySkill : public EntityIdentifierWithEnum<SecondarySkill, SecondarySkillBase>
 {
 public:
-	using IdentifierWithEnum<SecondarySkill, SecondarySkillBase>::IdentifierWithEnum;
+	using EntityIdentifierWithEnum<SecondarySkill, SecondarySkillBase>::EntityIdentifierWithEnum;
 	static std::string entityType();
 	static si32 decode(const std::string& identifier);
 	static std::string encode(const si32 index);
@@ -314,10 +211,10 @@ public:
 	const Skill * toEntity(const Services * services) const;
 };
 
-class DLL_LINKAGE PrimarySkill : public Identifier<PrimarySkill>
+class DLL_LINKAGE PrimarySkill : public StaticIdentifier<PrimarySkill>
 {
 public:
-	using Identifier<PrimarySkill>::Identifier;
+	using StaticIdentifier<PrimarySkill>::StaticIdentifier;
 
 	static const PrimarySkill NONE;
 	static const PrimarySkill ATTACK;
@@ -335,10 +232,10 @@ public:
 	static std::string entityType();
 };
 
-class DLL_LINKAGE FactionID : public Identifier<FactionID>
+class DLL_LINKAGE FactionID : public EntityIdentifier<FactionID>
 {
 public:
-	using Identifier<FactionID>::Identifier;
+	using EntityIdentifier<FactionID>::EntityIdentifier;
 
 	static const FactionID NONE;
 	static const FactionID DEFAULT;
@@ -413,10 +310,10 @@ public:
 	}
 };
 
-class DLL_LINKAGE BuildingID : public IdentifierWithEnum<BuildingID, BuildingIDBase>
+class DLL_LINKAGE BuildingID : public StaticIdentifierWithEnum<BuildingID, BuildingIDBase>
 {
 public:
-	using IdentifierWithEnum<BuildingID, BuildingIDBase>::IdentifierWithEnum;
+	using StaticIdentifierWithEnum<BuildingID, BuildingIDBase>::StaticIdentifierWithEnum;
 
 	static BuildingID HALL_LEVEL(unsigned int level)
 	{
@@ -578,10 +475,10 @@ public:
 	};
 };
 
-class DLL_LINKAGE MapObjectID : public IdentifierWithEnum<MapObjectID, MapObjectBaseID>
+class DLL_LINKAGE MapObjectID : public EntityIdentifierWithEnum<MapObjectID, MapObjectBaseID>
 {
 public:
-	using IdentifierWithEnum<MapObjectID, MapObjectBaseID>::IdentifierWithEnum;
+	using EntityIdentifierWithEnum<MapObjectID, MapObjectBaseID>::EntityIdentifierWithEnum;
 
 	static std::string encode(int32_t index);
 	static si32 decode(const std::string & identifier);
@@ -593,7 +490,7 @@ public:
 	}
 };
 
-class MapObjectSubID : public Identifier<MapObjectSubID>
+class DLL_LINKAGE MapObjectSubID : public Identifier<MapObjectSubID>
 {
 public:
 	constexpr MapObjectSubID(const IdentifierBase & value):
@@ -615,17 +512,37 @@ public:
 		return *this;
 	}
 
+	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 Identifier<RoadId>
+class DLL_LINKAGE RoadId : public EntityIdentifier<RoadId>
 {
 public:
-	using Identifier<RoadId>::Identifier;
+	using EntityIdentifier<RoadId>::EntityIdentifier;
+	static si32 decode(const std::string & identifier);
+	static std::string encode(const si32 index);
+	static std::string entityType();
 
 	static const RoadId NO_ROAD;
 	static const RoadId DIRT_ROAD;
@@ -635,10 +552,13 @@ public:
 	const RoadType * toEntity(const Services * service) const;
 };
 
-class DLL_LINKAGE RiverId : public Identifier<RiverId>
+class DLL_LINKAGE RiverId : public EntityIdentifier<RiverId>
 {
 public:
-	using Identifier<RiverId>::Identifier;
+	using EntityIdentifier<RiverId>::EntityIdentifier;
+	static si32 decode(const std::string & identifier);
+	static std::string encode(const si32 index);
+	static std::string entityType();
 
 	static const RiverId NO_RIVER;
 	static const RiverId WATER_RIVER;
@@ -658,10 +578,10 @@ public:
 	};
 };
 
-class EPathfindingLayer : public IdentifierWithEnum<EPathfindingLayer, EPathfindingLayerBase>
+class EPathfindingLayer : public StaticIdentifierWithEnum<EPathfindingLayer, EPathfindingLayerBase>
 {
 public:
-	using IdentifierWithEnum<EPathfindingLayer, EPathfindingLayerBase>::IdentifierWithEnum;
+	using StaticIdentifierWithEnum<EPathfindingLayer, EPathfindingLayerBase>::StaticIdentifierWithEnum;
 };
 
 class ArtifactPositionBase : public IdentifierBase
@@ -694,10 +614,10 @@ public:
 	DLL_LINKAGE static std::string encode(const si32 index);
 };
 
-class ArtifactPosition : public IdentifierWithEnum<ArtifactPosition, ArtifactPositionBase>
+class ArtifactPosition : public StaticIdentifierWithEnum<ArtifactPosition, ArtifactPositionBase>
 {
 public:
-	using IdentifierWithEnum<ArtifactPosition, ArtifactPositionBase>::IdentifierWithEnum;
+	using StaticIdentifierWithEnum<ArtifactPosition, ArtifactPositionBase>::StaticIdentifierWithEnum;
 
 	// TODO: Remove
 	constexpr operator int32_t () const
@@ -730,10 +650,10 @@ public:
 	DLL_LINKAGE const Artifact * toEntity(const Services * service) const;
 };
 
-class ArtifactID : public IdentifierWithEnum<ArtifactID, ArtifactIDBase>
+class ArtifactID : public EntityIdentifierWithEnum<ArtifactID, ArtifactIDBase>
 {
 public:
-	using IdentifierWithEnum<ArtifactID, ArtifactIDBase>::IdentifierWithEnum;
+	using EntityIdentifierWithEnum<ArtifactID, ArtifactIDBase>::EntityIdentifierWithEnum;
 
 	///json serialization helpers
 	DLL_LINKAGE static si32 decode(const std::string & identifier);
@@ -773,10 +693,10 @@ public:
 	DLL_LINKAGE const Creature * toEntity(const CreatureService * creatures) const;
 };
 
-class DLL_LINKAGE CreatureID : public IdentifierWithEnum<CreatureID, CreatureIDBase>
+class DLL_LINKAGE CreatureID : public EntityIdentifierWithEnum<CreatureID, CreatureIDBase>
 {
 public:
-	using IdentifierWithEnum<CreatureID, CreatureIDBase>::IdentifierWithEnum;
+	using EntityIdentifierWithEnum<CreatureID, CreatureIDBase>::EntityIdentifierWithEnum;
 
 	///json serialization helpers
 	static si32 decode(const std::string & identifier);
@@ -892,10 +812,10 @@ public:
 	const spells::Spell * toEntity(const spells::Service * service) const;
 };
 
-class DLL_LINKAGE SpellID : public IdentifierWithEnum<SpellID, SpellIDBase>
+class DLL_LINKAGE SpellID : public EntityIdentifierWithEnum<SpellID, SpellIDBase>
 {
 public:
-	using IdentifierWithEnum<SpellID, SpellIDBase>::IdentifierWithEnum;
+	using EntityIdentifierWithEnum<SpellID, SpellIDBase>::EntityIdentifierWithEnum;
 
 	///json serialization helpers
 	static si32 decode(const std::string & identifier);
@@ -904,10 +824,10 @@ public:
 };
 
 class BattleFieldInfo;
-class DLL_LINKAGE BattleField : public Identifier<BattleField>
+class DLL_LINKAGE BattleField : public EntityIdentifier<BattleField>
 {
 public:
-	using Identifier<BattleField>::Identifier;
+	using EntityIdentifier<BattleField>::EntityIdentifier;
 
 	static const BattleField NONE;
 	const BattleFieldInfo * getInfo() const;
@@ -916,10 +836,13 @@ public:
 	static std::string encode(const si32 index);
 };
 
-class DLL_LINKAGE BoatId : public Identifier<BoatId>
+class DLL_LINKAGE BoatId : public EntityIdentifier<BoatId>
 {
 public:
-	using Identifier<BoatId>::Identifier;
+	using EntityIdentifier<BoatId>::EntityIdentifier;
+
+	static si32 decode(const std::string & identifier);
+	static std::string encode(const si32 index);
 
 	static const BoatId NONE;
 	static const BoatId NECROPOLIS;
@@ -950,29 +873,29 @@ public:
 	};
 };
 
-class TerrainId : public IdentifierWithEnum<TerrainId, TerrainIdBase>
+class DLL_LINKAGE TerrainId : public EntityIdentifierWithEnum<TerrainId, TerrainIdBase>
 {
 public:
-	using IdentifierWithEnum<TerrainId, TerrainIdBase>::IdentifierWithEnum;
+	using EntityIdentifierWithEnum<TerrainId, TerrainIdBase>::EntityIdentifierWithEnum;
 
-	DLL_LINKAGE static si32 decode(const std::string & identifier);
+	static si32 decode(const std::string & identifier);
-	DLL_LINKAGE static std::string encode(const si32 index);
+	static std::string encode(const si32 index);
 	static std::string entityType();
 	const TerrainType * toEntity(const Services * service) const;
 };
 
 class ObstacleInfo;
-class Obstacle : public Identifier<Obstacle>
+class Obstacle : public EntityIdentifier<Obstacle>
 {
 public:
-	using Identifier<Obstacle>::Identifier;
+	using EntityIdentifier<Obstacle>::EntityIdentifier;
 	DLL_LINKAGE const ObstacleInfo * getInfo() const;
 };
 
-class DLL_LINKAGE SpellSchool : public Identifier<SpellSchool>
+class DLL_LINKAGE SpellSchool : public StaticIdentifier<SpellSchool>
 {
 public:
-	using Identifier<SpellSchool>::Identifier;
+	using StaticIdentifier<SpellSchool>::StaticIdentifier;
 
 	static const SpellSchool ANY;
 	static const SpellSchool AIR;
@@ -1005,10 +928,10 @@ public:
 	};
 };
 
-class DLL_LINKAGE GameResID : public IdentifierWithEnum<GameResID, GameResIDBase>
+class DLL_LINKAGE GameResID : public StaticIdentifierWithEnum<GameResID, GameResIDBase>
 {
 public:
-	using IdentifierWithEnum<GameResID, GameResIDBase>::IdentifierWithEnum;
+	using StaticIdentifierWithEnum<GameResID, GameResIDBase>::StaticIdentifierWithEnum;
 
 	static si32 decode(const std::string & identifier);
 	static std::string encode(const si32 index);
@@ -1029,12 +952,25 @@ public:
 	FactionID getFaction() const;
 
 	using Identifier<BuildingTypeUniqueID>::Identifier;
+
+	template <typename Handler>
+	void serialize(Handler & h, const int version)
+	{
+		FactionID faction = getFaction();
+		BuildingID building = getBuilding();
+
+		h & faction;
+		h & building;
+
+		if (!h.saving)
+			*this = BuildingTypeUniqueID(faction, building);
+	}
 };
 
-class DLL_LINKAGE CampaignScenarioID : public Identifier<CampaignScenarioID>
+class DLL_LINKAGE CampaignScenarioID : public StaticIdentifier<CampaignScenarioID>
 {
 public:
-	using Identifier<CampaignScenarioID>::Identifier;
+	using StaticIdentifier<CampaignScenarioID>::StaticIdentifier;
 
 	static si32 decode(const std::string & identifier);
 	static std::string encode(int32_t index);

lib/constants/IdentifierBase.h

@@ -9,6 +9,19 @@
  */
 #pragma once
 
+VCMI_LIB_NAMESPACE_BEGIN
+
+class IdentifierResolutionException : public std::runtime_error
+{
+public:
+	const std::string identifierName;
+
+	IdentifierResolutionException(const std::string & identifierName )
+		: std::runtime_error("Failed to resolve identifier " + identifierName)
+		, identifierName(identifierName)
+	{}
+};
+
 class IdentifierBase
 {
 protected:
@@ -21,6 +34,11 @@ protected:
 	{}
 
 	~IdentifierBase() = default;
+
+	/// Attempts to resolve identifier using provided entity type
+	/// Returns resolved numeric identifier
+	/// Throws IdentifierResolutionException on failure
+	static int32_t resolveIdentifier(const std::string & entityType, const std::string identifier);
 public:
 	int32_t num;
 
@@ -42,11 +60,6 @@ public:
 		}
 	};
 
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & num;
-	}
-
 	constexpr void advance(int change)
 	{
 		num += change;
@@ -62,3 +75,180 @@ public:
 		return os << dt.num;
 	}
 };
+
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+// Note: use template to force different type, blocking any Identifier<A> <=> Identifier<B> comparisons
+template<typename FinalClass>
+class Identifier : public IdentifierBase
+{
+	using BaseClass = IdentifierBase;
+public:
+	constexpr Identifier()
+	{}
+
+	explicit constexpr Identifier(int32_t value):
+		IdentifierBase(value)
+	{}
+
+	constexpr bool operator == (const Identifier & b) const { return BaseClass::num == b.num; }
+	constexpr bool operator <= (const Identifier & b) const { return BaseClass::num <= b.num; }
+	constexpr bool operator >= (const Identifier & b) const { return BaseClass::num >= b.num; }
+	constexpr bool operator != (const Identifier & b) const { return BaseClass::num != b.num; }
+	constexpr bool operator <  (const Identifier & b) const { return BaseClass::num <  b.num; }
+	constexpr bool operator >  (const Identifier & b) const { return BaseClass::num >  b.num; }
+
+	constexpr FinalClass & operator++()
+	{
+		++BaseClass::num;
+		return static_cast<FinalClass&>(*this);
+	}
+
+	constexpr FinalClass & operator--()
+	{
+		--BaseClass::num;
+		return static_cast<FinalClass&>(*this);
+	}
+
+	constexpr FinalClass operator--(int)
+	{
+		FinalClass ret(num);
+		--BaseClass::num;
+		return ret;
+	}
+
+	constexpr FinalClass operator++(int)
+	{
+		FinalClass ret(num);
+		++BaseClass::num;
+		return ret;
+	}
+};
+
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+template<typename FinalClass, typename BaseClass>
+class IdentifierWithEnum : public BaseClass
+{
+	using EnumType = typename BaseClass::Type;
+
+	static_assert(std::is_same_v<std::underlying_type_t<EnumType>, int32_t>, "Entity Identifier must use int32_t");
+public:
+	constexpr EnumType toEnum() const
+	{
+		return static_cast<EnumType>(BaseClass::num);
+	}
+
+	constexpr IdentifierWithEnum(const EnumType & enumValue)
+	{
+		BaseClass::num = static_cast<int32_t>(enumValue);
+	}
+
+	constexpr IdentifierWithEnum(int32_t _num = -1)
+	{
+		BaseClass::num = _num;
+	}
+
+	constexpr bool operator == (const EnumType & b) const { return BaseClass::num == static_cast<int32_t>(b); }
+	constexpr bool operator <= (const EnumType & b) const { return BaseClass::num <= static_cast<int32_t>(b); }
+	constexpr bool operator >= (const EnumType & b) const { return BaseClass::num >= static_cast<int32_t>(b); }
+	constexpr bool operator != (const EnumType & b) const { return BaseClass::num != static_cast<int32_t>(b); }
+	constexpr bool operator <  (const EnumType & b) const { return BaseClass::num <  static_cast<int32_t>(b); }
+	constexpr bool operator >  (const EnumType & b) const { return BaseClass::num >  static_cast<int32_t>(b); }
+
+	constexpr bool operator == (const IdentifierWithEnum & b) const { return BaseClass::num == b.num; }
+	constexpr bool operator <= (const IdentifierWithEnum & b) const { return BaseClass::num <= b.num; }
+	constexpr bool operator >= (const IdentifierWithEnum & b) const { return BaseClass::num >= b.num; }
+	constexpr bool operator != (const IdentifierWithEnum & b) const { return BaseClass::num != b.num; }
+	constexpr bool operator <  (const IdentifierWithEnum & b) const { return BaseClass::num <  b.num; }
+	constexpr bool operator >  (const IdentifierWithEnum & b) const { return BaseClass::num >  b.num; }
+
+	constexpr FinalClass & operator++()
+	{
+		++BaseClass::num;
+		return static_cast<FinalClass&>(*this);
+	}
+
+	constexpr FinalClass operator++(int)
+	{
+		FinalClass ret(BaseClass::num);
+		++BaseClass::num;
+		return ret;
+	}
+};
+
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+template<typename FinalClass>
+class EntityIdentifier : public Identifier<FinalClass>
+{
+public:
+	using Identifier<FinalClass>::Identifier;
+
+	template <typename Handler>
+	void serialize(Handler &h, const int version)
+	{
+		auto * finalClass = static_cast<FinalClass*>(this);
+		std::string value;
+
+		if (h.saving)
+			value = FinalClass::encode(finalClass->num);
+
+		h & value;
+
+		if (!h.saving)
+			finalClass->num = FinalClass::decode(value);
+	}
+};
+
+template<typename FinalClass, typename BaseClass>
+class EntityIdentifierWithEnum : public IdentifierWithEnum<FinalClass, BaseClass>
+{
+public:
+	using IdentifierWithEnum<FinalClass, BaseClass>::IdentifierWithEnum;
+
+	template <typename Handler>
+	void serialize(Handler &h, const int version)
+	{
+		auto * finalClass = static_cast<FinalClass*>(this);
+		std::string value;
+
+		if (h.saving)
+			value = FinalClass::encode(finalClass->num);
+
+		h & value;
+
+		if (!h.saving)
+			finalClass->num = FinalClass::decode(value);
+	}
+};
+
+template<typename FinalClass>
+class StaticIdentifier : public Identifier<FinalClass>
+{
+public:
+	using Identifier<FinalClass>::Identifier;
+
+	template <typename Handler>
+	void serialize(Handler &h, const int version)
+	{
+		auto * finalClass = static_cast<FinalClass*>(this);
+		h & finalClass->num;
+	}
+};
+
+template<typename FinalClass, typename BaseClass>
+class StaticIdentifierWithEnum : public IdentifierWithEnum<FinalClass, BaseClass>
+{
+public:
+	using IdentifierWithEnum<FinalClass, BaseClass>::IdentifierWithEnum;
+
+	template <typename Handler>
+	void serialize(Handler &h, const int version)
+	{
+		auto * finalClass = static_cast<FinalClass*>(this);
+		h & finalClass->num;
+	}
+};
+
+VCMI_LIB_NAMESPACE_END

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;
@@ -1411,9 +1406,9 @@ bool CGameState::checkForVictory(const PlayerColor & player, const EventConditio
 		}
 		case EventCondition::HAVE_BUILDING:
 		{
-			if (condition.object) // specific town
+			if (condition.objectID != ObjectInstanceID::NONE) // specific town
 			{
-				const auto * t = dynamic_cast<const CGTownInstance *>(condition.object);
+				const auto * t = getTown(condition.objectID);
 				return (t->tempOwner == player && t->hasBuilt(condition.objectType.as<BuildingID>()));
 			}
 			else // any town
@@ -1428,12 +1423,12 @@ bool CGameState::checkForVictory(const PlayerColor & player, const EventConditio
 		}
 		case EventCondition::DESTROY:
 		{
-			if (condition.object) // mode A - destroy specific object of this type
+			if (condition.objectID != ObjectInstanceID::NONE) // mode A - destroy specific object of this type
 			{
-				if(const auto * hero = dynamic_cast<const CGHeroInstance *>(condition.object))
+				if(const auto * hero = getHero(condition.objectID))
 					return boost::range::find(gs->map->heroesOnMap, hero) == gs->map->heroesOnMap.end();
 				else
-					return getObj(condition.object->id) == nullptr;
+					return getObj(condition.objectID) == nullptr;
 			}
 			else
 			{
@@ -1451,9 +1446,9 @@ bool CGameState::checkForVictory(const PlayerColor & player, const EventConditio
 			// NOTE: cgameinfocallback specified explicitly in order to get const version
 			const auto & team = CGameInfoCallback::getPlayerTeam(player)->players;
 
-			if (condition.object) // mode A - flag one specific object, like town
+			if (condition.objectID != ObjectInstanceID::NONE) // mode A - flag one specific object, like town
 			{
-				return team.count(condition.object->tempOwner) != 0;
+				return team.count(getObjInstance(condition.objectID)->tempOwner) != 0;
 			}
 			else
 			{
@@ -1468,7 +1463,7 @@ bool CGameState::checkForVictory(const PlayerColor & player, const EventConditio
 		}
 		case EventCondition::TRANSPORT:
 		{
-			const auto * t = dynamic_cast<const CGTownInstance *>(condition.object);
+			const auto * t = getTown(condition.objectID);
 			return (t->visitingHero && t->visitingHero->hasArt(condition.objectType.as<ArtifactID>())) ||
 				   (t->garrisonHero && t->garrisonHero->hasArt(condition.objectType.as<ArtifactID>()));
 		}

lib/mapObjects/CGHeroInstance.cpp

@@ -1793,11 +1793,8 @@ bool CGHeroInstance::isMissionCritical() const
 
 		auto const & testFunctor = [&](const EventCondition & condition)
 		{
-			if ((condition.condition == EventCondition::CONTROL) && condition.object)
+			if ((condition.condition == EventCondition::CONTROL) && condition.objectID != ObjectInstanceID::NONE)
-			{
+				return (id != condition.objectID);
-				const auto * hero = dynamic_cast<const CGHeroInstance *>(condition.object);
-				return (hero != this);
-			}
 
 			if(condition.condition == EventCondition::IS_HUMAN)
 				return true;

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

@@ -448,19 +448,19 @@ void CMap::checkForObjectives()
 
 				case EventCondition::HAVE_BUILDING:
 					if (isInTheMap(cond.position))
-						cond.object = getObjectiveObjectFrom(cond.position, Obj::TOWN);
+						cond.objectID = getObjectiveObjectFrom(cond.position, Obj::TOWN)->id;
 					break;
 
 				case EventCondition::CONTROL:
 					if (isInTheMap(cond.position))
-						cond.object = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>());
+						cond.objectID = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>())->id;
 
-					if (cond.object)
+					if (cond.objectID != ObjectInstanceID::NONE)
 					{
-						const auto * town = dynamic_cast<const CGTownInstance *>(cond.object);
+						const auto * town = dynamic_cast<const CGTownInstance *>(objects[cond.objectID].get());
 						if (town)
 							event.onFulfill.replaceRawString(town->getNameTranslated());
-						const auto * hero = dynamic_cast<const CGHeroInstance *>(cond.object);
+						const auto * hero = dynamic_cast<const CGHeroInstance *>(objects[cond.objectID].get());
 						if (hero)
 							event.onFulfill.replaceRawString(hero->getNameTranslated());
 					}
@@ -468,17 +468,17 @@ void CMap::checkForObjectives()
 
 				case EventCondition::DESTROY:
 					if (isInTheMap(cond.position))
-						cond.object = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>());
+						cond.objectID = getObjectiveObjectFrom(cond.position, cond.objectType.as<MapObjectID>())->id;
 
-					if (cond.object)
+					if (cond.objectID != ObjectInstanceID::NONE)
 					{
-						const auto * hero = dynamic_cast<const CGHeroInstance *>(cond.object);
+						const auto * hero = dynamic_cast<const CGHeroInstance *>(objects[cond.objectID].get());
 						if (hero)
 							event.onFulfill.replaceRawString(hero->getNameTranslated());
 					}
 					break;
 				case EventCondition::TRANSPORT:
-					cond.object = getObjectiveObjectFrom(cond.position, Obj::TOWN);
+					cond.objectID = getObjectiveObjectFrom(cond.position, Obj::TOWN)->id;
 					break;
 				//break; case EventCondition::DAYS_PASSED:
 				//break; case EventCondition::IS_HUMAN:
@@ -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/mapping/CMapHeader.cpp

@@ -68,7 +68,6 @@ bool PlayerInfo::hasCustomMainHero() const
 }
 
 EventCondition::EventCondition(EWinLoseType condition):
-	object(nullptr),
 	value(-1),
 	position(-1, -1, -1),
 	condition(condition)
@@ -76,7 +75,6 @@ EventCondition::EventCondition(EWinLoseType condition):
 }
 
 EventCondition::EventCondition(EWinLoseType condition, si32 value, TargetTypeID objectType, const int3 & position):
-	object(nullptr),
 	value(value),
 	objectType(objectType),
 	position(position),

lib/mapping/CMapHeader.h

@@ -120,7 +120,7 @@ struct DLL_LINKAGE EventCondition
 	EventCondition(EWinLoseType condition = STANDARD_WIN);
 	EventCondition(EWinLoseType condition, si32 value, TargetTypeID objectType, const int3 & position = int3(-1, -1, -1));
 
-	const CGObjectInstance * object; // object that was at specified position or with instance name on start
+	ObjectInstanceID objectID; // object that was at specified position or with instance name on start
 	si32 value;
 	TargetTypeID objectType;
 	std::string objectInstanceName;
@@ -130,7 +130,7 @@ struct DLL_LINKAGE EventCondition
 	template <typename Handler>
 	void serialize(Handler & h, const int version)
 	{
-		h & object;
+		h & objectID;
 		h & value;
 		h & objectType;
 		h & position;

lib/mapping/MapIdentifiersH3M.cpp

@@ -37,6 +37,9 @@ void MapIdentifiersH3M::loadMapping(std::map<IdentifierID, IdentifierID> & resul
 
 void MapIdentifiersH3M::loadMapping(const JsonNode & mapping)
 {
+	if (!mapping["supported"].Bool())
+		throw std::runtime_error("Unsupported map format!");
+
 	for (auto entryFaction : mapping["buildings"].Struct())
 	{
 		FactionID factionID (*VLC->identifiers()->getIdentifier(entryFaction.second.meta, "faction", entryFaction.first));

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;
@@ -1215,7 +1221,7 @@ void RemoveObject::applyGs(CGameState *gs)
 	{
 		auto patcher = [&](EventCondition cond) -> EventExpression::Variant
 		{
-			if (cond.object == obj)
+			if (cond.objectID == obj->id)
 			{
 				if (cond.condition == EventCondition::DESTROY)
 				{
@@ -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
@@ -53,10 +52,10 @@ enum class ObjProperty : int8_t
 	REWARD_CLEARED
 };
 
-class NumericID : public Identifier<NumericID>
+class NumericID : public StaticIdentifier<NumericID>
 {
 public:
-	using Identifier<NumericID>::Identifier;
+	using StaticIdentifier<NumericID>::StaticIdentifier;
 
 	static si32 decode(const std::string & identifier)
 	{

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)

lib/serializer/CSerializer.h

@@ -14,8 +14,8 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-const ui32 SERIALIZATION_VERSION = 830;
+const ui32 SERIALIZATION_VERSION = 831;
-const ui32 MINIMAL_SERIALIZATION_VERSION = 830;
+const ui32 MINIMAL_SERIALIZATION_VERSION = 831;
 const std::string SAVEGAME_MAGIC = "VCMISVG";
 
 class CHero;

server/CGameHandler.cpp

@@ -1802,9 +1802,20 @@ bool CGameHandler::load(const std::string & filename)
 		lobby->announceMessage(errorMsg);
 		return false;
 	}
+	catch(const IdentifierResolutionException & e)
+	{
+		logGlobal->error("Failed to load game: %s", e.what());
+		MetaString errorMsg;
+		errorMsg.appendTextID("vcmi.server.errors.unknownEntity");
+		errorMsg.replaceRawString(e.identifierName);
+		lobby->announceMessage(errorMsg.toString());//FIXME: should be localized on client side
+		return false;
+	}
+
 	catch(const std::exception & e)
 	{
 		logGlobal->error("Failed to load game: %s", e.what());
+		lobby->announceMessage(std::string("Failed to load game: ") + e.what());
 		return false;
 	}
 	gs->preInit(VLC);