Remove marketModes as member

marketModes are now generated in runtime and are not a member of
IMarket. Was not a bad change, but towns load buildings before town type
is randomized, leading to case where market modes are not actually known
when building is added to town (like random towns with market built)

Since altar requires CArtifactSet for work, IMarket will now always
contain it, but it will only be accessible if market supports altar
mode.

client/widgets/markets/CAltarArtifacts.cpp

@@ -199,7 +199,7 @@ void CAltarArtifacts::onSlotClickPressed(const std::shared_ptr<CTradeableItem> &
 
 	if(const auto pickedArtInst = heroArts->getPickedArtifact())
 	{
-		if(pickedArtInst->canBePutAt(altarArtifactsStorage.get()))
+		if(pickedArtInst->canBePutAt(altarArtifactsStorage))
 		{
 			if(pickedArtInst->artType->isTradable())
 			{

client/widgets/markets/CAltarArtifacts.h

@@ -26,7 +26,7 @@ public:
 	void putBackArtifacts();
 
 private:
-	std::shared_ptr<CArtifactSet> altarArtifactsStorage;
+	const CArtifactSet * altarArtifactsStorage;
 	std::shared_ptr<CButton> sacrificeBackpackButton;
 	std::shared_ptr<CArtifactsOfHeroAltar> heroArts;
 	std::map<std::shared_ptr<CTradeableItem>, const CArtifactInstance*> tradeSlotsMap;

lib/IGameCallback.cpp

@@ -284,7 +284,7 @@ CArtifactSet * CNonConstInfoCallback::getArtSet(const ArtifactLocation & loc)
 	else if(auto market = getMarket(loc.artHolder))
 	{
 		if(auto artSet = market->getArtifactsStorage())
-			return artSet.get();
+			return artSet;
 	}
 	else if(auto army = getArmyInstance(loc.artHolder))
 	{

lib/mapObjectConstructors/CommonConstructors.cpp

@@ -243,7 +243,6 @@ CGMarket * MarketInstanceConstructor::createObject(IGameCallback * cb) const
 
 void MarketInstanceConstructor::initializeObject(CGMarket * market) const
 {
-	market->addMarketMode(marketModes);
 	market->marketEfficiency = marketEfficiency;
 	
 	if(auto university = dynamic_cast<CGUniversity*>(market))
@@ -257,6 +256,11 @@ void MarketInstanceConstructor::initializeObject(CGMarket * market) const
 	}
 }
 
+const std::set<EMarketMode> & MarketInstanceConstructor::availableModes() const
+{
+	return marketModes;
+}
+
 void MarketInstanceConstructor::randomizeObject(CGMarket * object, vstd::RNG & rng) const
 {
 	JsonRandom randomizer(object->cb);

lib/mapObjectConstructors/CommonConstructors.h

@@ -14,6 +14,7 @@
 
 #include "../mapObjects/MiscObjects.h"
 #include "../mapObjects/CGCreature.h"
+#include "../mapObjects/CGHeroInstance.h"
 #include "../mapObjects/ObstacleSetHandler.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -127,6 +128,8 @@ public:
 	void initializeObject(CGMarket * object) const override;
 	void randomizeObject(CGMarket * object, vstd::RNG & rng) const override;
 
+	const std::set<EMarketMode> & availableModes() const;
+
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CGMarket.cpp

@@ -19,6 +19,7 @@
 #include "../CSkillHandler.h"
 #include "../mapObjectConstructors/AObjectTypeHandler.h"
 #include "../mapObjectConstructors/CObjectClassesHandler.h"
+#include "../mapObjectConstructors/CommonConstructors.h"
 #include "../networkPacks/PacksForClient.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -48,6 +49,14 @@ int CGMarket::availableUnits(EMarketMode mode, int marketItemSerial) const
 	return -1;
 }
 
+std::set<EMarketMode> CGMarket::availableModes() const
+{
+	const auto & baseHandler = getObjectHandler();
+	const auto & ourHandler = std::dynamic_pointer_cast<MarketInstanceConstructor>(baseHandler);
+
+	return ourHandler->availableModes();
+}
+
 CGMarket::CGMarket(IGameCallback *cb):
 	CGObjectInstance(cb)
 {}

lib/mapObjects/CGMarket.h

@@ -29,11 +29,17 @@ public:
 	ObjectInstanceID getObjInstanceID() const override;
 	int getMarketEfficiency() const override;
 	int availableUnits(EMarketMode mode, int marketItemSerial) const override; //-1 if unlimited
+	std::set<EMarketMode> availableModes() const override;
 
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & static_cast<CGObjectInstance&>(*this);
-		h & static_cast<IMarket&>(*this);
+		if (h.version < Handler::Version::NEW_MARKETS)
+		{
+			std::set<EMarketMode> marketModes;
+			h & marketModes;
+		}
+
 		h & marketEfficiency;
 		if (h.version < Handler::Version::NEW_MARKETS)
 		{
@@ -47,7 +53,6 @@ public:
 	template <typename Handler> void serializeArtifactsAltar(Handler &h)
 	{
 		serialize(h);
-		IMarket::serializeArtifactsAltar(h);
 	}
 };
 

lib/mapObjects/CGTownInstance.cpp

@@ -938,20 +938,19 @@ void CGTownInstance::addBuilding(const BuildingID & buildingID)
 	if(buildingID == BuildingID::NONE)
 		return;
 
-	const auto townType = (*VLC->townh)[getFaction()]->town;
-	if(const auto & building = townType->buildings.find(buildingID); building != townType->buildings.end())
-	{
-		builtBuildings.insert(buildingID);
-		addMarketMode(building->second->marketModes);
-	}
+	builtBuildings.insert(buildingID);
 }
 
-void CGTownInstance::postDeserializeMarketFix()
+std::set<EMarketMode> CGTownInstance::availableModes() const
 {
-	// re-add all buildings to recreate existing market modes
-	auto buildingsBak = builtBuildings;
-	for (auto building : buildingsBak)
-		addBuilding(building);
+	std::set<EMarketMode> result;
+	for (const auto & buildingID : builtBuildings)
+	{
+		const auto * buildingPtr = town->buildings.at(buildingID).get();
+		result.insert(buildingPtr->marketModes.begin(), buildingPtr->marketModes.end());
+	}
+
+	return result;
 }
 
 void CGTownInstance::removeBuilding(const BuildingID & buildingID)
@@ -959,17 +958,12 @@ void CGTownInstance::removeBuilding(const BuildingID & buildingID)
 	if(!vstd::contains(builtBuildings, buildingID))
 		return;
 
-	if(const auto & building = town->buildings.find(buildingID); building != town->buildings.end())
-	{
-		builtBuildings.erase(buildingID);
-		removeMarketMode(building->second->marketModes);
-	}
+	builtBuildings.erase(buildingID);
 }
 
 void CGTownInstance::removeAllBuildings()
 {
 	builtBuildings.clear();
-	removeAllMarketModes();
 }
 
 std::set<BuildingID> CGTownInstance::getBuildings() const

lib/mapObjects/CGTownInstance.h

@@ -77,9 +77,6 @@ public:
 	template <typename Handler> void serialize(Handler &h)
 	{
 		h & static_cast<CGDwelling&>(*this);
-		if (h.version >= Handler::Version::NEW_MARKETS)
-			h & static_cast<IMarket&>(*this);
-
 		h & nameTextId;
 		h & built;
 		h & destroyed;
@@ -118,9 +115,6 @@ public:
 			town = faction ? faction->town : nullptr;
 		}
 
-		if (!h.saving && h.version < Handler::Version::NEW_MARKETS)
-			postDeserializeMarketFix();
-
 		h & townAndVis;
 		BONUS_TREE_DESERIALIZATION_FIX
 
@@ -140,7 +134,6 @@ public:
 	void updateMoraleBonusFromArmy() override;
 	void deserializationFix();
 	void postDeserialize();
-	void postDeserializeMarketFix();
 	void recreateBuildingsBonuses();
 	void setVisitingHero(CGHeroInstance *h);
 	void setGarrisonedHero(CGHeroInstance *h);
@@ -160,6 +153,7 @@ public:
 	EGeneratorState shipyardStatus() const override;
 	const IObjectInterface * getObject() const override;
 	int getMarketEfficiency() const override; //=market count
+	std::set<EMarketMode> availableModes() const override;
 	std::vector<TradeItemBuy> availableItemsIds(EMarketMode mode) const override;
 	ObjectInstanceID getObjInstanceID() const override;
 	void updateAppearance();

lib/mapObjects/IMarket.cpp

@@ -22,7 +22,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 bool IMarket::allowsTrade(const EMarketMode mode) const
 {
-	return vstd::contains(marketModes, mode);
+	return vstd::contains(availableModes(), mode);
 }
 
 bool IMarket::getOffer(int id1, int id2, int &val1, int &val2, EMarketMode mode) const
@@ -140,42 +140,19 @@ int IMarket::availableUnits(const EMarketMode mode, const int marketItemSerial)
 	}
 }
 
-void IMarket::addMarketMode(const EMarketMode mode)
+IMarket::IMarket()
+	:altarArtifactsStorage(std::make_unique<CArtifactSetAltar>())
 {
-	marketModes.insert(mode);
-
-	if(mode == EMarketMode::ARTIFACT_EXP)
-		altarArtifactsStorage = std::make_shared<CArtifactSetAltar>();
-}
-
-void IMarket::addMarketMode(const std::set<EMarketMode> & modes)
-{
-	for(const auto & mode : modes)
-		addMarketMode(mode);
-}
-
-void IMarket::removeMarketMode(const EMarketMode mode)
-{
-	marketModes.erase(mode);
-
-	if(mode == EMarketMode::ARTIFACT_EXP)
-		altarArtifactsStorage.reset();
 }
 
-void IMarket::removeMarketMode(const std::set<EMarketMode> & modes)
-{
-	for(const auto & mode : modes)
-		removeMarketMode(mode);
-}
-
-void IMarket::removeAllMarketModes()
-{
-	marketModes.clear();
-}
+IMarket::~IMarket() = default;
 
-std::shared_ptr<CArtifactSet> IMarket::getArtifactsStorage() const
+CArtifactSet * IMarket::getArtifactsStorage() const
 {
-	return altarArtifactsStorage;
+	if (availableModes().count(EMarketMode::ARTIFACT_EXP))
+		return altarArtifactsStorage.get();
+	else
+		return nullptr;
 }
 
 std::vector<TradeItemBuy> IMarket::availableItemsIds(const EMarketMode mode) const
@@ -192,9 +169,4 @@ std::vector<TradeItemBuy> IMarket::availableItemsIds(const EMarketMode mode) con
 	return ret;
 }
 
-std::set<EMarketMode> IMarket::availableModes() const
-{
-	return marketModes;
-}
-
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/IMarket.h

@@ -15,9 +15,12 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-class DLL_LINKAGE IMarket : public virtual Serializeable
+class DLL_LINKAGE IMarket : public virtual Serializeable, boost::noncopyable
 {
 public:
+	IMarket();
+	~IMarket();
+
 	class CArtifactSetAltar : public CArtifactSet
 	{
 	public:
@@ -29,36 +32,12 @@ public:
 	virtual bool allowsTrade(const EMarketMode mode) const;
 	virtual int availableUnits(const EMarketMode mode, const int marketItemSerial) const; //-1 if unlimited
 	virtual std::vector<TradeItemBuy> availableItemsIds(const EMarketMode mode) const;
-	void addMarketMode(const EMarketMode mode);
-	void addMarketMode(const std::set<EMarketMode> & modes);
-	void removeMarketMode(const EMarketMode mode);
-	void removeMarketMode(const std::set<EMarketMode> & modes);
-	void removeAllMarketModes();
-	std::set<EMarketMode> availableModes() const;
-	std::shared_ptr<CArtifactSet> getArtifactsStorage() const;
+	virtual std::set<EMarketMode> availableModes() const = 0;
+	CArtifactSet * getArtifactsStorage() const;
 	bool getOffer(int id1, int id2, int &val1, int &val2, EMarketMode mode) const; //val1 - how many units of id1 player has to give to receive val2 units
 
-	template <typename Handler> void serialize(Handler & h)
-	{
-		h & marketModes;
-
-		if(vstd::contains(marketModes, EMarketMode::ARTIFACT_EXP))
-		{
-			if (!h.saving)
-				altarArtifactsStorage = std::make_shared<CArtifactSetAltar>();
-
-			h & *altarArtifactsStorage;
-		}
-	}
-
-	template <typename Handler> void serializeArtifactsAltar(Handler & h)
-	{
-		h & *altarArtifactsStorage;
-	}
-
 private:
-	std::shared_ptr<CArtifactSetAltar> altarArtifactsStorage;
-	std::set<EMarketMode> marketModes;
+	std::unique_ptr<CArtifactSetAltar> altarArtifactsStorage;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/mapping/MapFormatH3M.cpp

@@ -2198,7 +2198,6 @@ CGObjectInstance * CMapLoaderH3M::readTown(const int3 & position, std::shared_pt
 	bool hasCustomBuildings = reader->readBool();
 	if(hasCustomBuildings)
 	{
-		object->subID = faction.value();
 		std::set<BuildingID> builtBuildings;
 		reader->readBitmaskBuildings(builtBuildings, faction);
 		for(const auto & building : builtBuildings)