Slight tweaking of logging of h3m edge cases

lib/mapping/MapFormatH3M.cpp

@@ -195,7 +195,7 @@ void CMapLoaderH3M::readPlayerInfo()
 		if(features.levelSOD)
 			reader->skipUnused(1); //TODO: check meaning?
 
-		std::set<TFaction> allowedFactions;
+		std::set<FactionID> allowedFactions;
 
 		reader->readBitmask(allowedFactions, features.factionsBytes, features.factionsCount, false);
 
@@ -331,7 +331,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 
 			if(playersOnMap == 1)
 			{
-				logGlobal->warn("Map %s has only one player but allows normal victory?", mapHeader->name);
+				logGlobal->warn("Map %s: Only one player exists, but normal victory allowed!", mapName);
 				allowNormalVictory = false; // makes sense? Not much. Works as H3? Yes!
 			}
 		}
@@ -847,13 +847,13 @@ bool CMapLoaderH3M::loadArtifactToSlot(CGHeroInstance * hero, int slot)
 
 	if(!art)
 	{
-		logGlobal->warn("Invalid artifact in hero's backpack, ignoring...");
+		logGlobal->warn("Map '%s': Invalid artifact in hero's backpack, ignoring...", mapName);
 		return false;
 	}
 
 	if(art->isBig() && slot >= GameConstants::BACKPACK_START)
 	{
-		logGlobal->warn("A big artifact (war machine) in hero's backpack, ignoring...");
+		logGlobal->warn("Map '%s': A big artifact (war machine) in hero's backpack, ignoring...", mapName);
 		return false;
 	}
 
@@ -868,7 +868,7 @@ bool CMapLoaderH3M::loadArtifactToSlot(CGHeroInstance * hero, int slot)
 	}
 	else
 	{
-		logGlobal->debug("Artifact '%s' can't be put at the slot %d", artifact->artType->getNameTranslated(), slot);
+		logGlobal->warn("Map '%s': Artifact '%s' can't be put at the slot %d", mapName, artifact->artType->getNameTranslated(), slot);
 		return false;
 	}
 
@@ -1070,17 +1070,17 @@ CGObjectInstance * CMapLoaderH3M::readScholar()
 	object->bonusType = static_cast<CGScholar::EBonusType>(reader->readUInt8());
 	object->bonusID = reader->readUInt8();
 	reader->skipZero(6);
-	return object;
-}
-
-CGObjectInstance * CMapLoaderH3M::readGarrison(const int3 & mapPosition)
-{
-	auto * object = new CGGarrison();
-
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
-	readCreatureSet(object, 7);
-	if(features.levelAB)
-		object->removableUnits = reader->readBool();
+	return object;
+}
+
+CGObjectInstance * CMapLoaderH3M::readGarrison(const int3 & mapPosition)
+{
+	auto * object = new CGGarrison();
+
+	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	readCreatureSet(object, 7);
+	if(features.levelAB)
+		object->removableUnits = reader->readBool();
 	else
 		object->removableUnits = true;
 
@@ -1129,26 +1129,26 @@ CGObjectInstance * CMapLoaderH3M::readResource(const int3 & mapPosition, std::sh
 
 CGObjectInstance * CMapLoaderH3M::readMine(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
-	auto * object = new CGMine();
-	if(objectTemplate->subid < 7)
-	{
-		setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
-	}
-	else
-	{
+	auto * object = new CGMine();
+	if(objectTemplate->subid < 7)
+	{
+		setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	}
+	else
+	{
 		object->setOwner(PlayerColor::NEUTRAL);
 		reader->readBitmask(object->abandonedMineResources, features.resourcesBytes, features.resourcesCount, false);
 	}
 	return object;
 }
 
-CGObjectInstance * CMapLoaderH3M::readDwelling(const int3 & position)
-{
-	auto * object = new CGDwelling();
-	setOwnerAndValidate(position, object, reader->readPlayer32());
-	return object;
-}
-
+CGObjectInstance * CMapLoaderH3M::readDwelling(const int3 & position)
+{
+	auto * object = new CGDwelling();
+	setOwnerAndValidate(position, object, reader->readPlayer32());
+	return object;
+}
+
 CGObjectInstance * CMapLoaderH3M::readDwellingRandom(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	auto * object = new CGDwelling();
@@ -1167,13 +1167,13 @@ CGObjectInstance * CMapLoaderH3M::readDwellingRandom(const int3 & mapPosition, s
 			break;
 		default:
 			throw std::runtime_error("Invalid random dwelling format");
-	}
-	spec->owner = object;
-
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
-
-	//216 and 217
-	if(auto * castleSpec = dynamic_cast<CCreGenAsCastleInfo *>(spec))
+	}
+	spec->owner = object;
+
+	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+
+	//216 and 217
+	if(auto * castleSpec = dynamic_cast<CCreGenAsCastleInfo *>(spec))
 	{
 		castleSpec->instanceId = "";
 		castleSpec->identifier = reader->readUInt32();
@@ -1218,13 +1218,13 @@ CGObjectInstance * CMapLoaderH3M::readShrine()
 }
 
 CGObjectInstance * CMapLoaderH3M::readHeroPlaceholder(const int3 & mapPosition)
-{
-	auto * object = new CGHeroPlaceholder();
-
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer());
-
-	HeroTypeID htid = reader->readHero(); //hero type id
-	object->subID = htid.getNum();
+{
+	auto * object = new CGHeroPlaceholder();
+
+	setOwnerAndValidate(mapPosition, object, reader->readPlayer());
+
+	HeroTypeID htid = reader->readHero(); //hero type id
+	object->subID = htid.getNum();
 
 	if(htid.getNum() == -1)
 	{
@@ -1252,7 +1252,7 @@ CGObjectInstance * CMapLoaderH3M::readGeneric(const int3 & mapPosition, std::sha
 	if(VLC->objtypeh->knownSubObjects(objectTemplate->id).count(objectTemplate->subid))
 		return VLC->objtypeh->getHandlerFor(objectTemplate->id, objectTemplate->subid)->create(objectTemplate);
 
-	logGlobal->warn("Unrecognized object: %d:%d ('%s') at %s on map '%s'", objectTemplate->id.toEnum(), objectTemplate->subid, objectTemplate->animationFile, mapPosition.toString(), map->name);
+	logGlobal->warn("Map '%s': Unrecognized object %d:%d ('%s') at %s found!", mapName, objectTemplate->id.toEnum(), objectTemplate->subid, objectTemplate->animationFile, mapPosition.toString());
 	return new CGObjectInstance();
 }
 
@@ -1268,16 +1268,16 @@ CGObjectInstance * CMapLoaderH3M::readQuestGuard(const int3 & mapPosition)
 {
 	auto * guard = new CGQuestGuard();
 	readQuest(guard, mapPosition);
-	return guard;
-}
-
-CGObjectInstance * CMapLoaderH3M::readShipyard(const int3 & mapPosition)
-{
-	auto * object = new CGShipyard();
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
-	return object;
-}
-
+	return guard;
+}
+
+CGObjectInstance * CMapLoaderH3M::readShipyard(const int3 & mapPosition)
+{
+	auto * object = new CGShipyard();
+	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	return object;
+}
+
 CGObjectInstance * CMapLoaderH3M::readBorderGuard()
 {
 	return new CGBorderGuard();
@@ -1293,16 +1293,16 @@ CGObjectInstance * CMapLoaderH3M::readBorderGate(const int3 & mapPosition, std::
 		return new CGObjectInstance();
 
 	logGlobal->warn("Map '%s: Quest gates at %s are not implemented!", mapName, mapPosition.toString());
-	return readQuestGuard(mapPosition);
-}
-
-CGObjectInstance * CMapLoaderH3M::readLighthouse(const int3 & mapPosition)
-{
-	auto * object = new CGLighthouse();
-	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
-	return object;
-}
-
+	return readQuestGuard(mapPosition);
+}
+
+CGObjectInstance * CMapLoaderH3M::readLighthouse(const int3 & mapPosition)
+{
+	auto * object = new CGLighthouse();
+	setOwnerAndValidate(mapPosition, object, reader->readPlayer32());
+	return object;
+}
+
 CGObjectInstance * CMapLoaderH3M::readBank(const int3 & mapPosition, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	if(features.levelHOTA3)
@@ -1376,13 +1376,13 @@ CGObjectInstance * CMapLoaderH3M::readObject(std::shared_ptr<const ObjectTemplat
 			return readWitchHut();
 		case Obj::SCHOLAR:
 			return readScholar();
-
-		case Obj::GARRISON:
-		case Obj::GARRISON2:
-			return readGarrison(mapPosition);
-
-		case Obj::ARTIFACT:
-		case Obj::RANDOM_ART:
+
+		case Obj::GARRISON:
+		case Obj::GARRISON2:
+			return readGarrison(mapPosition);
+
+		case Obj::ARTIFACT:
+		case Obj::RANDOM_ART:
 		case Obj::RANDOM_TREASURE_ART:
 		case Obj::RANDOM_MINOR_ART:
 		case Obj::RANDOM_MAJOR_ART:
@@ -1424,13 +1424,13 @@ CGObjectInstance * CMapLoaderH3M::readObject(std::shared_ptr<const ObjectTemplat
 			return readDwellingRandom(mapPosition, objectTemplate);
 
 		case Obj::QUEST_GUARD:
-			return readQuestGuard(mapPosition);
-
-		case Obj::SHIPYARD:
-			return readShipyard(mapPosition);
-
-		case Obj::HERO_PLACEHOLDER:
-			return readHeroPlaceholder(mapPosition);
+			return readQuestGuard(mapPosition);
+
+		case Obj::SHIPYARD:
+			return readShipyard(mapPosition);
+
+		case Obj::HERO_PLACEHOLDER:
+			return readHeroPlaceholder(mapPosition);
 
 		case Obj::BORDERGUARD:
 			return readBorderGuard();
@@ -1439,13 +1439,13 @@ CGObjectInstance * CMapLoaderH3M::readObject(std::shared_ptr<const ObjectTemplat
 			return readBorderGate(mapPosition, objectTemplate);
 
 		case Obj::PYRAMID:
-			return readPyramid(mapPosition, objectTemplate);
-
-		case Obj::LIGHTHOUSE:
-			return readLighthouse(mapPosition);
-
-		case Obj::CREATURE_BANK:
-		case Obj::DERELICT_SHIP:
+			return readPyramid(mapPosition, objectTemplate);
+
+		case Obj::LIGHTHOUSE:
+			return readLighthouse(mapPosition);
+
+		case Obj::CREATURE_BANK:
+		case Obj::DERELICT_SHIP:
 		case Obj::DRAGON_UTOPIA:
 		case Obj::CRYPT:
 		case Obj::SHIPWRECK:
@@ -1538,40 +1538,40 @@ void CMapLoaderH3M::readCreatureSet(CCreatureSet * out, int number)
 		out->putStack(SlotID(index), result);
 	}
 
-	out->validTypes(true);
-}
-
-void CMapLoaderH3M::setOwnerAndValidate(const int3 & mapPosition, CGObjectInstance * object, const PlayerColor & owner)
-{
-	assert(owner.isValidPlayer() || owner == PlayerColor::NEUTRAL);
-
-	if(owner == PlayerColor::NEUTRAL)
-	{
-		object->setOwner(PlayerColor::NEUTRAL);
-		return;
-	}
-
-	if(!owner.isValidPlayer())
-	{
-		object->setOwner(PlayerColor::NEUTRAL);
-		logGlobal->warn("Map '%s': Object at %s - owned by invalid player %d! Will be set to neutral!", mapName, mapPosition.toString(), int(owner.getNum()));
-		return;
-	}
-
-	if(!mapHeader->players[owner.getNum()].canAnyonePlay())
-	{
-		object->setOwner(PlayerColor::NEUTRAL);
-		logGlobal->warn("Map '%s': Object at %s - owned by non-existing player %d! Will be set to neutral!", mapName, mapPosition.toString(), int(owner.getNum())
-		);
-		return;
-	}
-
-	object->setOwner(owner);
-}
-
-CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
-{
-	auto * object = new CGHeroInstance();
+	out->validTypes(true);
+}
+
+void CMapLoaderH3M::setOwnerAndValidate(const int3 & mapPosition, CGObjectInstance * object, const PlayerColor & owner)
+{
+	assert(owner.isValidPlayer() || owner == PlayerColor::NEUTRAL);
+
+	if(owner == PlayerColor::NEUTRAL)
+	{
+		object->setOwner(PlayerColor::NEUTRAL);
+		return;
+	}
+
+	if(!owner.isValidPlayer())
+	{
+		object->setOwner(PlayerColor::NEUTRAL);
+		logGlobal->warn("Map '%s': Object at %s - owned by invalid player %d! Will be set to neutral!", mapName, mapPosition.toString(), int(owner.getNum()));
+		return;
+	}
+
+	if(!mapHeader->players[owner.getNum()].canAnyonePlay())
+	{
+		object->setOwner(PlayerColor::NEUTRAL);
+		logGlobal->warn("Map '%s': Object at %s - owned by non-existing player %d! Will be set to neutral!", mapName, mapPosition.toString(), int(owner.getNum())
+		);
+		return;
+	}
+
+	object->setOwner(owner);
+}
+
+CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const ObjectInstanceID & objectInstanceID)
+{
+	auto * object = new CGHeroInstance();
 
 	if(features.levelAB)
 	{
@@ -1592,13 +1592,13 @@ CGObjectInstance * CMapLoaderH3M::readHero(const int3 & mapPosition, const Objec
 			delete object;
 			object = elem;
 			break;
-		}
-	}
-
-	setOwnerAndValidate(mapPosition, object, owner);
-	object->portrait = object->subID;
-
-	for(auto & elem : map->disposedHeroes)
+		}
+	}
+
+	setOwnerAndValidate(mapPosition, object, owner);
+	object->portrait = object->subID;
+
+	for(auto & elem : map->disposedHeroes)
 	{
 		if(elem.heroId == object->subID)
 		{
@@ -1973,14 +1973,14 @@ void CMapLoaderH3M::readQuest(IQuestObject * guard, const int3 & position)
 CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_ptr<const ObjectTemplate> objectTemplate)
 {
 	auto * object = new CGTownInstance();
-	if(features.levelAB)
-		object->identifier = reader->readUInt32();
-
-	setOwnerAndValidate(position, object, reader->readPlayer());
-
-	bool hasName = reader->readBool();
-	if(hasName)
-		object->setNameTranslated(readLocalizedString(TextIdentifier("town", position.x, position.y, position.z, "name")));
+	if(features.levelAB)
+		object->identifier = reader->readUInt32();
+
+	setOwnerAndValidate(position, object, reader->readPlayer());
+
+	bool hasName = reader->readBool();
+	if(hasName)
+		object->setNameTranslated(readLocalizedString(TextIdentifier("town", position.x, position.y, position.z, "name")));
 
 	bool hasGarrison = reader->readBool();
 	if(hasGarrison)

lib/serializer/BinaryDeserializer.h

@@ -118,7 +118,8 @@ class DLL_LINKAGE BinaryDeserializer : public CLoaderBase
 	{
 		ui32 length;
 		load(length);
-		if(length > 500000)
+		//NOTE: also used for h3m's embedded in campaigns, so it may be quite large in some cases (e.g. XXL maps with multiple objects)
+		if(length > 1000000)
 		{
 			logGlobal->warn("Warning: very big length: %d", length);
 			reader->reportState(logGlobal);