Merge pull request #6349 from IvanSavenko/rmg_object_fix

Fix issues with object customization in rmg

config/schemas/template.json

@@ -90,9 +90,20 @@
 							}
 						},
 						"bannedObjects": {
-							"type": "array",
-							"items": {
-								"type": "string"
+							"type": "object",
+							"additionalProperties": {
+								"anyOf" : [
+									{
+										"type": "boolean"
+									},
+									{
+										"type": "object",
+										"additionalProperties": {
+											"type": "boolean"
+										}
+									}
+								]
+								
 							}
 						},
 						"commonObjects": {
@@ -100,7 +111,10 @@
 							"items": {
 								"type": "object",
 								"properties": {
-									"id": {
+									"type": {
+										"type": "string"
+									},
+									"subtype": {
 										"type": "string"
 									},
 									"rmg": {
@@ -119,7 +133,7 @@
 										"required": ["value", "rarity"]
 									}
 								},
-								"required": ["id", "rmg"]
+								"required": ["type", "rmg"]
 							}
 						}
 					}

docs/modders/Random_Map_Template.md

@@ -216,18 +216,53 @@
 		// All of objects of this kind will be removed from zone
 		// Possible values: "all", "none", "creatureBank", "bonus", "dwelling", "resource", "resourceGenerator", "spellScroll", "randomArtifact", "pandorasBox", "questArtifact", "seerHut", "other
 		"bannedCategories" : ["all", "dwelling", "creatureBank", "other"],
+
 		// Specify object types and subtypes
-		"bannedObjects" :["core:object.randomArtifactRelic"],
-		// Configure individual common objects - overrides banned objects
+		"bannedObjects" : {
+			// ban a specific object from base game or from any dependent mod. Object with such ID must exist
+			"randomArtifactRelic" : true,
+			
+			// ban object named townGate from mod 'hota.mapobjects'. Mod can be used without explicit dependency
+			// If mod with such name is not loaded, this entry will be ignored and will have no effect
+			"hota.mapobjects:townGate" : true,
+			
+			// ban only land version of Cartographer. Other versions, such as water and subterra cartographers may still appear on map
+			"cartographer" : {
+				"cartographerLand" : true
+			}
+		},
+
+		// Configure individual common objects. 
+		// Any object in this list will be excluded from regular placement rules, similarly to bannedObjects
 		"commonObjects":
 		[
 			{
-				"id" : "core:object.creatureBank.dragonFlyHive",
+				// configure water cartographer properties
+				"type" : "cartographer",
+				"subtype" : "cartographerWater",
 				"rmg" : {
 					"value"		: 9000,
 					"rarity"	: 500,
 					"zoneLimit" : 2
 				}
+			},
+			{
+				// configure scholar properties. Behavior unspecified if there are multiple 'scholar' objects
+				"type" : "scholar",
+				"rmg" : {
+					"value"		: 900,
+					"rarity"	: 50,
+					"zoneLimit" : 2
+				}
+			},
+			{
+				// configure town gate (hota) properties. This entry will have no effect if mod is not enabled
+				"type" : "hota.mapobjects:townGate",
+				"rmg" : {
+					"value"		: 2000,
+					"rarity"	: 100,
+					"zoneLimit" : 2
+				}
 			}
 		]
 	}

lib/mapObjectConstructors/CObjectClassesHandler.cpp

@@ -433,7 +433,7 @@ CompoundMapObjectID CObjectClassesHandler::getCompoundIdentifier(const std::stri
 	if(id)
 	{
 		if (subtype.empty())
-			return CompoundMapObjectID(id.value(), 0);
+			return CompoundMapObjectID(id.value(), -1);
 
 		const auto & object = mapObjectTypes.at(id.value());
 		std::optional<si32> subID = LIBRARY->identifiers()->getIdentifier(scope, object->getJsonKey(), subtype);
@@ -449,7 +449,7 @@ CompoundMapObjectID CObjectClassesHandler::getCompoundIdentifier(const std::stri
 
 CompoundMapObjectID CObjectClassesHandler::getCompoundIdentifier(const std::string & objectName) const
 {
-	std::string subtype = "object"; //Default for objects with no subIds
+	std::string subtype;
 	std::string type;
 
 	auto scopeAndFullName = vstd::splitStringToPair(objectName, ':');

lib/rmg/ObjectConfig.cpp

@@ -30,143 +30,139 @@ void ObjectConfig::addBannedObject(const CompoundMapObjectID & objid)
 	logGlobal->info("Banned object of type %d.%d", objid.primaryID, objid.secondaryID);
 }
 
-void ObjectConfig::addCustomObject(const ObjectInfo & object, const CompoundMapObjectID & objid)
+void ObjectConfig::addCustomObject(const ObjectInfo & object)
 {
 	customObjects.push_back(object);
 	auto & lastObject = customObjects.back();
-	lastObject.setAllTemplates(objid.primaryID, objid.secondaryID);
+	lastObject.setAllTemplates(object.primaryID, object.secondaryID);
+
+	// also ban object to prevent default configuration from being used in this zone
+	bannedObjects.push_back(CompoundMapObjectID(object.primaryID, object.secondaryID));
 
 	assert(lastObject.templates.size() > 0);
-	logGlobal->info("Added custom object of type %d.%d", objid.primaryID, objid.secondaryID);
+	logGlobal->info("Added custom object of type %d.%d", object.primaryID, object.secondaryID);
 }
 
 void ObjectConfig::serializeJson(JsonSerializeFormat & handler)
 {
-	// TODO: We need serializer utility for list of enum values
-
-	static const boost::bimap<EObjectCategory, std::string> OBJECT_CATEGORY_STRINGS = boost::assign::list_of<boost::bimap<EObjectCategory, std::string>::relation>
-		(EObjectCategory::OTHER, "other")
-		(EObjectCategory::ALL, "all")
-		(EObjectCategory::NONE, "none")
-		(EObjectCategory::CREATURE_BANK, "creatureBank")
-		(EObjectCategory::BONUS, "bonus")
-		(EObjectCategory::DWELLING, "dwelling")
-		(EObjectCategory::RESOURCE, "resource")
-		(EObjectCategory::RESOURCE_GENERATOR, "resourceGenerator")
-		(EObjectCategory::SPELL_SCROLL, "spellScroll")
-		(EObjectCategory::RANDOM_ARTIFACT, "randomArtifact")
-		(EObjectCategory::PANDORAS_BOX, "pandorasBox")
-		(EObjectCategory::QUEST_ARTIFACT, "questArtifact")
-		(EObjectCategory::SEER_HUT, "seerHut");
-
-
-	// TODO: Separate into individual methods to enforce RAII destruction?
+	static const std::map<std::string, EObjectCategory> OBJECT_CATEGORY_STRINGS = {
+		{ "other", EObjectCategory::OTHER},
+		{ "all", EObjectCategory::ALL},
+		{ "none", EObjectCategory::NONE},
+		{ "creatureBank", EObjectCategory::CREATURE_BANK},
+		{ "bonus", EObjectCategory::BONUS},
+		{ "dwelling", EObjectCategory::DWELLING},
+		{ "resource", EObjectCategory::RESOURCE},
+		{ "resourceGenerator", EObjectCategory::RESOURCE_GENERATOR},
+		{ "spellScroll", EObjectCategory::SPELL_SCROLL},
+		{ "randomArtifact", EObjectCategory::RANDOM_ARTIFACT},
+		{ "pandorasBox", EObjectCategory::PANDORAS_BOX},
+		{ "questArtifact", EObjectCategory::QUEST_ARTIFACT},
+		{ "seerHut", EObjectCategory::SEER_HUT}
+	};
+
+	const JsonNode & config = handler.getCurrent();
+	const JsonNode & configBannedCategories = config["bannedCategories"];
+	const JsonNode & configBannedObjects = config["bannedObjects"];
+	const JsonNode & configCommonObjects = config["commonObjects"];
+
+	for(const auto & node : configBannedCategories.Vector())
+	{
+		auto it = OBJECT_CATEGORY_STRINGS.find(node.String());
+		if(it != OBJECT_CATEGORY_STRINGS.end())
+			bannedObjectCategories.push_back(it->second);
+	}
+
+	if(configBannedObjects.isVector())
 	{
-		auto categories = handler.enterArray("bannedCategories");
-		if (handler.saving)
+		// MOD COMPATIBILITY - 1.6 format
+
+		for(const auto & node : configBannedObjects.Vector())
 		{
-			for (const auto& category : bannedObjectCategories)
-			{
-				auto str = OBJECT_CATEGORY_STRINGS.left.at(category);
-				categories.serializeString(categories.size(), str);
-			}
+			LIBRARY->objtypeh->resolveObjectCompoundId(node.String(),
+				[this](CompoundMapObjectID objid)
+				{
+					addBannedObject(objid);
+				}
+			);
 		}
-		else
+	}
+	else
+	{
+		for(const auto & node : configBannedObjects.Struct())
 		{
-			std::vector<std::string> categoryNames;
-			categories.serializeArray(categoryNames);
-
-			for (const auto & categoryName : categoryNames)
+			LIBRARY->identifiers()->requestIdentifierIfFound(node.second.getModScope(), "object", node.first, [this, node](int primaryID)
 			{
-				auto it = OBJECT_CATEGORY_STRINGS.right.find(categoryName);
-				if (it != OBJECT_CATEGORY_STRINGS.right.end())
+				if (node.second.Bool())
+					addBannedObject(CompoundMapObjectID(primaryID, -1));
+
+				for (const auto & subNode : node.second.Struct())
 				{
-					bannedObjectCategories.push_back(it->second);
+					const std::string jsonKey = LIBRARY->objtypeh->getJsonKey(primaryID);
+
+					LIBRARY->identifiers()->requestIdentifierIfFound(node.second.getModScope(), jsonKey, subNode.first, [this, primaryID](int secondaryID)
+					{
+						addBannedObject(CompoundMapObjectID(primaryID, secondaryID));
+					});
 				}
-			}
+			});
 		}
 	}
 
-	// FIXME: Doesn't seem to use this field at all
-	
+	for (const auto & objectConfig : configCommonObjects.Vector())
 	{
-		auto bannedObjectData = handler.enterArray("bannedObjects");	
-		if (handler.saving)
-		{
-
-			// FIXME: Do we even need to serialize / store banned objects?
-			/*
-			for (const auto & object : bannedObjects)
-			{
-				// TODO: Translate id back to string?
+		auto rmg = objectConfig["rmg"].Struct();
 
+		// TODO: Use common code with default rmg config
 
-				JsonNode node;
-				node.String() = LIBRARY->objtypeh->getHandlerFor(object.primaryID, object.secondaryID);
-				// TODO: Check if AI-generated code is right
+		ObjectInfo object;
 
+		// TODO: Configure basic generateObject function
+		object.value = rmg["value"].Integer();
+		object.probability = rmg["rarity"].Integer();
+		object.maxPerZone = rmg["zoneLimit"].Integer();
+		if (object.maxPerZone == 0)
+			object.maxPerZone = std::numeric_limits<int>::max();
 
-			}
-			// handler.serializeRaw("bannedObjects", node, std::nullopt);
-
-			*/
-		}
-		else
+		if (objectConfig["id"].isNull())
 		{
-			std::vector<std::string> objectNames;
-			bannedObjectData.serializeArray(objectNames);
-
-			for (const auto & objectName : objectNames)
+			LIBRARY->identifiers()->requestIdentifierIfFound("object", objectConfig["type"], [this, object, objectConfig](int primaryID)
 			{
-				LIBRARY->objtypeh->resolveObjectCompoundId(objectName,
-					[this](CompoundMapObjectID objid)
+				if (objectConfig["subtype"].isNull())
+				{
+					auto objectWithID = object;
+					objectWithID.primaryID = primaryID;
+					objectWithID.secondaryID = 0;
+					addCustomObject(object);
+				}
+				else
+				{
+					const std::string jsonKey = LIBRARY->objtypeh->getJsonKey(primaryID);
+
+					LIBRARY->identifiers()->requestIdentifierIfFound(jsonKey, objectConfig["subtype"], [this, primaryID, object](int secondaryID)
 					{
-						addBannedObject(objid);
-					}
-				);
-				
-			}
+						auto objectWithID = object;
+						objectWithID.primaryID = primaryID;
+						objectWithID.secondaryID = secondaryID;
+						addCustomObject(object);
+					});
+				}
+			});
 		}
-	}
-
-	auto commonObjectData = handler.getCurrent()["commonObjects"].Vector();	
-	if (handler.saving)
-	{
-
-		//TODO?
-	}
-	else
-	{
-		for (const auto & objectConfig : commonObjectData)
+		else
 		{
+			// MOD COMPATIBILITY - 1.6 format
 			auto objectName = objectConfig["id"].String();
-			auto rmg = objectConfig["rmg"].Struct();
-
-			// TODO: Use common code with default rmg config
-			auto objectValue = rmg["value"].Integer();
-			auto objectProbability = rmg["rarity"].Integer();
 
-			auto objectMaxPerZone = rmg["zoneLimit"].Integer();
-			if (objectMaxPerZone == 0)
+			LIBRARY->objtypeh->resolveObjectCompoundId(objectName, [this, object](CompoundMapObjectID objid)
 			{
-				objectMaxPerZone = std::numeric_limits<int>::max();
-			}
-
-			LIBRARY->objtypeh->resolveObjectCompoundId(objectName,
-
-				[this, objectValue, objectProbability, objectMaxPerZone](CompoundMapObjectID objid)
-				{
-					ObjectInfo object(objid.primaryID, objid.secondaryID);
-					
-					// TODO: Configure basic generateObject function
-
-					object.value = objectValue;
-					object.probability = objectProbability;
-					object.maxPerZone = objectMaxPerZone;
-					addCustomObject(object, objid);
-				}
-			);
-			
+				auto objectWithID = object;
+				objectWithID.primaryID = objid.primaryID;
+				objectWithID.secondaryID = objid.secondaryID;
+				if (objectWithID.secondaryID == -1)
+					objectWithID.secondaryID = 0;
+				addCustomObject(objectWithID);
+			});
 		}
 	}
 }
@@ -186,4 +182,4 @@ const std::vector<ObjectConfig::EObjectCategory> & ObjectConfig::getBannedObject
 	return bannedObjectCategories;
 }
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

lib/rmg/ObjectConfig.h

@@ -40,7 +40,7 @@ public:
 	};
 
 	void addBannedObject(const CompoundMapObjectID & objid);
-	void addCustomObject(const ObjectInfo & object, const CompoundMapObjectID & objid);
+	void addCustomObject(const ObjectInfo & object);
 	void clearBannedObjects();
 	void clearCustomObjects();
 	const std::vector<CompoundMapObjectID> & getBannedObjects() const;
@@ -58,4 +58,4 @@ private:
 	std::vector<ObjectInfo> customObjects;
 };
 
-VCMI_LIB_NAMESPACE_END
+VCMI_LIB_NAMESPACE_END

lib/rmg/ObjectInfo.h

@@ -20,6 +20,7 @@ class CGObjectInstance;
 
 struct DLL_LINKAGE ObjectInfo
 {
+	ObjectInfo() = default;
 	ObjectInfo(si32 ID, si32 subID);
 	ObjectInfo(CompoundMapObjectID id);
 	ObjectInfo(const ObjectInfo & other);

lib/rmg/modificators/TreasurePlacer.cpp

@@ -1159,7 +1159,8 @@ void TreasurePlacer::ObjectPool::patchWithZoneConfig(const Zone & zone, Treasure
 			for (const auto & templ : object.templates)
 			{
 				CompoundMapObjectID key = object.getCompoundID();
-				if (bannedObjectsSet.count(key))
+				CompoundMapObjectID keyGroup( key.primaryID, -1);
+				if (bannedObjectsSet.count(key) || bannedObjectsSet.count(keyGroup))
 				{
 					// FIXME: Stopped working, nothing is banned
 					logGlobal->info("Banning object %s from possible objects", templ->stringID);