Merge pull request #7992 from IvanSavenko/h3m-fixes

Fix issues with HotA h3m parsing and clean up parser code

docs/modders/Lua_Reference/HeroInstance.md

@@ -86,6 +86,18 @@ Returns the hero's mastery of the given secondary skill.
 
 - returns `integer` — Mastery level (0 = none, 1 = basic, 2 = advanced, 3 = expert).
 
+### getHeroType
+
+Returns the hero type (Orrin, Kyrre, Astral...) of this hero.
+
+- returns [`HeroType`](HeroType.md) — Type of the hero, comparable with the result of Services:getHeroTypeByName.
+
+### getHeroClass
+
+Returns the hero class of this hero.
+
+- returns [`HeroClass`](HeroClass.md) — Class of the hero, comparable with the result of Services:getHeroClassByName.
+
 ### hasArtifact
 
 Returns whether the hero owns the given artifact, either as equipped or in the backpack.

docs/modders/Lua_Reference/TownInstance.md

@@ -13,3 +13,9 @@ Returns the player color that owns this town, or the neutral player when it is u
 Returns the buildings that have been built in this town, upgrades of other buildings among them.
 
 - returns [`Building[]`](Building.md) — Every building standing in this town.
+
+### getFaction
+
+Returns the faction (Castle, Rampart, Tower...) of this town.
+
+- returns [`Faction`](Faction.md) — Faction of the town, comparable with the result of Services:getFactionByName.

docs/modders/Lua_Reference/api.lua

@@ -1229,6 +1229,14 @@ function HeroInstance:getPrimarySkill(skill) end
 ---@return integer # Mastery level (0 = none, 1 = basic, 2 = advanced, 3 = expert).
 function HeroInstance:getSecondarySkill(skill) end
 
+---Returns the hero type (Orrin, Kyrre, Astral...) of this hero.
+---@return HeroType # Type of the hero, comparable with the result of Services:getHeroTypeByName.
+function HeroInstance:getHeroType() end
+
+---Returns the hero class of this hero.
+---@return HeroClass # Class of the hero, comparable with the result of Services:getHeroClassByName.
+function HeroInstance:getHeroClass() end
+
 ---Returns whether the hero owns the given artifact, either as equipped or in the backpack.
 ---@param artifact integer # Artifact JSON key.
 ---@return boolean # True if the hero owns the artifact.
@@ -1638,6 +1646,10 @@ function TownInstance:getOwner() end
 ---@return Building[] # Every building standing in this town.
 function TownInstance:getBuildings() end
 
+---Returns the faction (Castle, Rampart, Tower...) of this town.
+---@return Faction # Faction of the town, comparable with the result of Services:getFactionByName.
+function TownInstance:getFaction() end
+
 ---Represents a creature stack participating in the current battle. Provides access to the live combat state — position, owner, current health, applied bonuses, ability checks. To stage modifications, copy into a UnitState, edit it, then commit via server.
 ---@class Unit
 local Unit = {}

lib/campaign/CampaignHandler.cpp

@@ -351,13 +351,14 @@ void CampaignHandler::readHeaderFromMemory( CampaignHeader & ret, CBinaryReader
 				logGlobal->warn("Map '%s': This map is forced to use specific hota version!", filename);
 		}
 
-		[[maybe_unused]] int32_t unknownB = reader.readInt8();
-		[[maybe_unused]] int32_t unknownC = reader.readInt32();
+		int unknownB = reader.readInt8(); // 1 in nearly all campaigns
+		int unknownC = reader.readInt32(); // 0 in official campaigns, varying values in user-made ones
 		ret.numberOfScenarios = reader.readInt32();
 
-		assert(unknownB == 1);
-		assert(unknownC == 0);
-		assert(ret.numberOfScenarios <= 8);
+		if (unknownB != 1 || unknownC != 0)
+			logGlobal->warn("Campaign '%s': unknown header fields have unexpected values %d and %d", filename, unknownB, unknownC);
+
+		assert(ret.numberOfScenarios <= 32);
 	}
 
 	const auto & mapping = LIBRARY->mapFormat->getMapping(ret.version);
@@ -366,7 +367,8 @@ void CampaignHandler::readHeaderFromMemory( CampaignHeader & ret, CBinaryReader
 	if(ret.version != CampaignVersion::Chr)
 	{
 		ret.campaignRegions = *LIBRARY->campaignRegions->getByIndex(mapping.remap(campaignMapId));
-		if(ret.version != CampaignVersion::HotA)
+		// HotA campaigns on a non-HotA region map may store 0 scenarios
+		if(ret.version != CampaignVersion::HotA || ret.numberOfScenarios == 0)
 			ret.numberOfScenarios = ret.campaignRegions.regionsCount();
 	}
 	ret.name.appendTextID(readLocalizedString(ret, reader, filename, modName, encoding, "name"));
@@ -408,14 +410,16 @@ CampaignScenario CampaignHandler::readScenarioFromMemory( CBinaryReader & reader
 	CampaignScenario ret;
 	ret.mapName = reader.readBaseString();
 	reader.readUInt32(); //packedMapSize - not used
-	if(header.numberOfScenarios > 8) //unholy alliance
+	if(header.numberOfScenarios > 16) // HotA custom campaigns
 	{
+		if(header.numberOfScenarios <= 24)
+			logGlobal->warn("Campaign '%s': unverified size of scenario preconditions for %d scenarios, assuming 4 bytes", header.filename, header.numberOfScenarios);
+		ret.loadPreconditionRegions(reader.readUInt32());
+	}
+	else if(header.numberOfScenarios > 8) //unholy alliance
 		ret.loadPreconditionRegions(reader.readUInt16());
-	}
 	else
-	{
 		ret.loadPreconditionRegions(reader.readUInt8());
-	}
 	ret.regionColor = reader.readUInt8();
 	ret.difficulty = reader.readUInt8();
 	assert(ret.difficulty < 5);

lib/mapObjects/Quest.cpp

@@ -365,7 +365,7 @@ void Quest::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeBool("repeatedQuest", repeatedQuest, false);
 
 	handler.serializeInt("timeLimit", lastDay, -1);
-	handler.serializeString("questGiverName", questGiverNameTextID);
+	handler.serializeString("timeoutText", timeoutTextID);
 	handler.serializeStruct("limiter", mission);
 
 	// kill quests have a single target; kept as a scalar "killTarget" key for map
@@ -604,7 +604,8 @@ void SeerHut::init(vstd::RNG & rand)
 {
 	auto names = LIBRARY->generaltexth->findStringsWithPrefix("core.seerhut.names");
 
-	seerNameTextID = *RandomGeneratorUtil::nextItem(names, rand);
+	if(seerNameTextID.empty())
+		seerNameTextID = *RandomGeneratorUtil::nextItem(names, rand);
 
 	bool h3BugTakesArmy = cb->getSettings().getBoolean(EGameSettings::MAP_OBJECTS_H3_BUG_QUEST_TAKES_ENTIRE_ARMY);
 	for(const auto & q : allQuests())
@@ -683,13 +684,7 @@ void QuestSource::defineDefaultTexts(Quest & q)
 
 std::string SeerHut::getQuestGiverName() const
 {
-	if(seerNameTextID.empty()) // quest guards have no seer of their own
-		return {};
-
-	if(!isEmpty() && !getQuest().questGiverNameTextID.empty())
-		return getQuest().questGiverNameTextID;
-
-	return seerNameTextID;
+	return seerNameTextID; // empty for quest guards, which have no seer of their own
 }
 
 void SeerHut::setSeerName(CMap & map, const std::string & newName)
@@ -882,6 +877,7 @@ void SeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 
 	//quest and reward
 	CRewardableObject::serializeJsonOptions(handler);
+	handler.serializeString("seerName", seerNameTextID);
 
 	bool oldVersion = false;
 	{

lib/mapObjects/Quest.h

@@ -74,8 +74,8 @@ public:
 	std::string heroNameTextID; //backup of hero name identifier, the hero itself is gone by then
 	HeroTypeID heroPortrait;
 
-	/// Map-defined name of this quest's giver, overriding the seer hut's own name. Only seer huts show it.
-	std::string questGiverNameTextID;
+	/// Map-defined message for the quest running out of time
+	std::string timeoutTextID;
 
 	MetaString firstVisitText;
 	MetaString nextVisitText;
@@ -134,7 +134,7 @@ public:
 			h & scriptHintText;
 		}
 		if(h.hasFeature(Handler::Version::SEER_HUT_NAME_TEXT_ID))
-			h & questGiverNameTextID;
+			h & timeoutTextID; // older saves hold a quest giver name here instead
 		// legacy "text was customized" flags; now derived on the fly from text
 		// emptiness in initObj. Kept on the wire for save compatibility.
 		bool isCustomFirst = !firstVisitText.empty();

lib/mapping/HotaScriptConverter.cpp

@@ -66,6 +66,7 @@ enum class HotaScriptCondition : int32_t
 	NOT_EQUAL = 10,
 	CURRENT_PLAYER = 11,
 	HERO_OWNER = 12,
+	HERO_CLASS = 13,
 	PLAYER_DEFEATED_MONSTER = 14,
 	PLAYER_DEFEATED_HERO = 15,
 	HERO_SECONDARY_SKILL = 16,
@@ -73,7 +74,8 @@ enum class HotaScriptCondition : int32_t
 	PLAYER_OWNS_TOWN = 18,
 	PLAYER_IS_HUMAN = 19,
 	PLAYER_STARTING_FACTION = 20,
-	TOWN_IS_NEUTRAL = 21
+	TOWN_IS_NEUTRAL = 21,
+	HERO_TYPE = 22
 };
 
 enum class HotaScriptExpression : int32_t
@@ -110,10 +112,10 @@ static std::string boolStr(bool value)
 
 /// Renders a string as a quoted Lua literal. Only used for editor-supplied variable names, which
 /// are expected to be plain identifiers - anything needing escaping means a malformed map.
-static std::string luaString(const std::string & value)
+std::string HotaScriptConverter::luaString(const std::string & value) const
 {
 	if(value.find_first_of("\\\"\n\r") != std::string::npos)
-		throw std::runtime_error("Script variable name contains unsupported characters: " + value);
+		throw std::runtime_error("Map '" + mapName + "': Script variable name contains unsupported characters: " + value);
 
 	return '"' + value + '"';
 }
@@ -157,7 +159,7 @@ static std::string entityRef(const std::string & lookup, IdentifierType identifi
 }
 
 /// EXECUTE_EVENT stores the target bucket as an integer in map order.
-static std::string bucketName(int eventType)
+std::string HotaScriptConverter::bucketName(int eventType) const
 {
 	switch(eventType)
 	{
@@ -165,7 +167,7 @@ static std::string bucketName(int eventType)
 		case 1: return "playerEvents";
 		case 2: return "townEvents";
 		case 3: return "questEvents";
-		default: throw std::runtime_error("Unknown event bucket code:" + std::to_string(eventType));
+		default: throw unsupported("Unknown event bucket code " + std::to_string(eventType));
 	}
 }
 
@@ -238,6 +240,15 @@ local function changeResources(server, player, amounts, sign)
 	end
 end
 
+-- Sets the player's treasury to the given resource set, in the same order as changeResources.
+local function setResources(game, server, player, amounts)
+	for index, key in ipairs(RESOURCES) do
+		local resource = LIBRARY:getResourceByName(key)
+		local delta = amounts[index] - game:getResource(player, resource)
+		if delta ~= 0 then server:giveResource(player, resource, delta) end
+	end
+end
+
 -- HotA object-identity predicates. The engine only offers the lookups; the comparison lives here.
 local function heroOwner(game, heroType, player)
 	local hero = game:getHeroByType(heroType)
@@ -427,7 +438,9 @@ std::string HotaScriptConverter::loadImageList(int count)
 	{
 		int imageType = reader.readInt32();
 		int imageSubtype = reader.readInt32();
-		std::string amount = loadExpression();
+		bool isVariable = reader.readBool();
+		int value = reader.readInt32();
+		std::string amount = isVariable ? "game:getMapVariable(" + varRef(value) + ")" : num(value);
 		if(i != 0)
 			result += ", ";
 		result += "{" + num(imageType) + ", " + num(imageSubtype) + ", " + amount + "}";
@@ -568,7 +581,13 @@ std::string HotaScriptConverter::loadActions(int indent)
 				if(mode == 1 || mode == 2)
 				{
 					reader.readBool(); // show OR between images
-					reader.readInt32(); // unknown
+					// TODO: images shown besides the two choices, unclear where HotA displays them
+					int extraImagesCount = reader.readInt32();
+					if(extraImagesCount != 0)
+					{
+						loadImageList(extraImagesCount);
+						logGlobal->warn("Map '%s': question with %d extra images is not implemented!", mapName, extraImagesCount);
+					}
 				}
 
 				result += pad + "server:showQuestion{\n";
@@ -701,16 +720,29 @@ std::string HotaScriptConverter::loadActions(int indent)
 			}
 			case HotaScriptActions::CONSTRUCT_BUILDING:
 			{
-				BuildingID building = reader.readBuilding32(std::nullopt);
-				int unknownA = reader.readInt16(); // faction ID?
-				int unknownB = reader.readInt16(); // faction building ID?
+				int rawBuilding = reader.readInt32();
+				int rawFaction = reader.readInt16(); // -1 for buildings common to all factions
+				reader.readInt16(); // unknown, seen as -1 and 0
 				reader.readBool(); // showMessage flag
-				// the building was read without faction context, so a set faction field would mean the emitted
-				// building id is the wrong one - refuse rather than erect something else
-				if(unknownA != -1 || unknownB != -1)
-					throw unsupported("CONSTRUCT_BUILDING with faction fields set to " + num(unknownA) + "/" + num(unknownB));
 
-				result += pad + "server:constructBuilding(town, " + num(building.getNum()) + ")\n";
+				std::optional<FactionID> faction;
+				if(rawFaction != -1)
+				{
+					faction = reader.toFaction(rawFaction);
+					if(!faction->hasValue())
+						throw unsupported("CONSTRUCT_BUILDING with unknown faction " + num(rawFaction));
+				}
+
+				BuildingID building = reader.toBuilding(rawBuilding, faction);
+				if(building == BuildingID::NONE)
+					throw unsupported("CONSTRUCT_BUILDING with unknown building " + num(rawBuilding) + " for faction " + num(rawFaction));
+
+				std::string construct = "server:constructBuilding(town, " + num(building.getNum()) + ")";
+				// a faction-specific id names a building slot that holds something else in other factions
+				if(faction)
+					result += pad + "if town:getFaction() == " + entityRef("getFactionByName", *faction) + " then " + construct + " end\n";
+				else
+					result += pad + construct + "\n";
 				break;
 			}
 			case HotaScriptActions::EXECUTE_EVENT:
@@ -720,9 +752,12 @@ std::string HotaScriptConverter::loadActions(int indent)
 				std::string targetBucket = bucketName(eventType);
 
 				// The call forwards the caller's locals, so the target handler must take the same
-				// parameters - a town handler invoked from a hero event would bind `object` to `town`
-				const std::string & args = bucketTraits(currentBucket).args;
-				if(bucketTraits(targetBucket).args != args)
+				// parameters - a town handler invoked from a hero event would bind `object` to `town`.
+				// Player handlers are the exception: every other bucket has `player` as a local, taken from the owner
+				std::string args = bucketTraits(currentBucket).args;
+				if(targetBucket == "playerEvents")
+					args = bucketTraits(targetBucket).args;
+				else if(bucketTraits(targetBucket).args != args)
 					throw unsupported("EXECUTE_EVENT targeting " + targetBucket + ", which takes different parameters");
 
 				result += pad + "Map:" + eventHandlerName(targetBucket, eventID) + "(" + args + ")\n";
@@ -730,7 +765,7 @@ std::string HotaScriptConverter::loadActions(int indent)
 			}
 			case HotaScriptActions::RESOURCES:
 			{
-				int mode = reader.readInt8(); // 0 = give, 1 = take
+				int mode = reader.readInt8(); // 0 = give, 1 = take, 2 = set
 				std::string amounts;
 				for(int i = 0; i < 7; ++i)
 				{
@@ -739,15 +774,12 @@ std::string HotaScriptConverter::loadActions(int indent)
 					amounts += loadExpression();
 				}
 				reader.readBool(); // showMessage flag
-				// spell and movement points read the same field as 0 = add, 1 = subtract, 2 = set the total,
-				// so mode 2 likely sets the treasury - unverified, no known map uses it here
-				if(mode != 0 && mode != 1)
-				{
-					logGlobal->warn("Map '%s': RESOURCES event with unknown mode %d, possibly 'set total'!", mapName, mode);
+				if(mode == 2)
+					result += pad + "setResources(game, server, player, {" + amounts + "})\n";
+				else if(mode == 0 || mode == 1)
+					result += pad + "changeResources(server, player, {" + amounts + "}, " + (mode == 0 ? "1" : "-1") + ")\n";
+				else
 					throw unsupported("RESOURCES with mode " + num(mode));
-				}
-
-				result += pad + "changeResources(server, player, {" + amounts + "}, " + (mode == 0 ? "1" : "-1") + ")\n";
 				break;
 			}
 			case HotaScriptActions::PRIMARY_SKILL:
@@ -787,7 +819,7 @@ std::string HotaScriptConverter::loadActions(int indent)
 				break;
 			}
 			default:
-				throw std::runtime_error("Unknown event action code:" + std::to_string(static_cast<int>(actionType)));
+				throw unsupported("Unknown event action code " + std::to_string(static_cast<int>(actionType)));
 		}
 	}
 	return result;
@@ -801,6 +833,10 @@ std::string HotaScriptConverter::loadCondition()
 
 std::string HotaScriptConverter::loadConditionInternal()
 {
+	// HotA editor writes 0 when the condition has no target object selected
+	// TODO: unknown whether HotA treats such condition as always true or as always false
+	constexpr uint32_t noTargetObject = 0;
+
 	auto conditionCode = static_cast<HotaScriptCondition>(reader.readInt32());
 	switch(conditionCode)
 	{
@@ -872,6 +908,10 @@ std::string HotaScriptConverter::loadConditionInternal()
 		}
 		case HotaScriptCondition::TOWN_IS_NEUTRAL:
 			return "(town:getOwner() == ENUM.PlayerColor.neutral)";
+		case HotaScriptCondition::HERO_CLASS:
+			return "(hero:getHeroClass() == " + entityRef("getHeroClassByName", reader.readHeroClass32()) + ")";
+		case HotaScriptCondition::HERO_TYPE:
+			return "(hero:getHeroType() == " + entityRef("getHeroTypeByName", reader.readHero32()) + ")";
 		case HotaScriptCondition::PLAYER_DEFEATED:
 		{
 			PlayerColor conditionPlayer = reader.readPlayer32();
@@ -889,25 +929,24 @@ std::string HotaScriptConverter::loadConditionInternal()
 			return "playerStartingFaction(game, resolvePlayer(" + num(conditionPlayer.getNum()) + ", player), " + entityRef("getFactionByName", faction) + ")";
 		}
 		case HotaScriptCondition::PLAYER_DEFEATED_MONSTER:
-		{
-			PlayerColor conditionPlayer = reader.readPlayer32();
-			uint32_t targetObjectID = reader.readUInt32();
-			return "playerDestroyedObject(game, resolvePlayer(" + num(conditionPlayer.getNum()) + ", player), " + questObjectRef(targetObjectID) + ")";
-		}
 		case HotaScriptCondition::PLAYER_DEFEATED_HERO:
 		{
 			PlayerColor conditionPlayer = reader.readPlayer32();
 			uint32_t targetObjectID = reader.readUInt32();
+			if(targetObjectID == noTargetObject)
+				return "false";
 			return "playerDestroyedObject(game, resolvePlayer(" + num(conditionPlayer.getNum()) + ", player), " + questObjectRef(targetObjectID) + ")";
 		}
 		case HotaScriptCondition::PLAYER_OWNS_TOWN:
 		{
 			PlayerColor conditionPlayer = reader.readPlayer32();
 			uint32_t targetObjectID = reader.readUInt32();
+			if(targetObjectID == noTargetObject)
+				return "false";
 			return "playerOwnsTown(game, resolvePlayer(" + num(conditionPlayer.getNum()) + ", player), " + questObjectRef(targetObjectID) + ")";
 		}
 		default:
-			throw std::runtime_error("Unknown event condition code:" + std::to_string(static_cast<int>(conditionCode)));
+			throw unsupported("Unknown event condition code " + std::to_string(static_cast<int>(conditionCode)));
 	}
 }
 
@@ -990,7 +1029,7 @@ std::string HotaScriptConverter::loadExpressionInternal()
 			return "hero:ownedArtifacts(" + entityRef("getArtifactByName", artifact) + ")";
 		}
 		default:
-			throw std::runtime_error("Unknown event expression code:" + std::to_string(static_cast<int>(expressionCode)));
+			throw unsupported("Unknown event expression code " + std::to_string(static_cast<int>(expressionCode)));
 	}
 }
 

lib/mapping/HotaScriptConverter.h

@@ -37,7 +37,7 @@ public:
 
 	/// Stable key of the Lua handler for an event bucket + id. Shared by the converter (which emits
 	/// the handler table key) and the H3M loader (which tags objects/events with the handler to fire).
-	std::string eventHandlerName(const std::string & bucket, int eventID);
+	static std::string eventHandlerName(const std::string & bucket, int eventID);
 
 private:
 	MapReaderH3M & reader;
@@ -71,6 +71,9 @@ private:
 	/// Error for a field whose meaning is not known yet, naming the map and the event being converted
 	std::runtime_error unsupported(const std::string & message) const;
 
+	std::string luaString(const std::string & value) const;
+	std::string bucketName(int eventType) const;
+
 	std::string loadImageList(int count);
 	std::string localizedText(const std::string & role); ///< reads a text field, returns a Lua MetaString table holding its identifier
 

lib/mapping/MapFeaturesH3M.h

@@ -29,36 +29,36 @@ public:
 
 	// number of bytes in bitmask of appropriate type
 
-	int factionsBytes;
-	int heroesBytes;
-	int artifactsBytes;
-	int resourcesBytes;
-	int skillsBytes;
-	int spellsBytes;
-	int buildingsBytes;
+	int factionsBytes = 0;
+	int heroesBytes = 0;
+	int artifactsBytes = 0;
+	int resourcesBytes = 0;
+	int skillsBytes = 0;
+	int spellsBytes = 0;
+	int buildingsBytes = 0;
 
 	// total number of elements of appropriate type
 
-	int factionsCount;
-	int heroesCount;
-	int heroesPortraitsCount;
-	int artifactsCount;
-	int resourcesCount;
-	int creaturesCount;
-	int spellsCount;
-	int skillsCount;
-	int terrainsCount;
-	int roadsCount;
-	int riversCount;
-	int artifactSlotsCount;
-	int buildingsCount;
+	int factionsCount = 0;
+	int heroesCount = 0;
+	int heroesPortraitsCount = 0;
+	int artifactsCount = 0;
+	int resourcesCount = 0;
+	int creaturesCount = 0;
+	int spellsCount = 0;
+	int skillsCount = 0;
+	int terrainsCount = 0;
+	int roadsCount = 0;
+	int riversCount = 0;
+	int artifactSlotsCount = 0;
+	int buildingsCount = 0;
 
 	// identifier that should be treated as "invalid", usually - '-1'
 
-	int heroIdentifierInvalid;
-	int artifactIdentifierInvalid;
-	int creatureIdentifierInvalid;
-	int spellIdentifierInvalid;
+	int heroIdentifierInvalid = 0;
+	int artifactIdentifierInvalid = 0;
+	int creatureIdentifierInvalid = 0;
+	int spellIdentifierInvalid = 0;
 
 	// features from which map format are available
 

lib/mapping/MapFormatH3M.h

@@ -20,6 +20,7 @@ class HotaScriptConverter;
 class MetaString;
 class CArtifactInstance;
 class CArmedInstance;
+class CRewardableObject;
 class CGObjectInstance;
 class SeerHut;
 class QuestSource;
@@ -219,7 +220,6 @@ private:
 	std::shared_ptr<CGObjectInstance> readGrail(const int3 & position);
 	std::shared_ptr<CGObjectInstance> readHotaBattleLocation(const int3 & position);
 	std::shared_ptr<CGObjectInstance> readQuestGuard(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
-	std::shared_ptr<CGObjectInstance> readQuestGate(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
 	std::shared_ptr<CGObjectInstance> readShipyard(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate);
 	std::shared_ptr<CGObjectInstance> readLighthouse(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate);
 	std::shared_ptr<CGObjectInstance> readGeneric(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
@@ -236,6 +236,22 @@ private:
 	std::shared_ptr<CGObjectInstance> readBlackMarket(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
 	std::shared_ptr<CGObjectInstance> readUniversity(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate);
 
+	/// Reward settings shared by several HotA objects. Fields not used by specific object or content are often garbage
+	struct HotaRewardBlock
+	{
+		int32_t content;
+		int32_t artifact;
+		int32_t amountA;
+		int8_t resourceA;
+		int32_t amountB;
+		int8_t resourceB;
+	};
+
+	HotaRewardBlock readHotaRewardBlock();
+	void presetEntityVariable(CRewardableObject & object, const std::string & category, const std::string & name, const std::string & jsonKey);
+	/// presets resource and its amount, if resource is valid
+	void presetResourceVariable(CRewardableObject & object, int8_t resourceRaw, int32_t amount, const std::string & suffix);
+
 	/**
 	 * Reads a creature set.
 	 *
@@ -268,10 +284,13 @@ private:
 	*/
 	void readMessageAndGuards(MetaString & message, CArmedInstance * guards, const int3 & position, const ObjectInstanceID & idToBeGiven);
 
+	/// reads reference to HotA event system handler and returns its script name
+	std::string readEventHandler(const std::string & bucket);
+
 	/// reads string from input stream and converts it to unicode
 	std::string readBasicString();
-	/// reads name that map maker gave to quest giver, if map format has one
-    void readQuestGiverName(Quest & quest, const int3 & position, int questIndex);
+	/// reads custom message for quest running out of time, if map format has one
+	void readQuestTimeoutMessage(Quest & quest, const int3 & position, int questIndex);
 
 	/// reads string from input stream, converts it to unicode and attempts to translate it
 	std::string readLocalizedString(const TextIdentifier & identifier);
@@ -291,14 +310,10 @@ private:
 	std::map<si32, ObjectInstanceID> questIdentifierToId;
 	std::map<Quest*, si32> questsToResolve;
 
-	/** ptr to the map object which gets filled by data from the buffer */
-	CMap * map;
-
-	/**
-	 * ptr to the map header object which gets filled by data from the buffer.
-	 * (when loading a map then the mapHeader ptr points to the same object)
-	 */
-	std::unique_ptr<CMapHeader> mapHeader;
+	/// map being loaded, owned by caller. Null when only header is loaded
+	CMap * map = nullptr;
+	/// header being loaded, owned by caller. Points to the same object as map when full map is loaded
+	CMapHeader * mapHeader = nullptr;
 	std::unique_ptr<MapReaderH3M> reader;
 	std::unique_ptr<HotaScriptConverter> scriptConverter;
 	CInputStream * inputStream;

lib/mapping/MapIdentifiersH3M.cpp

@@ -117,7 +117,7 @@ void MapIdentifiersH3M::loadMapping(const JsonNode & mapping)
 	loadMapping(mappingCampaignRegions, mapping["campaignRegions"], "campaignRegion");
 }
 
-void MapIdentifiersH3M::remapTemplate(ObjectTemplate & objectTemplate)
+void MapIdentifiersH3M::remapTemplate(ObjectTemplate & objectTemplate, const std::string & mapName)
 {
 	auto name = objectTemplate.animationFile;
 
@@ -144,7 +144,7 @@ void MapIdentifiersH3M::remapTemplate(ObjectTemplate & objectTemplate)
 
 	if (LIBRARY->objtypeh->knownObjects().count(objectTemplate.id) == 0)
 	{
-		logGlobal->warn("Unknown object found: %d | %d (%s)", objectTemplate.id, objectTemplate.subid, objectTemplate.animationFile.getName());
+		logGlobal->warn("Map '%s': Unknown object found: %d | %d (%s)", mapName, objectTemplate.id, objectTemplate.subid, objectTemplate.animationFile.getName());
 
 		objectTemplate.id = Obj::NOTHING;
 		objectTemplate.subid = {};
@@ -153,7 +153,7 @@ void MapIdentifiersH3M::remapTemplate(ObjectTemplate & objectTemplate)
 	{
 		if (LIBRARY->objtypeh->knownSubObjects(objectTemplate.id).count(objectTemplate.subid) == 0)
 		{
-			logGlobal->warn("Unknown subobject found: %d | %d", objectTemplate.id, objectTemplate.subid);
+			logGlobal->warn("Map '%s': Unknown subobject found: %d | %d", mapName, objectTemplate.id, objectTemplate.subid);
 			objectTemplate.subid = {};
 		}
 	}

lib/mapping/MapIdentifiersH3M.h

@@ -55,7 +55,7 @@ class DLL_LINKAGE MapIdentifiersH3M
 public:
 	void loadMapping(const JsonNode & mapping);
 
-	void remapTemplate(ObjectTemplate & objectTemplate);
+	void remapTemplate(ObjectTemplate & objectTemplate, const std::string & mapName);
 
 	AudioPath remapCampaignMusic(int index) const;
 	std::pair<VideoPath, VideoPath> remapCampaignVideo(int index) const;

lib/mapping/MapReaderH3M.cpp

@@ -45,8 +45,19 @@ Identifier MapReaderH3M::remapIdentifier(const Identifier & identifier)
 	return remapper.remap(identifier);
 }
 
-MapReaderH3M::MapReaderH3M(CInputStream * stream)
+template<class Identifier>
+Identifier MapReaderH3M::validateIdentifier(const Identifier & identifier, int32_t count, const std::string & typeName)
+{
+	if(identifier == Identifier::NONE || (identifier.getNum() >= 0 && identifier.getNum() < count))
+		return identifier;
+
+	logGlobal->warn("Map '%s': Map contains invalid %s %d!", mapName, typeName, identifier.getNum());
+	return Identifier::NONE;
+}
+
+MapReaderH3M::MapReaderH3M(CInputStream * stream, const std::string & mapName)
 	: reader(std::make_unique<CBinaryReader>(stream))
+	, mapName(mapName)
 {
 }
 
@@ -75,7 +86,7 @@ ArtifactID MapReaderH3M::readArtifact()
 	if (result.getNum() < features.artifactsCount)
 		return remapIdentifier(result);
 
-	logGlobal->warn("Map contains invalid artifact %d. Will be removed!", result.getNum());
+	logGlobal->warn("Map '%s': Map contains invalid artifact %d. Will be removed!", mapName, result.getNum());
 	return ArtifactID::NONE;
 }
 
@@ -89,22 +100,18 @@ ArtifactID MapReaderH3M::readArtifact8()
 	if (result.getNum() < features.artifactsCount)
 		return remapIdentifier(result);
 
-	logGlobal->warn("Map contains invalid artifact %d. Will be removed!", result.getNum());
+	logGlobal->warn("Map '%s': Map contains invalid artifact %d. Will be removed!", mapName, result.getNum());
 	return ArtifactID::NONE;
 }
 
 ArtifactID MapReaderH3M::readArtifact32()
 {
-	ArtifactID result(reader->readInt32());
+	return toArtifact(reader->readInt32());
+}
 
-	if(result == ArtifactID::NONE)
-		return ArtifactID::NONE;
-
-	if (result.getNum() < features.artifactsCount)
-		return remapIdentifier(result);
-
-	logGlobal->warn("Map contains invalid artifact %d. Will be removed!", result.getNum());
-	return ArtifactID::NONE;
+ArtifactID MapReaderH3M::toArtifact(int32_t raw)
+{
+	return remapIdentifier(validateIdentifier(ArtifactID(raw), features.artifactsCount, "artifact"));
 }
 
 HeroTypeID MapReaderH3M::readHero()
@@ -112,18 +119,15 @@ HeroTypeID MapReaderH3M::readHero()
 	HeroTypeID result(reader->readUInt8());
 
 	if(result.getNum() == features.heroIdentifierInvalid)
-		return HeroTypeID(-1);
+		return HeroTypeID::NONE;
 
-	assert(result.getNum() < features.heroesCount);
-	return remapIdentifier(result);
+	return remapIdentifier(validateIdentifier(result, features.heroesCount, "hero"));
 }
 
 HeroTypeID MapReaderH3M::readHero32()
 {
 	HeroTypeID result(reader->readInt32());
-
-	assert(result.getNum() < features.heroesCount);
-	return remapIdentifier(result);
+	return remapIdentifier(validateIdentifier(result, features.heroesCount, "hero"));
 }
 
 HeroTypeID MapReaderH3M::readHeroPortrait()
@@ -135,7 +139,7 @@ HeroTypeID MapReaderH3M::readHeroPortrait()
 
 	if (result.getNum() >= features.heroesPortraitsCount)
 	{
-		logGlobal->warn("Map contains invalid hero portrait ID %d. Will be reset!", result.getNum() );
+		logGlobal->warn("Map '%s': Map contains invalid hero portrait ID %d. Will be reset!", mapName, result.getNum() );
 		return HeroTypeID::NONE;
 	}
 
@@ -160,9 +164,9 @@ CreatureID MapReaderH3M::readCreature32(const std::string & context)
 		return randomIndex;
 
 	if(context.empty())
-		logGlobal->warn("Map contains invalid creature %d. Will be removed!", result.getNum());
+		logGlobal->warn("Map '%s': Map contains invalid creature %d. Will be removed!", mapName, result.getNum());
 	else
-		logGlobal->warn("Map contains invalid creature %d in %s. Will be removed!", result.getNum(), context);
+		logGlobal->warn("Map '%s': Map contains invalid creature %d in %s. Will be removed!", mapName, result.getNum(), context);
 	return CreatureID::NONE;
 }
 
@@ -189,17 +193,25 @@ CreatureID MapReaderH3M::readCreature(const std::string & context)
 		return randomIndex;
 
 	if(context.empty())
-		logGlobal->warn("Map contains invalid creature %d. Will be removed!", result.getNum());
+		logGlobal->warn("Map '%s': Map contains invalid creature %d. Will be removed!", mapName, result.getNum());
 	else
-		logGlobal->warn("Map contains invalid creature %d in %s. Will be removed!", result.getNum(), context);
+		logGlobal->warn("Map '%s': Map contains invalid creature %d in %s. Will be removed!", mapName, result.getNum(), context);
 	return CreatureID::NONE;
 }
 
 FactionID MapReaderH3M::readFaction32()
 {
-	FactionID result(readInt32());
-	assert(result.getNum() < features.factionsCount);
-	return remapIdentifier(result);
+	return toFaction(readInt32());
+}
+
+FactionID MapReaderH3M::toFaction(int32_t raw)
+{
+	return remapIdentifier(validateIdentifier(FactionID(raw), features.factionsCount, "faction"));
+}
+
+HeroClassID MapReaderH3M::readHeroClass32()
+{
+	return remapIdentifier(HeroClassID(readInt32()));
 }
 
 TerrainId MapReaderH3M::readTerrain()
@@ -211,8 +223,8 @@ TerrainId MapReaderH3M::readTerrain()
 		// Map uses terrain index we don't support (e.g. extended terrain from other editor/mod)
 		// Fallback: clamp to last known terrain
 		logGlobal->warn(
-			"MapReaderH3M::readTerrain: map uses terrain %u but only %u types are supported. "
-			"Clamping to %u.",
+			"Map '%s': Map uses terrain %u but only %u types are supported. Clamping to %u.",
+			mapName,
 			raw, features.terrainsCount, features.terrainsCount ? features.terrainsCount - 1 : 0);
 
 		raw = features.terrainsCount ? features.terrainsCount - 1 : 0;
@@ -233,8 +245,8 @@ RoadId MapReaderH3M::readRoad()
 		// Map uses extended road type not supported by current config.
 		// Fallback: use the last supported road type (usually cobblestone).
 		logGlobal->warn(
-			"MapReaderH3M::readRoad: map uses road type %u but only %u types are supported. "
-			"Clamping to %u.",
+			"Map '%s': Map uses road type %u but only %u types are supported. Clamping to %u.",
+			mapName,
 			type, features.roadsCount, features.roadsCount);
 
 		type = features.roadsCount;
@@ -256,8 +268,8 @@ RiverId MapReaderH3M::readRiver()
 		const uint8_t fallback = features.riversCount ? features.riversCount : 0;
 
 		logGlobal->warn(
-			"MapReaderH3M::readRiver: map uses river type %u but only %u types are supported. "
-			"Clamping to %u.",
+			"Map '%s': Map uses river type %u but only %u types are supported. Clamping to %u.",
+			mapName,
 			type, features.riversCount, fallback);
 
 		type = fallback;
@@ -269,29 +281,25 @@ RiverId MapReaderH3M::readRiver()
 PrimarySkill MapReaderH3M::readPrimary()
 {
 	PrimarySkill result(readUInt8());
-	assert(result <= PrimarySkill::KNOWLEDGE );
-	return result;
+	return validateIdentifier(result, GameConstants::PRIMARY_SKILLS, "primary skill");
 }
 
 PrimarySkill MapReaderH3M::readPrimary32()
 {
 	PrimarySkill result(readInt32());
-	assert(result <= PrimarySkill::KNOWLEDGE );
-	return result;
+	return validateIdentifier(result, GameConstants::PRIMARY_SKILLS, "primary skill");
 }
 
 SecondarySkill MapReaderH3M::readSkill()
 {
 	SecondarySkill result(readUInt8());
-	assert(result.getNum() < features.skillsCount);
-	return remapIdentifier(result);
+	return remapIdentifier(validateIdentifier(result, features.skillsCount, "secondary skill"));
 }
 
 SecondarySkill MapReaderH3M::readSkill32()
 {
 	SecondarySkill result(readInt32());
-	assert(result.getNum() < features.skillsCount);
-	return remapIdentifier(result);
+	return remapIdentifier(validateIdentifier(result, features.skillsCount, "secondary skill"));
 }
 
 SpellID MapReaderH3M::readSpell()
@@ -302,40 +310,39 @@ SpellID MapReaderH3M::readSpell()
 	if(result.getNum() == features.spellIdentifierInvalid - 1)
 		return SpellID::PRESET;
 
-	assert(result.getNum() < features.spellsCount);
-	return remapIdentifier(result);
+	return remapIdentifier(validateIdentifier(result, features.spellsCount, "spell"));
 }
 
 SpellID MapReaderH3M::readSpell16()
 {
-	SpellID result(readInt16());
-	if(result.getNum() == features.spellIdentifierInvalid)
-		return SpellID::NONE;
-	assert(result.getNum() < features.spellsCount);
-	return result;
+	return toSpell(readInt16());
 }
 
 SpellID MapReaderH3M::readSpell32()
 {
-	SpellID result(readInt32());
-	if(result.getNum() == features.spellIdentifierInvalid)
+	return toSpell(readInt32());
+}
+
+SpellID MapReaderH3M::toSpell(int32_t raw)
+{
+	if(raw == features.spellIdentifierInvalid)
 		return SpellID::NONE;
-	assert(result.getNum() < features.spellsCount);
-	return result;
+	return validateIdentifier(SpellID(raw), features.spellsCount, "spell");
 }
 
 GameResID MapReaderH3M::readGameResID()
 {
-	GameResID result(readInt8());
-	assert(result.getNum() < features.resourcesCount);
-	return result;
+	return toGameResID(readInt8());
 }
 
 GameResID MapReaderH3M::readGameResID32()
 {
-	GameResID result(readInt32());
-	assert(result.getNum() < features.resourcesCount);
-	return result;
+	return toGameResID(readInt32());
+}
+
+GameResID MapReaderH3M::toGameResID(int32_t raw)
+{
+	return validateIdentifier(GameResID(raw), features.resourcesCount, "resource");
 }
 
 PlayerColor MapReaderH3M::readPlayer()
@@ -347,7 +354,7 @@ PlayerColor MapReaderH3M::readPlayer()
 
 	if (value >= PlayerColor::PLAYER_LIMIT_I)
 	{
-		logGlobal->warn("Map contains invalid player ID %d. Will be reset!", value );
+		logGlobal->warn("Map '%s': Map contains invalid player ID %d. Will be reset!", mapName, value);
 		return PlayerColor::NEUTRAL;
 	}
 
@@ -361,9 +368,13 @@ PlayerColor MapReaderH3M::readPlayer32()
 	if (value == 255)
 		return PlayerColor::NEUTRAL;
 
+	// HotA scripts use it as "current hero" marker
+	if (value == 254)
+		return PlayerColor::UNFLAGGABLE;
+
 	if (value >= PlayerColor::PLAYER_LIMIT_I)
 	{
-		logGlobal->warn("Map contains invalid player ID %d. Will be reset!", value );
+		logGlobal->warn("Map '%s': Map contains invalid player ID %d. Will be reset!", mapName, value);
 		return PlayerColor::NEUTRAL;
 	}
 
@@ -376,6 +387,11 @@ BuildingID MapReaderH3M::readBuilding32(std::optional<FactionID> faction)
 	return remapper.remapBuilding(faction, value);
 }
 
+BuildingID MapReaderH3M::toBuilding(int32_t raw, std::optional<FactionID> faction)
+{
+	return remapper.remapBuilding(faction, BuildingID(raw));
+}
+
 void MapReaderH3M::readBitmaskBuildings(std::set<BuildingID> & dest, std::optional<FactionID> faction)
 {
 	std::set<BuildingID> h3m;
@@ -422,9 +438,10 @@ void MapReaderH3M::readBitmaskHeroesSized(std::set<HeroTypeID> & dest, bool inve
 {
 	uint32_t heroesCount = readUInt32();
 	uint32_t heroesBytes = (heroesCount + 7) / 8;
-	assert(heroesCount <= features.heroesCount);
+	if(heroesCount > features.heroesCount)
+		logGlobal->warn("Map '%s': Map contains %d heroes, but only %d are supported. Extra heroes will be ignored!", mapName, heroesCount, features.heroesCount);
 
-	readBitmask<HeroTypeID>(dest, heroesBytes, heroesCount, invert);
+	readBitmask<HeroTypeID>(dest, heroesBytes, std::min<int>(heroesCount, features.heroesCount), invert);
 }
 
 void MapReaderH3M::readBitmaskArtifacts(std::set<ArtifactID> &dest, bool invert)
@@ -436,9 +453,10 @@ void MapReaderH3M::readBitmaskArtifactsSized(std::set<ArtifactID> &dest, bool in
 {
 	uint32_t artifactsCount = reader->readUInt32();
 	uint32_t artifactsBytes = (artifactsCount + 7) / 8;
-	assert(artifactsCount <= features.artifactsCount);
+	if(artifactsCount > features.artifactsCount)
+		logGlobal->warn("Map '%s': Map contains %d artifacts, but only %d are supported. Extra artifacts will be ignored!", mapName, artifactsCount, features.artifactsCount);
 
-	readBitmask<ArtifactID>(dest, artifactsBytes, artifactsCount, invert);
+	readBitmask<ArtifactID>(dest, artifactsBytes, std::min<int>(artifactsCount, features.artifactsCount), invert);
 }
 
 void MapReaderH3M::readBitmaskSpells(std::set<SpellID> & dest, bool invert)
@@ -495,7 +513,7 @@ std::shared_ptr<ObjectTemplate> MapReaderH3M::readObjectTemplate()
 
 void MapReaderH3M::remapTemplate(ObjectTemplate & tmpl)
 {
-	remapper.remapTemplate(tmpl);
+	remapper.remapTemplate(tmpl, mapName);
 }
 
 void MapReaderH3M::skipUnused(size_t amount)
@@ -505,15 +523,13 @@ void MapReaderH3M::skipUnused(size_t amount)
 
 void MapReaderH3M::skipZero(size_t amount)
 {
-#ifdef NDEBUG
-	skipUnused(amount);
-#else
 	for(size_t i = 0; i < amount; ++i)
 	{
 		uint8_t value = reader->readUInt8();
+		if(value != 0)
+			logGlobal->warn("Map '%s': Expected zero byte, but %d found!", mapName, static_cast<int>(value));
 		assert(value == 0);
 	}
-#endif
 }
 
 void MapReaderH3M::readResources(TResources & resources)
@@ -529,7 +545,8 @@ bool MapReaderH3M::readBool()
 	if(raw != 0 && raw != 1)
 	{
 		logGlobal->warn(
-			"MapReaderH3M: invalid bool value %u in H3M data, using LSB (%u) as value",
+			"Map '%s': Invalid bool value %u, using LSB (%u) as value",
+			mapName,
 			static_cast<unsigned>(raw),
 			static_cast<unsigned>(raw & 1));
 	}
@@ -537,16 +554,35 @@ bool MapReaderH3M::readBool()
 	return (raw & 1) != 0;
 }
 
+int32_t MapReaderH3M::readInt32Checked(int32_t lowerLimit, int32_t upperLimit)
+{
+	int32_t result = readInt32();
+	int32_t resultClamped = std::clamp(result, lowerLimit, upperLimit);
+	if (result != resultClamped)
+		logGlobal->warn("Map '%s': Map contains out of range value %d! Expected %d-%d", mapName, result, lowerLimit, upperLimit);
+
+	return resultClamped;
+}
+
 int8_t MapReaderH3M::readInt8Checked(int8_t lowerLimit, int8_t upperLimit)
 {
 	int8_t result = readInt8();
 	int8_t resultClamped = std::clamp(result, lowerLimit, upperLimit);
 	if (result != resultClamped)
-		logGlobal->warn("Map contains out of range value %d! Expected %d-%d", static_cast<int>(result), static_cast<int>(lowerLimit), static_cast<int>(upperLimit));
+		logGlobal->warn("Map '%s': Map contains out of range value %d! Expected %d-%d", mapName, static_cast<int>(result), static_cast<int>(lowerLimit), static_cast<int>(upperLimit));
 
 	return resultClamped;
 }
 
+int8_t MapReaderH3M::readInt8Strict(int8_t lowerLimit, int8_t upperLimit)
+{
+	int8_t result = readInt8();
+	if (result < lowerLimit || result > upperLimit)
+		throw std::runtime_error("Map '" + mapName + "': Map contains out of range value " + std::to_string(result) + ", expected " + std::to_string(lowerLimit) + "-" + std::to_string(upperLimit));
+
+	return result;
+}
+
 uint8_t MapReaderH3M::readUInt8()
 {
 	return reader->readUInt8();

lib/mapping/MapReaderH3M.h

@@ -24,7 +24,7 @@ enum class EMapFormat : uint8_t;
 class MapReaderH3M
 {
 public:
-	explicit MapReaderH3M(CInputStream * stream);
+	MapReaderH3M(CInputStream * stream, const std::string & mapName);
 
 	void setFormatLevel(const MapFormatFeaturesH3M & features);
 	void setIdentifierRemapper(const MapIdentifiersH3M & remapper);
@@ -38,6 +38,7 @@ public:
 	HeroTypeID readHero();
 	HeroTypeID readHero32();
 	HeroTypeID readHeroPortrait();
+	HeroClassID readHeroClass32();
 	FactionID readFaction32();
 	TerrainId readTerrain();
 	RoadId readRoad();
@@ -52,8 +53,17 @@ public:
 	GameResID readGameResID();
 	GameResID readGameResID32();
 	PlayerColor readPlayer();
+
 	PlayerColor readPlayer32();
 
+	/// validate and remap already read raw value, for fields that may contain garbage depending on other fields
+	ArtifactID toArtifact(int32_t raw);
+	SpellID toSpell(int32_t raw);
+	GameResID toGameResID(int32_t raw);
+	FactionID toFaction(int32_t raw);
+	/// faction of nullopt means building id that is common to all factions
+	BuildingID toBuilding(int32_t raw, std::optional<FactionID> faction);
+
 	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);
@@ -80,13 +90,17 @@ public:
 
 	uint8_t readUInt8();
 	int8_t readInt8();
+	/// clamps out of range value with a warning
 	int8_t readInt8Checked(int8_t lowerLimit, int8_t upperLimit);
+	/// throws on out of range value, for values that define layout of following data
+	int8_t readInt8Strict(int8_t lowerLimit, int8_t upperLimit);
 
 	int16_t readInt16();
 	uint16_t readUInt16();
 
 	uint32_t readUInt32();
 	int32_t readInt32();
+	int32_t readInt32Checked(int32_t lowerLimit, int32_t upperLimit);
 
 	std::string readBaseString();
 
@@ -94,6 +108,9 @@ private:
 	template<class Identifier>
 	Identifier remapIdentifier(const Identifier & identifier);
 
+	template<class Identifier>
+	Identifier validateIdentifier(const Identifier & identifier, int32_t count, const std::string & typeName);
+
 	template<class Identifier>
 	void readBitmask(std::set<Identifier> & dest, int bytesToRead, int objectsToRead, bool invert);
 
@@ -101,4 +118,5 @@ private:
 	MapIdentifiersH3M remapper;
 
 	std::unique_ptr<CBinaryReader> reader;
+	std::string mapName;
 };

lib/serializer/ESerializationVersion.h

@@ -59,7 +59,7 @@ enum class ESerializationVersion : int32_t
 	RECORD_TEXTS_METASTRING, // highscore scenario name and statistics map name are stored unresolved, to be rendered by the reader
 	BONUS_TARGET_SOURCE_ID, // bonus can restrict PERCENT_TO_TARGET_TYPE to a single source ID
 	SECONDARY_SKILL_OFFER_COOLDOWN, // hero stores level at which skills with offer cooldown were gained
-	SEER_HUT_NAME_TEXT_ID, // seer name is stored as text identifier, and a quest may override it with a map-defined name
+	SEER_HUT_NAME_TEXT_ID, // seer name is stored as text identifier, and a quest stores map-defined timeout message
 	SIMTURNS_CONTACT_STATUS, // game state stores players that still play simultaneous turns without contact
 	BATTLE_RESTORE_ARTIFACT_CHARGES, // battle stores initial charges of artifacts to restore them if battle is cancelled
 	CREATURE_REFUSED_JOINING_REMOVED, // wandering creatures no longer store a refused offer to join, the visit tracks it

luascript/api/adventure/HeroInstance.cpp

@@ -11,12 +11,15 @@
 
 #include "HeroInstance.h"
 
+#include <vcmi/HeroType.h>
+
 #include "../Registry.h"
 #include "../library/BonusBearerBindings.h"
 
 #include "StackInstance.h"
 
 #include "../../../lib/CCreatureHandler.h"
+#include "../../../lib/GameLibrary.h"
 #include "../../../lib/entities/artifact/CArtifactInstance.h"
 
 namespace scripting::api
@@ -51,6 +54,12 @@ void HeroInstanceProxy::registerMethods(MethodRegistrar & R)
 		{{"skill", "Secondary skill JSON key."}},
 		{"Mastery level (0 = none, 1 = basic, 2 = advanced, 3 = expert)."},
 		"Returns the hero's mastery of the given secondary skill.");
+	R.function<&HeroInstanceProxy::getHeroType>("getHeroType",
+		{"Type of the hero, comparable with the result of Services:getHeroTypeByName."},
+		"Returns the hero type (Orrin, Kyrre, Astral...) of this hero.");
+	R.function<&HeroInstanceProxy::getHeroClass>("getHeroClass",
+		{"Class of the hero, comparable with the result of Services:getHeroClassByName."},
+		"Returns the hero class of this hero.");
 	R.function<&HeroInstanceProxy::hasArtifact>("hasArtifact",
 		{{"artifact", "Artifact JSON key."}},
 		{"True if the hero owns the artifact."},
@@ -85,6 +94,16 @@ int64_t HeroInstanceProxy::getExperience(const CGHeroInstance & hero)
 	return hero.exp;
 }
 
+const HeroType * HeroInstanceProxy::getHeroType(const CGHeroInstance & hero)
+{
+	return hero.getHeroTypeID().toEntity(LIBRARY);
+}
+
+const HeroClass * HeroInstanceProxy::getHeroClass(const CGHeroInstance & hero)
+{
+	return hero.getHeroClassID().toEntity(LIBRARY);
+}
+
 bool HeroInstanceProxy::hasArtifact(const CGHeroInstance & hero, ArtifactID artifact)
 {
 	return hero.hasArt(artifact);

luascript/api/adventure/HeroInstance.h

@@ -26,6 +26,8 @@ class HeroInstanceProxy : public RawPointerWrapper<const CGHeroInstance, HeroIns
 	static bool isFemale(const CGHeroInstance & hero);
 	static int getLevel(const CGHeroInstance & hero);
 	static int64_t getExperience(const CGHeroInstance & hero);
+	static const HeroType * getHeroType(const CGHeroInstance & hero);
+	static const HeroClass * getHeroClass(const CGHeroInstance & hero);
 	static bool hasArtifact(const CGHeroInstance & hero, ArtifactID artifact);
 	static int ownedArtifacts(const CGHeroInstance & hero, ArtifactID artifact);
 	static int creatureCountInArmy(const CGHeroInstance & hero, CreatureID creature);

luascript/api/adventure/TownInstance.cpp

@@ -11,6 +11,9 @@
 
 #include "TownInstance.h"
 
+#include <vcmi/Faction.h>
+
+#include "../../../lib/GameLibrary.h"
 #include "../../../lib/entities/building/CBuilding.h"
 #include "../../../lib/entities/faction/CTown.h"
 #include "../../../lib/mapObjects/CGObjectInstance.h"
@@ -25,6 +28,14 @@ void TownInstanceProxy::registerMethods(MethodRegistrar & R)
 	R.function<&TownInstanceProxy::getBuildings>("getBuildings",
 		{"Every building standing in this town."},
 		"Returns the buildings that have been built in this town, upgrades of other buildings among them.");
+	R.function<&TownInstanceProxy::getFaction>("getFaction",
+		{"Faction of the town, comparable with the result of Services:getFactionByName."},
+		"Returns the faction (Castle, Rampart, Tower...) of this town.");
+}
+
+const Faction * TownInstanceProxy::getFaction(const CGTownInstance & town)
+{
+	return town.getFactionID().toEntity(LIBRARY);
 }
 
 std::vector<const CBuilding *> TownInstanceProxy::getBuildings(const CGTownInstance & town)

luascript/api/adventure/TownInstance.h

@@ -29,6 +29,7 @@ public:
 	static void registerMethods(MethodRegistrar & R);
 
 	static std::vector<const CBuilding *> getBuildings(const CGTownInstance & town);
+	static const Faction * getFaction(const CGTownInstance & town);
 };
 
 }

test/quest/CQuestSeerMultiTest.cpp

@@ -236,7 +236,7 @@ TEST_F(QuestSeerMultiTest, ActiveQuestIsGlobalAcrossPlayers)
 
 // ---- quest giver name -------------------------------------------------------
 
-TEST_F(QuestSeerMultiTest, GiverName_activeQuestOverridesRolledSeerName)
+TEST_F(QuestSeerMultiTest, GiverName_isRolledSeerName)
 {
 	const int3 guardPos(9, 9, 0);
 
@@ -248,23 +248,12 @@ TEST_F(QuestSeerMultiTest, GiverName_activeQuestOverridesRolledSeerName)
 	auto * seer = expectAt<SeerHut>(kSeerPos);
 	ASSERT_EQ(seer->allQuests().size(), 2u);
 
-	Quest & active = seer->getQuest();
-	Quest & inactive = *(seer->allQuests().front().get() == &active ? seer->allQuests().back() : seer->allQuests().front());
-
-	// without an override the hut falls back to the name rolled on map start
+	// the hut uses the name rolled on map start
 	EXPECT_FALSE(seer->seerNameTextID.empty());
 	EXPECT_EQ(seer->getQuestGiverName(), seer->seerNameTextID);
 
-	// a name on some other quest of the same hut must not leak into the active one
-	inactive.questGiverNameTextID = "map.test.inactiveSeer";
-	EXPECT_EQ(seer->getQuestGiverName(), seer->seerNameTextID);
-
-	active.questGiverNameTextID = "map.test.activeSeer";
-	EXPECT_EQ(seer->getQuestGiverName(), "map.test.activeSeer");
-
-	// quest guards name no seer, even when their quest carries a name
+	// quest guards name no seer
 	auto * guard = expectAt<QuestGuard>(guardPos);
-	guard->getQuest().questGiverNameTextID = "map.test.guard";
 	EXPECT_TRUE(guard->getQuestGiverName().empty());
 }