Fixes for configurable markets support

- string "speech" can now be translated
- removed "title" string, VCMI will now use object name instead
- moved configuration of all "markets" into a separate json file
- added schema for validation of market objects
- removed serialization of translated strings from University

client/windows/GUIClasses.cpp

@@ -951,8 +951,8 @@ CUniversityWindow::CUniversityWindow(const CGHeroInstance * _hero, BuildingID bu
 	else if(auto uni = dynamic_cast<const CGUniversity *>(_market); uni->appearance)
 	{
 		titlePic = std::make_shared<CAnimImage>(uni->appearance->animationFile, 0, 0, 0, 0, CShowableAnim::CREATURE_MODE);
-		titleStr = uni->title;
-		speechStr = uni->speech;
+		titleStr = uni->getObjectName();
+		speechStr = uni->getSpeechTranslated();
 	}
 	else
 	{

config/gameConfig.json

@@ -56,6 +56,7 @@
 		"config/objects/lighthouse.json",
 		"config/objects/magicSpring.json",
 		"config/objects/magicWell.json",
+		"config/objects/markets.json",
 		"config/objects/moddables.json",
 		"config/objects/observatory.json",
 		"config/objects/pyramid.json",

config/objects/generic.json

@@ -18,115 +18,6 @@
 		}
 	},
 
-	"altarOfSacrifice" : {
-		"index" :2, 
-		"handler" : "market",
-		"base" : {
-			"sounds" : {
-				"visit" : ["MYSTERY"]
-			}
-		},
-		"types" : {
-			"object" : {
-				"index" : 0,
-				"aiValue" : 100,
-				"rmg" : {
-					"zoneLimit"	: 1,
-					"value"		: 100,
-					"rarity"	: 20
-				},
-				"modes" : ["creature-experience", "artifact-experience"]
-			}
-		}
-	},
-	"tradingPost" : {
-		"index" :221, 
-		"handler" : "market",
-		"base" : {
-			"sounds" : {
-				"ambient" : ["LOOPMARK"],
-				"visit" : ["STORE"]
-			}
-		},
-		"types" : {
-			"object" : {
-				"index" : 0,
-				"aiValue" : 100,
-				"rmg" : {
-					"zoneLimit"	: 1,
-					"value"		: 100,
-					"rarity"	: 100
-				},
-				"modes" : ["resource-resource", "resource-player"],
-				"efficiency" : 5,
-				"title" : "core.genrltxt.159"
-			}
-		}
-	},
-	"tradingPostDUPLICATE"		: {
-		"index" :99, 
-		"handler" : "market",
-		"base" : {
-			"sounds" : {
-				"ambient" : ["LOOPMARK"],
-				"visit" : ["STORE"]
-			}
-		},
-		"types" : {
-			"object" : {
-				"index" : 0,
-				"aiValue" : 100,
-				"rmg" : {
-					"zoneLimit"	: 1,
-					"value"		: 100,
-					"rarity"	: 100
-				},
-				"modes" : ["resource-resource", "resource-player"],
-				"efficiency" : 5,
-				"title" : "core.genrltxt.159"
-			}
-		}
-	},
-	"freelancersGuild" : {
-		"index" :213, 
-		"handler" : "market",
-		"types" : {
-			"object" : {
-				"index" : 0,
-				"aiValue" : 100,
-				"rmg" : {
-					"zoneLimit"	: 1,
-					"value"		: 100,
-					"rarity"	: 100
-				},
-				"modes" : ["creature-resource"]
-			}
-		}
-	},
-
-	"blackMarket" : {
-		"index" :7, 
-		"handler" : "market",
-		"base" : {
-			"sounds" : {
-				"ambient" : ["LOOPMARK"],
-				"visit" : ["MYSTERY"]
-			}
-		},
-		"types" : {
-			"object" : {
-				"index" : 0,
-				"aiValue" : 8000,
-				"rmg" : {
-					"value"		: 8000,
-					"rarity"	: 20
-				},
-				"modes" : ["resource-artifact"],
-				"title" : "core.genrltxt.349"
-			}
-		}
-	},
-
 	"pandoraBox" : {
 		"index" :6,
 		"handler" : "pandora",
@@ -393,35 +284,6 @@
 			}
 		}
 	},
-	"university" : {
-		"index" :104, 
-		"handler" : "market",
-		"base" : {
-			"sounds" : {
-				"visit" : ["GAZEBO"]
-			}
-		},
-		"types" : {
-			"object" : {
-				"index" : 0,
-				"aiValue" : 2500,
-				"rmg" : {
-					"value"		: 2500,
-					"rarity"	: 20
-				},
-				"modes" : ["resource-skill"],
-				"title" : "core.genrltxt.602",
-				"speech" : "core.genrltxt.603",
-				"offer": 
-				[
-					{ "noneOf" : ["necromancy"] },
-					{ "noneOf" : ["necromancy"] },
-					{ "noneOf" : ["necromancy"] },
-					{ "noneOf" : ["necromancy"] }
-				]
-			}
-		}
-	},
 	"questGuard" : {
 		"index" :215,
 		"handler" : "questGuard",

config/objects/markets.json

@@ -0,0 +1,138 @@
+{
+	"altarOfSacrifice" : {
+		"index" :2, 
+		"handler" : "market",
+		"base" : {
+			"sounds" : {
+				"visit" : ["MYSTERY"]
+			}
+		},
+		"types" : {
+			"object" : {
+				"index" : 0,
+				"aiValue" : 100,
+				"rmg" : {
+					"zoneLimit"	: 1,
+					"value"		: 100,
+					"rarity"	: 20
+				},
+				"modes" : ["creature-experience", "artifact-experience"]
+			}
+		}
+	},
+
+	"tradingPost" : {
+		"index" :221, 
+		"handler" : "market",
+		"base" : {
+			"sounds" : {
+				"ambient" : ["LOOPMARK"],
+				"visit" : ["STORE"]
+			}
+		},
+		"types" : {
+			"object" : {
+				"index" : 0,
+				"aiValue" : 100,
+				"rmg" : {
+					"zoneLimit"	: 1,
+					"value"		: 100,
+					"rarity"	: 100
+				},
+				"modes" : ["resource-resource", "resource-player"],
+				"efficiency" : 5
+			}
+		}
+	},
+
+	"tradingPostDUPLICATE"		: {
+		"index" :99, 
+		"handler" : "market",
+		"base" : {
+			"sounds" : {
+				"ambient" : ["LOOPMARK"],
+				"visit" : ["STORE"]
+			}
+		},
+		"types" : {
+			"object" : {
+				"index" : 0,
+				"aiValue" : 100,
+				"rmg" : {
+					"zoneLimit"	: 1,
+					"value"		: 100,
+					"rarity"	: 100
+				},
+				"modes" : ["resource-resource", "resource-player"],
+				"efficiency" : 5
+			}
+		}
+	},
+
+	"freelancersGuild" : {
+		"index" :213, 
+		"handler" : "market",
+		"types" : {
+			"object" : {
+				"index" : 0,
+				"aiValue" : 100,
+				"rmg" : {
+					"zoneLimit"	: 1,
+					"value"		: 100,
+					"rarity"	: 100
+				},
+				"modes" : ["creature-resource"]
+			}
+		}
+	},
+
+	"blackMarket" : {
+		"index" :7, 
+		"handler" : "market",
+		"base" : {
+			"sounds" : {
+				"ambient" : ["LOOPMARK"],
+				"visit" : ["MYSTERY"]
+			}
+		},
+		"types" : {
+			"object" : {
+				"index" : 0,
+				"aiValue" : 8000,
+				"rmg" : {
+					"value"		: 8000,
+					"rarity"	: 20
+				},
+				"modes" : ["resource-artifact"]
+			}
+		}
+	},
+	"university" : {
+		"index" :104, 
+		"handler" : "market",
+		"base" : {
+			"sounds" : {
+				"visit" : ["GAZEBO"]
+			}
+		},
+		"types" : {
+			"object" : {
+				"index" : 0,
+				"aiValue" : 2500,
+				"rmg" : {
+					"value"		: 2500,
+					"rarity"	: 20
+				},
+				"modes" : ["resource-skill"],
+				"speech" : "@core.genrltxt.603",
+				"offer": 
+				[
+					{ "noneOf" : ["necromancy"] },
+					{ "noneOf" : ["necromancy"] },
+					{ "noneOf" : ["necromancy"] },
+					{ "noneOf" : ["necromancy"] }
+				]
+			}
+		}
+	}
+}

config/schemas/market.json

@@ -0,0 +1,50 @@
+{
+	"type" : "object",
+	"$schema" : "http://json-schema.org/draft-04/schema",
+	"title" : "VCMI map object format",
+	"description" : "Description of map object class",
+	"required" : [ "modes" ],
+
+	"additionalProperties" : false,
+
+	"properties" : {
+		"description" : {
+			"description" : "Message that will be shown on right-clicking this object",
+			"type" : "string"
+		},
+		
+		"speech" : {
+			"description" : "Message that will be shown to player on visiting this object",
+			"type" : "string"
+		},
+
+		"modes" : {
+			"type" : "array",
+			"items" : {
+				"enum" : [ "resource-resource", "resource-player", "creature-resource", "resource-artifact", "artifact-resource", "artifact-experience", "creature-experience", "creature-undead", "resource-skill" ],
+				"type" : "string"
+			}
+		},
+		"efficiency" : {
+			"type" : "number",
+			"minimum" : 1,
+			"maximum" : 9
+		},
+		"offer" : {
+			"type" : "array"
+		},
+
+		// Properties that might appear since this node is shared with object config
+		"compatibilityIdentifiers" : { },
+		"blockedVisitable" : { },
+		"removable" : { },
+		"aiValue" : { },
+		"index" : { },
+		"base" : { },
+		"name" : { },
+		"rmg" : { },
+		"templates" : { },
+		"battleground" : { },
+		"sounds" : { }
+	}
+}

lib/mapObjectConstructors/CommonConstructors.cpp

@@ -17,8 +17,10 @@
 #include "../TerrainHandler.h"
 #include "../VCMI_Lib.h"
 
+#include "../CConfigHandler.h"
 #include "../entities/faction/CTownHandler.h"
 #include "../entities/hero/CHeroClass.h"
+#include "../json/JsonUtils.h"
 #include "../mapObjects/CGHeroInstance.h"
 #include "../mapObjects/CGMarket.h"
 #include "../mapObjects/CGTownInstance.h"
@@ -242,10 +244,28 @@ AnimationPath BoatInstanceConstructor::getBoatAnimationName() const
 
 void MarketInstanceConstructor::initTypeData(const JsonNode & input)
 {
+	if (settings["mods"]["validation"].String() != "off")
+		JsonUtils::validate(input, "vcmi:market", getJsonKey());
+
 	if (!input["description"].isNull())
 	{
-		description = input["description"].String();
-		VLC->generaltexth->registerString(input.getModScope(), TextIdentifier(getBaseTextID(), "description"), description);
+		std::string description = input["description"].String();
+		descriptionTextID = TextIdentifier(getBaseTextID(), "description").get();
+		VLC->generaltexth->registerString( input.getModScope(), descriptionTextID, input["description"]);
+	}
+
+	if (!input["speech"].isNull())
+	{
+		std::string speech = input["speech"].String();
+		if (!speech.empty() && speech.at(0) == '@')
+		{
+			speechTextID = speech.substr(1);
+		}
+		else
+		{
+			speechTextID = TextIdentifier(getBaseTextID(), "speech").get();
+			VLC->generaltexth->registerString( input.getModScope(), speechTextID, input["speech"]);
+		}
 	}
 
 	for(auto & element : input["modes"].Vector())
@@ -256,14 +276,11 @@ void MarketInstanceConstructor::initTypeData(const JsonNode & input)
 	
 	marketEfficiency = input["efficiency"].isNull() ? 5 : input["efficiency"].Integer();
 	predefinedOffer = input["offer"];
-	
-	title = input["title"].String();
-	speech = input["speech"].String();
 }
 
 bool MarketInstanceConstructor::hasDescription() const
 {
-	return !description.empty();
+	return !descriptionTextID.empty();
 }
 
 CGMarket * MarketInstanceConstructor::createObject(IGameCallback * cb) const
@@ -283,21 +300,6 @@ CGMarket * MarketInstanceConstructor::createObject(IGameCallback * cb) const
 	return new CGMarket(cb);
 }
 
-void MarketInstanceConstructor::initializeObject(CGMarket * market) const
-{
-	market->marketEfficiency = marketEfficiency;
-	
-	if(auto university = dynamic_cast<CGUniversity*>(market))
-	{
-		university->title = market->getObjectName();
-		if(!title.empty())
-			university->title = VLC->generaltexth->translate(title);
-
-		if(!speech.empty())
-			university->speech = VLC->generaltexth->translate(speech);
-	}
-}
-
 const std::set<EMarketMode> & MarketInstanceConstructor::availableModes() const
 {
 	return marketModes;
@@ -315,4 +317,15 @@ void MarketInstanceConstructor::randomizeObject(CGMarket * object, vstd::RNG & r
 	}
 }
 
+std::string MarketInstanceConstructor::getSpeechTranslated() const
+{
+	assert(marketModes.count(EMarketMode::RESOURCE_SKILL));
+	return VLC->generaltexth->translate(speechTextID);
+}
+
+int MarketInstanceConstructor::getMarketEfficiency() const
+{
+	return marketEfficiency;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/mapObjectConstructors/CommonConstructors.h

@@ -115,25 +115,23 @@ public:
 
 class MarketInstanceConstructor : public CDefaultObjectTypeHandler<CGMarket>
 {
-protected:
-	void initTypeData(const JsonNode & config) override;
+	std::string descriptionTextID;
+	std::string speechTextID;
 	
 	std::set<EMarketMode> marketModes;
 	JsonNode predefinedOffer;
 	int marketEfficiency;
-	
-	std::string description;
-	std::string title;
-	std::string speech;
-	
+
+	void initTypeData(const JsonNode & config) override;
 public:
 	CGMarket * createObject(IGameCallback * cb) const override;
-	void initializeObject(CGMarket * object) const override;
 	void randomizeObject(CGMarket * object, vstd::RNG & rng) const override;
 
 	const std::set<EMarketMode> & availableModes() const;
 	bool hasDescription() const;
 
+	std::string getSpeechTranslated() const;
+	int getMarketEfficiency() const;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CGMarket.cpp

@@ -57,7 +57,7 @@ std::string CGMarket::getPopupText(const CGHeroInstance * hero) const
 
 int CGMarket::getMarketEfficiency() const
 {
-	return marketEfficiency;
+	return getMarketHandler()->getMarketEfficiency();
 }
 
 int CGMarket::availableUnits(EMarketMode mode, int marketItemSerial) const
@@ -125,6 +125,11 @@ std::vector<TradeItemBuy> CGUniversity::availableItemsIds(EMarketMode mode) cons
 	}
 }
 
+std::string CGUniversity::getSpeechTranslated() const
+{
+	return getMarketHandler()->getSpeechTranslated();
+}
+
 void CGUniversity::onHeroVisit(const CGHeroInstance * h) const
 {
 	cb->showObjectWindow(this, EOpenWindowMode::UNIVERSITY_WINDOW, h, true);

lib/mapObjects/CGMarket.h

@@ -19,11 +19,10 @@ class MarketInstanceConstructor;
 
 class DLL_LINKAGE CGMarket : public CGObjectInstance, public IMarket
 {
+protected:
 	std::shared_ptr<MarketInstanceConstructor> getMarketHandler() const;
 
 public:
-	int marketEfficiency;
-	
 	CGMarket(IGameCallback *cb);
 	///IObjectInterface
 	void onHeroVisit(const CGHeroInstance * h) const override; //open trading window
@@ -48,7 +47,12 @@ public:
 			h & marketModes;
 		}
 
-		h & marketEfficiency;
+		if (h.version < Handler::Version::MARKET_TRANSLATION_FIX)
+		{
+			int unused = 0;
+			h & unused;
+		}
+
 		if (h.version < Handler::Version::NEW_MARKETS)
 		{
 			std::string speech;
@@ -103,8 +107,8 @@ class DLL_LINKAGE CGUniversity : public CGMarket
 {
 public:
 	using CGMarket::CGMarket;
-	std::string speech; //currently shown only in university
-	std::string title;
+
+	std::string getSpeechTranslated() const;
 
 	std::vector<TradeItemBuy> skills; //available skills
 
@@ -115,10 +119,11 @@ public:
 	{
 		h & static_cast<CGMarket&>(*this);
 		h & skills;
-		if (h.version >= Handler::Version::NEW_MARKETS)
+		if (h.version >= Handler::Version::NEW_MARKETS && h.version < Handler::Version::MARKET_TRANSLATION_FIX)
 		{
-			h & speech;
-			h & title;
+			std::string temp;
+			h & temp;
+			h & temp;
 		}
 	}
 };

lib/serializer/ESerializationVersion.h

@@ -68,6 +68,7 @@ enum class ESerializationVersion : int32_t
 	REMOVE_VLC_POINTERS, // 869 removed remaining pointers to VLC entities
 	FOLDER_NAME_REWORK, // 870 - rework foldername
 	REWARDABLE_GUARDS, // 871 - fix missing serialization of guards in rewardable objects
+	MARKET_TRANSLATION_FIX, // 872 - remove serialization of markets translateable strings
 	
-	CURRENT = REWARDABLE_GUARDS
+	CURRENT = MARKET_TRANSLATION_FIX
 };