VCMI will now correctly resolve identifiers in unexpected form

lib/CModHandler.cpp

@@ -65,20 +65,6 @@ void CIdentifierStorage::checkIdentifier(std::string & ID)
 	}
 }
 
-CIdentifierStorage::ObjectCallback::ObjectCallback(std::string localScope,
-												   std::string remoteScope,
-												   std::string type,
-												   std::string name,
-												   std::function<void(si32)> callback,
-												   bool optional):
-	localScope(std::move(localScope)),
-	remoteScope(std::move(remoteScope)),
-	type(std::move(type)),
-	name(std::move(name)),
-	callback(std::move(callback)),
-	optional(optional)
-{}
-
 void CIdentifierStorage::requestIdentifier(ObjectCallback callback)
 {
 	checkIdentifier(callback.type);
@@ -92,54 +78,87 @@ void CIdentifierStorage::requestIdentifier(ObjectCallback callback)
 		resolveIdentifier(callback);
 }
 
-void CIdentifierStorage::requestIdentifier(const std::string & scope, const std::string & type, const std::string & name, const std::function<void(si32)> & callback)
+CIdentifierStorage::ObjectCallback CIdentifierStorage::ObjectCallback::fromNameWithType(const std::string & scope, const std::string & fullName, const std::function<void(si32)> & callback, bool optional)
 {
-	auto pair = vstd::splitStringToPair(name, ':'); // remoteScope:name
+	assert(!scope.empty());
 
-	requestIdentifier(ObjectCallback(scope, pair.first, type, pair.second, callback, false));
+	auto scopeAndFullName = vstd::splitStringToPair(fullName, ':');
+	auto typeAndName = vstd::splitStringToPair(scopeAndFullName.second, '.');
+
+	if (scope == scopeAndFullName.first)
+		logMod->debug("Target scope for identifier '%s' is redundant! Identifier already defined in mod '%s'", fullName, scope);
+
+	ObjectCallback result;
+	result.localScope = scope;
+	result.remoteScope = scopeAndFullName.first;
+	result.type = typeAndName.first;
+	result.name = typeAndName.second;
+	result.callback = callback;
+	result.optional = optional;
+	return result;
 }
 
-void CIdentifierStorage::requestIdentifier(const std::string & scope, const std::string & fullName, const std::function<void(si32)> & callback)
+CIdentifierStorage::ObjectCallback CIdentifierStorage::ObjectCallback::fromNameAndType(const std::string & scope, const std::string & type, const std::string & fullName, const std::function<void(si32)> & callback, bool optional)
 {
+	assert(!scope.empty());
+
 	auto scopeAndFullName = vstd::splitStringToPair(fullName, ':');
 	auto typeAndName = vstd::splitStringToPair(scopeAndFullName.second, '.');
 
-	requestIdentifier(ObjectCallback(scope, scopeAndFullName.first, typeAndName.first, typeAndName.second, callback, false));
+	if(!typeAndName.first.empty())
+	{
+		if (typeAndName.first != type)
+			logMod->error("Identifier '%s' from mod '%s' requested with different type! Type '%s' expected!", fullName, scope, type);
+		else
+			logMod->debug("Target type for identifier '%s' defined in mod '%s' is redundant!", fullName, scope);
+	}
+
+	if (scope == scopeAndFullName.first)
+		logMod->debug("Target scope for identifier '%s' is redundant! Identifier already defined in mod '%s'", fullName, scope);
+
+	ObjectCallback result;
+	result.localScope = scope;
+	result.remoteScope = scopeAndFullName.first;
+	result.type = type;
+	result.name = typeAndName.second;
+	result.callback = callback;
+	result.optional = optional;
+	return result;
 }
 
-void CIdentifierStorage::requestIdentifier(const std::string & type, const JsonNode & name, const std::function<void(si32)> & callback)
+void CIdentifierStorage::requestIdentifier(const std::string & scope, const std::string & type, const std::string & name, const std::function<void(si32)> & callback)
 {
-	auto pair = vstd::splitStringToPair(name.String(), ':'); // remoteScope:name
+	requestIdentifier(ObjectCallback::fromNameAndType(scope, type, name, callback, false));
+}
 
-	requestIdentifier(ObjectCallback(name.meta, pair.first, type, pair.second, callback, false));
+void CIdentifierStorage::requestIdentifier(const std::string & scope, const std::string & fullName, const std::function<void(si32)> & callback)
+{
+	requestIdentifier(ObjectCallback::fromNameWithType(scope, fullName, callback, false));
 }
 
-void CIdentifierStorage::requestIdentifier(const JsonNode & name, const std::function<void(si32)> & callback)
+void CIdentifierStorage::requestIdentifier(const std::string & type, const JsonNode & name, const std::function<void(si32)> & callback)
 {
-	auto pair  = vstd::splitStringToPair(name.String(), ':'); // remoteScope:<type.name>
-	auto pair2 = vstd::splitStringToPair(pair.second,   '.'); // type.name
+	requestIdentifier(ObjectCallback::fromNameAndType(name.meta, type, name.String(), callback, false));
+}
 
-	requestIdentifier(ObjectCallback(name.meta, pair.first, pair2.first, pair2.second, callback, false));
+void CIdentifierStorage::requestIdentifier(const JsonNode & name, const std::function<void(si32)> & callback)
+{
+	requestIdentifier(ObjectCallback::fromNameWithType(name.meta, name.String(), callback, false));
 }
 
 void CIdentifierStorage::tryRequestIdentifier(const std::string & scope, const std::string & type, const std::string & name, const std::function<void(si32)> & callback)
 {
-	auto pair = vstd::splitStringToPair(name, ':'); // remoteScope:name
-
-	requestIdentifier(ObjectCallback(scope, pair.first, type, pair.second, callback, true));
+	requestIdentifier(ObjectCallback::fromNameAndType(scope, type, name, callback, true));
 }
 
 void CIdentifierStorage::tryRequestIdentifier(const std::string & type, const JsonNode & name, const std::function<void(si32)> & callback)
 {
-	auto pair = vstd::splitStringToPair(name.String(), ':'); // remoteScope:name
-
-	requestIdentifier(ObjectCallback(name.meta, pair.first, type, pair.second, callback, true));
+	requestIdentifier(ObjectCallback::fromNameAndType(name.meta, type, name.String(), callback, true));
 }
 
 boost::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope, const std::string & type, const std::string & name, bool silent)
 {
-	auto pair = vstd::splitStringToPair(name, ':'); // remoteScope:name
-	auto idList = getPossibleIdentifiers(ObjectCallback(scope, pair.first, type, pair.second, std::function<void(si32)>(), silent));
+	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(scope, type, name, std::function<void(si32)>(), silent));
 
 	if (idList.size() == 1)
 		return idList.front().id;
@@ -151,8 +170,7 @@ boost::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scop
 
 boost::optional<si32> CIdentifierStorage::getIdentifier(const std::string & type, const JsonNode & name, bool silent)
 {
-	auto pair = vstd::splitStringToPair(name.String(), ':'); // remoteScope:name
-	auto idList = getPossibleIdentifiers(ObjectCallback(name.meta, pair.first, type, pair.second, std::function<void(si32)>(), silent));
+	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameAndType(name.meta, type, name.String(), std::function<void(si32)>(), silent));
 
 	if (idList.size() == 1)
 		return idList.front().id;
@@ -164,28 +182,24 @@ boost::optional<si32> CIdentifierStorage::getIdentifier(const std::string & type
 
 boost::optional<si32> CIdentifierStorage::getIdentifier(const JsonNode & name, bool silent)
 {
-	auto pair  = vstd::splitStringToPair(name.String(), ':'); // remoteScope:<type.name>
-	auto pair2 = vstd::splitStringToPair(pair.second,   '.'); // type.name
-	auto idList = getPossibleIdentifiers(ObjectCallback(name.meta, pair.first, pair2.first, pair2.second, std::function<void(si32)>(), silent));
+	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 of type %s from mod %s", name.String(), pair2.first, name.meta);
+		logMod->error("Failed to resolve identifier %s from mod %s", name.String(), name.meta);
 
 	return boost::optional<si32>();
 }
 
 boost::optional<si32> CIdentifierStorage::getIdentifier(const std::string & scope, const std::string & fullName, bool silent)
 {
-	auto pair  = vstd::splitStringToPair(fullName, ':'); // remoteScope:<type.name>
-	auto pair2 = vstd::splitStringToPair(pair.second,   '.'); // type.name
-	auto idList = getPossibleIdentifiers(ObjectCallback(scope, pair.first, pair2.first, pair2.second, std::function<void(si32)>(), silent));
+	auto idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(scope, fullName, 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", fullName, pair2.first, scope);
+		logMod->error("Failed to resolve identifier %s from mod %s", fullName, scope);
 
 	return boost::optional<si32>();
 }

lib/CModHandler.h

@@ -45,10 +45,14 @@ class DLL_LINKAGE CIdentifierStorage
 		std::function<void(si32)> callback;
 		bool optional;
 
-		ObjectCallback(std::string localScope, std::string remoteScope,
-		               std::string type, std::string name,
-		               std::function<void(si32)> callback,
-		               bool optional);
+		/// Builds callback from identifier in form "targetMod:type.name"
+		static ObjectCallback fromNameWithType(const std::string & scope, const std::string & fullName, const std::function<void(si32)> & callback, bool optional);
+
+		/// Builds callback from identifier in form "targetMod:name"
+		static ObjectCallback fromNameAndType(const std::string & scope, const std::string & type, const std::string & fullName, const std::function<void(si32)> & callback, bool optional);
+
+	private:
+		ObjectCallback() = default;
 	};
 
 	struct ObjectData // entry created on ID registration