Feature: Town Negative Bonuses. Fix: Negative bonus bearers should not be affected

client/widgets/MiscWidgets.cpp

@@ -404,18 +404,22 @@ void MoraleLuckBox::set(const IBonusBearer * node)
 		text += "\n" + noLuck->Description();
 		bonusValue = 0;
 	}
-	else if(modifierList->empty())
-		text += CGI->generaltexth->arraytxt[noneTxtId];//no modifiers
 	else
 	{
-		for(auto& elem : *modifierList)
+		bool isListActual = false;
+		std::string addInfo = "";
+
+		for(auto & bonus : * modifierList)
 		{
-			if(elem->val != 0)
-				//no bonuses with value 0
-				text += "\n" + elem->Description();
+			if(bonus->val && bonus->effectRange != Bonus::ONLY_ENEMY_ARMY)
+			{
+				addInfo += "\n" + bonus->Description();
+				isListActual = true;
+			}
 		}
+		text = isListActual ? text + addInfo
+			: text + CGI->generaltexth->arraytxt[noneTxtId];//no modifiers
 	}
-
 	std::string imageName;
 	if (small)
 		imageName = morale ? "IMRL30": "ILCK30";

lib/HeroBonus.cpp

@@ -428,6 +428,9 @@ int BonusList::totalValue() const
 
 	for(std::shared_ptr<Bonus> b : bonuses)
 	{
+		if(b->effectRange == Bonus::ONLY_ENEMY_ARMY)
+			continue;
+
 		switch(b->valType)
 		{
 		case Bonus::BASE_NUMBER:
@@ -452,7 +455,6 @@ int BonusList::totalValue() const
 			{
 				vstd::amax(indepMax, b->val);
 			}
-
 			break;
 		case Bonus::INDEPENDENT_MIN:
 			if (!hasIndepMin)
@@ -464,7 +466,6 @@ int BonusList::totalValue() const
 			{
 				vstd::amin(indepMin, b->val);
 			}
-
 			break;
 		}
 	}
@@ -595,11 +596,14 @@ void BonusList::insert(BonusList::TInternalContainer::iterator position, BonusLi
 	changed();
 }
 
-CSelector IBonusBearer::anaffectedByMoraleSelector
-	= Selector::type()(Bonus::NON_LIVING)
-		.Or(Selector::type()(Bonus::UNDEAD))
-		.Or(Selector::type()(Bonus::NO_MORALE))
-		.Or(Selector::type()(Bonus::SIEGE_WEAPON));
+CSelector IBonusBearer::anaffectedByMoraleSelector =
+Selector::type()(Bonus::NON_LIVING)
+.Or(Selector::type()(Bonus::UNDEAD))
+.Or(Selector::type()(Bonus::SIEGE_WEAPON))
+.Or(Selector::effectRange()(Bonus::ONLY_ENEMY_ARMY).Not()
+	.And(Selector::type()(Bonus::NO_MORALE)
+		.Or(Selector::type()(Bonus::BLOCK_MORALE))
+	));
 
 CSelector IBonusBearer::moraleSelector = Selector::type()(Bonus::MORALE);
 CSelector IBonusBearer::luckSelector = Selector::type()(Bonus::LUCK);

lib/HeroBonus.h

@@ -56,6 +56,11 @@ public:
 		auto thisCopy = *this;
 		return [thisCopy, rhs](const Bonus *b) mutable { return thisCopy(b) || rhs(b); };
 	}
+	CSelector Not() const
+	{
+		auto thisCopy = *this;
+		return [thisCopy](const Bonus *b) mutable { return !thisCopy(b); };
+	}
 
 	bool operator()(const Bonus *b) const
 	{

lib/battle/BattleInfo.cpp

@@ -186,6 +186,45 @@ struct RangeGenerator
 	std::function<int()> myRand;
 };
 
+void BattleInfo::addOnlyEnemyArmyBonuses(const BonusList & bonusList, BattleInfo * curB, BattleInfo::BattleSide side)
+{
+	for(auto b : bonusList)
+	{
+		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));
+		curB->addNewBonus(bCopy);
+	}
+}
+
+void BattleInfo::setupBonusesFromTown(const CGTownInstance * town, BattleInfo * curB)
+{
+	assert(town);
+	for(auto building : town->builtBuildings)
+	{
+		const auto & bonuses = town->town->buildings.at(building)->buildingBonuses;
+		addOnlyEnemyArmyBonuses(bonuses, curB, BattleInfo::ATTACKER);
+	}
+}
+
+void BattleInfo::setupBonusesFromUnits(BattleInfo * curB)
+{
+	for(int i = 0; i < 2; i++)
+	{
+		TNodes nodes;
+		curB->battleGetArmyObject(i)->getRedAncestors(nodes);
+		for(CBonusSystemNode * n : nodes)
+		{
+			const auto & bonuses = n->getExportedBonusList();
+			addOnlyEnemyArmyBonuses(bonuses, curB, (BattleInfo::BattleSide)!i);
+		}
+	}
+}
+
 BattleInfo * BattleInfo::setupBattle(int3 tile, ETerrainType terrain, BFieldType battlefieldType, const CArmedInstance * armies[2], const CGHeroInstance * heroes[2], bool creatureBank, const CGTownInstance * town)
 {
 	CMP_stack cmpst;
@@ -528,8 +567,8 @@ BattleInfo * BattleInfo::setupBattle(int3 tile, ETerrainType terrain, BFieldType
 	//overlay premies given
 
 	//native terrain bonuses
-	auto nativeTerrain = std::make_shared<CreatureTerrainLimiter>();
-
+	static auto nativeTerrain = std::make_shared<CreatureTerrainLimiter>();
+	
 	curB->addNewBonus(std::make_shared<Bonus>(Bonus::ONE_BATTLE, Bonus::STACKS_SPEED, Bonus::TERRAIN_NATIVE, 1, 0, 0)->addLimiter(nativeTerrain));
 	curB->addNewBonus(std::make_shared<Bonus>(Bonus::ONE_BATTLE, Bonus::PRIMARY_SKILL, Bonus::TERRAIN_NATIVE, 1, 0, PrimarySkill::ATTACK)->addLimiter(nativeTerrain));
 	curB->addNewBonus(std::make_shared<Bonus>(Bonus::ONE_BATTLE, Bonus::PRIMARY_SKILL, Bonus::TERRAIN_NATIVE, 1, 0, PrimarySkill::DEFENSE)->addLimiter(nativeTerrain));
@@ -555,31 +594,13 @@ BattleInfo * BattleInfo::setupBattle(int3 tile, ETerrainType terrain, BFieldType
 	else
 		curB->tacticDistance = 0;
 
-
-	// workaround — bonuses affecting only enemy - DOES NOT WORK
-	for(int i = 0; i < 2; i++)
-	{
-		TNodes nodes;
-		curB->battleGetArmyObject(i)->getRedAncestors(nodes);
-		for(CBonusSystemNode *n : nodes)
-		{
-			for(auto b : n->getExportedBonusList())
-			{
-				if(b->effectRange == Bonus::ONLY_ENEMY_ARMY/* && b->propagator && b->propagator->shouldBeAttached(curB)*/)
-				{
-					auto bCopy = std::make_shared<Bonus>(*b);
-					bCopy->effectRange = Bonus::NO_LIMIT;
-					bCopy->propagator.reset();
-					bCopy->limiter.reset(new StackOwnerLimiter(curB->sides[!i].color));
-					curB->addNewBonus(bCopy);
-				}
-			}
-		}
-	}
-
+	if(town)
+		setupBonusesFromTown(town, curB);
+	setupBonusesFromUnits(curB);
 	return curB;
 }
 
+
 const CGHeroInstance * BattleInfo::getHero(PlayerColor player) const
 {
 	for(int i = 0; i < sides.size(); i++)

lib/battle/BattleInfo.h

@@ -22,6 +22,11 @@ class CStackBasicDescriptor;
 class DLL_LINKAGE BattleInfo : public CBonusSystemNode, public CBattleInfoCallback, public IBattleState
 {
 public:
+	enum BattleSide
+	{
+		ATTACKER = 0,
+		DEFENDER
+	};
 	std::array<SideInBattle, 2> sides; //sides[0] - attacker, sides[1] - defender
 	si32 round, activeStack;
 	const CGTownInstance * town; //used during town siege, nullptr if this is not a siege (note that fortless town IS also a siege)
@@ -133,6 +138,8 @@ 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 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;
@@ -141,6 +148,9 @@ public:
 
 protected:
 	scripting::Pool * getContextPool() const override;
+
+private:
+	static void addOnlyEnemyArmyBonuses(const BonusList & bonusList, BattleInfo * curB, BattleInfo::BattleSide side);
 };
 
 

lib/mapObjects/CGTownInstance.cpp

@@ -1265,6 +1265,8 @@ void CGTownInstance::recreateBuildingsBonuses()
 
 		for(auto bonus : building->buildingBonuses)
 		{
+			if(bonus->effectRange == Bonus::ONLY_ENEMY_ARMY) //will be added in the 'setupBattle' to Battle node.
+				continue;
 			if(bonus->propagator != nullptr && bonus->propagator->getPropagatorType() == ALL_CREATURES)
 				VLC->creh->addBonusForAllCreatures(bonus);
 			else