Minor fixes

client/lobby/CBonusSelection.cpp

@@ -161,7 +161,7 @@ void CBonusSelection::createBonusesIcons()
 			break;
 		case CampaignBonusType::BUILDING:
 		{
-			int faction = -1;
+			FactionID faction;
 			for(auto & elem : CSH->si->playerInfos)
 			{
 				if(elem.second.isControlledByHuman())

lib/CBuildingHandler.cpp

@@ -12,7 +12,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-BuildingID CBuildingHandler::campToERMU(int camp, int townType, const std::set<BuildingID> & builtBuildings)
+BuildingID CBuildingHandler::campToERMU(int camp, FactionID townType, const std::set<BuildingID> & builtBuildings)
 {
 	static const std::vector<BuildingID> campToERMU = 
 	{
@@ -47,13 +47,13 @@ BuildingID CBuildingHandler::campToERMU(int camp, int townType, const std::set<B
 
 		if (i < 5) // last two levels don't have reserved horde ID. Yet another H3C weirdeness
 		{
-			if (vstd::contains(hordeLvlsPerTType[townType], i))
+			if (vstd::contains(hordeLvlsPerTType[townType.getNum()], i))
 			{
 				if (camp == curPos)
 				{
-					if (hordeLvlsPerTType[townType][0] == i)
+					if (hordeLvlsPerTType[townType.getNum()][0] == i)
 					{
-						BuildingID dwellingID(BuildingID::DWELL_UP_FIRST + hordeLvlsPerTType[townType][0]);
+						BuildingID dwellingID(BuildingID::DWELL_UP_FIRST + hordeLvlsPerTType[townType.getNum()][0]);
 
 						if(vstd::contains(builtBuildings, dwellingID)) //if upgraded dwelling is built
 							return BuildingID::HORDE_1_UPGR;
@@ -62,9 +62,9 @@ BuildingID CBuildingHandler::campToERMU(int camp, int townType, const std::set<B
 					}
 					else
 					{
-						if(hordeLvlsPerTType[townType].size() > 1)
+						if(hordeLvlsPerTType[townType.getNum()].size() > 1)
 						{
-							BuildingID dwellingID(BuildingID::DWELL_UP_FIRST + hordeLvlsPerTType[townType][1]);
+							BuildingID dwellingID(BuildingID::DWELL_UP_FIRST + hordeLvlsPerTType[townType.getNum()][1]);
 
 							if(vstd::contains(builtBuildings, dwellingID)) //if upgraded dwelling is built
 								return BuildingID::HORDE_2_UPGR;

lib/CBuildingHandler.h

@@ -16,7 +16,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 class DLL_LINKAGE CBuildingHandler
 {
 public:
-	static BuildingID campToERMU(int camp, int townType, const std::set<BuildingID> & builtBuildings);
+	static BuildingID campToERMU(int camp, FactionID townType, const std::set<BuildingID> & builtBuildings);
 };
 
 VCMI_LIB_NAMESPACE_END

lib/CCreatureHandler.cpp

@@ -263,7 +263,7 @@ bool CCreature::isDoubleWide() const
  */
 bool CCreature::isGood () const
 {
-	return VLC->factions()->getByIndex(faction)->getAlignment() == EAlignment::GOOD;
+	return VLC->factions()->getById(faction)->getAlignment() == EAlignment::GOOD;
 }
 
 /**
@@ -272,7 +272,7 @@ bool CCreature::isGood () const
  */
 bool CCreature::isEvil () const
 {
-	return VLC->factions()->getByIndex(faction)->getAlignment() == EAlignment::EVIL;
+	return VLC->factions()->getById(faction)->getAlignment() == EAlignment::EVIL;
 }
 
 si32 CCreature::maxAmount(const TResources &res) const //how many creatures can be bought
@@ -1350,21 +1350,13 @@ CCreatureHandler::~CCreatureHandler()
 
 CreatureID CCreatureHandler::pickRandomMonster(CRandomGenerator & rand, int tier) const
 {
-	int r = 0;
-	if(tier == -1) //pick any allowed creature
-	{
-		do
-		{
-			r = (*RandomGeneratorUtil::nextItem(objects, rand))->getId();
-		} while (objects[r] && objects[r]->special); // find first "not special" creature
-	}
-	else
-	{
-		assert(vstd::iswithin(tier, 1, 7));
 	std::vector<CreatureID> allowed;
 	for(const auto & creature : objects)
 	{
-			if(!creature->special && creature->level == tier)
+		if(creature->special)
+			continue;
+
+		if (creature->level == tier || tier == -1)
 			allowed.push_back(creature->getId());
 	}
 
@@ -1375,9 +1367,6 @@ CreatureID CCreatureHandler::pickRandomMonster(CRandomGenerator & rand, int tier
 	}
 
 	return *RandomGeneratorUtil::nextItem(allowed, rand);
-	}
-	assert (r >= 0); //should always be, but it crashed
-	return CreatureID(r);
 }
 
 

lib/CHeroHandler.cpp

@@ -151,7 +151,7 @@ bool CHeroClass::isMagicHero() const
 
 EAlignment CHeroClass::getAlignment() const
 {
-	return VLC->factions()->getByIndex(faction)->getAlignment();
+	return VLC->factions()->getById(faction)->getAlignment();
 }
 
 int32_t CHeroClass::getIndex() const

lib/CSkillHandler.cpp

@@ -120,7 +120,7 @@ DLL_LINKAGE std::ostream & operator<<(std::ostream & out, const CSkill::LevelInf
 
 DLL_LINKAGE std::ostream & operator<<(std::ostream & out, const CSkill & skill)
 {
-	out << "Skill(" << (int)skill.id << "," << skill.identifier << "): [";
+	out << "Skill(" << skill.id.getNum() << "," << skill.identifier << "): [";
 	for(int i=0; i < skill.levels.size(); i++)
 		out << (i ? "," : "") << skill.levels[i];
 	return out << "]";
@@ -233,7 +233,7 @@ CSkill * CSkillHandler::loadFromJson(const std::string & scope, const JsonNode &
 		skillAtLevel.iconMedium = levelNode["images"]["medium"].String();
 		skillAtLevel.iconLarge = levelNode["images"]["large"].String();
 	}
-	logMod->debug("loaded secondary skill %s(%d)", identifier, (int)skill->id);
+	logMod->debug("loaded secondary skill %s(%d)", identifier, skill->id.getNum());
 
 	return skill;
 }

lib/constants/EntityIdentifiers.h

@@ -401,10 +401,6 @@ class BuildingID : public IdentifierWithEnum<BuildingID, BuildingIDBase>
 {
 public:
 	using IdentifierWithEnum<BuildingID, BuildingIDBase>::IdentifierWithEnum;
-
-	DLL_LINKAGE static si32 decode(const std::string & identifier);
-	DLL_LINKAGE static std::string encode(const si32 index);
-	static std::string entityType();
 };
 
 class MapObjectBaseID : public IdentifierBase

lib/gameState/CGameState.cpp

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

lib/mapObjects/CBank.cpp

@@ -271,7 +271,7 @@ void CBank::doVisit(const CGHeroInstance * hero) const
 				loot.appendRawString("%d %s");
 				loot.replaceNumber(bc->resources[it]);
 				loot.replaceLocalString(EMetaText::RES_NAMES, it);
-				cb->giveResource(hero->getOwner(), static_cast<EGameResID>(it), bc->resources[it]);
+				cb->giveResource(hero->getOwner(), it, bc->resources[it]);
 			}
 		}
 		//grant artifacts

lib/mapObjects/CGDwelling.cpp

@@ -344,7 +344,7 @@ void CGDwelling::updateGuards() const
 	//default condition - creatures are of level 5 or higher
 	for (auto creatureEntry : creatures)
 	{
-		if (VLC->creatures()->getByIndex(creatureEntry.second.at(0))->getLevel() >= 5 && ID != Obj::REFUGEE_CAMP)
+		if (VLC->creatures()->getById(creatureEntry.second.at(0))->getLevel() >= 5 && ID != Obj::REFUGEE_CAMP)
 		{
 			guarded = true;
 			break;

lib/mapObjects/CGHeroInstance.cpp

@@ -1679,7 +1679,7 @@ void CGHeroInstance::serializeCommonOptions(JsonSerializeFormat & handler)
 		{
 			auto addSkill = [this](const std::string & skillId, const std::string & levelId)
 			{
-				const int rawId = CSkillHandler::decodeSkill(skillId);
+				const int rawId = SecondarySkill::decode(skillId);
 				if(rawId < 0)
 				{
 					logGlobal->error("Invalid secondary skill %s", skillId);

lib/mapObjects/CGPandoraBox.cpp

@@ -257,7 +257,7 @@ void CGPandoraBox::serializeJsonOptions(JsonSerializeFormat & handler)
 				const std::string skillName = p.first;
 				const std::string levelId = p.second.String();
 				
-				const int rawId = CSkillHandler::decodeSkill(skillName);
+				const int rawId = SecondarySkill::decode(skillName);
 				if(rawId < 0)
 				{
 					logGlobal->error("Invalid secondary skill %s", skillName);

lib/mapObjects/CGTownInstance.cpp

@@ -584,7 +584,7 @@ void CGTownInstance::newTurn(CRandomGenerator & rand) const
 					
 					TQuantity count = creatureGrowth(i);
 					if (!count) // no dwelling
-						count = VLC->creatures()->getByIndex(c)->getGrowth();
+						count = VLC->creatures()->getById(c)->getGrowth();
 					
 					{//no lower tiers or above current month
 						

lib/mapObjects/MiscObjects.cpp

@@ -119,7 +119,7 @@ void CGMine::initObj(CRandomGenerator & rand)
 	}
 	else
 	{
-		producedResource = GameResID(getObjTypeIndex());
+		producedResource = GameResID(getObjTypeIndex().getNum());
 	}
 	producedQuantity = defaultResProduction();
 }
@@ -773,7 +773,7 @@ void CGArtifact::initObj(CRandomGenerator & rand)
 
 std::string CGArtifact::getObjectName() const
 {
-	return VLC->artifacts()->getByIndex(getArtifact())->getNameTranslated();
+	return VLC->artifacts()->getById(getArtifact())->getNameTranslated();
 }
 
 void CGArtifact::onHeroVisit(const CGHeroInstance * h) const

lib/mapping/CMap.cpp

@@ -72,7 +72,18 @@ void CCastleEvent::serializeJson(JsonSerializeFormat & handler)
 {
 	CMapEvent::serializeJson(handler);
 
-	handler.serializeIdArray("buildings", buildings);
+	// TODO: handler.serializeIdArray("buildings", buildings);
+	{
+		std::vector<BuildingID> temp(buildings.begin(), buildings.end());
+		auto a = handler.enterArray("buildings");
+		a.syncSize(temp);
+		for(int i = 0; i < temp.size(); ++i)
+		{
+			a.serializeInt(i, temp[i]);
+			buildings.insert(temp[i]);
+		}
+	}
+
 	{
 		auto a = handler.enterArray("creatures");
 		a.syncSize(creatures);

lib/mapping/MapFormatH3M.cpp

@@ -883,7 +883,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 		}
 		map->predefinedHeroes.emplace_back(hero);
 
-		logGlobal->debug("Map '%s': Hero predefined in map: %s", mapName, VLC->heroh->getByIndex(hero->subID)->getJsonKey());
+		logGlobal->debug("Map '%s': Hero predefined in map: %s", mapName, VLC->heroh->getById(hero->getHeroType())->getJsonKey());
 	}
 }
 

lib/mapping/MapFormatJson.cpp

@@ -831,7 +831,7 @@ void CMapFormatJson::serializeOptions(JsonSerializeFormat & handler)
 
 	serializePredefinedHeroes(handler);
 
-	handler.serializeLIC("allowedAbilities", &CSkillHandler::decodeSkill, &CSkillHandler::encodeSkill, VLC->skillh->getDefaultAllowed(), map->allowedAbilities);
+	handler.serializeLIC("allowedAbilities", &SecondarySkill::decode, &SecondarySkill::encode, VLC->skillh->getDefaultAllowed(), map->allowedAbilities);
 
 	handler.serializeLIC("allowedArtifacts",  &ArtifactID::decode, &ArtifactID::encode, VLC->arth->getDefaultAllowed(), map->allowedArtifact);
 

lib/rewardable/Reward.cpp

@@ -133,7 +133,7 @@ void Rewardable::Reward::serializeJson(JsonSerializeFormat & handler)
 		std::vector<std::pair<SecondarySkill, si32>> fieldValue(secondary.begin(), secondary.end());
 		a.serializeStruct<std::pair<SecondarySkill, si32>>(fieldValue, [](JsonSerializeFormat & h, std::pair<SecondarySkill, si32> & e)
 		{
-			h.serializeId("skill", e.first, SecondarySkill{}, VLC->skillh->decodeSkill, VLC->skillh->encodeSkill);
+			h.serializeId("skill", e.first);
 			h.serializeId("level", e.second, 0, [](const std::string & i){return vstd::find_pos(NSecondarySkill::levels, i);}, [](si32 i){return NSecondarySkill::levels.at(i);});
 		});
 		a.syncSize(fieldValue);

lib/rmg/CMapGenOptions.cpp

@@ -187,7 +187,7 @@ const std::map<PlayerColor, CMapGenOptions::CPlayerSettings> & CMapGenOptions::g
 	return players;
 }
 
-void CMapGenOptions::setStartingTownForPlayer(const PlayerColor & color, si32 town)
+void CMapGenOptions::setStartingTownForPlayer(const PlayerColor & color, FactionID town)
 {
 	auto it = players.find(color);
 	if(it == players.end()) assert(0);

lib/rmg/CMapGenOptions.h

@@ -114,7 +114,7 @@ public:
 	/// The first player colors belong to standard players and the last player colors belong to comp only players.
 	/// All standard players are by default of type EPlayerType::AI.
 	const std::map<PlayerColor, CPlayerSettings> & getPlayersSettings() const;
-	void setStartingTownForPlayer(const PlayerColor & color, si32 town);
+	void setStartingTownForPlayer(const PlayerColor & color, FactionID town);
 	/// Sets a player type for a standard player. A standard player is the opposite of a computer only player. The
 	/// values which can be chosen for the player type are EPlayerType::AI or EPlayerType::HUMAN.
 	void setPlayerTypeForStandardPlayer(const PlayerColor & color, EPlayerType playerType);