Merge pull request #4712 from IvanSavenko/detect_conflict

Detection of potential conflicts between mods

config/schemas/settings.json

@@ -3,7 +3,7 @@
 {
 	"type" : "object",
 	"$schema" : "http://json-schema.org/draft-04/schema",
-	"required" : [ "general", "video", "adventure", "battle", "input", "server", "logging", "launcher", "lobby", "gameTweaks" ],
+	"required" : [ "general", "video", "adventure", "battle", "input", "server", "logging", "launcher", "lobby", "gameTweaks", "mods" ],
 	"definitions" : {
 		"logLevelEnum" : {
 			"type" : "string",
@@ -149,6 +149,23 @@
 				}
 			}
 		},
+		
+		"mods" : {
+			"type" : "object",
+			"additionalProperties" : false,
+			"default" : {},
+			"required" : [ 
+				"validation"
+			],
+			"properties" : {
+				"validation" : {
+					"type" : "string",
+					"enum" : [ "off", "basic", "full" ],
+					"default" : "basic"
+				}
+			}
+		},
+		
 		"video" : {
 			"type" : "object",
 			"additionalProperties" : false,

launcher/settingsView/csettingsview_moc.cpp

@@ -182,6 +182,13 @@ void CSettingsView::loadSettings()
 	else
 		ui->buttonFontScalable->setChecked(true);
 
+	if (settings["mods"]["validation"].String() == "off")
+		ui->buttonValidationOff->setChecked(true);
+	else if (settings["mods"]["validation"].String() == "basic")
+		ui->buttonValidationBasic->setChecked(true);
+	else
+		ui->buttonValidationFull->setChecked(true);
+
 	loadToggleButtonSettings();
 }
 
@@ -791,3 +798,21 @@ void CSettingsView::on_buttonFontOriginal_clicked(bool checked)
 	Settings node = settings.write["video"]["fontsType"];
 	node->String() = "original";
 }
+
+void CSettingsView::on_buttonValidationOff_clicked(bool checked)
+{
+	Settings node = settings.write["mods"]["validation"];
+	node->String() = "off";
+}
+
+void CSettingsView::on_buttonValidationBasic_clicked(bool checked)
+{
+	Settings node = settings.write["mods"]["validation"];
+	node->String() = "basic";
+}
+
+void CSettingsView::on_buttonValidationFull_clicked(bool checked)
+{
+	Settings node = settings.write["mods"]["validation"];
+	node->String() = "full";
+}

launcher/settingsView/csettingsview_moc.h

@@ -83,19 +83,20 @@ private slots:
 	void on_sliderToleranceDistanceController_valueChanged(int value);
 	void on_lineEditGameLobbyHost_textChanged(const QString &arg1);
 	void on_spinBoxNetworkPortLobby_valueChanged(int arg1);
-
 	void on_sliderControllerSticksAcceleration_valueChanged(int value);
-
 	void on_sliderControllerSticksSensitivity_valueChanged(int value);
-
-	//void on_buttonTtfFont_toggled(bool value);
-
 	void on_sliderScalingFont_valueChanged(int value);
-
 	void on_buttonFontAuto_clicked(bool checked);
 	void on_buttonFontScalable_clicked(bool checked);
 	void on_buttonFontOriginal_clicked(bool checked);
 
+
+	void on_buttonValidationOff_clicked(bool checked);
+
+	void on_buttonValidationBasic_clicked(bool checked);
+
+	void on_buttonValidationFull_clicked(bool checked);
+
 private:
 	Ui::CSettingsView * ui;
 

lib/json/JsonUtils.cpp

@@ -296,4 +296,28 @@ JsonNode JsonUtils::assembleFromFiles(const std::string & filename)
 	return result;
 }
 
+void JsonUtils::detectConflicts(JsonNode & result, const JsonNode & left, const JsonNode & right, const std::string & keyName)
+{
+	switch (left.getType())
+	{
+		case JsonNode::JsonType::DATA_NULL:
+		case JsonNode::JsonType::DATA_BOOL:
+		case JsonNode::JsonType::DATA_FLOAT:
+		case JsonNode::JsonType::DATA_INTEGER:
+		case JsonNode::JsonType::DATA_STRING:
+		case JsonNode::JsonType::DATA_VECTOR: // NOTE: comparing vectors as whole - since merge will overwrite it in its entirety
+		{
+			result[keyName][left.getModScope()] = left;
+			result[keyName][right.getModScope()] = right;
+			return;
+		}
+		case JsonNode::JsonType::DATA_STRUCT:
+		{
+			for(const auto & node : left.Struct())
+				if (!right[node.first].isNull())
+					detectConflicts(result, node.second, right[node.first], keyName + "/" + node.first);
+		}
+	}
+}
+
 VCMI_LIB_NAMESPACE_END

lib/json/JsonUtils.h

@@ -74,6 +74,12 @@ namespace JsonUtils
 	/// get schema by json URI: vcmi:<name of file in schemas directory>#<entry in file, optional>
 	/// example: schema "vcmi:settings" is used to check user settings
 	DLL_LINKAGE const JsonNode & getSchema(const std::string & URI);
+
+	/// detects potential conflicts - json entries present in both nodes
+	/// returns JsonNode that contains list of conflicting keys
+	/// For each conflict - list of conflicting mods and list of conflicting json values
+	/// result[pathToKey][modID] -> node that was conflicting
+	DLL_LINKAGE void detectConflicts(JsonNode & result, const JsonNode & left, const JsonNode & right, const std::string & keyName);
 }
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjectConstructors/CRewardableConstructor.cpp

@@ -14,6 +14,7 @@
 #include "../mapObjects/CRewardableObject.h"
 #include "../texts/CGeneralTextHandler.h"
 #include "../IGameCallback.h"
+#include "../CConfigHandler.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -25,7 +26,8 @@ void CRewardableConstructor::initTypeData(const JsonNode & config)
 	if (!config["name"].isNull())
 		VLC->generaltexth->registerString( config.getModScope(), getNameTextID(), config["name"]);
 
-	JsonUtils::validate(config, "vcmi:rewardable", getJsonKey());
+	if (settings["mods"]["validation"].String() != "off")
+		JsonUtils::validate(config, "vcmi:rewardable", getJsonKey());
 	
 }
 

lib/modding/CModHandler.cpp

@@ -17,6 +17,7 @@
 #include "ModIncompatibility.h"
 
 #include "../CCreatureHandler.h"
+#include "../CConfigHandler.h"
 #include "../CStopWatch.h"
 #include "../GameSettings.h"
 #include "../ScriptHandler.h"
@@ -331,10 +332,38 @@ void CModHandler::loadModFilesystems()
 
 	coreMod->updateChecksum(calculateModChecksum(ModScope::scopeBuiltin(), CResourceHandler::get(ModScope::scopeBuiltin())));
 
+	std::map<std::string, ISimpleResourceLoader *> modFilesystems;
+
 	for(std::string & modName : activeMods)
+		modFilesystems[modName] = genModFilesystem(modName, allMods[modName].config);
+
+	for(std::string & modName : activeMods)
+		CResourceHandler::addFilesystem("data", modName, modFilesystems[modName]);
+
+	if (settings["mods"]["validation"].String() == "full")
 	{
-		CModInfo & mod = allMods[modName];
-		CResourceHandler::addFilesystem("data", modName, genModFilesystem(modName, mod.config));
+		for(std::string & leftModName : activeMods)
+		{
+			for(std::string & rightModName : activeMods)
+			{
+				if (leftModName == rightModName)
+					continue;
+
+				if (getModDependencies(leftModName).count(rightModName) || getModDependencies(rightModName).count(leftModName))
+					continue;
+
+				const auto & filter = [](const ResourcePath &path){return path.getType() != EResType::DIRECTORY;};
+
+				std::unordered_set<ResourcePath> leftResources = modFilesystems[leftModName]->getFilteredFiles(filter);
+				std::unordered_set<ResourcePath> rightResources = modFilesystems[rightModName]->getFilteredFiles(filter);
+
+				for (auto const & leftFile : leftResources)
+				{
+					if (rightResources.count(leftFile))
+						logMod->warn("Potential confict detected between '%s' and '%s': both mods add file '%s'", leftModName, rightModName, leftFile.getOriginalName());
+				}
+			}
+		}
 	}
 }
 
@@ -370,6 +399,12 @@ std::string CModHandler::getModLanguage(const TModID& modId) const
 	return allMods.at(modId).baseLanguage;
 }
 
+std::set<TModID> CModHandler::getModDependencies(const TModID & modId) const
+{
+	bool isModFound;
+	return getModDependencies(modId, isModFound);
+}
+
 std::set<TModID> CModHandler::getModDependencies(const TModID & modId, bool & isModFound) const
 {
 	auto it = allMods.find(modId);

lib/modding/CModHandler.h

@@ -64,6 +64,7 @@ public:
 
 	std::string getModLanguage(const TModID & modId) const;
 
+	std::set<TModID> getModDependencies(const TModID & modId) const;
 	std::set<TModID> getModDependencies(const TModID & modId, bool & isModFound) const;
 
 	/// returns list of all (active) mods

lib/modding/ContentTypeHandler.cpp

@@ -17,6 +17,7 @@
 #include "../BattleFieldHandler.h"
 #include "../CArtHandler.h"
 #include "../CCreatureHandler.h"
+#include "../CConfigHandler.h"
 #include "../entities/faction/CTownHandler.h"
 #include "../texts/CGeneralTextHandler.h"
 #include "../CHeroHandler.h"
@@ -39,9 +40,9 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-ContentTypeHandler::ContentTypeHandler(IHandlerBase * handler, const std::string & objectName):
+ContentTypeHandler::ContentTypeHandler(IHandlerBase * handler, const std::string & entityName):
 	handler(handler),
-	objectName(objectName),
+	entityName(entityName),
 	originalData(handler->loadLegacyData())
 {
 	for(auto & node : originalData)
@@ -79,6 +80,9 @@ bool ContentTypeHandler::preloadModData(const std::string & modName, const JsonN
 			logMod->trace("Patching object %s (%s) from %s", objectName, remoteName, modName);
 			JsonNode & remoteConf = modData[remoteName].patches[objectName];
 
+			if (!remoteConf.isNull() && settings["mods"]["validation"].String() != "off")
+				JsonUtils::detectConflicts(conflictList, remoteConf, entry.second, objectName);
+
 			JsonUtils::merge(remoteConf, entry.second);
 		}
 	}
@@ -93,7 +97,7 @@ bool ContentTypeHandler::loadMod(const std::string & modName, bool validate)
 	auto performValidate = [&,this](JsonNode & data, const std::string & name){
 		handler->beforeValidate(data);
 		if (validate)
-			result &= JsonUtils::validate(data, "vcmi:" + objectName, name);
+			result &= JsonUtils::validate(data, "vcmi:" + entityName, name);
 	};
 
 	// apply patches
@@ -113,7 +117,7 @@ bool ContentTypeHandler::loadMod(const std::string & modName, bool validate)
 			// - another mod attempts to add object into this mod (technically can be supported, but might lead to weird edge cases)
 			// - another mod attempts to edit object from this mod that no longer exist - DANGER since such patch likely has very incomplete data
 			// so emit warning and skip such case
-			logMod->warn("Mod '%s' attempts to edit object '%s' of type '%s' from mod '%s' but no such object exist!", data.getModScope(), name, objectName, modName);
+			logMod->warn("Mod '%s' attempts to edit object '%s' of type '%s' from mod '%s' but no such object exist!", data.getModScope(), name, entityName, modName);
 			continue;
 		}
 
@@ -159,31 +163,70 @@ void ContentTypeHandler::loadCustom()
 
 void ContentTypeHandler::afterLoadFinalization()
 {
-	for (auto const & data : modData)
+	if (settings["mods"]["validation"].String() != "off")
 	{
-		if (data.second.modData.isNull())
+		for (auto const & data : modData)
 		{
-			for (auto node : data.second.patches.Struct())
-				logMod->warn("Mod '%s' have added patch for object '%s' from mod '%s', but this mod was not loaded or has no new objects.", node.second.getModScope(), node.first, data.first);
-		}
-
-		for(auto & otherMod : modData)
-		{
-			if (otherMod.first == data.first)
-				continue;
-
-			if (otherMod.second.modData.isNull())
-				continue;
-
-			for(auto & otherObject : otherMod.second.modData.Struct())
+			if (data.second.modData.isNull())
 			{
-				if (data.second.modData.Struct().count(otherObject.first))
+				for (auto node : data.second.patches.Struct())
+					logMod->warn("Mod '%s' have added patch for object '%s' from mod '%s', but this mod was not loaded or has no new objects.", node.second.getModScope(), node.first, data.first);
+			}
+
+			for(auto & otherMod : modData)
+			{
+				if (otherMod.first == data.first)
+					continue;
+
+				if (otherMod.second.modData.isNull())
+					continue;
+
+				for(auto & otherObject : otherMod.second.modData.Struct())
 				{
-					logMod->warn("Mod '%s' have added object with name '%s' that is also available in mod '%s'", data.first, otherObject.first, otherMod.first);
-					logMod->warn("Two objects with same name were loaded. Please use form '%s:%s' if mod '%s' needs to modify this object instead", otherMod.first, otherObject.first, data.first);
+					if (data.second.modData.Struct().count(otherObject.first))
+					{
+						logMod->warn("Mod '%s' have added object with name '%s' that is also available in mod '%s'", data.first, otherObject.first, otherMod.first);
+						logMod->warn("Two objects with same name were loaded. Please use form '%s:%s' if mod '%s' needs to modify this object instead", otherMod.first, otherObject.first, data.first);
+					}
 				}
 			}
 		}
+
+		for (const auto& [conflictPath, conflictModData] : conflictList.Struct())
+		{
+			std::set<std::string> conflictingMods;
+			std::set<std::string> resolvedConflicts;
+
+			for (auto const & conflictModData : conflictModData.Struct())
+				conflictingMods.insert(conflictModData.first);
+
+			for (auto const & modID : conflictingMods)
+				resolvedConflicts.merge(VLC->modh->getModDependencies(modID));
+
+			vstd::erase_if(conflictingMods, [&resolvedConflicts](const std::string & entry){ return resolvedConflicts.count(entry);});
+
+			if (conflictingMods.size() < 2)
+				continue; // all conflicts were resolved - either via compatibility patch (mod that depends on 2 conflicting mods) or simple mod that depends on another one
+
+			bool allEqual = true;
+
+			for (auto const & modID : conflictingMods)
+			{
+				if (conflictModData[modID] != conflictModData[*conflictingMods.begin()])
+				{
+					allEqual = false;
+					break;
+				}
+			}
+
+			if (allEqual)
+				continue; // conflict still present, but all mods use the same value for conflicting entry - permit it
+
+			logMod->warn("Potential confict in '%s'", conflictPath);
+
+			for (auto const & modID : conflictingMods)
+				logMod->warn("Mod '%s' - value set to %s", modID, conflictModData[modID].toCompactString());
+		}
 	}
 
 	handler->afterLoadFinalization();
@@ -249,7 +292,7 @@ void CContentHandler::afterLoadFinalization()
 
 void CContentHandler::preloadData(CModInfo & mod)
 {
-	bool validate = (mod.validation != CModInfo::PASSED);
+	bool validate = validateMod(mod);
 
 	// print message in format [<8-symbols checksum>] <modname>
 	auto & info = mod.getVerificationInfo();
@@ -266,7 +309,7 @@ void CContentHandler::preloadData(CModInfo & mod)
 
 void CContentHandler::load(CModInfo & mod)
 {
-	bool validate = (mod.validation != CModInfo::PASSED);
+	bool validate = validateMod(mod);
 
 	if (!loadMod(mod.identifier, validate))
 		mod.validation = CModInfo::FAILED;
@@ -287,4 +330,18 @@ const ContentTypeHandler & CContentHandler::operator[](const std::string & name)
 	return handlers.at(name);
 }
 
+bool CContentHandler::validateMod(const CModInfo & mod) const
+{
+	if (settings["mods"]["validation"].String() == "full")
+		return true;
+
+	if (mod.validation == CModInfo::PASSED)
+		return false;
+
+	if (settings["mods"]["validation"].String() == "off")
+		return false;
+
+	return true;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/modding/ContentTypeHandler.h

@@ -19,6 +19,8 @@ class CModInfo;
 /// internal type to handle loading of one data type (e.g. artifacts, creatures)
 class DLL_LINKAGE ContentTypeHandler
 {
+	JsonNode conflictList;
+
 public:
 	struct ModInfo
 	{
@@ -29,7 +31,7 @@ public:
 	};
 	/// handler to which all data will be loaded
 	IHandlerBase * handler;
-	std::string objectName;
+	std::string entityName;
 
 	/// contains all loaded H3 data
 	std::vector<JsonNode> originalData;
@@ -56,6 +58,7 @@ class DLL_LINKAGE CContentHandler
 
 	std::map<std::string, ContentTypeHandler> handlers;
 
+	bool validateMod(const CModInfo & mod) const;
 public:
 	void init();