Merge pull request #2886 from IvanSavenko/h3m_parser_fixes

H3m parser fixes

config/mapOverrides.json

@@ -75,6 +75,46 @@
 		"victoryIconIndex" : 11,
 		"victoryString" : "core.vcdesc.0"
 	},
+	"data/evil2:0" : { // A Gryphon's Heart
+		"defeatIconIndex" : 2,
+		"defeatString" : "core.lcdesc.3",
+		"triggeredEvents" : {
+			"specialDefeat" : {
+				"condition" : [
+					"allOf",
+					[ "isHuman", { "value" : 1 } ],
+					[ "daysPassed", { "value" : 84 } ]
+				],
+				"effect" : {
+					"messageToSend" : "core.genrltxt.5",
+					"type" : "defeat"
+				},
+				"message" : "core.genrltxt.254"
+			},
+			"specialVictory" : {
+				"condition" : [
+					"allOf",
+					[ "isHuman", { "value" : 1 } ],
+					[ "transport", { "position" : [ 16, 23, 0 ], "type" : 84 } ]
+				],
+				"effect" : {
+					"messageToSend" : "core.genrltxt.293",
+					"type" : "victory"
+				},
+				"message" : "core.genrltxt.292"
+			},
+			"standardDefeat" : {
+				"condition" : [ "daysWithoutTown", { "value" : 7 } ],
+				"effect" : {
+					"messageToSend" : "core.genrltxt.8",
+					"type" : "defeat"
+				},
+				"message" : "core.genrltxt.7"
+			}
+		},
+		"victoryIconIndex" : 10,
+		"victoryString" : "core.vcdesc.11"
+	},
 	"data/secret1:0" : { // The Grail
 		"defeatIconIndex" : 2,
 		"defeatString" : "core.lcdesc.3",

lib/MetaString.cpp

@@ -388,12 +388,11 @@ void MetaString::jsonDeserialize(const JsonNode & source)
 
 void MetaString::serializeJson(JsonSerializeFormat & handler)
 {
-	JsonNode attr;
 	if(handler.saving)
-		jsonSerialize(attr);
-	handler.serializeRaw("attributes", attr, std::nullopt);
+		jsonSerialize(const_cast<JsonNode&>(handler.getCurrent()));
+
 	if(!handler.saving)
-		jsonDeserialize(attr);
+		jsonDeserialize(handler.getCurrent());
 }
 
 VCMI_LIB_NAMESPACE_END

lib/mapping/MapFeaturesH3M.cpp

@@ -49,7 +49,7 @@ MapFormatFeaturesH3M MapFormatFeaturesH3M::getFeaturesROE()
 
 	result.factionsCount = 8;
 	result.heroesCount = 128;
-	result.heroesPortraitsCount = 128;
+	result.heroesPortraitsCount = 130; // +General Kendal, +Catherine (portrait-only in RoE)
 	result.artifactsCount = 127;
 	result.resourcesCount = 7;
 	result.creaturesCount = 118;

lib/mapping/MapFormatH3M.cpp

@@ -44,6 +44,7 @@ static std::string convertMapName(std::string input)
 {
 	boost::algorithm::to_lower(input);
 	boost::algorithm::trim(input);
+	boost::algorithm::erase_all(input, ".");
 
 	size_t slashPos = input.find_last_of('/');
 
@@ -345,27 +346,11 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		bool allowNormalVictory = reader->readBool();
 		bool appliesToAI = reader->readBool();
 
-		if(allowNormalVictory)
-		{
-			size_t playersOnMap = boost::range::count_if(
-				mapHeader->players,
-				[](const PlayerInfo & info)
-				{
-					return info.canAnyonePlay();
-				}
-			);
-
-			if(playersOnMap == 1)
-			{
-				logGlobal->warn("Map %s: Only one player exists, but normal victory allowed!", mapName);
-				allowNormalVictory = false; // makes sense? Not much. Works as H3? Yes!
-			}
-		}
-
 		switch(vicCondition)
 		{
 			case EVictoryConditionType::ARTIFACT:
 			{
+				assert(allowNormalVictory == true); // not selectable in editor
 				EventCondition cond(EventCondition::HAVE_ARTIFACT);
 				cond.objectType = reader->readArtifact();
 
@@ -404,6 +389,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			}
 			case EVictoryConditionType::BUILDCITY:
 			{
+				assert(appliesToAI == true); // not selectable in editor
 				EventExpression::OperatorAll oper;
 				EventCondition cond(EventCondition::HAVE_BUILDING);
 				cond.position = reader->readInt3();
@@ -421,6 +407,8 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			}
 			case EVictoryConditionType::BUILDGRAIL:
 			{
+				assert(allowNormalVictory == true); // not selectable in editor
+				assert(appliesToAI == true); // not selectable in editor
 				EventCondition cond(EventCondition::HAVE_BUILDING);
 				cond.objectType = BuildingID::GRAIL;
 				cond.position = reader->readInt3();
@@ -436,6 +424,10 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			}
 			case EVictoryConditionType::BEATHERO:
 			{
+				if (!allowNormalVictory)
+					logGlobal->warn("Map %s: Has 'beat hero' as victory condition, but 'allow normal victory' not set. Ignoring", mapName);
+				allowNormalVictory = true; // H3 behavior
+				assert(appliesToAI == false); // not selectable in editor
 				EventCondition cond(EventCondition::DESTROY);
 				cond.objectType = Obj::HERO;
 				cond.position = reader->readInt3();
@@ -462,6 +454,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			}
 			case EVictoryConditionType::BEATMONSTER:
 			{
+				assert(appliesToAI == true); // not selectable in editor
 				EventCondition cond(EventCondition::DESTROY);
 				cond.objectType = Obj::MONSTER;
 				cond.position = reader->readInt3();
@@ -500,6 +493,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			}
 			case EVictoryConditionType::TRANSPORTITEM:
 			{
+				assert(allowNormalVictory == true); // not selectable in editor
 				EventCondition cond(EventCondition::TRANSPORT);
 				cond.objectType = reader->readUInt8();
 				cond.position = reader->readInt3();
@@ -513,6 +507,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			}
 			case EVictoryConditionType::HOTA_ELIMINATE_ALL_MONSTERS:
 			{
+				assert(appliesToAI == false); // not selectable in editor
 				EventCondition cond(EventCondition::DESTROY);
 				cond.objectType = Obj::MONSTER;
 
@@ -526,6 +521,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 			}
 			case EVictoryConditionType::HOTA_SURVIVE_FOR_DAYS:
 			{
+				assert(appliesToAI == false); // not selectable in editor
 				EventCondition cond(EventCondition::DAYS_PASSED);
 				cond.value = reader->readUInt32();
 
@@ -541,6 +537,24 @@ void CMapLoaderH3M::readVictoryLossConditions()
 				assert(0);
 		}
 
+		if(allowNormalVictory)
+		{
+			size_t playersOnMap = boost::range::count_if(
+				mapHeader->players,
+				[](const PlayerInfo & info)
+				{
+					return info.canAnyonePlay();
+				}
+			);
+
+			assert(playersOnMap > 1);
+			if(playersOnMap == 1)
+			{
+				logGlobal->warn("Map %s: Only one player exists, but normal victory allowed!", mapName);
+				allowNormalVictory = false; // makes sense? Not much. Works as H3? Yes!
+			}
+		}
+
 		// if condition is human-only turn it into following construction: AllOf(human, condition)
 		if(!appliesToAI)
 		{
@@ -883,7 +897,7 @@ void CMapLoaderH3M::loadArtifactsOfHero(CGHeroInstance * hero)
 
 	if(!hero->artifactsWorn.empty() || !hero->artifactsInBackpack.empty())
 	{
-		logGlobal->warn("Hero %s at %s has set artifacts twice (in map properties and on adventure map instance). Using the latter set...", hero->getNameTranslated(), hero->pos.toString());
+		logGlobal->debug("Hero %s at %s has set artifacts twice (in map properties and on adventure map instance). Using the latter set...", hero->getNameTranslated(), hero->pos.toString());
 
 		hero->artifactsInBackpack.clear();
 		while(!hero->artifactsWorn.empty())
@@ -1551,6 +1565,9 @@ void CMapLoaderH3M::readObjects()
 		newObject->appearance = objectTemplate;
 		assert(objectInstanceID == ObjectInstanceID((si32)map->objects.size()));
 
+		if (newObject->isVisitable() && !map->isInTheMap(newObject->visitablePos()))
+			logGlobal->error("Map '%s': Object at %s - outside of map borders!", mapName, mapPosition.toString());
+
 		{
 			//TODO: define valid typeName and subtypeName for H3M maps
 			//boost::format fmt("%s_%d");
@@ -1703,7 +1720,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 		if(!object->secSkills.empty())
 		{
 			if(object->secSkills[0].first != SecondarySkill::DEFAULT)
-				logGlobal->warn("Hero %s subID=%d has set secondary skills twice (in map properties and on adventure map instance). Using the latter set...", object->getNameTextID(), object->subID);
+				logGlobal->debug("Map '%s': Hero %s subID=%d has set secondary skills twice (in map properties and on adventure map instance). Using the latter set...", mapName, object->getNameTextID(), object->subID);
 			object->secSkills.clear();
 		}
 
@@ -1775,7 +1792,7 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 			auto ps = object->getAllBonuses(Selector::type()(BonusType::PRIMARY_SKILL).And(Selector::sourceType()(BonusSource::HERO_BASE_SKILL)), nullptr);
 			if(ps->size())
 			{
-				logGlobal->warn("Hero %s subID=%d has set primary skills twice (in map properties and on adventure map instance). Using the latter set...", object->getNameTranslated(), object->subID );
+				logGlobal->debug("Hero %s subID=%d has set primary skills twice (in map properties and on adventure map instance). Using the latter set...", object->getNameTranslated(), object->subID );
 				for(const auto & b : *ps)
 					object->removeBonus(b);
 			}
@@ -1907,8 +1924,7 @@ void CMapLoaderH3M::readSeerHutQuest(CGSeerHut * hut, const int3 & position, con
 				auto rVal = reader->readUInt32();
 
 				assert(rId < features.resourcesCount);
-				assert((rVal & 0x00ffffff) == rVal);
-				
+
 				reward.resources[rId] = rVal;
 				break;
 			}
@@ -2172,7 +2188,7 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 
 		 uint8_t alignment = reader->readUInt8();
 
-		if(alignment != PlayerColor::NEUTRAL.getNum())
+		if(alignment != 255)
 		{
 			if(alignment < PlayerColor::PLAYER_LIMIT.getNum())
 			{

lib/mapping/MapFormatJson.cpp

@@ -870,6 +870,11 @@ void CMapPatcher::readPatchData()
 {
 	JsonDeserializer handler(mapObjectResolver.get(), input);
 	readTriggeredEvents(handler);
+
+	handler.serializeInt("defeatIconIndex", mapHeader->defeatIconIndex);
+	handler.serializeInt("victoryIconIndex", mapHeader->victoryIconIndex);
+	handler.serializeStruct("victoryString", mapHeader->victoryMessage);
+	handler.serializeStruct("defeatString", mapHeader->defeatMessage);
 }
 
 ///CMapLoaderJson

lib/mapping/MapIdentifiersH3M.cpp

@@ -53,11 +53,11 @@ void MapIdentifiersH3M::loadMapping(const JsonNode & mapping)
 
 	for (auto entryTemplate : mapping["templates"].Struct())
 	{
-		std::string h3mName = boost::to_lower_copy(entryTemplate.second.String());
-		std::string vcmiName = boost::to_lower_copy(entryTemplate.first);
+		AnimationPath h3mName = AnimationPath::builtinTODO(entryTemplate.second.String());
+		AnimationPath vcmiName = AnimationPath::builtinTODO(entryTemplate.first);
 
-		if (!CResourceHandler::get()->existsResource(AnimationPath::builtinTODO("SPRITES/" + vcmiName)))
-			logMod->warn("Template animation file %s was not found!", vcmiName);
+		if (!CResourceHandler::get()->existsResource(vcmiName.addPrefix("SPRITES/")))
+			logMod->warn("Template animation file %s was not found!", vcmiName.getOriginalName());
 
 		mappingObjectTemplate[h3mName] = vcmiName;
 	}
@@ -108,10 +108,10 @@ void MapIdentifiersH3M::loadMapping(const JsonNode & mapping)
 
 void MapIdentifiersH3M::remapTemplate(ObjectTemplate & objectTemplate)
 {
-	std::string name = boost::to_lower_copy(objectTemplate.animationFile.getName());
+	auto name = objectTemplate.animationFile;
 
 	if (mappingObjectTemplate.count(name))
-		objectTemplate.animationFile = AnimationPath::builtinTODO(mappingObjectTemplate.at(name));
+		objectTemplate.animationFile = mappingObjectTemplate.at(name);
 
 	ObjectTypeIdentifier objectType{ objectTemplate.id, objectTemplate.subid};
 

lib/mapping/MapIdentifiersH3M.h

@@ -11,6 +11,7 @@
 #pragma once
 
 #include "../GameConstants.h"
+#include "../filesystem/ResourcePath.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -43,7 +44,7 @@ class MapIdentifiersH3M
 	std::map<ArtifactID, ArtifactID> mappingArtifact;
 	std::map<SecondarySkill, SecondarySkill> mappingSecondarySkill;
 
-	std::map<std::string, std::string> mappingObjectTemplate;
+	std::map<AnimationPath, AnimationPath> mappingObjectTemplate;
 	std::map<ObjectTypeIdentifier, ObjectTypeIdentifier> mappingObjectIndex;
 
 	template<typename IdentifierID>

lib/mapping/MapReaderH3M.cpp

@@ -113,7 +113,12 @@ int32_t MapReaderH3M::readHeroPortrait()
 	if(result.getNum() == features.heroIdentifierInvalid)
 		return int32_t(-1);
 
-	assert(result.getNum() < features.heroesPortraitsCount);
+	if (result.getNum() >= features.heroesPortraitsCount)
+	{
+		logGlobal->warn("Map contains invalid hero portrait ID %d. Will be reset!", result.getNum() );
+		return int32_t(-1);
+	}
+
 	return remapper.remapPortrrait(result);
 }
 
@@ -199,7 +204,12 @@ PlayerColor MapReaderH3M::readPlayer()
 	if (value == 255)
 		return PlayerColor::NEUTRAL;
 
-	assert(value < PlayerColor::PLAYER_LIMIT_I);
+	if (value >= PlayerColor::PLAYER_LIMIT_I)
+	{
+		logGlobal->warn("Map contains invalid player ID %d. Will be reset!", value );
+		return PlayerColor::NEUTRAL;
+	}
+
 	return PlayerColor(value);
 }
 
@@ -210,7 +220,12 @@ PlayerColor MapReaderH3M::readPlayer32()
 	if (value == 255)
 		return PlayerColor::NEUTRAL;
 
-	assert(value < PlayerColor::PLAYER_LIMIT_I);
+	if (value >= PlayerColor::PLAYER_LIMIT_I)
+	{
+		logGlobal->warn("Map contains invalid player ID %d. Will be reset!", value );
+		return PlayerColor::NEUTRAL;
+	}
+
 	return PlayerColor(value);
 }