Mod system improvement Part I : Old saves support & MSVS build fix

lib/CTownHandler.cpp

@@ -25,11 +25,21 @@
 
 const int NAMES_PER_TOWN=16; // number of town names per faction in H3 files. Json can define any number
 
-CBuilding::CBuilding():
-	town(nullptr),mode(BUILD_NORMAL)
+const std::map<std::string, CBuilding::EBuildMode> CBuilding::MODES =
 {
+	{ "normal", CBuilding::BUILD_NORMAL },
+	{ "auto", CBuilding::BUILD_AUTO },
+	{ "special", CBuilding::BUILD_SPECIAL },
+	{ "grail", CBuilding::BUILD_GRAIL }
+};
 
-}
+const std::map<std::string, CBuilding::ETowerHeight> CBuilding::TOWER_TYPES =
+{
+	{ "low", CBuilding::HEIGHT_LOW },
+	{ "average", CBuilding::HEIGHT_AVERAGE },
+	{ "high", CBuilding::HEIGHT_HIGH },
+	{ "skyship", CBuilding::HEIGHT_SKYSHIP }
+};
 
 const std::string & CBuilding::Name() const
 {
@@ -83,6 +93,52 @@ void CBuilding::deserializeFix()
 	}
 }
 
+void CBuilding::update792(const BuildingID & bid, BuildingSubID::EBuildingSubID & subId, ETowerHeight & height)
+{
+	subId = BuildingSubID::NONE;
+	height = ETowerHeight::HEIGHT_NO_TOWER;
+
+	if(!bid.IsSpecialOrGrail() || town == nullptr || town->faction == nullptr || town->faction->identifier.empty())
+		return;
+
+	const auto buildingName = CTownHandler::getMappedValue<std::string, BuildingID>(bid, std::string(), MappedKeys::BUILDING_TYPES_TO_NAMES);
+
+	if(buildingName.empty())
+		return;
+
+	const auto & faction = town->faction->identifier;
+	auto factionsContent = VLC->modh->content["factions"];
+	auto & coreData = factionsContent.modData.at("core");
+	auto & coreFactions = coreData.modData;
+	auto & currentFaction = coreFactions[faction];
+
+	if (currentFaction.isNull())
+	{
+		const auto index = faction.find(':');
+		const std::string factionDir = index == std::string::npos ? faction : faction.substr(0, index);
+		const auto it = factionsContent.modData.find(factionDir);
+
+		if (it == factionsContent.modData.end())
+		{
+			logMod->warn("Warning: Update old save failed: Faction: '%s' is not found.", factionDir);
+			return;
+		}
+		const std::string modFaction = index == std::string::npos ? faction : faction.substr(index + 1);
+		currentFaction = it->second.modData[modFaction];
+	}
+
+	if (!currentFaction.isNull() && currentFaction.getType() == JsonNode::JsonType::DATA_STRUCT)
+	{
+		const auto & buildings = currentFaction["town"]["buildings"];
+		const auto & currentBuilding = buildings[buildingName];
+
+		subId = CTownHandler::getMappedValue<BuildingSubID::EBuildingSubID>(currentBuilding["type"], BuildingSubID::NONE, MappedKeys::SPECIAL_BUILDINGS);
+		height = CBuilding::HEIGHT_NO_TOWER;
+
+		if (subId == BuildingSubID::LOOKOUT_TOWER || bid == BuildingID::GRAIL)
+			height = CTownHandler::getMappedValue<CBuilding::ETowerHeight>(currentBuilding["height"], CBuilding::HEIGHT_NO_TOWER, CBuilding::TOWER_TYPES);
+	}
+}
 
 CFaction::CFaction()
 {
@@ -343,8 +399,8 @@ void CTownHandler::loadBuildingRequirements(CBuilding * building, const JsonNode
 	requirementsToLoad.push_back(hlp);
 }
 
-template<typename R>
-R CTownHandler::getMappedValue(const std::string key, const R defval, const std::map<std::string, R> & map, bool required) const
+template<typename R, typename K>
+R CTownHandler::getMappedValue(const K key, const R defval, const std::map<K, R> & map, bool required)
 {
 	auto it = map.find(key);
 
@@ -357,64 +413,17 @@ R CTownHandler::getMappedValue(const std::string key, const R defval, const std:
 }
 
 template<typename R>
-R CTownHandler::getMappedValue(const JsonNode & node, const R defval, const std::map<std::string, R> & map, bool required) const
+R CTownHandler::getMappedValue(const JsonNode & node, const R defval, const std::map<std::string, R> & map, bool required)
 {
 	if(!node.isNull() && node.getType() == JsonNode::JsonType::DATA_STRING)
-		return getMappedValue<R>(node.String(), defval, map, required);
+		return getMappedValue<R, std::string>(node.String(), defval, map, required);
 	return defval;
 }
 
 void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, const JsonNode & source)
 {
-	static const std::map<std::string, CBuilding::EBuildMode> MODES =
-	{
-		{ "normal", CBuilding::BUILD_NORMAL },
-		{ "auto", CBuilding::BUILD_AUTO },
-		{ "special", CBuilding::BUILD_SPECIAL },
-		{ "grail", CBuilding::BUILD_GRAIL }
-	};
-
-	static const std::map<std::string, BuildingID> BUILDING_TYPES =
-	{
-		{ "special1", BuildingID::SPECIAL_1 },
-		{ "special2", BuildingID::SPECIAL_2 },
-		{ "special3", BuildingID::SPECIAL_3 },
-		{ "special4", BuildingID::SPECIAL_4 },
-		{ "grail", BuildingID::GRAIL }
-	};
-
-	static const std::map<std::string, CBuilding::ETowerHeight> LOOKOUT_TYPES =
-	{
-		{ "low", CBuilding::HEIGHT_LOW },
-		{ "average", CBuilding::HEIGHT_AVERAGE },
-		{ "high", CBuilding::HEIGHT_HIGH },
-		{ "skyship", CBuilding::HEIGHT_SKYSHIP }
-	};
-
-	static const std::map<std::string, BuildingSubID::EBuildingSubID> SPECIAL_BUILDINGS =
-	{
-		{ "mysticPond", BuildingSubID::MYSTIC_POND },
-		{ "artifactMerchant", BuildingSubID::ARTIFACT_MERCHANT },
-		{ "freelancersGuild", BuildingSubID::FREELANCERS_GUILD },
-		{ "magicUniversity", BuildingSubID::MAGIC_UNIVERSITY },
-		{ "castleGate", BuildingSubID::CASTLE_GATE },
-		{ "creatureTransformer", BuildingSubID::CREATURE_TRANSFORMER },//only skeleton transformer yet
-		{ "portalOfSummoning", BuildingSubID::PORTAL_OF_SUMMONING },
-		{ "ballistaYard", BuildingSubID::BALLISTA_YARD },
-		{ "stables", BuildingSubID::STABLES },
-		{ "manaVortex", BuildingSubID::MANA_VORTEX },
-		{ "lookoutTower", BuildingSubID::LOOKOUT_TOWER },
-		{ "library", BuildingSubID::LIBRARY },
-		{ "brotherhoodOfSword", BuildingSubID::BROTHERHOOD_OF_SWORD },//morale garrison bonus
-		{ "fountainOfFortune", BuildingSubID::FOUNTAIN_OF_FORTUNE },//luck garrison bonus
-		{ "spellPowerGarrisonBonus", BuildingSubID::SPELL_POWER_GARRISON_BONUS },//such as 'stormclouds', but this name is not ok for good towns
-		{ "attackGarrisonBonus", BuildingSubID::ATTACK_GARRISON_BONUS },
-		{ "defenseGarrisonBonus", BuildingSubID::DEFENSE_GARRISON_BONUS },
-		{ "escapeTunnel", BuildingSubID::ESCAPE_TUNNEL }
-	};
-
 	auto ret = new CBuilding();
-	ret->bid = getMappedValue<BuildingID>(stringID, BuildingID::NONE, BUILDING_TYPES, false);
+	ret->bid = getMappedValue<BuildingID, std::string>(stringID, BuildingID::NONE, MappedKeys::BUILDING_NAMES_TO_TYPES, false);
 
 	if(ret->bid == BuildingID::NONE)
 		ret->bid = source["id"].isNull() ? BuildingID(BuildingID::NONE) : BuildingID(source["id"].Float());
@@ -424,14 +433,14 @@ void CTownHandler::loadBuilding(CTown * town, const std::string & stringID, cons
 
 	ret->mode = ret->bid == BuildingID::GRAIL
 		? CBuilding::BUILD_GRAIL
-		: getMappedValue<CBuilding::EBuildMode>(source["mode"], CBuilding::BUILD_NORMAL, MODES);
+		: getMappedValue<CBuilding::EBuildMode>(source["mode"], CBuilding::BUILD_NORMAL, CBuilding::MODES);
 
-	ret->subId = getMappedValue<BuildingSubID::EBuildingSubID>(source["type"], BuildingSubID::NONE, SPECIAL_BUILDINGS);
+	ret->subId = getMappedValue<BuildingSubID::EBuildingSubID>(source["type"], BuildingSubID::NONE, MappedKeys::SPECIAL_BUILDINGS);
 	ret->height = CBuilding::HEIGHT_NO_TOWER;
 
 	if(ret->subId == BuildingSubID::LOOKOUT_TOWER 
 		|| ret->bid == BuildingID::GRAIL) 
-		ret->height = getMappedValue<CBuilding::ETowerHeight>(source["height"], CBuilding::HEIGHT_NO_TOWER, LOOKOUT_TYPES);
+		ret->height = getMappedValue<CBuilding::ETowerHeight>(source["height"], CBuilding::HEIGHT_NO_TOWER, CBuilding::TOWER_TYPES);
 
 	ret->identifier = stringID;
 	ret->town = town;

lib/CTownHandler.h

@@ -65,7 +65,10 @@ public:
 		HEIGHT_SKYSHIP = std::numeric_limits<int>::max()  // grail, open entire map
 	} height;
 
-	CBuilding();
+	static const std::map<std::string, CBuilding::EBuildMode> MODES;
+	static const std::map<std::string, CBuilding::ETowerHeight> TOWER_TYPES;
+
+	CBuilding() : town(nullptr), mode(BUILD_NORMAL) {};
 
 	const std::string &Name() const;
 	const std::string &Description() const;
@@ -75,6 +78,8 @@ public:
 
 	// returns how many times build has to be upgraded to become build
 	si32 getDistance(BuildingID build) const;
+	/// input: faction, bid; output: subId, height;
+	void update792(const BuildingID & bid, BuildingSubID::EBuildingSubID & subId, ETowerHeight & height);
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -94,10 +99,9 @@ public:
 			h & subId;
 			h & height;
 		}
-		else if (!h.saving)
+		else if(!h.saving)
 		{
-			subId = BuildingSubID::NONE;
-			height = CBuilding::HEIGHT_NO_TOWER;
+			update792(bid, subId, height);
 		}
 		if(!h.saving)
 			deserializeFix();
@@ -352,12 +356,12 @@ class DLL_LINKAGE CTownHandler : public IHandlerBase
 
 	void loadRandomFaction();
 
+public:
+	template<typename R, typename K>
+	static R getMappedValue(const K key, const R defval, const std::map<K, R> & map, bool required = true);
 	template<typename R>
-	R getMappedValue(const std::string key, const R defval, const std::map<std::string, R> & map, bool required = true) const;
-	template<typename R>
-	R getMappedValue(const JsonNode& node, const R defval, const std::map<std::string, R> & map, bool required = true) const;
+	static R getMappedValue(const JsonNode & node, const R defval, const std::map<std::string, R> & map, bool required = true);
 
-public:
 	std::vector<ConstTransitivePtr<CFaction> > factions;
 
 	CTown * randomTown;

lib/GameConstants.h

@@ -409,11 +409,18 @@ public:
 	BuildingID(EBuildingID _num = NONE) : num(_num)
 	{}
 
+	STRONG_INLINE
+	bool IsSpecialOrGrail() const
+	{
+		return num == SPECIAL_1 || num == SPECIAL_2 || num == SPECIAL_3 || num == SPECIAL_4 || num == GRAIL;
+	}
+
 	ID_LIKE_CLASS_COMMON(BuildingID, EBuildingID)
 
 	EBuildingID num;
 };
 
+ID_LIKE_OPERATORS(BuildingID, BuildingID::EBuildingID)
 
 namespace BuildingSubID
 {
@@ -443,7 +450,49 @@ namespace BuildingSubID
 	};
 }
 
-ID_LIKE_OPERATORS(BuildingID, BuildingID::EBuildingID)
+namespace MappedKeys
+{
+
+	static const std::map<std::string, BuildingID> BUILDING_NAMES_TO_TYPES =
+	{
+		{ "special1", BuildingID::SPECIAL_1 },
+		{ "special2", BuildingID::SPECIAL_2 },
+		{ "special3", BuildingID::SPECIAL_3 },
+		{ "special4", BuildingID::SPECIAL_4 },
+		{ "grail", BuildingID::GRAIL }
+	};
+
+	static const std::map<BuildingID, std::string> BUILDING_TYPES_TO_NAMES =
+	{
+		{ BuildingID::SPECIAL_1, "special1", },
+		{ BuildingID::SPECIAL_2, "special2" },
+		{ BuildingID::SPECIAL_3, "special3" },
+		{ BuildingID::SPECIAL_4, "special4" },
+		{ BuildingID::GRAIL, "grail"}
+	};
+
+	static const std::map<std::string, BuildingSubID::EBuildingSubID> SPECIAL_BUILDINGS =
+	{
+		{ "mysticPond", BuildingSubID::MYSTIC_POND },
+		{ "artifactMerchant", BuildingSubID::ARTIFACT_MERCHANT },
+		{ "freelancersGuild", BuildingSubID::FREELANCERS_GUILD },
+		{ "magicUniversity", BuildingSubID::MAGIC_UNIVERSITY },
+		{ "castleGate", BuildingSubID::CASTLE_GATE },
+		{ "creatureTransformer", BuildingSubID::CREATURE_TRANSFORMER },//only skeleton transformer yet
+		{ "portalOfSummoning", BuildingSubID::PORTAL_OF_SUMMONING },
+		{ "ballistaYard", BuildingSubID::BALLISTA_YARD },
+		{ "stables", BuildingSubID::STABLES },
+		{ "manaVortex", BuildingSubID::MANA_VORTEX },
+		{ "lookoutTower", BuildingSubID::LOOKOUT_TOWER },
+		{ "library", BuildingSubID::LIBRARY },
+		{ "brotherhoodOfSword", BuildingSubID::BROTHERHOOD_OF_SWORD },//morale garrison bonus
+		{ "fountainOfFortune", BuildingSubID::FOUNTAIN_OF_FORTUNE },//luck garrison bonus
+		{ "spellPowerGarrisonBonus", BuildingSubID::SPELL_POWER_GARRISON_BONUS },//such as 'stormclouds', but this name is not ok for good towns
+		{ "attackGarrisonBonus", BuildingSubID::ATTACK_GARRISON_BONUS },
+		{ "defenseGarrisonBonus", BuildingSubID::DEFENSE_GARRISON_BONUS },
+		{ "escapeTunnel", BuildingSubID::ESCAPE_TUNNEL }
+	};
+}
 
 namespace EAiTactic
 {

lib/NetPacksBase.h

@@ -171,7 +171,7 @@ struct Component
 {
 	enum EComponentType {PRIM_SKILL, SEC_SKILL, RESOURCE, CREATURE, ARTIFACT, EXPERIENCE, SPELL, MORALE, LUCK, BUILDING, HERO_PORTRAIT, FLAG};
 	ui16 id, subtype; //id uses ^^^ enums, when id==EXPPERIENCE subtype==0 means exp points and subtype==1 levels)
-	si64 val; // + give; - take
+	si32 val; // + give; - take
 	si16 when; // 0 - now; +x - within x days; -x - per x days
 
 	template <typename Handler> void serialize(Handler &h, const int version)
@@ -186,7 +186,7 @@ struct Component
 	{
 	}
 	DLL_LINKAGE explicit Component(const CStackBasicDescriptor &stack);
-	Component(Component::EComponentType Type, ui16 Subtype, si64 Val, si16 When)
+	Component(Component::EComponentType Type, ui16 Subtype, si32 Val, si16 When)
 		:id(Type),subtype(Subtype),val(Val),when(When)
 	{
 	}

lib/mapObjects/CGTownInstance.cpp

@@ -435,17 +435,15 @@ void CGDwelling::serializeJsonOptions(JsonSerializeFormat & handler)
 	}
 }
 
+TPropagatorPtr CGTownInstance::emptyPropagator = TPropagatorPtr();
+
 int CGTownInstance::getSightRadius() const //returns sight distance
 {
 	auto ret = CBuilding::HEIGHT_NO_TOWER;
 
 	for(const auto & bid : builtBuildings)
 	{
-		if(bid == BuildingID::SPECIAL_1
-			|| bid == BuildingID::SPECIAL_2
-			|| bid == BuildingID::SPECIAL_3
-			|| bid == BuildingID::SPECIAL_4
-			|| bid == BuildingID::GRAIL)
+		if(bid.IsSpecialOrGrail())
 		{
 			auto height = town->buildings.at(bid)->height;
 			if(ret < height)
@@ -797,6 +795,81 @@ void CGTownInstance::initObj(CRandomGenerator & rand)
 	updateAppearance();
 }
 
+void CGTownInstance::updateBonusingBuildings()
+{
+	if (this->town->faction != nullptr)
+	{
+		//firstly, update subtype for the Bonusing objects, which are already stored in the bonusing list
+		for (auto building : bonusingBuildings)
+		{
+			switch (this->town->faction->index)
+			{
+			case ETownType::CASTLE:
+				if (building->getBuildingType() == BuildingID::SPECIAL_2)
+					building->setBuildingSubtype(BuildingSubID::STABLES);
+				break;
+
+			case ETownType::DUNGEON:
+				if(building->getBuildingType() == BuildingID::SPECIAL_2)
+					building->setBuildingSubtype(BuildingSubID::MANA_VORTEX);
+				break;
+			}
+		}
+	}
+	//secondly, supplement bonusing buildings list and active bonuses; subtypes for these objects are already set in update792
+	for (auto & kvp : town->buildings)
+	{
+		auto & building = kvp.second;
+
+		switch (building->subId)
+		{
+		case BuildingSubID::PORTAL_OF_SUMMONING:
+			creatures.resize(GameConstants::CREATURES_PER_TOWN + 1);
+			break;
+		///'hasBuilt' checking for COPW bonuses is in the COPWBonus::onHeroVisit
+		case BuildingSubID::STABLES:
+			if(getBonusingBuilding(building->subId) == nullptr)
+				bonusingBuildings.push_back(new COPWBonus(BuildingID::STABLES, BuildingSubID::STABLES, this));
+			break;
+
+		case BuildingSubID::MANA_VORTEX:
+			if(getBonusingBuilding(building->subId) == nullptr)
+				bonusingBuildings.push_back(new COPWBonus(BuildingID::MANA_VORTEX, BuildingSubID::MANA_VORTEX, this));
+			break;
+		///add new bonus if bonusing building was built in the user added towns:
+		case BuildingSubID::BROTHERHOOD_OF_SWORD:
+			if(!hasBuiltInOldWay(ETownType::CASTLE, BuildingID::BROTHERHOOD))
+				addBonusIfBuilt(BuildingID::BROTHERHOOD, BuildingSubID::BROTHERHOOD_OF_SWORD, Bonus::MORALE, +2);
+			break;
+
+		case BuildingSubID::FOUNTAIN_OF_FORTUNE:
+			if(!hasBuiltInOldWay(ETownType::RAMPART, BuildingID::FOUNTAIN_OF_FORTUNE))
+				addBonusIfBuilt(BuildingID::FOUNTAIN_OF_FORTUNE, BuildingSubID::FOUNTAIN_OF_FORTUNE, Bonus::LUCK, +2);
+			break;
+
+		case BuildingSubID::SPELL_POWER_GARRISON_BONUS:
+			if(!hasBuiltInOldWay(ETownType::INFERNO, BuildingID::STORMCLOUDS))
+				addBonusIfBuilt(BuildingID::STORMCLOUDS, BuildingSubID::SPELL_POWER_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::SPELL_POWER);
+			break;
+
+		case BuildingSubID::ATTACK_GARRISON_BONUS:
+			if(!hasBuiltInOldWay(ETownType::FORTRESS, BuildingID::BLOOD_OBELISK))
+				addBonusIfBuilt(BuildingID::BLOOD_OBELISK, BuildingSubID::ATTACK_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::ATTACK);
+			break;
+
+		case BuildingSubID::DEFENSE_GARRISON_BONUS:
+			if(!hasBuiltInOldWay(ETownType::FORTRESS, BuildingID::GLYPHS_OF_FEAR))
+				addBonusIfBuilt(BuildingID::GLYPHS_OF_FEAR, BuildingSubID::DEFENSE_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::DEFENSE);
+			break;
+		}
+	}
+}
+
+bool CGTownInstance::hasBuiltInOldWay(ETownType::ETownType type, BuildingID bid) const
+{
+	return (this->town->faction != nullptr && this->town->faction->index == type && hasBuilt(bid));
+}
+
 void CGTownInstance::newTurn(CRandomGenerator & rand) const
 {
 	if (cb->getDate(Date::DAY_OF_WEEK) == 1) //reset on new week
@@ -1122,13 +1195,13 @@ void CGTownInstance::recreateBuildingsBonuses()
 		removeBonus(b);
 
 	//tricky! -> checks tavern only if no bratherhood of sword or not a castle
-	if(!addBonusIfBuilt(BuildingSubID::BROTHERHOOD_OF_SWORD, Bonus::MORALE, +2))
+	if(!addBonusIfBuilt(BuildingID::BROTHERHOOD, BuildingSubID::BROTHERHOOD_OF_SWORD, Bonus::MORALE, +2))
 		addBonusIfBuilt(BuildingID::TAVERN, Bonus::MORALE, +1);
 
-	addBonusIfBuilt(BuildingSubID::FOUNTAIN_OF_FORTUNE, Bonus::LUCK, +2); //fountain of fortune
-	addBonusIfBuilt(BuildingSubID::SPELL_POWER_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::SPELL_POWER);//works as Brimstone Clouds
-	addBonusIfBuilt(BuildingSubID::ATTACK_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::ATTACK);//works as Blood Obelisk
-	addBonusIfBuilt(BuildingSubID::DEFENSE_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::DEFENSE);//works as Glyphs of Fear
+	addBonusIfBuilt(BuildingID::FOUNTAIN_OF_FORTUNE, BuildingSubID::FOUNTAIN_OF_FORTUNE, Bonus::LUCK, +2); //fountain of fortune
+	addBonusIfBuilt(BuildingID::STORMCLOUDS, BuildingSubID::SPELL_POWER_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::SPELL_POWER);//works as Brimstone Clouds
+	addBonusIfBuilt(BuildingID::BLOOD_OBELISK, BuildingSubID::ATTACK_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::ATTACK);//works as Blood Obelisk
+	addBonusIfBuilt(BuildingID::GLYPHS_OF_FEAR, BuildingSubID::DEFENSE_GARRISON_BONUS, Bonus::PRIMARY_SKILL, +2, PrimarySkill::DEFENSE);//works as Glyphs of Fear
 
 	if(subID == ETownType::CASTLE) //castle
 	{
@@ -1164,50 +1237,47 @@ void CGTownInstance::recreateBuildingsBonuses()
 	}
 }
 
-bool CGTownInstance::addBonusIfBuilt(BuildingSubID::EBuildingSubID building, Bonus::BonusType type, int val, int subtype)
+bool CGTownInstance::addBonusIfBuilt(BuildingID bid, BuildingSubID::EBuildingSubID subId, Bonus::BonusType type, int val, int subtype)
 {
-	bool ret = false;
+	bool hasBuilt = false;
+	std::ostringstream descr;
 
-	if (hasBuilt(building))
+	for (const auto & bid : builtBuildings)
 	{
-		std::ostringstream descr;
-
-		for (const auto & it : town->buildings)
+		if (town->buildings.at(bid)->subId == subId)
 		{
-			if (it.second->subId == building)
-			{
-				descr << it.second->Name();
-				break;
-			}
+			descr << town->buildings.at(bid)->Name();
+			hasBuilt = true;
+			break;
 		}
-		auto b = std::make_shared<Bonus>(Bonus::PERMANENT, type, Bonus::TOWN_STRUCTURE, val, building, descr.str(), subtype);
-		addNewBonus(b); //looks like a propagator is not necessary in this case
-		ret = true;
 	}
-	return ret;
+	if(hasBuilt)
+		hasBuilt = addBonusImpl(bid, type, val, emptyPropagator, descr.str(), subtype);
+	return hasBuilt;
 }
 
 bool CGTownInstance::addBonusIfBuilt(BuildingID building, Bonus::BonusType type, int val, int subtype)
 {
-	static auto emptyPropagator = TPropagatorPtr();
 	return addBonusIfBuilt(building, type, val, emptyPropagator, subtype);
 }
 
 bool CGTownInstance::addBonusIfBuilt(BuildingID building, Bonus::BonusType type, int val, TPropagatorPtr & prop, int subtype)
 {
-	if(hasBuilt(building))
-	{
-		std::ostringstream descr;
-		descr << town->buildings.at(building)->Name();
-
-		auto b = std::make_shared<Bonus>(Bonus::PERMANENT, type, Bonus::TOWN_STRUCTURE, val, building, descr.str(), subtype);
-		if(prop)
-			b->addPropagator(prop);
-		addNewBonus(b);
-		return true;
-	}
+	if(!hasBuilt(building))
+		return false;
+	
+	std::ostringstream descr;
+	descr << town->buildings.at(building)->Name();
+	return addBonusImpl(building, type, val, prop, descr.str(), subtype);
+}
 
-	return false;
+bool CGTownInstance::addBonusImpl(BuildingID building, Bonus::BonusType type, int val, TPropagatorPtr & prop, const std::string & description, int subtype)
+{
+	auto b = std::make_shared<Bonus>(Bonus::PERMANENT, type, Bonus::TOWN_STRUCTURE, val, building, description, subtype);
+	if(prop)
+		b->addPropagator(prop);
+	addNewBonus(b);
+	return true;
 }
 
 void CGTownInstance::setVisitingHero(CGHeroInstance *h)
@@ -1402,7 +1472,7 @@ CBuilding::TRequired CGTownInstance::genBuildingRequirements(BuildingID buildID,
 	return ret;
 }
 
-void CGTownInstance::addHeroToStructureVisitors( const CGHeroInstance *h, si64 structureInstanceID ) const
+void CGTownInstance::addHeroToStructureVisitors(const CGHeroInstance *h, si64 structureInstanceID ) const
 {
 	if(visitingHero == h)
 		cb->setObjProperty(id, ObjProperty::STRUCTURE_ADD_VISITING_HERO, structureInstanceID); //add to visitors
@@ -1583,7 +1653,7 @@ void COPWBonus::setProperty(ui8 what, ui32 val)
 	}
 }
 
-void COPWBonus::onHeroVisit (const CGHeroInstance * h) const
+void COPWBonus::onHeroVisit(const CGHeroInstance * h) const
 {
 	ObjectInstanceID heroID = h->id;
 	if (town->hasBuilt(bID))

lib/mapObjects/CGTownInstance.h

@@ -107,6 +107,18 @@ public:
 		return bType;
 	}
 
+	STRONG_INLINE
+	const BuildingID & getBuildingType() const
+	{
+		return bID;
+	}
+
+	STRONG_INLINE
+	void setBuildingSubtype(BuildingSubID::EBuildingSubID subId)
+	{
+		bType = subId;
+	}
+
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & bID;
@@ -114,8 +126,6 @@ public:
 
 		if(version >= 792)
 			h & bType;
-		else if(!h.saving)
-			bType = BuildingSubID::NONE;
 	}
 protected:
 	BuildingID bID; //from buildig list
@@ -240,6 +250,9 @@ public:
 			}
 			return false;
 		});
+
+		if(!h.saving && version < 792)
+			updateBonusingBuildings();
 	}
 	//////////////////////////////////////////////////////////////////////////
 
@@ -248,7 +261,8 @@ public:
 	void updateMoraleBonusFromArmy() override;
 	void deserializationFix();
 	void recreateBuildingsBonuses();
-	bool addBonusIfBuilt(BuildingSubID::EBuildingSubID building, Bonus::BonusType type, int val, int subtype = -1);
+	///bid: param to bind a building with a bonus, subId: param to check if already built
+	bool addBonusIfBuilt(BuildingID bid, BuildingSubID::EBuildingSubID subId, Bonus::BonusType type, int val, int subtype = -1);
 	bool addBonusIfBuilt(BuildingID building, Bonus::BonusType type, int val, TPropagatorPtr &prop, int subtype = -1); //returns true if building is built and bonus has been added
 	bool addBonusIfBuilt(BuildingID building, Bonus::BonusType type, int val, int subtype = -1); //convienence version of above
 	void setVisitingHero(CGHeroInstance *h);
@@ -317,8 +331,12 @@ public:
 	void afterAddToMap(CMap * map) override;
 	static void reset();
 protected:
+	static TPropagatorPtr emptyPropagator;
 	void setPropertyDer(ui8 what, ui32 val) override;
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;
 private:
 	int getDwellingBonus(const std::vector<CreatureID>& creatureIds, const std::vector<ConstTransitivePtr<CGDwelling> >& dwellings) const;
+	void updateBonusingBuildings();
+	bool hasBuiltInOldWay(ETownType::ETownType type, BuildingID bid) const;
+	bool addBonusImpl(BuildingID building, Bonus::BonusType type, int val, TPropagatorPtr & prop, const std::string & description, int subtype = -1);
 };

lib/mapObjects/CObjectClassesHandler.cpp

@@ -214,7 +214,7 @@ CObjectClassesHandler::ObjectContainter * CObjectClassesHandler::loadFromJson(co
 	if(json["defaultAiValue"].isNull())
 		obj->groupDefaultAiValue = boost::none;
 	else
-		obj->groupDefaultAiValue = static_cast<si32>(json["defaultAiValue"].Integer());
+		obj->groupDefaultAiValue = static_cast<boost::optional<si32>>(json["defaultAiValue"].Integer());
 
 	for (auto entry : json["types"].Struct())
 	{
@@ -475,7 +475,7 @@ void AObjectTypeHandler::init(const JsonNode & input, boost::optional<std::strin
 	if(input["aiValue"].isNull())
 		aiValue = boost::none;
 	else
-		aiValue = static_cast<si32>(input["aiValue"].Integer());
+		aiValue = static_cast<boost::optional<si32>>(input["aiValue"].Integer());
 
 	initTypeData(input);
 }

server/CGameHandler.cpp

@@ -2521,9 +2521,8 @@ void CGameHandler::heroVisitCastle(const CGTownInstance * obj, const CGHeroInsta
 
 void CGameHandler::visitCastleObjects(const CGTownInstance * t, const CGHeroInstance * h)
 {
-	std::vector<CGTownBuilding*>::const_iterator i;
-	for (i = t->bonusingBuildings.begin(); i != t->bonusingBuildings.end(); i++)
-		(*i)->onHeroVisit (h);
+	for (auto building : t->bonusingBuildings)
+		building->onHeroVisit(h);
 }
 
 void CGameHandler::stopHeroVisitCastle(const CGTownInstance * obj, const CGHeroInstance * hero)