Fix: Visiting hero should receive town bonuses during town siege

lib/CCreatureHandler.cpp

@@ -1322,14 +1322,20 @@ CreatureID CCreatureHandler::pickRandomMonster(CRandomGenerator & rand, int tier
 	return CreatureID(r);
 }
 
-void CCreatureHandler::addBonusForTier(int tier, std::shared_ptr<Bonus> b)
+void CCreatureHandler::addBonusForTier(int tier, const std::shared_ptr<Bonus> & b)
 {
 	assert(vstd::iswithin(tier, 1, 7));
 	creaturesOfLevel[tier].addNewBonus(b);
 }
 
-void CCreatureHandler::addBonusForAllCreatures(std::shared_ptr<Bonus> b)
+void CCreatureHandler::addBonusForAllCreatures(const std::shared_ptr<Bonus> & b)
 {
+	const auto & exportedBonuses = allCreatures.getExportedBonusList();
+	for(const auto & bonus : exportedBonuses)
+	{
+		if(bonus->type == b->type && bonus->subtype == b->subtype)
+			return;
+	}
 	allCreatures.addNewBonus(b);
 }
 

lib/CCreatureHandler.h

@@ -278,8 +278,8 @@ public:
 
 	void deserializationFix();
 	CreatureID pickRandomMonster(CRandomGenerator & rand, int tier = -1) const; //tier <1 - CREATURES_PER_TOWN> or -1 for any
-	void addBonusForTier(int tier, std::shared_ptr<Bonus> b); //tier must be <1-7>
-	void addBonusForAllCreatures(std::shared_ptr<Bonus> b);
+	void addBonusForTier(int tier, const std::shared_ptr<Bonus> & b); //tier must be <1-7>
+	void addBonusForAllCreatures(const std::shared_ptr<Bonus> & b); //due to CBonusSystem::addNewBonus(const std::shared_ptr<Bonus>& b);
 	void removeBonusesFromAllCreatures();
 	void restoreAllCreaturesNodeType794(); //restore ALL_CREATURES node type for old saves
 

lib/CTownHandler.cpp

@@ -377,7 +377,11 @@ JsonNode readBuilding(CLegacyConfigParser & parser)
 	return ret;
 }
 
-TPropagatorPtr CTownHandler::emptyPropagator = std::make_shared<CPropagatorNodeType>();
+TPropagatorPtr & CTownHandler::emptyPropagator()
+{
+	static TPropagatorPtr emptyProp(nullptr);
+	return emptyProp;
+}
 
 std::vector<JsonNode> CTownHandler::loadLegacyData(size_t dataSize)
 {
@@ -618,7 +622,7 @@ void CTownHandler::addBonusesForVanilaBuilding(CBuilding * building)
 
 std::shared_ptr<Bonus> CTownHandler::createBonus(CBuilding * build, Bonus::BonusType type, int val, int subtype)
 {
-	return createBonus(build, type, val, emptyPropagator, subtype);
+	return createBonus(build, type, val, emptyPropagator(), subtype);
 }
 
 std::shared_ptr<Bonus> CTownHandler::createBonus(CBuilding * build, Bonus::BonusType type, int val, TPropagatorPtr & prop, int subtype)
@@ -657,7 +661,7 @@ void CTownHandler::loadSpecialBuildingBonuses(const JsonNode & source, BonusList
 		//JsonUtils::parseBuildingBonus produces UNKNOWN type propagator instead of empty.
 		if(bonus->propagator != nullptr
 			&& bonus->propagator->getPropagatorType() == CBonusSystemNode::ENodeTypes::UNKNOWN)
-				bonus->addPropagator(emptyPropagator);
+				bonus->addPropagator(emptyPropagator());
 		building->addNewBonus(bonus, bonusList);
 	}
 }

lib/CTownHandler.h

@@ -388,7 +388,7 @@ class DLL_LINKAGE CTownHandler : public CHandlerBase<FactionID, Faction, CFactio
 	const static ETerrainType::EETerrainType defaultEvilTerrain = ETerrainType::EETerrainType::LAVA;
 	const static ETerrainType::EETerrainType defaultNeutralTerrain = ETerrainType::EETerrainType::ROUGH;
 
-	static TPropagatorPtr emptyPropagator;
+	static TPropagatorPtr & emptyPropagator();
 
 	void initializeRequirements();
 	void initializeOverridden();

lib/HeroBonus.cpp

@@ -26,7 +26,6 @@
 #include "battle/BattleInfo.h"
 
 #define FOREACH_PARENT(pname) 	TNodes lparents; getParents(lparents); for(CBonusSystemNode *pname : lparents)
-#define FOREACH_CPARENT(pname) 	TCNodes lparents; getParents(lparents); for(const CBonusSystemNode *pname : lparents)
 #define FOREACH_RED_CHILD(pname) 	TNodes lchildren; getRedChildren(lchildren); for(CBonusSystemNode *pname : lchildren)
 #define FOREACH_RED_PARENT(pname) 	TNodes lparents; getRedParents(lparents); for(CBonusSystemNode *pname : lparents)
 
@@ -873,26 +872,25 @@ void CBonusSystemNode::getParents(TNodes &out)
 	}
 }
 
-void CBonusSystemNode::getBonusesRec(BonusList &out, const CSelector &selector, const CSelector &limit) const
+void CBonusSystemNode::getAllParents(TCNodes & out) const /*retrieves list of parent nodes (nodes to inherit bonuses from) */
 {
-	BonusList beforeUpdate;
-	FOREACH_CPARENT(p)
+	for(auto & cparent : parents)
 	{
-		p->getBonusesRec(beforeUpdate, selector, limit);
+		const CBonusSystemNode * parent = cparent;
+		out.insert(parent);
+		parent->getAllParents(out);
 	}
-	bonuses.getBonuses(beforeUpdate, selector, limit);
-
-	for(auto b : beforeUpdate)
-		out.push_back(update(b));
 }
 
 void CBonusSystemNode::getAllBonusesRec(BonusList &out) const
 {
 	BonusList beforeUpdate;
-	FOREACH_CPARENT(p)
-	{
-		p->getAllBonusesRec(beforeUpdate);
-	}
+	TCNodes lparents;
+	getAllParents(lparents);
+
+	for(auto parent : lparents)
+		parent->bonuses.getAllBonuses(beforeUpdate);
+
 	bonuses.getAllBonuses(beforeUpdate);
 
 	for(auto b : beforeUpdate)

lib/HeroBonus.h

@@ -769,7 +769,6 @@ private:
 	mutable std::map<std::string, TBonusListPtr > cachedRequests;
 	mutable boost::mutex sync;
 
-	void getBonusesRec(BonusList &out, const CSelector &selector, const CSelector &limit) const;
 	void getAllBonusesRec(BonusList &out) const;
 	TConstBonusListPtr getAllBonusesWithoutCaching(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root = nullptr) const;
 	std::shared_ptr<Bonus> update(const std::shared_ptr<Bonus> & b) const;
@@ -793,6 +792,7 @@ public:
 	void getRedAncestors(TNodes &out);
 	void getRedChildren(TNodes &out);
 	void getRedDescendants(TNodes &out);
+	void getAllParents(TCNodes & out) const;
 	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector &selector);
 
 	void attachTo(CBonusSystemNode *parent);

lib/battle/BattleInfo.cpp

@@ -186,38 +186,70 @@ struct RangeGenerator
 	std::function<int()> myRand;
 };
 
-void BattleInfo::addOnlyEnemyArmyBonuses(const BonusList & bonusList, BattleInfo * curB, BattleInfo::BattleSide side)
+
+std::shared_ptr<Bonus> BattleInfo::makeBonusForEnemy(const std::shared_ptr<Bonus> & b, PlayerColor & enemyColor)
+{
+	auto bCopy = std::make_shared<Bonus>(*b);
+	bCopy->effectRange = Bonus::NO_LIMIT;
+	bCopy->propagator.reset();
+	bCopy->limiter.reset(new StackOwnerLimiter(enemyColor));
+	return bCopy;
+}
+
+void BattleInfo::addOnlyEnemyArmyBonuses(const BonusList & bonusList, BattleInfo * curB, BattleInfo::BattleSide enemySide)
 {
-	for(auto b : bonusList)
+	for(const auto & b : bonusList) //don't copy shared_ptr object, don't change it
 	{
 		if(b->effectRange != Bonus::ONLY_ENEMY_ARMY)
 			continue;
 
-		auto bCopy = std::make_shared<Bonus>(*b);
-		bCopy->effectRange = Bonus::NO_LIMIT;
-		bCopy->propagator.reset();
-		bCopy->limiter.reset(new StackOwnerLimiter(curB->sides[side].color));
+		auto bCopy = makeBonusForEnemy(b, curB->sides[enemySide].color);
 		curB->addNewBonus(bCopy);
 	}
 }
 
-void BattleInfo::setupBonusesFromTown(const CGTownInstance * town, BattleInfo * curB)
+void BattleInfo::setupBonusesFromTownToEnemy(const CGTownInstance * town, BattleInfo * curB)
 {
 	assert(town);
-	for(auto building : town->builtBuildings)
+	for(const auto & building : town->builtBuildings)
 	{
 		const auto & bonuses = town->town->buildings.at(building)->buildingBonuses;
 		addOnlyEnemyArmyBonuses(bonuses, curB, BattleInfo::ATTACKER);
 	}
 }
 
-void BattleInfo::setupBonusesFromUnits(BattleInfo * curB)
+void BattleInfo::setupBonusesFromTownToBothSides(const CGTownInstance * town, BattleInfo * curB)
+{
+	assert(town);
+	for(const auto & building : town->builtBuildings)
+	{
+		const auto & bonuses = town->town->buildings.at(building)->buildingBonuses;
+
+		for(const auto & b : bonuses)
+		{
+			if(b->effectRange == Bonus::ONLY_ENEMY_ARMY)
+			{
+				auto bCopy = makeBonusForEnemy(b, curB->sides[BattleInfo::ATTACKER].color);
+				curB->addNewBonus(bCopy);
+			}
+			else
+			{
+				auto bCopy = std::make_shared<Bonus>(*b);
+				bCopy->propagator.reset();
+				bCopy->limiter.reset(new StackOwnerLimiter(curB->sides[BattleInfo::DEFENDER].color));
+				curB->addNewBonus(bCopy);
+			}
+		}
+	}
+}
+
+void BattleInfo::setupBonusesFromUnitsToEnemy(BattleInfo * curB)
 {
 	for(int i = 0; i < 2; i++)
 	{
 		TNodes nodes;
 		curB->battleGetArmyObject(i)->getRedAncestors(nodes);
-		for(CBonusSystemNode * n : nodes)
+		for(auto n : nodes)
 		{
 			const auto & bonuses = n->getExportedBonusList();
 			addOnlyEnemyArmyBonuses(bonuses, curB, (BattleInfo::BattleSide)!i);
@@ -595,8 +627,13 @@ BattleInfo * BattleInfo::setupBattle(int3 tile, ETerrainType terrain, BFieldType
 		curB->tacticDistance = 0;
 
 	if(town)
-		setupBonusesFromTown(town, curB);
-	setupBonusesFromUnits(curB);
+	{
+		if(town->visitingHero == heroes[BattleSide::DEFENDER])
+			setupBonusesFromTownToBothSides(town, curB);
+		else
+			setupBonusesFromTownToEnemy(town, curB);
+	}
+	setupBonusesFromUnitsToEnemy(curB);
 	return curB;
 }
 

lib/battle/BattleInfo.h

@@ -138,8 +138,9 @@ public:
 
 	void localInit();
 
-	static void setupBonusesFromTown(const CGTownInstance * town, BattleInfo * curB); //besieged city may contain buildings with negative bonuses for enemy army.
-	static void setupBonusesFromUnits(BattleInfo * curB); //besieged city may contain buildings with negative bonuses for enemy army.
+	static void setupBonusesFromTownToEnemy(const CGTownInstance * town, BattleInfo * curB); //besieged city may contain buildings with negative bonuses for enemy army
+	static void setupBonusesFromTownToBothSides(const CGTownInstance * town, BattleInfo * curB); //if visiting hero is town defender during siege, he/she should receive bonuses
+	static void setupBonusesFromUnitsToEnemy(BattleInfo * curB);
 	static BattleInfo * setupBattle(int3 tile, ETerrainType terrain, BFieldType battlefieldType, const CArmedInstance * armies[2], const CGHeroInstance * heroes[2], bool creatureBank, const CGTownInstance * town);
 
 	ui8 whatSide(PlayerColor player) const;
@@ -150,7 +151,8 @@ protected:
 	scripting::Pool * getContextPool() const override;
 
 private:
-	static void addOnlyEnemyArmyBonuses(const BonusList & bonusList, BattleInfo * curB, BattleInfo::BattleSide side);
+	static void addOnlyEnemyArmyBonuses(const BonusList & bonusList, BattleInfo * curB, BattleInfo::BattleSide enemySide);
+	STRONG_INLINE static std::shared_ptr<Bonus> makeBonusForEnemy(const std::shared_ptr<Bonus> & b, PlayerColor & enemyColor);
 };