Browse Source

Improvements for json validation for mods

- Implemented validation for `targetCondition` in spells
- Implemented validation for `mapObject` in towns/heroes
- Fixed validation of zone connections in RMG
- Added workarounds to prevent assertions triggering on invalid mods
- Erase 'base' entries from json before validation (but after applying
them to derived keys)

Should have no effect on mod behavior/support, but may cause new
detections for mods that were broken in either 1.6 or 1.7
Ivan Savenko 4 months ago
parent
commit
78b39688c5

+ 1 - 1
config/schemas/biome.json

@@ -4,7 +4,7 @@
 	"title" : "VCMI map obstacle set format",
 	"description" : "Description of map object set, used only as sub-schema of object",
 	"required" : ["biome", "templates"],
-	"additionalProperties" : true, // may have type-dependant properties
+	"additionalProperties" : false,
 	"properties" : {
 		"biome" : {
 			"type" : "object",

+ 0 - 5
config/schemas/hero.json

@@ -119,11 +119,6 @@
 			"description" : "Description of hero specialty using bonus system",
 			"additionalProperties" : false,
 			"properties" : { 
-				"base" : {
-					"type" : "object",
-					"additionalProperties" : true,
-					"description" : "Section that will be added into every bonus instance, for use in specialties with multiple similar bonuses."
-				},
 				"bonuses" : {
 					"type" : "object",
 					"description" : "List of bonuses added by this specialty. See bonus format for more details",

+ 1 - 2
config/schemas/heroClass.json

@@ -54,8 +54,7 @@
 			}
 		},
 		"mapObject" : {
-			// TODO: this entry should be merged with corresponding base entry in hero object type and validated as objectType
-			// "$ref" : "objectType.json",
+			// NOTE: this entry is merged with corresponding base entry in hero object type and validated as objectType
 			"type" : "object",
 			"properties" : {
 				"filters" : {

+ 6 - 6
config/schemas/spell.json

@@ -633,7 +633,12 @@
 		},
 		"targetCondition" : {
 			"type" : "object",
-			"additionalProperties" : true
+			"additionalProperties" : false,
+			"properties" : {
+				"noneOf" : { "type" : "object", "additionalProperties" : { "type" : "string", "enum" : [ "absolute", "normal" ] } },
+				"anyOf" :  { "type" : "object", "additionalProperties" : { "type" : "string", "enum" : [ "absolute", "normal" ] } },
+				"allOf" :  { "type" : "object", "additionalProperties" : { "type" : "string", "enum" : [ "absolute", "normal" ] } }
+			}
 		},
 		"animation" : {"$ref" : "#/definitions/animation"},
 		"graphics" : {
@@ -681,11 +686,6 @@
 			 "type" : "object",
 			 "additionalProperties" : false,
 			 "properties" : {
-				"base" : {
-					"type" : "object",
-					"description" : "will be merged with all levels",
-					"additionalProperties" : true
-				},
 				"none" : {
 					"$ref" : "#/definitions/levelInfo"
 				},

+ 6 - 1
config/schemas/template.json

@@ -153,7 +153,7 @@
 		"connection" :
 		{
 			"required" : ["a", "b"],
-			"additionalProperties" : true,
+			"additionalProperties" : false,
 			"properties" : {
 				"a" : {
 					"type" : "string"
@@ -170,6 +170,11 @@
 				{
 					"type" : "string",
 					"enum" : ["wide", "fictive", "repulsive", "forcePortal"]
+				},
+				"road":
+				{
+					"type" : "string",
+					"enum" : ["true", "false", "random"]
 				}
 			}
 		},

+ 2 - 0
lib/CSkillHandler.cpp

@@ -316,6 +316,8 @@ void CSkillHandler::beforeValidate(JsonNode & object)
 	inheritNode("basic");
 	inheritNode("advanced");
 	inheritNode("expert");
+
+	object.Struct().erase("base");
 }
 
 std::set<SecondarySkill> CSkillHandler::getDefaultAllowed() const

+ 1 - 1
lib/entities/hero/CHeroClassHandler.cpp

@@ -123,9 +123,9 @@ std::shared_ptr<CHeroClass> CHeroClassHandler::loadFromJson(const std::string &
 	LIBRARY->identifiers()->requestIdentifier(scope, "object", "hero", [=](si32 index) {
 		JsonNode classConf = node["mapObject"];
 		classConf["heroClass"].String() = identifier;
+		classConf["heroClass"].setModScope(scope);
 		if (!node["compatibilityIdentifiers"].isNull())
 			classConf["compatibilityIdentifiers"] = node["compatibilityIdentifiers"];
-		classConf.setModScope(scope);
 		LIBRARY->objtypeh->loadSubObject(identifier, classConf, index, heroClass->getIndex());
 	});
 

+ 3 - 2
lib/entities/hero/CHeroHandler.cpp

@@ -207,9 +207,9 @@ void CHeroHandler::beforeValidate(JsonNode & object)
 	JsonNode & specialtyNode = object["specialty"];
 	if(specialtyNode.getType() == JsonNode::JsonType::DATA_STRUCT)
 	{
-		const JsonNode & base = specialtyNode["base"];
-		if(!base.isNull())
+		if(specialtyNode.Struct().count("base") != 0)
 		{
+			const JsonNode & base = specialtyNode["base"];
 			if(specialtyNode["bonuses"].isNull())
 			{
 				logMod->warn("specialty has base without bonuses");
@@ -220,6 +220,7 @@ void CHeroHandler::beforeValidate(JsonNode & object)
 				for(std::pair<std::string, JsonNode> keyValue : bonuses)
 					JsonUtils::inherit(bonuses[keyValue.first], base);
 			}
+			specialtyNode.Struct().erase("base");
 		}
 	}
 }

+ 4 - 2
lib/json/JsonBonus.cpp

@@ -536,14 +536,16 @@ static std::shared_ptr<const ILimiter> parseCreatureTypeLimiter(const JsonNode &
 		creatureLimiter->setCreature(CreatureID(creature));
 	});
 
-	creatureLimiter->includeUpgrades = upgradesNode.Bool();
-
 	if (upgradesNode.isString())
 	{
 		logGlobal->warn("CREATURE_TYPE_LIMITER: parameter 'includeUpgrades' is invalid! expected boolean, but string '%s' found!", upgradesNode.String());
 		if (upgradesNode.String() == "true") // MOD COMPATIBILITY - broken mod, compensating
 			creatureLimiter->includeUpgrades = true;
 	}
+	else
+	{
+		creatureLimiter->includeUpgrades = upgradesNode.Bool();
+	}
 
 	return creatureLimiter;
 }

+ 6 - 0
lib/mapObjectConstructors/CObjectClassesHandler.cpp

@@ -356,6 +356,12 @@ void CObjectClassesHandler::loadSubObject(const std::string & identifier, JsonNo
 	}
 
 	JsonUtils::inherit(config, mapObjectTypes.at(ID.getNum())->base);
+	for (auto & templ : config["templates"].Struct())
+		JsonUtils::inherit(templ.second, config["base"]);
+
+	if (settings["mods"]["validation"].String() != "off")
+		JsonUtils::validate(config, "vcmi:objectType", identifier);
+
 	loadSubObject(config.getModScope(), identifier, config, mapObjectTypes.at(ID.getNum()).get(), subID.getNum());
 }
 

+ 2 - 0
lib/spells/CSpellHandler.cpp

@@ -1028,6 +1028,8 @@ void CSpellHandler::beforeValidate(JsonNode & object)
 	inheritNode("basic");
 	inheritNode("advanced");
 	inheritNode("expert");
+
+	levels.Struct().erase("base");
 }
 
 std::set<SpellID> CSpellHandler::getDefaultAllowed() const

+ 3 - 0
lib/spells/TargetCondition.cpp

@@ -513,6 +513,9 @@ void TargetCondition::loadConditions(const JsonNode & source, bool exclusive, bo
 
 		const JsonNode & value = keyValue.second;
 
+		if (!value.isString())
+			continue;
+
 		if(value.String() == "absolute")
 			isAbsolute = true;
 		else if(value.String() == "normal")