Merge pull request #1865 from IvanSavenko/identifiers_resolution_fixes

Identifiers resolution fixes (1.3)

config/factions/castle.json

@@ -147,7 +147,7 @@
 			"horde" : [ 2, -1 ],
 			"mageGuild" : 4,
 			"warMachine" : "ballista",
-			"moatAbility" : "core:spell.castleMoat",
+			"moatAbility" : "castleMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :

config/factions/conflux.json

@@ -152,7 +152,7 @@
 			"mageGuild" : 5,
 			"primaryResource" : "mercury",
 			"warMachine" : "ballista",
-			"moatAbility" : "core:spell.castleMoat",
+			"moatAbility" : "castleMoat",
 
 			"buildings" :
 			{

config/factions/dungeon.json

@@ -148,7 +148,7 @@
 			"mageGuild" : 5,
 			"primaryResource" : "sulfur",
 			"warMachine" : "ballista",
-			"moatAbility" : "core:spell.dungeonMoat",
+			"moatAbility" : "dungeonMoat",
 
 
 			"buildings" :

config/factions/fortress.json

@@ -147,7 +147,7 @@
 			"horde" : [ 0, -1 ],
 			"mageGuild" : 3,
 			"warMachine" : "firstAidTent",
-			"moatAbility" : "core:spell.fortressMoat",
+			"moatAbility" : "fortressMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :

config/factions/inferno.json

@@ -149,7 +149,7 @@
 			"mageGuild" : 5,
 			"primaryResource" : "mercury",
 			"warMachine" : "ammoCart",
-			"moatAbility" : "core:spell.infernoMoat",
+			"moatAbility" : "infernoMoat",
 
 			"buildings" :
 			{

config/factions/necropolis.json

@@ -152,7 +152,7 @@
 			"horde" : [ 0, -1 ],
 			"mageGuild" : 5,
 			"warMachine" : "firstAidTent",
-			"moatAbility" : "core:spell.necropolisMoat",
+			"moatAbility" : "necropolisMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :

config/factions/rampart.json

@@ -152,7 +152,7 @@
 			"mageGuild" : 5,
 			"primaryResource" : "crystal",
 			"warMachine" : "firstAidTent",
-			"moatAbility" : "core:spell.rampartMoat",
+			"moatAbility" : "rampartMoat",
 
 			"buildings" :
 			{

config/factions/stronghold.json

@@ -145,7 +145,7 @@
 			"horde" : [ 0, -1 ],
 			"mageGuild" : 3,
 			"warMachine" : "ammoCart",
-			"moatAbility" : "core:spell.strongholdMoat",
+			"moatAbility" : "strongholdMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :

config/factions/tower.json

@@ -147,7 +147,7 @@
 			"primaryResource" : "gems",
 			"mageGuild" : 5,
 			"warMachine" : "ammoCart",
-			"moatAbility" : "core:spell.towerMoat",
+			"moatAbility" : "towerMoat",
 
 			"buildings" :
 			{

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);
 }
 
+CIdentifierStorage::ObjectCallback CIdentifierStorage::ObjectCallback::fromNameWithType(const std::string & scope, 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, '.');
+
+	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;
+}
+
+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, '.');
+
+	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 & scope, const std::string & type, const std::string & name, const std::function<void(si32)> & callback)
 {
-	auto pair = vstd::splitStringToPair(name, ':'); // remoteScope:name
+	requestIdentifier(ObjectCallback::fromNameAndType(scope, type, name, callback, false));
-
-	requestIdentifier(ObjectCallback(scope, pair.first, type, pair.second, callback, false));
 }
 
 void CIdentifierStorage::requestIdentifier(const std::string & scope, const std::string & fullName, const std::function<void(si32)> & callback)
 {
-	auto scopeAndFullName = vstd::splitStringToPair(fullName, ':');
+	requestIdentifier(ObjectCallback::fromNameWithType(scope, fullName, callback, false));
-	auto typeAndName = vstd::splitStringToPair(scopeAndFullName.second, '.');
-
-	requestIdentifier(ObjectCallback(scope, scopeAndFullName.first, typeAndName.first, typeAndName.second, callback, false));
 }
 
 void CIdentifierStorage::requestIdentifier(const std::string & type, const JsonNode & name, const std::function<void(si32)> & callback)
 {
-	auto pair = vstd::splitStringToPair(name.String(), ':'); // remoteScope:name
+	requestIdentifier(ObjectCallback::fromNameAndType(name.meta, type, name.String(), callback, false));
-
-	requestIdentifier(ObjectCallback(name.meta, pair.first, type, pair.second, callback, false));
 }
 
 void CIdentifierStorage::requestIdentifier(const JsonNode & name, const std::function<void(si32)> & callback)
 {
-	auto pair  = vstd::splitStringToPair(name.String(), ':'); // remoteScope:<type.name>
+	requestIdentifier(ObjectCallback::fromNameWithType(name.meta, name.String(), callback, false));
-	auto pair2 = vstd::splitStringToPair(pair.second,   '.'); // type.name
-
-	requestIdentifier(ObjectCallback(name.meta, pair.first, pair2.first, pair2.second, 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::fromNameAndType(scope, type, name, callback, true));
-
-	requestIdentifier(ObjectCallback(scope, pair.first, type, pair.second, 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::fromNameAndType(name.meta, type, name.String(), callback, true));
-
-	requestIdentifier(ObjectCallback(name.meta, pair.first, type, pair.second, 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::fromNameAndType(scope, type, name, std::function<void(si32)>(), silent));
-	auto idList = getPossibleIdentifiers(ObjectCallback(scope, pair.first, type, pair.second, 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::fromNameAndType(name.meta, type, name.String(), std::function<void(si32)>(), silent));
-	auto idList = getPossibleIdentifiers(ObjectCallback(name.meta, pair.first, type, pair.second, 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 idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(name.meta, name.String(), std::function<void(si32)>(), silent));
-	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));
 
 	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 idList = getPossibleIdentifiers(ObjectCallback::fromNameWithType(scope, fullName, std::function<void(si32)>(), silent));
-	auto pair2 = vstd::splitStringToPair(pair.second,   '.'); // type.name
-	auto idList = getPossibleIdentifiers(ObjectCallback(scope, pair.first, pair2.first, pair2.second, 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,
+		/// Builds callback from identifier in form "targetMod:type.name"
-		               std::string type, std::string name,
+		static ObjectCallback fromNameWithType(const std::string & scope, const std::string & fullName, const std::function<void(si32)> & callback, bool optional);
-		               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

lib/CTownHandler.cpp

@@ -885,10 +885,20 @@ void CTownHandler::loadTown(CTown * town, const JsonNode & source)
 		town->namesCount += 1;
 	}
 
-	VLC->modh->identifiers.requestIdentifier(source["moatAbility"], [=](si32 ability)
+	if (!source["moatAbility"].isNull()) // VCMI 1.2 compatibility code
+	{
+		VLC->modh->identifiers.requestIdentifier( "spell", source["moatAbility"], [=](si32 ability)
 		{
 			town->moatAbility = SpellID(ability);
 		});
+	}
+	else
+	{
+		VLC->modh->identifiers.requestIdentifier( source.meta, "spell", "castleMoat", [=](si32 ability)
+		{
+			town->moatAbility = SpellID(ability);
+		});
+	}
 
 	//  Horde building creature level
 	for(const JsonNode &node : source["horde"].Vector())

lib/mapping/CMapService.cpp

@@ -136,6 +136,8 @@ static JsonNode loadPatches(std::string path)
 	JsonNode node = JsonUtils::assembleFromFiles(std::move(path));
 	for (auto & entry : node.Struct())
 		JsonUtils::validate(entry.second, "vcmi:mapHeader", "patch for " + entry.first);
+
+	node.setMeta(CModHandler::scopeMap());
 	return node;
 }
 

lib/spells/effects/Moat.cpp

@@ -129,7 +129,6 @@ void Moat::placeObstacles(ServerCallback * server, const Mechanics * m, const Ef
 {
 	assert(m->battle()->battleGetDefendedTown());
 	assert(m->casterSide == BattleSide::DEFENDER); // Moats are always cast by defender
-	assert(turnsRemaining < 0); // Moats should lasts infininte number of turns
 
 	BattleObstaclesChanged pack;