Remove bitmasks of PlayerColor's. Add encode/decode methods

lib/constants/EntityIdentifiers.cpp

@@ -228,6 +228,16 @@ std::string PlayerColor::getStrCap(bool L10n) const
 	return ret;
 }
 
+si32 PlayerColor::decode(const std::string & identifier)
+{
+	return vstd::find_pos(GameConstants::PLAYER_COLOR_NAMES, identifier);
+}
+
+std::string PlayerColor::encode(const si32 index)
+{
+	return GameConstants::PLAYER_COLOR_NAMES[index];
+}
+
 si32 FactionID::decode(const std::string & identifier)
 {
 	auto rawId = VLC->identifiers()->getIdentifier(ModScope::scopeGame(), entityType(), identifier);

lib/constants/EntityIdentifiers.h

@@ -59,6 +59,11 @@ public:
 		}
 	};
 
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & num;
+	}
+
 	constexpr void advance(int change)
 	{
 		num += change;
@@ -87,11 +92,6 @@ public:
 		:IdentifierBase(_num)
 	{}
 
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & BaseClass::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; }
@@ -122,21 +122,11 @@ class IdentifierWithEnum : public BaseClass
 
 	static_assert(std::is_same_v<std::underlying_type_t<EnumType>, int32_t>, "Entity Identifier must use int32_t");
 public:
-	constexpr int32_t getNum() const
-	{
-		return BaseClass::num;
-	}
-
 	constexpr EnumType toEnum() const
 	{
 		return static_cast<EnumType>(BaseClass::num);
 	}
 
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & BaseClass::num;
-	}
-
 	constexpr IdentifierWithEnum(const EnumType & enumValue)
 	{
 		BaseClass::num = static_cast<int32_t>(enumValue);
@@ -230,7 +220,7 @@ public:
 	}
 };
 
-class PlayerColor : public Identifier<PlayerColor>
+class DLL_LINKAGE PlayerColor : public Identifier<PlayerColor>
 {
 public:
 	using Identifier<PlayerColor>::Identifier;
@@ -240,19 +230,20 @@ public:
 		PLAYER_LIMIT_I = 8,
 	};
 
-	using Mask = uint8_t;
+	static const PlayerColor SPECTATOR; //252
+	static const PlayerColor CANNOT_DETERMINE; //253
+	static const PlayerColor UNFLAGGABLE; //254 - neutral objects (pandora, banks)
+	static const PlayerColor NEUTRAL; //255
+	static const PlayerColor PLAYER_LIMIT; //player limit per map
 
-	DLL_LINKAGE static const PlayerColor SPECTATOR; //252
-	DLL_LINKAGE static const PlayerColor CANNOT_DETERMINE; //253
-	DLL_LINKAGE static const PlayerColor UNFLAGGABLE; //254 - neutral objects (pandora, banks)
-	DLL_LINKAGE static const PlayerColor NEUTRAL; //255
-	DLL_LINKAGE static const PlayerColor PLAYER_LIMIT; //player limit per map
+	bool isValidPlayer() const; //valid means < PLAYER_LIMIT (especially non-neutral)
+	bool isSpectator() const;
 
-	DLL_LINKAGE bool isValidPlayer() const; //valid means < PLAYER_LIMIT (especially non-neutral)
-	DLL_LINKAGE bool isSpectator() const;
+	std::string getStr(bool L10n = false) const;
+	std::string getStrCap(bool L10n = false) const;
 
-	DLL_LINKAGE std::string getStr(bool L10n = false) const;
-	DLL_LINKAGE std::string getStrCap(bool L10n = false) const;
+	static si32 decode(const std::string& identifier);
+	static std::string encode(const si32 index);
 };
 
 class TeamID : public Identifier<TeamID>

lib/gameState/TavernHeroesPool.cpp

@@ -74,7 +74,7 @@ void TavernHeroesPool::setHeroForPlayer(PlayerColor player, TavernHeroSlot slot,
 bool TavernHeroesPool::isHeroAvailableFor(HeroTypeID hero, PlayerColor color) const
 {
 	if (perPlayerAvailability.count(hero))
-		return perPlayerAvailability.at(hero) & (1 << color.getNum());
+		return perPlayerAvailability.at(hero).count(color) != 0;
 
 	return true;
 }
@@ -131,7 +131,7 @@ void TavernHeroesPool::addHeroToPool(CGHeroInstance * hero)
 	heroesPool[HeroTypeID(hero->subID)] = hero;
 }
 
-void TavernHeroesPool::setAvailability(HeroTypeID hero, PlayerColor::Mask mask)
+void TavernHeroesPool::setAvailability(HeroTypeID hero, std::set<PlayerColor> mask)
 {
 	perPlayerAvailability[hero] = mask;
 }

lib/gameState/TavernHeroesPool.h

@@ -44,7 +44,7 @@ class DLL_LINKAGE TavernHeroesPool
 
 	/// 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> perPlayerAvailability;
+	std::map<HeroTypeID, std::set<PlayerColor>> perPlayerAvailability;
 
 	/// list of heroes currently available in taverns
 	std::vector<TavernSlot> currentTavern;
@@ -71,7 +71,7 @@ public:
 	void addHeroToPool(CGHeroInstance * hero);
 
 	/// Marks hero as available to only specific set of players
-	void setAvailability(HeroTypeID hero, PlayerColor::Mask mask);
+	void setAvailability(HeroTypeID hero, std::set<PlayerColor> mask);
 
 	/// Makes hero available in tavern of specified player
 	void setHeroForPlayer(PlayerColor player, TavernHeroSlot slot, HeroTypeID hero, CSimpleArmy & army, TavernSlotRole role);

lib/mapObjects/CGPandoraBox.cpp

@@ -443,8 +443,9 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 
 void CGEvent::onHeroVisit( const CGHeroInstance * h ) const
 {
-	if(!(availableFor & (1 << h->tempOwner.getNum())))
+	if(availableFor.count(h->tempOwner) == 0)
 		return;
+
 	if(cb->getPlayerSettings(h->tempOwner)->isControlledByHuman())
 	{
 		if(humanActivate)
@@ -490,20 +491,7 @@ void CGEvent::serializeJsonOptions(JsonSerializeFormat & handler)
 	handler.serializeBool<bool>("aIActivable", computerActivate, true, false, false);
 	handler.serializeBool<bool>("humanActivable", humanActivate, true, false, true);
 	handler.serializeBool<bool>("removeAfterVisit", removeAfterVisit, true, false, false);
-
-	{
-		auto decodePlayer = [](const std::string & id)->si32
-		{
-			return vstd::find_pos(GameConstants::PLAYER_COLOR_NAMES, id);
-		};
-
-		auto encodePlayer = [](si32 idx)->std::string
-		{
-			return GameConstants::PLAYER_COLOR_NAMES[idx];
-		};
-
-		handler.serializeIdArray<ui8, PlayerColor::PLAYER_LIMIT_I>("availableFor", availableFor, GameConstants::ALL_PLAYERS, decodePlayer, encodePlayer);
-    }
+	handler.serializeIdArray("availableFor", availableFor);
 }
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CGPandoraBox.h

@@ -72,7 +72,7 @@ class DLL_LINKAGE CGEvent : public CGPandoraBox  //event objects
 {
 public:
 	bool removeAfterVisit = false; //true if event is removed after occurring
-	ui8 availableFor = 0; //players whom this event is available for
+	std::set<PlayerColor> availableFor; //players whom this event is available for
 	bool computerActivate = false; //true if computer player can activate this event
 	bool humanActivate = false; //true if human player can activate this event
 

lib/mapping/CMap.cpp

@@ -35,7 +35,7 @@ void Rumor::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeString("text", text);
 }
 
-DisposedHero::DisposedHero() : heroId(0), portrait(255), players(0)
+DisposedHero::DisposedHero() : heroId(0), portrait(255)
 {
 
 }

lib/mapping/CMap.h

@@ -59,7 +59,7 @@ struct DLL_LINKAGE DisposedHero
 	HeroTypeID heroId;
 	HeroTypeID portrait; /// The portrait id of the hero, -1 is default.
 	std::string name;
-	PlayerColor::Mask players; /// Who can hire this hero (bitfield).
+	std::set<PlayerColor> players; /// Who can hire this hero (bitfield).
 
 	template <typename Handler>
 	void serialize(Handler & h, const int version)

lib/mapping/MapFormatH3M.cpp

@@ -692,7 +692,7 @@ void CMapLoaderH3M::readDisposedHeroes()
 			map->disposedHeroes[g].heroId = reader->readHero().getNum();
 			map->disposedHeroes[g].portrait = reader->readHeroPortrait();
 			map->disposedHeroes[g].name = readLocalizedString(TextIdentifier("header", "heroes", map->disposedHeroes[g].heroId));
-			map->disposedHeroes[g].players = reader->readUInt8();
+			reader->readBitmaskPlayers(map->disposedHeroes[g].players, false);
 		}
 	}
 }
@@ -995,7 +995,7 @@ CGObjectInstance * CMapLoaderH3M::readEvent(const int3 & mapPosition)
 
 	readBoxContent(object, mapPosition);
 
-	object->availableFor = reader->readUInt8();
+	reader->readBitmaskPlayers(object->availableFor, false);
 	object->computerActivate = reader->readBool();
 	object->removeAfterVisit = reader->readBool();
 

lib/mapping/MapFormatJson.cpp

@@ -729,18 +729,16 @@ void CMapFormatJson::readDisposedHeroes(JsonSerializeFormat & handler)
 	{
 		HeroTypeID type(HeroTypeID::decode(entry.first));
 
-		ui8 mask = 0;
+		std::set<PlayerColor> mask;
 
 		for(const JsonNode & playerData : entry.second["availableFor"].Vector())
 		{
 			PlayerColor player = PlayerColor(vstd::find_pos(GameConstants::PLAYER_COLOR_NAMES, playerData.String()));
 			if(player.isValidPlayer())
-			{
-				mask |= 1 << player.getNum();
-			}
+				mask.insert(player);
 		}
 
-		if(mask != 0 && mask != GameConstants::ALL_PLAYERS && type.getNum() >= 0)
+		if(!mask.empty() && mask.size() != PlayerColor::PLAYER_LIMIT_I && type.getNum() >= 0)
 		{
 			DisposedHero hero;
 
@@ -760,24 +758,14 @@ void CMapFormatJson::writeDisposedHeroes(JsonSerializeFormat & handler)
 
 	auto definitions = handler.enterStruct("predefinedHeroes");//DisposedHeroes are part of predefinedHeroes in VCMI map format
 
-	for(const DisposedHero & hero : map->disposedHeroes)
+	for(DisposedHero & hero : map->disposedHeroes)
 	{
 		std::string type = HeroTypeID::encode(hero.heroId);
 
 		auto definition = definitions->enterStruct(type);
 
 		JsonNode players(JsonNode::JsonType::DATA_VECTOR);
-
-		for(int playerNum = 0; playerNum < PlayerColor::PLAYER_LIMIT_I; playerNum++)
-		{
-			if((1 << playerNum) & hero.players)
-			{
-				JsonNode player(JsonNode::JsonType::DATA_STRING);
-				player.String() = GameConstants::PLAYER_COLOR_NAMES[playerNum];
-				players.Vector().push_back(player);
-			}
-		}
-		definition->serializeRaw("availableFor", players, std::nullopt);
+		definition->serializeIdArray("availableFor", hero.players);
 	}
 }
 

lib/mapping/MapReaderH3M.cpp

@@ -35,6 +35,12 @@ SpellID MapReaderH3M::remapIdentifier(const SpellID & identifier)
 	return identifier;
 }
 
+template<>
+PlayerColor MapReaderH3M::remapIdentifier(const PlayerColor & identifier)
+{
+	return identifier;
+}
+
 template<class Identifier>
 Identifier MapReaderH3M::remapIdentifier(const Identifier & identifier)
 {
@@ -227,6 +233,11 @@ void MapReaderH3M::readBitmaskFactions(std::set<FactionID> & dest, bool invert)
 	readBitmask(dest, features.factionsBytes, features.factionsCount, invert);
 }
 
+void MapReaderH3M::readBitmaskPlayers(std::set<PlayerColor> & dest, bool invert)
+{
+	readBitmask(dest, 1, 8, invert);
+}
+
 void MapReaderH3M::readBitmaskResources(std::set<GameResID> & dest, bool invert)
 {
 	readBitmask(dest, features.resourcesBytes, features.resourcesCount, invert);

lib/mapping/MapReaderH3M.h

@@ -47,6 +47,7 @@ public:
 
 	void readBitmaskBuildings(std::set<BuildingID> & dest, std::optional<FactionID> faction);
 	void readBitmaskFactions(std::set<FactionID> & dest, bool invert);
+	void readBitmaskPlayers(std::set<PlayerColor> & dest, bool invert);
 	void readBitmaskResources(std::set<GameResID> & dest, bool invert);
 	void readBitmaskHeroClassesSized(std::set<HeroClassID> & dest, bool invert);
 	void readBitmaskHeroes(std::vector<bool> & dest, bool invert);

lib/serializer/JsonSerializeFormat.h

@@ -335,81 +335,6 @@ public:
 		}
 	}
 
-	///si32-convertible identifier set <-> Json array of string
-	///Type U is only used for code & decode
-	///TODO: Auto deduce U based on T?
-	template <typename T, typename U = T>
-	void serializeIdArray(const std::string & fieldName, std::set<T> & value, const std::set<T> & defaultValue)
-	{
-		std::vector<si32> temp;
-
-		if(saving && value != defaultValue)
-		{
-			temp.reserve(value.size());
-
-			for(const T & vitem : value)
-			{
-				si32 item = static_cast<si32>(vitem);
-				temp.push_back(item);
-			}
-			serializeInternal(fieldName, temp, &U::decode, &U::encode);
-		}
-
-		if(!saving)
-		{
-			JsonNode node;
-			serializeRaw(fieldName, node, std::nullopt);
-			if(node.Vector().empty())
-			{
-				value = defaultValue;
-			}
-			else
-			{
-				value.clear();
-
-				for(const auto & id : node.Vector())
-				{
-					VLC->identifiers()->requestIdentifier(U::entityType(), id, [&value](int32_t identifier)
-					{
-						value.emplace(identifier);
-					});
-				}
-			}
-		}
-	}
-
-	///bitmask <-> Json array of string
-	template <typename T, int Size>
-	void serializeIdArray(const std::string & fieldName, T & value, const T & defaultValue, const TDecoder & decoder, const TEncoder & encoder)
-	{
-		static_assert(8 * sizeof(T) >= Size, "Mask size too small");
-
-		std::vector<si32> temp;
-		temp.reserve(Size);
-
-		if(saving && value != defaultValue)
-		{
-			for(si32 i = 0; i < Size; i++)
-				if(value & (1 << i))
-					temp.push_back(i);
-			serializeInternal(fieldName, temp, decoder, encoder);
-		}
-
-		if(!saving)
-		{
-			serializeInternal(fieldName, temp, decoder, encoder);
-
-			if(temp.empty())
-				value = defaultValue;
-			else
-			{
-				value = 0;
-				for(auto i : temp)
-					value |= (1 << i);
-			}
-		}
-	}
-
 	///si32-convertible instance identifier <-> Json string
 	template <typename T>
 	void serializeInstance(const std::string & fieldName, T & value, const T & defaultValue)