Merge pull request #3597 from IvanSavenko/bugfixing

Fixes for map/save loading

client/lobby/CSelectionBase.cpp

@@ -229,7 +229,7 @@ void InfoCard::changeSelection()
 	iconsLossCondition->setFrame(header->defeatIconIndex);
 	labelLossConditionText->setText(header->defeatMessage.toString());
 	flagbox->recreate();
-	labelDifficulty->setText(CGI->generaltexth->arraytxt[142 + mapInfo->mapHeader->difficulty]);
+	labelDifficulty->setText(CGI->generaltexth->arraytxt[142 + vstd::to_underlying(mapInfo->mapHeader->difficulty)]);
 	iconDifficulty->setSelected(SEL->getCurrentDifficulty());
 	if(SEL->screenType == ESelectionScreen::loadGame || SEL->screenType == ESelectionScreen::saveGame)
 		for(auto & button : iconDifficulty->buttons)

client/lobby/RandomMapTab.cpp

@@ -177,7 +177,7 @@ void RandomMapTab::updateMapInfoByHost()
 	mapInfo->mapHeader->version = EMapFormat::VCMI;
 	mapInfo->mapHeader->name.appendLocalString(EMetaText::GENERAL_TXT, 740);
 	mapInfo->mapHeader->description.appendLocalString(EMetaText::GENERAL_TXT, 741);
-	mapInfo->mapHeader->difficulty = 1; // Normal
+	mapInfo->mapHeader->difficulty = EMapDifficulty::NORMAL;
 	mapInfo->mapHeader->height = mapGenOptions->getHeight();
 	mapInfo->mapHeader->width = mapGenOptions->getWidth();
 	mapInfo->mapHeader->twoLevel = mapGenOptions->getHasTwoLevels();

config/heroes/tower.json

@@ -70,6 +70,7 @@
 	"torosar":
 	{
 		"index": 36,
+		"compatibilityIdentifiers" : [ "torosar " ],
 		"class" : "alchemist",
 		"female": false,
 		"spellbook": [ "magicArrow" ],

lib/CHeroHandler.cpp

@@ -326,6 +326,8 @@ CHeroClass * CHeroClassHandler::loadFromJson(const std::string & scope, const Js
 	{
 		JsonNode classConf = node["mapObject"];
 		classConf["heroClass"].String() = identifier;
+		if (!node["compatibilityIdentifiers"].isNull())
+			classConf["compatibilityIdentifiers"] = node["compatibilityIdentifiers"];
 		classConf.setMeta(scope);
 		VLC->objtypeh->loadSubObject(identifier, classConf, index, heroClass->getIndex());
 	});
@@ -756,6 +758,9 @@ void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNod
 	objects.emplace_back(object);
 
 	registerObject(scope, "hero", name, object->getIndex());
+
+	for(const auto & compatID : data["compatibilityIdentifiers"].Vector())
+		registerObject(scope, "hero", compatID.String(), object->getIndex());
 }
 
 void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNode & data, size_t index)
@@ -767,6 +772,8 @@ void CHeroHandler::loadObject(std::string scope, std::string name, const JsonNod
 	objects[index] = object;
 
 	registerObject(scope, "hero", name, object->getIndex());
+	for(const auto & compatID : data["compatibilityIdentifiers"].Vector())
+		registerObject(scope, "hero", compatID.String(), object->getIndex());
 }
 
 ui32 CHeroHandler::level (TExpType experience) const

lib/CHeroHandler.h

@@ -31,11 +31,11 @@ class CRandomGenerator;
 class JsonSerializeFormat;
 class BattleField;
 
-enum class EHeroGender : uint8_t
+enum class EHeroGender : int8_t
 {
+	DEFAULT = -1, // from h3m, instance has same gender as hero type
 	MALE = 0,
 	FEMALE = 1,
-	DEFAULT = 0xff // from h3m, instance has same gender as hero type
 };
 
 class DLL_LINKAGE CHero : public HeroType

lib/constants/EntityIdentifiers.h

@@ -330,7 +330,7 @@ public:
 	static BuildingID FORT_LEVEL(unsigned int level)
 	{
 		assert(level < 3);
-		return BuildingID(Type::TOWN_HALL + level);
+		return BuildingID(Type::FORT + level);
 	}
 
 	static std::string encode(int32_t index);

lib/constants/Enumerations.h

@@ -64,10 +64,10 @@ enum class EMarketMode : int8_t
 enum class EAiTactic : int8_t
 {
 	NONE = -1,
-	RANDOM,
+	RANDOM = 0,
-	WARRIOR,
+	WARRIOR = 1,
-	BUILDER,
+	BUILDER = 2,
-	EXPLORER
+	EXPLORER = 3
 };
 
 enum class EBuildingState : int8_t

lib/mapObjects/CGHeroInstance.cpp

@@ -1111,7 +1111,7 @@ std::string CGHeroInstance::getClassNameTextID() const
 {
 	if (isCampaignGem())
 		return "core.genrltxt.735";
-	return type->heroClass->getNameTranslated();
+	return type->heroClass->getNameTextID();
 }
 
 std::string CGHeroInstance::getNameTextID() const
@@ -1524,28 +1524,6 @@ std::string CGHeroInstance::getHeroTypeName() const
 
 void CGHeroInstance::afterAddToMap(CMap * map)
 {
-	if(ID != Obj::RANDOM_HERO)
-	{
-		auto existingHero = std::find_if(map->objects.begin(), map->objects.end(), [&](const CGObjectInstance * o) ->bool
-			{
-				return o && (o->ID == Obj::HERO || o->ID == Obj::PRISON) && o->subID == subID && o != this;
-			});
-
-		if(existingHero != map->objects.end())
-		{
-			if(settings["session"]["editor"].Bool())
-			{
-				logGlobal->warn("Hero is already on the map at %s", (*existingHero)->visitablePos().toString());
-			}
-			else
-			{
-				logGlobal->error("Hero is already on the map at %s", (*existingHero)->visitablePos().toString());
-
-				throw std::runtime_error("Hero is already on the map");
-			}
-		}
-	}
-
 	if(ID != Obj::PRISON)
 	{		
 		map->heroesOnMap.emplace_back(this);

lib/mapObjects/CGHeroInstance.h

@@ -23,7 +23,7 @@ class CGTownInstance;
 class CMap;
 struct TerrainTile;
 struct TurnInfo;
-enum class EHeroGender : uint8_t;
+enum class EHeroGender : int8_t;
 
 class DLL_LINKAGE CGHeroPlaceholder : public CGObjectInstance
 {

lib/mapping/CMap.cpp

@@ -42,8 +42,12 @@ DisposedHero::DisposedHero() : heroId(0), portrait(255)
 
 }
 
-CMapEvent::CMapEvent() : players(0), humanAffected(0), computerAffected(0),
+CMapEvent::CMapEvent()
-	firstOccurence(0), nextOccurence(0)
+	: players(0)
+	, humanAffected(false)
+	, computerAffected(false)
+	, firstOccurence(0)
+	, nextOccurence(0)
 {
 
 }

lib/mapping/CMapDefines.h

@@ -37,8 +37,8 @@ public:
 	MetaString message;
 	TResources resources;
 	ui8 players; // affected players, bit field?
-	ui8 humanAffected;
+	bool humanAffected;
-	ui8 computerAffected;
+	bool computerAffected;
 	ui32 firstOccurence;
 	ui32 nextOccurence; /// specifies after how many days the event will occur the next time; 0 if event occurs only one time
 

lib/mapping/CMapHeader.cpp

@@ -117,7 +117,7 @@ void CMapHeader::setupEvents()
 }
 
 CMapHeader::CMapHeader() : version(EMapFormat::VCMI), height(72), width(72),
-	twoLevel(true), difficulty(1), levelLimit(0), howManyTeams(0), areAnyPlayers(false)
+	twoLevel(true), difficulty(EMapDifficulty::NORMAL), levelLimit(0), howManyTeams(0), areAnyPlayers(false)
 {
 	setupEvents();
 	allowedHeroes = VLC->heroh->getDefaultAllowed();
@@ -149,7 +149,7 @@ void CMapHeader::registerMapStrings()
 	
 	if(maxStrings == 0 || mapLanguages.empty())
 	{
-		logGlobal->info("Map %s doesn't have any supported translation", name.toString());
+		logGlobal->trace("Map %s doesn't have any supported translation", name.toString());
 		return;
 	}
 	

lib/mapping/CMapHeader.h

@@ -191,6 +191,15 @@ struct DLL_LINKAGE TriggeredEvent
 	}
 };
 
+enum class EMapDifficulty : uint8_t
+{
+	EASY = 0,
+	NORMAL = 1,
+	HARD = 2,
+	EXPERT = 3,
+	IMPOSSIBLE = 4
+};
+
 /// The map header holds information about loss/victory condition,map format, version, players, height, width,...
 class DLL_LINKAGE CMapHeader
 {
@@ -218,7 +227,7 @@ public:
 	bool twoLevel; /// The default value is true.
 	MetaString name;
 	MetaString description;
-	ui8 difficulty; /// The default value is 1 representing a normal map difficulty.
+	EMapDifficulty difficulty;
 	/// Specifies the maximum level to reach for a hero. A value of 0 states that there is no
 	///	maximum level for heroes. This is the default value.
 	ui8 levelLimit;

lib/mapping/MapFormatH3M.cpp

@@ -184,10 +184,10 @@ void CMapLoaderH3M::readHeader()
 	mapHeader->twoLevel = reader->readBool();
 	mapHeader->name.appendTextID(readLocalizedString("header.name"));
 	mapHeader->description.appendTextID(readLocalizedString("header.description"));
-	mapHeader->difficulty = reader->readInt8();
+	mapHeader->difficulty = static_cast<EMapDifficulty>(reader->readInt8Checked(0, 4));
 
 	if(features.levelAB)
-		mapHeader->levelLimit = reader->readUInt8();
+		mapHeader->levelLimit = reader->readInt8Checked(0, std::min(100u, VLC->heroh->maxSupportedLevel()));
 	else
 		mapHeader->levelLimit = 0;
 
@@ -218,7 +218,7 @@ void CMapLoaderH3M::readPlayerInfo()
 			continue;
 		}
 
-		playerInfo.aiTactic = static_cast<EAiTactic>(reader->readUInt8());
+		playerInfo.aiTactic = static_cast<EAiTactic>(reader->readInt8Checked(-1, 3));
 
 		if(features.levelSOD)
 			reader->skipUnused(1); //TODO: check meaning?
@@ -261,8 +261,8 @@ void CMapLoaderH3M::readPlayerInfo()
 		if(features.levelAB)
 		{
 			reader->skipUnused(1); //TODO: check meaning?
-			uint32_t heroCount = reader->readUInt32();
+			size_t heroCount = reader->readUInt32();
-			for(int pp = 0; pp < heroCount; ++pp)
+			for(size_t pp = 0; pp < heroCount; ++pp)
 			{
 				SHeroName vv;
 				vv.heroId = reader->readHero();
@@ -274,39 +274,13 @@ void CMapLoaderH3M::readPlayerInfo()
 	}
 }
 
-enum class EVictoryConditionType : uint8_t
-{
-	ARTIFACT = 0,
-	GATHERTROOP = 1,
-	GATHERRESOURCE = 2,
-	BUILDCITY = 3,
-	BUILDGRAIL = 4,
-	BEATHERO = 5,
-	CAPTURECITY = 6,
-	BEATMONSTER = 7,
-	TAKEDWELLINGS = 8,
-	TAKEMINES = 9,
-	TRANSPORTITEM = 10,
-	HOTA_ELIMINATE_ALL_MONSTERS = 11,
-	HOTA_SURVIVE_FOR_DAYS = 12,
-	WINSTANDARD = 255
-};
-
-enum class ELossConditionType : uint8_t
-{
-	LOSSCASTLE = 0,
-	LOSSHERO = 1,
-	TIMEEXPIRES = 2,
-	LOSSSTANDARD = 255
-};
-
 void CMapLoaderH3M::readVictoryLossConditions()
 {
 	mapHeader->triggeredEvents.clear();
 	mapHeader->victoryMessage.clear();
 	mapHeader->defeatMessage.clear();
 
-	auto vicCondition = static_cast<EVictoryConditionType>(reader->readUInt8());
+	auto vicCondition = static_cast<EVictoryConditionType>(reader->readInt8Checked(-1, 12));
 
 	EventCondition victoryCondition(EventCondition::STANDARD_WIN);
 	EventCondition defeatCondition(EventCondition::DAYS_WITHOUT_TOWN);
@@ -395,9 +369,9 @@ void CMapLoaderH3M::readVictoryLossConditions()
 				EventExpression::OperatorAll oper;
 				EventCondition cond(EventCondition::HAVE_BUILDING);
 				cond.position = reader->readInt3();
-				cond.objectType = BuildingID::HALL_LEVEL(reader->readUInt8() + 1);
+				cond.objectType = BuildingID::HALL_LEVEL(reader->readInt8Checked(0,3) + 1);
 				oper.expressions.emplace_back(cond);
-				cond.objectType = BuildingID::FORT_LEVEL(reader->readUInt8());
+				cond.objectType = BuildingID::FORT_LEVEL(reader->readInt8Checked(0, 2));
 				oper.expressions.emplace_back(cond);
 
 				specialVictory.effect.toOtherMessage.appendTextID("core.genrltxt.283");
@@ -578,7 +552,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 	}
 
 	// Read loss conditions
-	auto lossCond = static_cast<ELossConditionType>(reader->readUInt8());
+	auto lossCond = static_cast<ELossConditionType>(reader->readInt8Checked(-1, 2));
 	if(lossCond == ELossConditionType::LOSSSTANDARD)
 	{
 		mapHeader->defeatIconIndex = 3;
@@ -685,9 +659,9 @@ void CMapLoaderH3M::readAllowedHeroes()
 
 	if(features.levelAB)
 	{
-		uint32_t placeholdersQty = reader->readUInt32();
+		size_t placeholdersQty = reader->readUInt32();
 
-		for (uint32_t i = 0; i < placeholdersQty; ++i)
+		for (size_t i = 0; i < placeholdersQty; ++i)
 		{
 			auto heroID = reader->readHero();
 			mapHeader->reservedCampaignHeroes.insert(heroID);
@@ -700,9 +674,9 @@ void CMapLoaderH3M::readDisposedHeroes()
 	// Reading disposed heroes (20 bytes)
 	if(features.levelSOD)
 	{
-		ui8 disp = reader->readUInt8();
+		size_t disp = reader->readUInt8();
 		map->disposedHeroes.resize(disp);
-		for(int g = 0; g < disp; ++g)
+		for(size_t g = 0; g < disp; ++g)
 		{
 			map->disposedHeroes[g].heroId = reader->readHero();
 			map->disposedHeroes[g].portrait = reader->readHeroPortrait();
@@ -801,10 +775,10 @@ void CMapLoaderH3M::readAllowedSpellsAbilities()
 
 void CMapLoaderH3M::readRumors()
 {
-	uint32_t rumorsCount = reader->readUInt32();
+	size_t rumorsCount = reader->readUInt32();
 	assert(rumorsCount < 1000); // sanity check
 
-	for(int it = 0; it < rumorsCount; it++)
+	for(size_t it = 0; it < rumorsCount; it++)
 	{
 		Rumor ourRumor;
 		ourRumor.name = readBasicString();
@@ -853,7 +827,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 			for(int yy = 0; yy < howMany; ++yy)
 			{
 				hero->secSkills[yy].first = reader->readSkill();
-				hero->secSkills[yy].second = reader->readUInt8();
+				hero->secSkills[yy].second = reader->readInt8Checked(1,3);
 			}
 		}
 
@@ -864,7 +838,7 @@ void CMapLoaderH3M::readPredefinedHeroes()
 			hero->biographyCustomTextId = readLocalizedString(TextIdentifier("heroes", heroID, "biography"));
 
 		// 0xFF is default, 00 male, 01 female
-		hero->gender = static_cast<EHeroGender>(reader->readUInt8());
+		hero->gender = static_cast<EHeroGender>(reader->readInt8Checked(-1, 1));
 		assert(hero->gender == EHeroGender::MALE || hero->gender == EHeroGender::FEMALE || hero->gender == EHeroGender::DEFAULT);
 
 		bool hasCustomSpells = reader->readBool();
@@ -910,8 +884,8 @@ void CMapLoaderH3M::loadArtifactsOfHero(CGHeroInstance * hero)
 
 	// bag artifacts
 	// number of artifacts in hero's bag
-	int amount = reader->readUInt16();
+	size_t amount = reader->readUInt16();
-	for(int i = 0; i < amount; ++i)
+	for(size_t i = 0; i < amount; ++i)
 	{
 		loadArtifactToSlot(hero, ArtifactPosition::BACKPACK_START + static_cast<int>(hero->artifactsInBackpack.size()));
 	}
@@ -1038,33 +1012,33 @@ void CMapLoaderH3M::readBoxContent(CGPandoraBox * object, const int3 & mapPositi
 
 	reward.heroExperience = reader->readUInt32();
 	reward.manaDiff = reader->readInt32();
-	if(auto val = reader->readUInt8())
+	if(auto val = reader->readInt8Checked(-3, 3))
 		reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
-	if(auto val = reader->readUInt8())
+	if(auto val = reader->readInt8Checked(-3, 3))
 		reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, val, BonusSourceID(idToBeGiven));
 
 	reader->readResourses(reward.resources);
 	for(int x = 0; x < GameConstants::PRIMARY_SKILLS; ++x)
 		reward.primary.at(x) = reader->readUInt8();
 
-	int gabn = reader->readUInt8(); //number of gained abilities
+	size_t gabn = reader->readUInt8(); //number of gained abilities
-	for(int oo = 0; oo < gabn; ++oo)
+	for(size_t oo = 0; oo < gabn; ++oo)
 	{
 		auto rId = reader->readSkill();
-		auto rVal = reader->readUInt8();
+		auto rVal = reader->readInt8Checked(1,3);
 
 		reward.secondary[rId] = rVal;
 	}
-	int gart = reader->readUInt8(); //number of gained artifacts
+	size_t gart = reader->readUInt8(); //number of gained artifacts
-	for(int oo = 0; oo < gart; ++oo)
+	for(size_t oo = 0; oo < gart; ++oo)
 		reward.artifacts.push_back(reader->readArtifact());
 
-	int gspel = reader->readUInt8(); //number of gained spells
+	size_t gspel = reader->readUInt8(); //number of gained spells
-	for(int oo = 0; oo < gspel; ++oo)
+	for(size_t oo = 0; oo < gspel; ++oo)
 		reward.spells.push_back(reader->readSpell());
 
-	int gcre = reader->readUInt8(); //number of gained creatures
+	size_t gcre = reader->readUInt8(); //number of gained creatures
-	for(int oo = 0; oo < gcre; ++oo)
+	for(size_t oo = 0; oo < gcre; ++oo)
 	{
 		auto rId = reader->readCreature();
 		auto rVal = reader->readUInt16();
@@ -1094,7 +1068,7 @@ CGObjectInstance * CMapLoaderH3M::readMonster(const int3 & mapPosition, const Ob
 	//type will be set during initialization
 	object->putStack(SlotID(0), hlp);
 
-	object->character = reader->readInt8();
+	object->character = reader->readInt8Checked(0, 4);
 
 	bool hasMessage = reader->readBool();
 	if(hasMessage)
@@ -1192,17 +1166,17 @@ CGObjectInstance * CMapLoaderH3M::readWitchHut(const int3 & position, std::share
 
 CGObjectInstance * CMapLoaderH3M::readScholar(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	enum class ScholarBonusType : uint8_t {
+	enum class ScholarBonusType : int8_t {
+		RANDOM = -1,
 		PRIM_SKILL = 0,
 		SECONDARY_SKILL = 1,
 		SPELL = 2,
-		RANDOM = 255
 	};
 
 	auto * object = readGeneric(position, objectTemplate);
 	auto * rewardable = dynamic_cast<CRewardableObject*>(object);
 
-	uint8_t bonusTypeRaw = reader->readUInt8();
+	uint8_t bonusTypeRaw = reader->readInt8Checked(-1, 2);
 	auto bonusType = static_cast<ScholarBonusType>(bonusTypeRaw);
 	auto bonusID = reader->readUInt8();
 
@@ -1477,7 +1451,7 @@ CGObjectInstance * CMapLoaderH3M::readBank(const int3 & mapPosition, std::shared
 		int32_t guardsPresetIndex = reader->readInt32();
 
 		// presence of upgraded stack: -1 = random, 0 = never, 1 = always
-		int8_t upgradedStackPresence = reader->readInt8();
+		int8_t upgradedStackPresence = reader->readInt8Checked(-1, 1);
 
 		assert(vstd::iswithin(guardsPresetIndex, -1, 4));
 		assert(vstd::iswithin(upgradedStackPresence, -1, 1));
@@ -1807,7 +1781,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 		for(int i = 0; i < skillsCount; ++i)
 		{
 			object->secSkills[i].first = reader->readSkill();
-			object->secSkills[i].second = reader->readUInt8();
+			object->secSkills[i].second = reader->readInt8Checked(1,3);
 		}
 	}
 
@@ -1815,7 +1789,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 	if(hasGarison)
 		readCreatureSet(object, 7);
 
-	object->formation = static_cast<EArmyFormation>(reader->readUInt8());
+	object->formation = static_cast<EArmyFormation>(reader->readInt8Checked(0, 1));
 	assert(object->formation == EArmyFormation::LOOSE || object->formation == EArmyFormation::TIGHT);
 
 	loadArtifactsOfHero(object);
@@ -1828,7 +1802,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 		if(hasCustomBiography)
 			object->biographyCustomTextId = readLocalizedString(TextIdentifier("heroes", object->subID, "biography"));
 
-		object->gender = static_cast<EHeroGender>(reader->readUInt8());
+		object->gender = static_cast<EHeroGender>(reader->readInt8Checked(-1, 1));
 		assert(object->gender == EHeroGender::MALE || object->gender == EHeroGender::FEMALE || object->gender == EHeroGender::DEFAULT);
 	}
 	else
@@ -1938,30 +1912,12 @@ enum class ESeerHutRewardType : uint8_t
 	CREATURE = 10,
 };
 
-enum class EQuestMission {
-	NONE = 0,
-	LEVEL = 1,
-	PRIMARY_SKILL = 2,
-	KILL_HERO = 3,
-	KILL_CREATURE = 4,
-	ARTIFACT = 5,
-	ARMY = 6,
-	RESOURCES = 7,
-	HERO = 8,
-	PLAYER = 9,
-	HOTA_MULTI = 10,
-	// end of H3 missions
-	KEYMASTER = 100,
-	HOTA_HERO_CLASS = 101,
-	HOTA_REACH_DATE = 102
-};
-
 void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, const ObjectInstanceID & idToBeGiven)
 {
 	EQuestMission missionType = EQuestMission::NONE;
 	if(features.levelAB)
 	{
-		missionType = static_cast<EQuestMission>(readQuest(hut, position));
+		missionType = readQuest(hut, position);
 	}
 	else
 	{
@@ -1981,7 +1937,7 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con
 
 	if(missionType != EQuestMission::NONE)
 	{
-		auto rewardType = static_cast<ESeerHutRewardType>(reader->readUInt8());
+		auto rewardType = static_cast<ESeerHutRewardType>(reader->readInt8Checked(0, 10));
 		Rewardable::VisitInfo vinfo;
 		auto & reward = vinfo.reward;
 		switch(rewardType)
@@ -2003,36 +1959,34 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con
 			}
 			case ESeerHutRewardType::MORALE:
 			{
-				reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readUInt8(), BonusSourceID(idToBeGiven));
+				reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::MORALE, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
 				break;
 			}
 			case ESeerHutRewardType::LUCK:
 			{
-				reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readUInt8(), BonusSourceID(idToBeGiven));
+				reward.bonuses.emplace_back(BonusDuration::ONE_BATTLE, BonusType::LUCK, BonusSource::OBJECT_INSTANCE, reader->readInt8Checked(-3, 3), BonusSourceID(idToBeGiven));
 				break;
 			}
 			case ESeerHutRewardType::RESOURCES:
 			{
-				auto rId = reader->readUInt8();
+				auto rId = reader->readGameResID();
 				auto rVal = reader->readUInt32();
 
-				assert(rId < features.resourcesCount);
-
 				reward.resources[rId] = rVal;
 				break;
 			}
 			case ESeerHutRewardType::PRIMARY_SKILL:
 			{
-				auto rId = reader->readUInt8();
+				auto rId = reader->readPrimary();
 				auto rVal = reader->readUInt8();
 
-				reward.primary.at(rId) = rVal;
+				reward.primary.at(rId.getNum()) = rVal;
 				break;
 			}
 			case ESeerHutRewardType::SECONDARY_SKILL:
 			{
 				auto rId = reader->readSkill();
-				auto rVal = reader->readUInt8();
+				auto rVal = reader->readInt8Checked(1,3);
 
 				reward.secondary[rId] = rVal;
 				break;
@@ -2071,11 +2025,11 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con
 	}
 }
 
-int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position)
+EQuestMission CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position)
 {
-	auto missionId = reader->readUInt8();
+	auto missionId = static_cast<EQuestMission>(reader->readInt8Checked(0, 10));
 
-	switch(static_cast<EQuestMission>(missionId))
+	switch(missionId)
 	{
 		case EQuestMission::NONE:
 			return missionId;
@@ -2100,8 +2054,8 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position)
 		}
 		case EQuestMission::ARTIFACT:
 		{
-			int artNumber = reader->readUInt8();
+			size_t artNumber = reader->readUInt8();
-			for(int yy = 0; yy < artNumber; ++yy)
+			for(size_t yy = 0; yy < artNumber; ++yy)
 			{
 				auto artid = reader->readArtifact();
 				guard->quest->mission.artifacts.push_back(artid);
@@ -2111,9 +2065,9 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position)
 		}
 		case EQuestMission::ARMY:
 		{
-			int typeNumber = reader->readUInt8();
+			size_t typeNumber = reader->readUInt8();
 			guard->quest->mission.creatures.resize(typeNumber);
-			for(int hh = 0; hh < typeNumber; ++hh)
+			for(size_t hh = 0; hh < typeNumber; ++hh)
 			{
 				guard->quest->mission.creatures[hh].type = reader->readCreature().toCreature();
 				guard->quest->mission.creatures[hh].count = reader->readUInt16();
@@ -2143,7 +2097,7 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position)
 
 			if(missionSubID == 0)
 			{
-				missionId = int(EQuestMission::HOTA_HERO_CLASS);
+				missionId = EQuestMission::HOTA_HERO_CLASS;
 				std::set<HeroClassID> heroClasses;
 				reader->readBitmaskHeroClassesSized(heroClasses, false);
 				for(auto & hc : heroClasses)
@@ -2152,7 +2106,7 @@ int CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position)
 			}
 			if(missionSubID == 1)
 			{
-				missionId = int(EQuestMission::HOTA_REACH_DATE);
+				missionId = EQuestMission::HOTA_REACH_DATE;
 				guard->quest->mission.daysPassed = reader->readUInt32() + 1;
 				break;
 			}
@@ -2194,7 +2148,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 	if(hasGarrison)
 		readCreatureSet(object, 7);
 
-	object->formation = static_cast<EArmyFormation>(reader->readUInt8());
+	object->formation = static_cast<EArmyFormation>(reader->readInt8Checked(0, 1));
 	assert(object->formation == EArmyFormation::LOOSE || object->formation == EArmyFormation::TIGHT);
 
 	bool hasCustomBuildings = reader->readBool();
@@ -2252,7 +2206,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 		else
 			event.humanAffected = true;
 
-		event.computerAffected = reader->readUInt8();
+		event.computerAffected = reader->readBool();
 		event.firstOccurence = reader->readUInt16();
 		event.nextOccurence = reader->readUInt8();
 

lib/mapping/MapFormatH3M.h

@@ -35,6 +35,50 @@ class SpellID;
 class PlayerColor;
 class int3;
 
+enum class EQuestMission {
+	NONE = 0,
+	LEVEL = 1,
+	PRIMARY_SKILL = 2,
+	KILL_HERO = 3,
+	KILL_CREATURE = 4,
+	ARTIFACT = 5,
+	ARMY = 6,
+	RESOURCES = 7,
+	HERO = 8,
+	PLAYER = 9,
+	HOTA_MULTI = 10,
+	// end of H3 missions
+	KEYMASTER = 100,
+	HOTA_HERO_CLASS = 101,
+	HOTA_REACH_DATE = 102
+};
+
+enum class EVictoryConditionType : int8_t
+{
+	WINSTANDARD = -1,
+	ARTIFACT = 0,
+	GATHERTROOP = 1,
+	GATHERRESOURCE = 2,
+	BUILDCITY = 3,
+	BUILDGRAIL = 4,
+	BEATHERO = 5,
+	CAPTURECITY = 6,
+	BEATMONSTER = 7,
+	TAKEDWELLINGS = 8,
+	TAKEMINES = 9,
+	TRANSPORTITEM = 10,
+	HOTA_ELIMINATE_ALL_MONSTERS = 11,
+	HOTA_SURVIVE_FOR_DAYS = 12
+};
+
+enum class ELossConditionType : int8_t
+{
+	LOSSSTANDARD = -1,
+	LOSSCASTLE = 0,
+	LOSSHERO = 1,
+	TIMEEXPIRES = 2
+};
+
 class DLL_LINKAGE CMapLoaderH3M : public IMapLoader
 {
 public:
@@ -204,7 +248,7 @@ private:
 	 *
 	 * @param guard the quest guard where that quest should be applied to
 	 */
-	int readQuest(IQuestObject * guard, const int3 & position);
+	EQuestMission readQuest(IQuestObject * guard, const int3 & position);
 
 	void readSeerHutQuest(CGSeerHut * hut, const int3 & position, const ObjectInstanceID & idToBeGiven);
 

lib/mapping/MapFormatJson.cpp

@@ -1156,6 +1156,21 @@ void CMapLoaderJson::readObjects()
 	{
 		return a->getObjTypeIndex() < b->getObjTypeIndex();
 	});
+
+
+	std::set<HeroTypeID> debugHeroesOnMap;
+	for (auto const & object : map->objects)
+	{
+		if(object->ID != Obj::HERO && object->ID != Obj::PRISON)
+			continue;
+
+		auto * hero = dynamic_cast<const CGHeroInstance *>(object.get());
+
+		if (debugHeroesOnMap.count(hero->getHeroType()))
+			logGlobal->error("Hero is already on the map at %s", hero->visitablePos().toString());
+
+		debugHeroesOnMap.insert(hero->getHeroType());
+	}
 }
 
 void CMapLoaderJson::readTranslations()

lib/mapping/MapReaderH3M.cpp

@@ -183,6 +183,13 @@ RiverId MapReaderH3M::readRiver()
 	return result;
 }
 
+PrimarySkill MapReaderH3M::readPrimary()
+{
+	PrimarySkill result(readUInt8());
+	assert(result <= PrimarySkill::KNOWLEDGE );
+	return result;
+}
+
 SecondarySkill MapReaderH3M::readSkill()
 {
 	SecondarySkill result(readUInt8());
@@ -400,32 +407,35 @@ bool MapReaderH3M::readBool()
 	return result != 0;
 }
 
-ui8 MapReaderH3M::readUInt8()
+int8_t MapReaderH3M::readInt8Checked(int8_t lowerLimit, int8_t upperLimit)
+{
+	int8_t result = readInt8();
+	assert(result >= lowerLimit);
+	assert(result <= upperLimit);
+	return std::clamp(result, lowerLimit, upperLimit);
+}
+
+uint8_t MapReaderH3M::readUInt8()
 {
 	return reader->readUInt8();
 }
 
-si8 MapReaderH3M::readInt8()
+int8_t MapReaderH3M::readInt8()
 {
 	return reader->readInt8();
 }
 
-ui16 MapReaderH3M::readUInt16()
+uint16_t MapReaderH3M::readUInt16()
 {
 	return reader->readUInt16();
 }
 
-si16 MapReaderH3M::readInt16()
+uint32_t MapReaderH3M::readUInt32()
-{
-	return reader->readInt16();
-}
-
-ui32 MapReaderH3M::readUInt32()
 {
 	return reader->readUInt32();
 }
 
-si32 MapReaderH3M::readInt32()
+int32_t MapReaderH3M::readInt32()
 {
 	return reader->readInt32();
 }
@@ -435,9 +445,4 @@ std::string MapReaderH3M::readBaseString()
 	return reader->readBaseString();
 }
 
-CBinaryReader & MapReaderH3M::getInternalReader()
-{
-	return *reader;
-}
-
 VCMI_LIB_NAMESPACE_END

lib/mapping/MapReaderH3M.h

@@ -40,6 +40,7 @@ public:
 	TerrainId readTerrain();
 	RoadId readRoad();
 	RiverId readRiver();
+	PrimarySkill readPrimary();
 	SecondarySkill readSkill();
 	SpellID readSpell();
 	SpellID readSpell32();
@@ -70,17 +71,17 @@ public:
 
 	bool readBool();
 
-	ui8 readUInt8();
+	uint8_t readUInt8();
-	si8 readInt8();
+	int8_t readInt8();
-	ui16 readUInt16();
+	int8_t readInt8Checked(int8_t lowerLimit, int8_t upperLimit);
-	si16 readInt16();
+
-	ui32 readUInt32();
+	uint16_t readUInt16();
-	si32 readInt32();
+
+	uint32_t readUInt32();
+	int32_t readInt32();
 
 	std::string readBaseString();
 
-	CBinaryReader & getInternalReader();
-
 private:
 	template<class Identifier>
 	Identifier remapIdentifier(const Identifier & identifier);

lib/registerTypes/RegisterTypesMapObjects.h

@@ -54,7 +54,6 @@ void registerTypesMapObjects(Serializer &s)
 	s.template registerType<CGObjectInstance, CGMarket>();
 		s.template registerType<CGMarket, CGBlackMarket>();
 		s.template registerType<CGMarket, CGUniversity>();
-		s.template registerType<CGMarket, CGArtifactsAltar>();
 	s.template registerType<CGObjectInstance, CGHeroPlaceholder>();
 
 	s.template registerType<CGObjectInstance, CArmedInstance>(); s.template registerType<CBonusSystemNode, CArmedInstance>(); s.template registerType<CCreatureSet, CArmedInstance>();
@@ -133,6 +132,8 @@ void registerTypesMapObjects(Serializer &s)
 
 	//s.template registerType<CObstacleInstance>();
 		s.template registerType<CObstacleInstance, SpellCreatedObstacle>();
+
+	s.template registerType<CGMarket, CGArtifactsAltar>();
 }
 
 VCMI_LIB_NAMESPACE_END

lib/rmg/CMapGenerator.cpp

@@ -434,7 +434,7 @@ void CMapGenerator::addHeaderInfo()
 	m.twoLevel = mapGenOptions.getHasTwoLevels();
 	m.name.appendLocalString(EMetaText::GENERAL_TXT, 740);
 	m.description.appendRawString(getMapDescription());
-	m.difficulty = 1;
+	m.difficulty = EMapDifficulty::NORMAL;
 	addPlayerInfo();
 	m.waterMap = (mapGenOptions.getWaterContent() != EWaterContent::EWaterContent::NONE);
 	m.banWaterContent();

lib/serializer/JsonSerializeFormat.cpp

@@ -135,9 +135,16 @@ void JsonSerializeFormat::readLICPart(const JsonNode & part, const JsonSerialize
 	{
 		const std::string & identifier = index.String();
 
-		const si32 rawId = decoder(identifier);
+		try
-		if(rawId != -1)
+		{
+			const si32 rawId = decoder(identifier);
 			value.insert(rawId);
+		}
+		catch (const IdentifierResolutionException & e)
+		{
+			// downgrade exception to warning (printed as part of exception generation
+			// this is usually caused by loading allowed heroes / artifacts list from vmap's
+		}
 	}
 }
 

mapeditor/mapsettings/generalsettings.cpp

@@ -35,23 +35,23 @@ void GeneralSettings::initialize(MapController & c)
 	//set difficulty
 	switch(controller->map()->difficulty)
 	{
-		case 0:
+		case EMapDifficulty::EASY:
 			ui->diffRadio1->setChecked(true);
 			break;
 
-		case 1:
+		case EMapDifficulty::NORMAL:
 			ui->diffRadio2->setChecked(true);
 			break;
 
-		case 2:
+		case EMapDifficulty::HARD:
 			ui->diffRadio3->setChecked(true);
 			break;
 
-		case 3:
+		case EMapDifficulty::EXPERT:
 			ui->diffRadio4->setChecked(true);
 			break;
 
-		case 4:
+		case EMapDifficulty::IMPOSSIBLE:
 			ui->diffRadio5->setChecked(true);
 			break;
 	};
@@ -67,11 +67,11 @@ void GeneralSettings::update()
 		controller->map()->levelLimit = 0;
 
 	//set difficulty
-	if(ui->diffRadio1->isChecked()) controller->map()->difficulty = 0;
+	if(ui->diffRadio1->isChecked()) controller->map()->difficulty = EMapDifficulty::EASY;
-	if(ui->diffRadio2->isChecked()) controller->map()->difficulty = 1;
+	if(ui->diffRadio2->isChecked()) controller->map()->difficulty = EMapDifficulty::NORMAL;
-	if(ui->diffRadio3->isChecked()) controller->map()->difficulty = 2;
+	if(ui->diffRadio3->isChecked()) controller->map()->difficulty = EMapDifficulty::HARD;
-	if(ui->diffRadio4->isChecked()) controller->map()->difficulty = 3;
+	if(ui->diffRadio4->isChecked()) controller->map()->difficulty = EMapDifficulty::EXPERT;
-	if(ui->diffRadio5->isChecked()) controller->map()->difficulty = 4;
+	if(ui->diffRadio5->isChecked()) controller->map()->difficulty = EMapDifficulty::IMPOSSIBLE;
 }
 
 void GeneralSettings::on_heroLevelLimitCheck_toggled(bool checked)

server/battles/BattleFlowProcessor.cpp

@@ -584,10 +584,10 @@ void BattleFlowProcessor::stackEnchantedTrigger(const CBattleInfoCallback & batt
 	auto bl = *(st->getBonuses(Selector::type()(BonusType::ENCHANTED)));
 	for(auto b : bl)
 	{
-		const CSpell * sp = b->subtype.as<SpellID>().toSpell();
+		if (!b->subtype.as<SpellID>().hasValue())
-		if(!sp)
 			continue;
 
+		const CSpell * sp = b->subtype.as<SpellID>().toSpell();
 		const int32_t val = bl.valOfBonuses(Selector::typeSubtype(b->type, b->subtype));
 		const int32_t level = ((val > 3) ? (val - 3) : val);