Implemented better error-reporting for issues commonly encountered by

modders

lib/modding/ContentTypeHandler.cpp

@@ -111,7 +111,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 from mod %s but no such object exist!", data.meta, name, modName);
+			logMod->warn("Mod '%s' attempts to edit object '%s' of type '%s' from mod '%s' but no such object exist!", data.meta, name, objectName, modName);
 			continue;
 		}
 
@@ -157,6 +157,33 @@ void ContentTypeHandler::loadCustom()
 
 void ContentTypeHandler::afterLoadFinalization()
 {
+	for (auto const & data : modData)
+	{
+		if (data.second.modData.isNull())
+		{
+			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.meta, 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.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);
+				}
+			}
+		}
+	}
+
 	handler->afterLoadFinalization();
 }
 

lib/modding/IdentifierStorage.cpp

@@ -197,58 +197,97 @@ std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope,
 {
 	assert(state != ELoadingState::LOADING);
 
-	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(scope, type, name, std::function<void(si32)>(), silent));
-
-	if (idList.size() == 1)
-		return idList.front().id;
-	if (!silent)
-		logMod->error("Failed to resolve identifier %s of type %s from mod %s", name , type ,scope);
-
-	return std::optional<si32>();
+	auto options = ObjectCallback::fromNameAndType(scope, type, name, std::function<void(si32)>(), silent);
+	return getIdentifierImpl(options, silent);
 }
 
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & type, const JsonNode & name, bool silent) const
 {
 	assert(state != ELoadingState::LOADING);
 
-	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(name.meta, type, name.String(), std::function<void(si32)>(), silent));
+	auto options = ObjectCallback::fromNameAndType(name.meta, type, name.String(), std::function<void(si32)>(), silent);
 
-	if (idList.size() == 1)
-		return idList.front().id;
-	if (!silent)
-		logMod->error("Failed to resolve identifier %s of type %s from mod %s", name.String(), type, name.meta);
-
-	return std::optional<si32>();
+	return getIdentifierImpl(options, silent);
 }
 
 std::optional<si32> CIdentifierStorage::getIdentifier(const JsonNode & name, bool silent) const
 {
 	assert(state != ELoadingState::LOADING);
 
-	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(name.meta, name.String(), std::function<void(si32)>(), silent));
-
-	if (idList.size() == 1)
-		return idList.front().id;
-	if (!silent)
-		logMod->error("Failed to resolve identifier %s from mod %s", name.String(), name.meta);
-
-	return std::optional<si32>();
+	auto options = ObjectCallback::fromNameWithType(name.meta, name.String(), std::function<void(si32)>(), silent);
+	return getIdentifierImpl(options, silent);
 }
 
 std::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope, const std::string & fullName, bool silent) const
 {
 	assert(state != ELoadingState::LOADING);
 
-	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(scope, fullName, std::function<void(si32)>(), silent));
+	auto options = ObjectCallback::fromNameWithType(scope, fullName, std::function<void(si32)>(), silent);
+	return getIdentifierImpl(options, silent);
+}
+
+std::optional<si32> CIdentifierStorage::getIdentifierImpl(const ObjectCallback & options, bool silent) const
+{
+	auto idList = getPossibleIdentifiers(options);
 
 	if (idList.size() == 1)
 		return idList.front().id;
 	if (!silent)
-		logMod->error("Failed to resolve identifier %s from mod %s", fullName, scope);
-
+		showIdentifierResolutionErrorDetails(options);
 	return std::optional<si32>();
 }
 
+void CIdentifierStorage::showIdentifierResolutionErrorDetails(const ObjectCallback & options) const
+{
+	auto idList = getPossibleIdentifiers(options);
+
+	logMod->error("Failed to resolve identifier '%s' of type '%s' from mod '%s'", options.name, options.type, options.localScope);
+	if (idList.empty())
+	{
+		// check whether identifier is unavailable due to a missing dependency on a mod
+		ObjectCallback testOptions = options;
+		testOptions.localScope = ModScope::scopeGame();
+		testOptions.remoteScope = {};
+
+		auto testList = getPossibleIdentifiers(testOptions);
+		if (testList.empty())
+		{
+			logMod->error("Identifier '%s' of type '%s' does not exists in any loaded mod!", options.name, options.type);
+		}
+		else
+		{
+			// such identifiers exists, but were not picked for some reason
+			if (options.remoteScope.empty())
+			{
+				// attempt to access identifier from mods that is not dependency
+				for (auto const & testOption : testList)
+				{
+					logMod->error("Identifier '%s' exists in mod %s", options.name, testOption.scope);
+					logMod->error("Please add mod '%s' as dependency of mod '%s' to access this identifier", testOption.scope, options.localScope);
+				}
+			}
+			else
+			{
+				// attempt to access identifier in form 'modName:object', but identifier is only present in different mod
+				for (auto const & testOption : testList)
+				{
+					logMod->error("Identifier '%s' exists in mod '%s' but identifier was explicitly requested from mod '%s'", options.name, testOption.scope, options.remoteScope);
+					logMod->error("Please use form '%s' or '%s:%s' to access this identifier", options.name, testOption.scope, options.name);
+				}
+			}
+		}
+	}
+	else
+	{
+		logMod->error("Multiple possible candidates:");
+		for (auto const & testOption : idList)
+		{
+			logMod->error("Identifier %s exists in mod %s", options.name, testOption.scope);
+			logMod->error("Please use '%s:%s' form to select this identifier", testOption.scope, options.remoteScope);
+		}
+	}
+}
+
 void CIdentifierStorage::registerObject(const std::string & scope, const std::string & type, const std::string & name, si32 identifier)
 {
 	assert(state != ELoadingState::FINISHED);
@@ -362,17 +401,7 @@ bool CIdentifierStorage::resolveIdentifier(const ObjectCallback & request) const
 	}
 
 	// error found. Try to generate some debug info
-	if(identifiers.empty())
-		logMod->error("Unknown identifier!");
-	else
-		logMod->error("Ambiguous identifier request!");
-
-	 logMod->error("Request for %s.%s from mod %s", request.type, request.name, request.localScope);
-
-	for(const auto & id : identifiers)
-	{
-		logMod->error("\tID is available in mod %s", id.scope);
-	}
+	showIdentifierResolutionErrorDetails(request);
 	return false;
 }
 
@@ -381,26 +410,16 @@ void CIdentifierStorage::finalize()
 	assert(state == ELoadingState::LOADING);
 
 	state = ELoadingState::FINALIZING;
-	bool errorsFound = false;
 
 	while ( !scheduledRequests.empty() )
 	{
 		// Use local copy since new requests may appear during resolving, invalidating any iterators
 		auto request = scheduledRequests.back();
 		scheduledRequests.pop_back();
-
-		if (!resolveIdentifier(request))
-			errorsFound = true;
+		resolveIdentifier(request);
 	}
 
-	debugDumpIdentifiers();
-
-	if (errorsFound)
-		logMod->error("All known identifiers were dumped into log file");
-
-	assert(errorsFound == false);
 	state = ELoadingState::FINISHED;
-
 }
 
 void CIdentifierStorage::debugDumpIdentifiers()

lib/modding/IdentifierStorage.h

@@ -69,6 +69,8 @@ class DLL_LINKAGE CIdentifierStorage
 	bool resolveIdentifier(const ObjectCallback & callback) const;
 	std::vector<ObjectData> getPossibleIdentifiers(const ObjectCallback & callback) const;
 
+	void showIdentifierResolutionErrorDetails(const ObjectCallback & callback) const;
+	std::optional<si32> getIdentifierImpl(const ObjectCallback & callback, bool silent) const;
 public:
 	CIdentifierStorage();
 	virtual ~CIdentifierStorage() = default;