Feature: Opposite Side Limiter

client/CPlayerInterface.cpp

@@ -1491,24 +1491,23 @@ void CPlayerInterface::objectPropertyChanged(const SetObjectProperty * sop)
 	{
 		const CGObjectInstance * obj = cb->getObj(sop->id);
 		std::set<int3> pos = obj->getBlockedPos();
-		for (auto & po : pos)
+
+		for(auto & po : pos)
 		{
-			if (cb->isVisible(po))
+			if(cb->isVisible(po))
 				adventureInt->minimap.showTile(po);
 		}
-
-		if (obj->ID == Obj::TOWN)
+		if(obj->ID == Obj::TOWN)
 		{
-			if (obj->tempOwner == playerID)
+			if(obj->tempOwner == playerID)
 				towns.push_back(static_cast<const CGTownInstance *>(obj));
 			else
 				towns -= obj;
+
 			adventureInt->townList.update();
 		}
-
 		assert(cb->getTownsInfo().size() == towns.size());
 	}
-
 }
 
 void CPlayerInterface::initializeHeroTownList()

client/Client.cpp

@@ -558,7 +558,9 @@ int CClient::sendRequest(const CPackForServer * request, PlayerColor player)
 
 void CClient::battleStarted(const BattleInfo * info)
 {
+	BattleInfo::adjustOppositeBonuses(const_cast<BattleInfo *>(info));
 	setBattle(info);
+
 	for(auto & battleCb : battleCallbacks)
 	{
 		if(vstd::contains_if(info->sides, [&](const SideInBattle& side) {return side.color == battleCb.first; })

client/Client.h

@@ -202,7 +202,7 @@ public:
 	bool moveStack(const StackLocation & src, const StackLocation & dst, TQuantity count = -1) override {return false;}
 
 	void removeAfterVisit(const CGObjectInstance * object) override {};
-
+	bool garrisonSwapOnSiege(ObjectInstanceID tid) override {return false;};
 	void giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos) override {};
 	void giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) override {};
 	void putArtifact(const ArtifactLocation & al, const CArtifactInstance * a) override {};

client/NetPacksClient.cpp

@@ -526,7 +526,7 @@ void InfoWindow::applyCl(CClient *cl)
 		logNetwork->warn("We received InfoWindow for not our player...");
 }
 
-void SetObjectProperty::applyCl(CClient *cl)
+void SetObjectProperty::applyCl(CClient * cl)
 {
 	//inform all players that see this object
 	for(auto it = cl->playerint.cbegin(); it != cl->playerint.cend(); ++it)

client/widgets/MiscWidgets.cpp

@@ -411,7 +411,7 @@ void MoraleLuckBox::set(const IBonusBearer * node)
 
 		for(auto & bonus : * modifierList)
 		{
-			if(bonus->val && bonus->effectRange != Bonus::ONLY_ENEMY_ARMY)
+			if(bonus->val)
 			{
 				addInfo += "\n" + bonus->Description();
 				isListActual = true;

config/creatures/necropolis.json

@@ -339,10 +339,13 @@
 			{
 				"type" : "DRAGON_NATURE"
 			},
-			"const_lowers_morale" :
+			"descreaseMorale" :
 			{
+				"type" : "MORALE",
+				"effectRange": "ONLY_ENEMY_ARMY",
+				"val" : -1,
 				"stacking" : "Undead Dragons"
-			}
+			},
 		},
 		"upgrades": ["ghostDragon"],
 		"graphics" :
@@ -369,8 +372,11 @@
 			{
 				"type" : "DRAGON_NATURE"
 			},
-			"const_lowers_morale" :
+			"descreaseMorale" :
 			{
+				"type" : "MORALE",
+				"effectRange": "ONLY_ENEMY_ARMY",
+				"val" : -1,
 				"stacking" : "Undead Dragons"
 			},
 			"age" :

lib/CCreatureHandler.cpp

@@ -497,13 +497,6 @@ void CCreatureHandler::loadBonuses(JsonNode & creature, std::string bonuses)
 		node["propagator"].String() = "HERO";
 		creature["abilities"]["const_raises_morale"] = node;
 	}
-	if(hasAbility("const_lowers_morale"))
-	{
-		JsonNode node = makeBonusNode("MORALE");
-		node["val"].Float() = -1;
-		node["effectRange"].String() = "ONLY_ENEMY_ARMY";
-		creature["abilities"]["const_lowers_morale"] = node;
-	}
 }
 
 std::vector<JsonNode> CCreatureHandler::loadLegacyData(size_t dataSize)
@@ -870,6 +863,10 @@ void CCreatureHandler::loadCreatureJson(CCreature * creature, const JsonNode & c
 			if (!ability.second.isNull())
 			{
 				auto b = JsonUtils::parseBonus(ability.second);
+
+				if(b->effectRange == Bonus::ONLY_ENEMY_ARMY) //Opposite Side bonuses should not be exported from CREATURE node.
+					b->propagator.reset();
+
 				b->source = Bonus::CREATURE_ABILITY;
 				b->sid = creature->getIndex();
 				b->duration = Bonus::PERMANENT;

lib/CCreatureSet.cpp

@@ -630,10 +630,38 @@ void CStackInstance::setType(CreatureID creID)
 		setType((const CCreature*)nullptr);
 }
 
+
+void CStackInstance::copyOppositeBonusesFromCreature(const CCreature * creature)
+{
+	auto owner = _armyObj ? _armyObj->getOwner() : PlayerColor::NEUTRAL;
+
+	if(owner == PlayerColor::UNFLAGGABLE)
+		owner = PlayerColor::NEUTRAL;
+
+	auto & creatureBonuses = const_cast<CCreature *>(type)->getExportedBonusList();
+
+	for(auto & creatureBonus : creatureBonuses)
+	{
+		if(creatureBonus->effectRange == Bonus::ONLY_ENEMY_ARMY) //it has better prformance than dynamic_cast
+		{
+			auto bCopy = std::make_shared<Bonus>(*creatureBonus);
+			bCopy->propagator.reset(new CPropagatorNodeType(CBonusSystemNode::BATTLE));
+			bCopy->limiter.reset(new OppositeSideLimiter(owner));
+			this->addNewBonus(bCopy);
+		}
+	}
+}
+
+void CStackInstance::removeOppositeBonuses()
+{
+	removeBonuses(Selector::effectRange()(Bonus::ONLY_ENEMY_ARMY));
+}
+
 void CStackInstance::setType(const CCreature *c)
 {
 	if(type)
 	{
+		removeOppositeBonuses();
 		detachFrom(const_cast<CCreature*>(type));
 		if (type->isMyUpgrade(c) && VLC->modh->modules.STACK_EXP)
 			experience = static_cast<TExpType>(experience * VLC->creh->expAfterUpgrade / 100.0);
@@ -642,7 +670,11 @@ void CStackInstance::setType(const CCreature *c)
 	CStackBasicDescriptor::setType(c);
 
 	if(type)
-		attachTo(const_cast<CCreature*>(type));
+	{
+		auto creature = const_cast<CCreature*>(type);
+		copyOppositeBonusesFromCreature(creature);
+		attachTo(creature);
+	}
 }
 std::string CStackInstance::bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const
 {
@@ -662,7 +694,7 @@ std::string CStackInstance::bonusToGraphics(const std::shared_ptr<Bonus>& bonus)
 	return VLC->getBth()->bonusToGraphics(bonus);
 }
 
-void CStackInstance::setArmyObj(const CArmedInstance *ArmyObj)
+void CStackInstance::setArmyObj(const CArmedInstance * ArmyObj)
 {
 	if(_armyObj)
 		detachFrom(const_cast<CArmedInstance*>(_armyObj));
@@ -670,6 +702,24 @@ void CStackInstance::setArmyObj(const CArmedInstance *ArmyObj)
 	_armyObj = ArmyObj;
 	if(ArmyObj)
 	{
+		auto owner = _armyObj->getOwner();
+
+		if(owner == PlayerColor::UNFLAGGABLE)
+			owner = PlayerColor::NEUTRAL;
+
+		auto & bonusList = getExportedBonusList();
+
+		for(auto & bonus : bonusList)
+		{
+			if(bonus->effectRange != Bonus::ONLY_ENEMY_ARMY)
+				continue;
+
+			auto limPtr = bonus->limiter.get();
+			if(limPtr)
+				static_cast<OppositeSideLimiter *>(limPtr)->owner = owner;
+			else
+				logGlobal->error("setArmyObj. Limiter has been lost. Creature is %s", type ? type->nodeShortInfo() : std::string("No creature"));
+		}
 		attachTo(const_cast<CArmedInstance*>(_armyObj));
 	}
 }

lib/CCreatureSet.h

@@ -99,6 +99,10 @@ public:
 	ArtBearer::ArtBearer bearerType() const override; //from CArtifactSet
 	virtual std::string nodeName() const override; //from CBonusSystemnode
 	void deserializationFix();
+
+private:
+	void copyOppositeBonusesFromCreature(const CCreature * creature);
+	STRONG_INLINE void removeOppositeBonuses();
 };
 
 class DLL_LINKAGE CCommanderInstance : public CStackInstance

lib/HeroBonus.cpp

@@ -27,7 +27,6 @@
 
 #define FOREACH_PARENT(pname) 	TNodes lparents; getParents(lparents); for(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)
 
 #define BONUS_NAME(x) { #x, Bonus::x },
 	const std::map<std::string, Bonus::BonusType> bonusNameMap = { BONUS_LIST };
@@ -72,7 +71,8 @@ const std::map<std::string, TLimiterPtr> bonusLimiterMap =
 	{"DRAGON_NATURE", std::make_shared<HasAnotherBonusLimiter>(Bonus::DRAGON_NATURE)},
 	{"IS_UNDEAD", std::make_shared<HasAnotherBonusLimiter>(Bonus::UNDEAD)},
 	{"CREATURE_NATIVE_TERRAIN", std::make_shared<CreatureTerrainLimiter>()},
-	{"CREATURE_FACTION", std::make_shared<CreatureFactionLimiter>()}
+	{"CREATURE_FACTION", std::make_shared<CreatureFactionLimiter>()},
+	{"OPPOSITE_SIDE", std::make_shared<OppositeSideLimiter>()}
 };
 
 const std::map<std::string, TPropagatorPtr> bonusPropagatorMap =
@@ -427,9 +427,6 @@ 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:
@@ -528,7 +525,8 @@ void BonusList::getBonuses(BonusList & out, const CSelector &selector, const CSe
 	for (auto & b : bonuses)
 	{
 		//add matching bonuses that matches limit predicate or have NO_LIMIT if no given predicate
-		if(selector(b.get()) && ((!limit && b->effectRange == Bonus::NO_LIMIT) || ((bool)limit && limit(b.get()))))
+		auto noFightLimit = b->effectRange == Bonus::NO_LIMIT || b->effectRange == Bonus::ONLY_ENEMY_ARMY;
+		if(selector(b.get()) && ((!limit && noFightLimit) || ((bool)limit && limit(b.get()))))
 			out.push_back(b);
 	}
 }
@@ -599,10 +597,8 @@ 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))
-	));
+.Or(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);
@@ -792,6 +788,8 @@ int IBonusBearer::getPrimSkillLevel(PrimarySkill::PrimarySkill id) const
 	static const std::string keyAllSkills = "type_PRIMARY_SKILL";
 	auto allSkills = getBonuses(selectorAllSkills, keyAllSkills);
 	auto ret = allSkills->valOfBonuses(Selector::subtype()(id));
+	auto minSkillValue = (id == PrimarySkill::SPELL_POWER || id == PrimarySkill::KNOWLEDGE) ? 1 : 0;
+	vstd::amax(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect
 	return ret; //sp=0 works in old saves
 }
 
@@ -833,7 +831,47 @@ std::shared_ptr<const Bonus> IBonusBearer::getBonus(const CSelector &selector) c
 	return bonuses->getFirst(Selector::all);
 }
 
-std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector &selector)
+const CStack * retrieveStackBattle(const CBonusSystemNode * node)
+{
+	switch (node->getNodeType())
+	{
+	case CBonusSystemNode::STACK_BATTLE:
+		return static_cast<const CStack*>(node);
+	default:
+		return nullptr;
+	}
+}
+
+const CStackInstance * retrieveStackInstance(const CBonusSystemNode * node)
+{
+	switch (node->getNodeType())
+	{
+	case CBonusSystemNode::STACK_INSTANCE:
+		return (static_cast<const CStackInstance *>(node));
+	case CBonusSystemNode::STACK_BATTLE:
+		return (static_cast<const CStack*>(node))->base;
+	default:
+		return nullptr;
+	}
+}
+
+PlayerColor CBonusSystemNode::retrieveNodeOwner(const CBonusSystemNode * node)
+{
+	if (!node)
+		return PlayerColor::CANNOT_DETERMINE;
+
+	const CStack * stack = retrieveStackBattle(node);
+	if (stack)
+		return stack->owner;
+
+	const CStackInstance * csi = retrieveStackInstance(node);
+	if (csi && csi->armyObj)
+		return csi->armyObj->getOwner();
+
+	return PlayerColor::NEUTRAL;
+}
+
+std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & selector)
 {
 	auto ret = bonuses.getFirst(selector);
 	if(ret)
@@ -849,7 +887,7 @@ std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector &sel
 	return nullptr;
 }
 
-std::shared_ptr<const Bonus> CBonusSystemNode::getBonusLocalFirst( const CSelector &selector ) const
+std::shared_ptr<const Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & selector) const
 {
 	return (const_cast<CBonusSystemNode*>(this))->getBonusLocalFirst(selector);
 }
@@ -872,11 +910,10 @@ void CBonusSystemNode::getParents(TNodes &out)
 	}
 }
 
-void CBonusSystemNode::getAllParents(TCNodes & out) const /*retrieves list of parent nodes (nodes to inherit bonuses from) */
+void CBonusSystemNode::getAllParents(TCNodes & out) const //retrieves list of parent nodes (nodes to inherit bonuses from)
 {
-	for(auto & cparent : parents)
+	for(auto parent : parents)
 	{
-		const CBonusSystemNode * parent = cparent;
 		out.insert(parent);
 		parent->getAllParents(out);
 	}
@@ -1091,13 +1128,20 @@ void CBonusSystemNode::detachFrom(CBonusSystemNode *parent)
 			removedRedDescendant(parent);
 	}
 
-	parents -= parent;
+	if (vstd::contains(parents, parent))
+	{
+		parents -= parent;
+	}
+	else
+	{
+		logBonus->error("Error on Detach. Node %s (nodeType=%d) has not parent %s (nodeType=%d)"
+			, nodeShortInfo(), nodeType, parent->nodeShortInfo(), parent->nodeType);
+	}
 
 	if(!isHypothetic())
 	{
 		parent->childDetached(this);
 	}
-
 	CBonusSystemNode::treeHasChanged();
 }
 
@@ -1171,6 +1215,7 @@ bool CBonusSystemNode::actsAsBonusSourceOnly() const
 	case CREATURE:
 	case ARTIFACT:
 	case ARTIFACT_INSTANCE:
+	case TOWN:
 		return true;
 	default:
 		return false;
@@ -1209,12 +1254,12 @@ void CBonusSystemNode::newChildAttached(CBonusSystemNode *child)
 
 void CBonusSystemNode::childDetached(CBonusSystemNode *child)
 {
-	if (vstd::contains(children, child))
+	if(vstd::contains(children, child))
 		children -= child;
 	else
 	{
-		logBonus->error("Error! %s #cannot be detached from# %s", child->nodeName(), nodeName());
-		throw std::runtime_error("internal error");
+		logBonus->error("Error on Detach. Node %s (nodeType=%d) is not a child of %s (nodeType=%d)"
+			, child->nodeShortInfo(), child->nodeType, nodeShortInfo(), nodeType);
 	}
 }
 
@@ -1236,13 +1281,23 @@ std::string CBonusSystemNode::nodeName() const
 		: std::string("Bonus system node of type ") + typeid(*this).name();
 }
 
+std::string CBonusSystemNode::nodeShortInfo() const
+{
+	std::ostringstream str;
+	str << "'" << typeid(* this).name() << "'";
+	description.length() > 0 
+		? str << " (" << description << ")"
+		: str << " (no description)";
+	return str.str();
+}
+
 void CBonusSystemNode::deserializationFix()
 {
 	exportBonuses();
 
 }
 
-void CBonusSystemNode::getRedParents(TNodes &out)
+void CBonusSystemNode::getRedParents(TNodes & out)
 {
 	FOREACH_PARENT(pname)
 	{
@@ -1280,14 +1335,37 @@ void CBonusSystemNode::getRedChildren(TNodes &out)
 	}
 }
 
-void CBonusSystemNode::newRedDescendant(CBonusSystemNode *descendant)
+void CBonusSystemNode::newRedDescendant(CBonusSystemNode * descendant)
 {
 	for(auto b : exportedBonuses)
+	{
 		if(b->propagator)
 			descendant->propagateBonus(b);
+	}
+	TNodes redParents;
+	getRedAncestors(redParents); //get all red parents recursively
 
-	FOREACH_RED_PARENT(parent)
-		parent->newRedDescendant(descendant);
+	//it's not in the 'getRedParents' due to infinite recursion loop.
+	//it's actual only for battle, otherwise, when garrison hero is linking to the town, town's 'children' is already empty
+	if(descendant->nodeType == BATTLE && nodeType == TOWN) //acts as a pure bonus source, but has bonus bearers.
+	{
+		for(auto child : children)
+		{
+			for(auto b : child->exportedBonuses)
+			{
+				if(b->propagator && b->propagator->getPropagatorType() == BATTLE)
+					descendant->propagateBonus(b);
+			}
+		}
+	}
+	for(auto parent : redParents)
+	{
+		for(auto b : parent->exportedBonuses)
+		{
+			if(b->propagator)
+				descendant->propagateBonus(b);
+		}
+	}
 }
 
 void CBonusSystemNode::removedRedDescendant(CBonusSystemNode *descendant)
@@ -1296,15 +1374,26 @@ void CBonusSystemNode::removedRedDescendant(CBonusSystemNode *descendant)
 		if(b->propagator)
 			descendant->unpropagateBonus(b);
 
-	FOREACH_RED_PARENT(parent)
-		parent->removedRedDescendant(descendant);
+	TNodes redParents;
+	getRedAncestors(redParents); //get all red parents recursively
+
+	for (auto parent : redParents)
+	{
+		for (auto b : parent->exportedBonuses)
+			if (b->propagator)
+				descendant->unpropagateBonus(b);
+	}
 }
 
 void CBonusSystemNode::getRedAncestors(TNodes &out)
 {
 	getRedParents(out);
-	FOREACH_RED_PARENT(p)
-		p->getRedAncestors(out);
+
+	TNodes redParents; 
+	getRedParents(redParents);
+	
+	for(CBonusSystemNode * parent : redParents)
+		parent->getRedAncestors(out);
 }
 
 void CBonusSystemNode::getRedDescendants(TNodes &out)
@@ -1738,30 +1827,6 @@ namespace Selector
 	}
 }
 
-const CStack * retrieveStackBattle(const CBonusSystemNode * node)
-{
-	switch(node->getNodeType())
-	{
-	case CBonusSystemNode::STACK_BATTLE:
-		return static_cast<const CStack*>(node);
-	default:
-		return nullptr;
-	}
-}
-
-const CStackInstance * retrieveStackInstance(const CBonusSystemNode * node)
-{
-	switch(node->getNodeType())
-	{
-	case CBonusSystemNode::STACK_INSTANCE:
-		return (static_cast<const CStackInstance *>(node));
-	case CBonusSystemNode::STACK_BATTLE:
-		return (static_cast<const CStack*>(node))->base;
-	default:
-		return nullptr;
-	}
-}
-
 const CCreature * retrieveCreature(const CBonusSystemNode *node)
 {
 	switch(node->getNodeType())
@@ -2163,6 +2228,23 @@ StackOwnerLimiter::StackOwnerLimiter(PlayerColor Owner)
 {
 }
 
+OppositeSideLimiter::OppositeSideLimiter()
+	: owner(PlayerColor::CANNOT_DETERMINE)
+{
+}
+
+OppositeSideLimiter::OppositeSideLimiter(PlayerColor Owner)
+	: owner(Owner)
+{
+}
+
+int OppositeSideLimiter::limit(const BonusLimitationContext & context) const
+{
+	auto contextOwner = CBonusSystemNode::retrieveNodeOwner(& context.node);
+	auto decision = (owner == contextOwner || owner == PlayerColor::CANNOT_DETERMINE) ? ILimiter::DISCARD : ILimiter::ACCEPT;
+	return decision;
+}
+
 // Aggregate/Boolean Limiters
 
 void AggregateLimiter::add(TLimiterPtr limiter)

lib/HeroBonus.h

@@ -56,11 +56,6 @@ 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
 	{
@@ -745,7 +740,7 @@ public:
 	{
 		NONE = -1, 
 		UNKNOWN, STACK_INSTANCE, STACK_BATTLE, SPECIALTY, ARTIFACT, CREATURE, ARTIFACT_INSTANCE, HERO, PLAYER, TEAM,
-		TOWN_AND_VISITOR, BATTLE, COMMANDER, GLOBAL_EFFECTS, ALL_CREATURES
+		TOWN_AND_VISITOR, BATTLE, COMMANDER, GLOBAL_EFFECTS, ALL_CREATURES, TOWN
 	};
 private:
 	BonusList bonuses; //wielded bonuses (local or up-propagated here)
@@ -784,16 +779,18 @@ public:
 	TBonusListPtr limitBonuses(const BonusList &allBonuses) const; //same as above, returns out by val for convienence
 	TConstBonusListPtr getAllBonuses(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root = nullptr, const std::string &cachingStr = "") const override;
 	void getParents(TCNodes &out) const;  //retrieves list of parent nodes (nodes to inherit bonuses from),
-	std::shared_ptr<const Bonus> getBonusLocalFirst(const CSelector &selector) const;
+	std::shared_ptr<const Bonus> getBonusLocalFirst(const CSelector & selector) const;
 
 	//non-const interface
 	void getParents(TNodes &out);  //retrieves list of parent nodes (nodes to inherit bonuses from)
+
 	void getRedParents(TNodes &out);  //retrieves list of red parent nodes (nodes bonuses propagate from)
 	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);
+	static PlayerColor retrieveNodeOwner(const CBonusSystemNode * node);
+	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector & selector);
 
 	void attachTo(CBonusSystemNode *parent);
 	void detachFrom(CBonusSystemNode *parent);
@@ -817,6 +814,7 @@ public:
 	void reduceBonusDurations(const CSelector &s);
 	virtual std::string bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const {return "";}; //description or bonus name
 	virtual std::string nodeName() const;
+	virtual std::string nodeShortInfo() const;
 	bool isHypothetic() const { return isHypotheticNode; }
 
 	void deserializationFix();
@@ -1130,6 +1128,22 @@ public:
 	}
 };
 
+class DLL_LINKAGE OppositeSideLimiter : public ILimiter //applies only to creatures of enemy army during combat
+{
+public:
+	PlayerColor owner;
+	OppositeSideLimiter();
+	OppositeSideLimiter(PlayerColor Owner);
+
+	int limit(const BonusLimitationContext &context) const override;
+
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & static_cast<ILimiter&>(*this);
+		h & owner;
+	}
+};
+
 class DLL_LINKAGE RankRangeLimiter : public ILimiter //applies to creatures with min <= Rank <= max
 {
 public:

lib/IGameCallback.h

@@ -98,6 +98,7 @@ public:
 	virtual void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, int3 tile, bool creatureBank = false)=0; //if any of armies is hero, hero will be used
 	virtual void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, bool creatureBank = false)=0; //if any of armies is hero, hero will be used, visitable tile of second obj is place of battle
 	virtual bool moveHero(ObjectInstanceID hid, int3 dst, ui8 teleporting, bool transit = false, PlayerColor asker = PlayerColor::NEUTRAL)=0;
+	virtual bool garrisonSwapOnSiege(ObjectInstanceID tid)=0;
 	virtual void giveHeroBonus(GiveBonus * bonus)=0;
 	virtual void setMovePoints(SetMovePoints * smp)=0;
 	virtual void setManaPoints(ObjectInstanceID hid, int val)=0;

lib/JsonNode.cpp

@@ -751,6 +751,38 @@ std::shared_ptr<Bonus> JsonUtils::parseBuildingBonus(const JsonNode &ability, Bu
 	return b;
 }
 
+inline void createOppositeLimiter(Bonus * b)
+{
+	if(b->limiter)
+	{
+		if(!dynamic_cast<OppositeSideLimiter *>(b->limiter.get()))
+		{
+			logMod->error("Wrong Limiter will be ignored: The 'ONLY_ENEMY_ARMY' effectRange is only compatible with the 'OPPOSITE_SIDE' limiter.");
+			b->limiter.reset(new OppositeSideLimiter());
+		}
+	}
+	else
+	{
+		b->limiter = std::make_shared<OppositeSideLimiter>();
+	}
+}
+
+inline void createBattlePropagator(Bonus * b)
+{
+	if(b->propagator)
+	{
+		if(b->propagator->getPropagatorType() != CBonusSystemNode::BATTLE)
+		{
+			logMod->error("Wrong Propagator will be ignored: The 'ONLY_ENEMY_ARMY' effectRange is only compatible with the 'BATTLE_WIDE' propagator.");
+			b->propagator.reset(new CPropagatorNodeType(CBonusSystemNode::BATTLE));
+		}
+	}
+	else
+	{
+		b->propagator = std::make_shared<CPropagatorNodeType>(CBonusSystemNode::BATTLE);
+	}
+}
+
 bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 {
 	const JsonNode *value;
@@ -846,7 +878,16 @@ bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 			break;
 		}
 	}
-
+	if(b->effectRange == Bonus::ONLY_ENEMY_ARMY)
+	{
+		createBattlePropagator(b);
+		createOppositeLimiter(b);
+	}
+	else if(b->limiter && dynamic_cast<OppositeSideLimiter *>(b->limiter.get()))
+	{
+		createBattlePropagator(b);
+		b->effectRange = Bonus::ONLY_ENEMY_ARMY;
+	}
 	return true;
 }
 

lib/NetPacksLib.cpp

@@ -392,43 +392,44 @@ DLL_LINKAGE void RemoveObject::applyGs(CGameState *gs)
 	//unblock tiles
 	gs->map->removeBlockVisTiles(obj);
 
-	if(obj->ID==Obj::HERO)
-	{
-		CGHeroInstance *h = static_cast<CGHeroInstance*>(obj);
-		PlayerState *p = gs->getPlayerState(h->tempOwner);
-		gs->map->heroesOnMap -= h;
-		p->heroes -= h;
-		h->detachFrom(h->whereShouldBeAttached(gs));
-		h->tempOwner = PlayerColor::NEUTRAL; //no one owns beaten hero
-		vstd::erase_if(h->artifactsInBackpack, [](const ArtSlotInfo& asi)
+	if(obj->ID == Obj::HERO) //remove beaten hero
+	{
+		CGHeroInstance * beatenHero = static_cast<CGHeroInstance*>(obj);
+		PlayerState * p = gs->getPlayerState(beatenHero->tempOwner);
+		gs->map->heroesOnMap -= beatenHero;
+		p->heroes -= beatenHero;
+		beatenHero->detachFrom(beatenHero->whereShouldBeAttachedOnSiege(gs));
+		beatenHero->tempOwner = PlayerColor::NEUTRAL; //no one owns beaten hero
+		vstd::erase_if(beatenHero->artifactsInBackpack, [](const ArtSlotInfo& asi)
 		{
 			return asi.artifact->artType->id == ArtifactID::GRAIL;
 		});
 
-		if(h->visitedTown)
+		if(beatenHero->visitedTown)
 		{
-			if(h->inTownGarrison)
-				h->visitedTown->garrisonHero = nullptr;
+			if(beatenHero->visitedTown->garrisonHero == beatenHero)
+				beatenHero->visitedTown->garrisonHero = nullptr;
 			else
-				h->visitedTown->visitingHero = nullptr;
-			h->visitedTown = nullptr;
-		}
+				beatenHero->visitedTown->visitingHero = nullptr;
 
+			beatenHero->visitedTown = nullptr;
+			beatenHero->inTownGarrison = false;
+		}
 		//return hero to the pool, so he may reappear in tavern
-		gs->hpool.heroesPool[h->subID] = h;
-		if(!vstd::contains(gs->hpool.pavailable, h->subID))
-			gs->hpool.pavailable[h->subID] = 0xff;
+		gs->hpool.heroesPool[beatenHero->subID] = beatenHero;
+
+		if(!vstd::contains(gs->hpool.pavailable, beatenHero->subID))
+			gs->hpool.pavailable[beatenHero->subID] = 0xff;
 
 		gs->map->objects[id.getNum()] = nullptr;
 
 		//If hero on Boat is removed, the Boat disappears
-		if(h->boat)
+		if(beatenHero->boat)
 		{
-			gs->map->instanceNames.erase(h->boat->instanceName);
-			gs->map->objects[h->boat->id.getNum()].dellNull();
-			h->boat = nullptr;
+			gs->map->instanceNames.erase(beatenHero->boat->instanceName);
+			gs->map->objects[beatenHero->boat->id.getNum()].dellNull();
+			beatenHero->boat = nullptr;
 		}
-
 		return;
 	}
 

lib/battle/BattleInfo.cpp

@@ -186,73 +186,44 @@ struct RangeGenerator
 	std::function<int()> myRand;
 };
 
-
-std::shared_ptr<Bonus> BattleInfo::makeBonusForEnemy(const std::shared_ptr<Bonus> & b, PlayerColor & enemyColor)
+void BattleInfo::adjustOppositeBonuses(CBonusSystemNode * node, PlayerColor ownerColor)
 {
-	auto bCopy = std::make_shared<Bonus>(*b);
-	bCopy->effectRange = Bonus::NO_LIMIT;
-	bCopy->propagator.reset();
-	bCopy->limiter.reset(new StackOwnerLimiter(enemyColor));
-	return bCopy;
-}
+	auto & bonusList = node->getExportedBonusList();
+	if(bonusList.empty())
+		return;
 
-void BattleInfo::addOnlyEnemyArmyBonuses(const BonusList & bonusList, BattleInfo * curB, BattleInfo::BattleSide enemySide)
-{
-	for(const auto & b : bonusList) //don't copy shared_ptr object, don't change it
+	for(const auto & bonus : bonusList)
 	{
-		if(b->effectRange != Bonus::ONLY_ENEMY_ARMY)
+		if(bonus->effectRange != Bonus::ONLY_ENEMY_ARMY)
 			continue;
 
-		auto bCopy = makeBonusForEnemy(b, curB->sides[enemySide].color);
-		curB->addNewBonus(bCopy);
+		auto limPtr = bonus->limiter.get();
+		if(limPtr)
+			static_cast<OppositeSideLimiter *>(limPtr)->owner = ownerColor;
+		else
+			logGlobal->error("adjustOppositeBonuses. Limiter has been lost. Node is %s", node->nodeShortInfo());
 	}
 }
 
-void BattleInfo::setupBonusesFromTownToEnemy(const CGTownInstance * town, BattleInfo * curB)
+void BattleInfo::adjustOppositeBonuses(BattleInfo * curB)
 {
-	assert(town);
-	for(const auto & building : town->builtBuildings)
+	for(int i = 0; i < 2; i++)
 	{
-		const auto & bonuses = town->town->buildings.at(building)->buildingBonuses;
-		addOnlyEnemyArmyBonuses(bonuses, curB, BattleInfo::ATTACKER);
-	}
-}
+		TNodes nodes;
+		auto * army = curB->battleGetArmyObject(i);
+		auto ownerColor = curB->sides[i].color;
 
-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;
+		if(army->getNodeType() == CBonusSystemNode::HERO)
+			adjustOppositeBonuses(army, ownerColor);
+			
+		army->getRedAncestors(nodes);
 
-		for(const auto & b : bonuses)
+		for(auto node : nodes)
 		{
-			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);
-			}
-		}
-	}
-}
+			auto currentNodeType = node->getNodeType();
 
-void BattleInfo::setupBonusesFromUnitsToEnemy(BattleInfo * curB)
-{
-	for(int i = 0; i < 2; i++)
-	{
-		TNodes nodes;
-		curB->battleGetArmyObject(i)->getRedAncestors(nodes);
-		for(auto n : nodes)
-		{
-			const auto & bonuses = n->getExportedBonusList();
-			addOnlyEnemyArmyBonuses(bonuses, curB, (BattleInfo::BattleSide)!i);
+			if(currentNodeType == CBonusSystemNode::ARTIFACT || currentNodeType == CBonusSystemNode::TOWN)
+				adjustOppositeBonuses(node, ownerColor);
 		}
 	}
 }
@@ -626,14 +597,7 @@ BattleInfo * BattleInfo::setupBattle(int3 tile, ETerrainType terrain, BFieldType
 	else
 		curB->tacticDistance = 0;
 
-	if(town)
-	{
-		if(town->visitingHero == heroes[BattleSide::DEFENDER])
-			setupBonusesFromTownToBothSides(town, curB);
-		else
-			setupBonusesFromTownToEnemy(town, curB);
-	}
-	setupBonusesFromUnitsToEnemy(curB);
+	adjustOppositeBonuses(curB);
 	return curB;
 }
 

lib/battle/BattleInfo.h

@@ -137,10 +137,7 @@ public:
 	const CGHeroInstance * getHero(PlayerColor player) const; //returns fighting hero that belongs to given player
 
 	void localInit();
-
-	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 void adjustOppositeBonuses(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;
@@ -151,8 +148,7 @@ protected:
 	scripting::Pool * getContextPool() const override;
 
 private:
-	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);
+	inline static void adjustOppositeBonuses(CBonusSystemNode * node, PlayerColor ownerColor);
 };
 
 

lib/mapObjects/CGHeroInstance.cpp

@@ -1069,11 +1069,29 @@ void CGHeroInstance::deserializationFix()
 	artDeserializationFix(this);
 }
 
-CBonusSystemNode * CGHeroInstance::whereShouldBeAttached(CGameState *gs)
+CBonusSystemNode * CGHeroInstance::whereShouldBeAttachedOnSiege(const bool isBattleOutsideTown) const
+{
+	if(!visitedTown)
+		return nullptr;
+
+	return isBattleOutsideTown ? (CBonusSystemNode *)(& visitedTown->townAndVis)
+		: (CBonusSystemNode *)(visitedTown.get());
+
+}
+
+CBonusSystemNode * CGHeroInstance::whereShouldBeAttachedOnSiege(CGameState * gs)
 {
 	if(visitedTown)
+		return whereShouldBeAttachedOnSiege(visitedTown->isBattleOutsideTown(this));
+
+	return CArmedInstance::whereShouldBeAttached(gs);
+}
+
+CBonusSystemNode * CGHeroInstance::whereShouldBeAttached(CGameState * gs)
+{
+	if (visitedTown)
 	{
-		if(inTownGarrison)
+		if (inTownGarrison)
 			return visitedTown;
 		else
 			return &visitedTown->townAndVis;

lib/mapObjects/CGHeroInstance.h

@@ -246,9 +246,12 @@ public:
 	ArtBearer::ArtBearer bearerType() const override;
 
 	///IBonusBearer
-	CBonusSystemNode *whereShouldBeAttached(CGameState *gs) override;
+	CBonusSystemNode * whereShouldBeAttached(CGameState *gs) override;
 	std::string nodeName() const override;
 
+	CBonusSystemNode * whereShouldBeAttachedOnSiege(const bool isBattleOutsideTown) const;
+	CBonusSystemNode * whereShouldBeAttachedOnSiege(CGameState * gs);
+
 	///spells::Caster
 	int32_t getCasterUnitId() const override;
 	int32_t getSpellSchoolLevel(const spells::Spell * spell, int32_t * outSelectedSchool = nullptr) const override;

lib/mapObjects/CGTownInstance.cpp

@@ -592,7 +592,6 @@ int CGTownInstance::getDwellingBonus(const std::vector<CreatureID>& creatureIds,
 TResources CGTownInstance::dailyIncome() const
 {
 	TResources ret;
-
 	for (auto & p : town->buildings)
 	{
 		BuildingID buildingUpgrade;
@@ -604,28 +603,28 @@ TResources CGTownInstance::dailyIncome() const
 				buildingUpgrade = p2.first;
 			}
 		}
-
 		if (!hasBuilt(buildingUpgrade)&&(hasBuilt(p.first)))
 		{
 			ret += p.second->produce;
 		}
-
 	}
-
 	return ret;
 }
+
 bool CGTownInstance::hasFort() const
 {
 	return hasBuilt(BuildingID::FORT);
 }
+
 bool CGTownInstance::hasCapitol() const
 {
 	return hasBuilt(BuildingID::CAPITOL);
 }
+
 CGTownInstance::CGTownInstance()
 	:CGDwelling(), IShipyard(this), IMarket(this), town(nullptr), builded(0), destroyed(0), identifier(0), alignment(0xff)
 {
-
+	this->setNodeType(CBonusSystemNode::TOWN);
 }
 
 CGTownInstance::~CGTownInstance()
@@ -653,43 +652,43 @@ bool CGTownInstance::needsLastStack() const
 	else return false;
 }
 
+void CGTownInstance::setOwner(const PlayerColor player) const
+{
+	removeCapitols(player);
+	cb->setOwner(this, player);
+}
+
 void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const
 {
-	if( !cb->gameState()->getPlayerRelations( getOwner(), h->getOwner() ))//if this is enemy
+	if(!cb->gameState()->getPlayerRelations( getOwner(), h->getOwner() ))//if this is enemy
 	{
 		if(armedGarrison() || visitingHero)
 		{
-			const CGHeroInstance *defendingHero = nullptr;
-			const CArmedInstance *defendingArmy = this;
-
-			if(visitingHero)
-				defendingHero = visitingHero;
-			else if(garrisonHero)
-				defendingHero = garrisonHero;
+			const CGHeroInstance * defendingHero = visitingHero ? visitingHero : garrisonHero;
+			const CArmedInstance * defendingArmy = defendingHero ? (CArmedInstance *)defendingHero : (CArmedInstance *)this;
+			const bool isBattleOutside = isBattleOutsideTown(defendingHero); //defendingHero && garrisonHero && defendingHero != garrisonHero;
 
-			if(defendingHero)
-				defendingArmy = defendingHero;
+			if(!isBattleOutside && visitingHero && defendingHero == visitingHero)
+			{
+				//we have two approaches to merge armies: mergeGarrisonOnSiege() and used in the CGameHandler::garrisonSwap(ObjectInstanceID tid)
+				auto nodeSiege = defendingHero->whereShouldBeAttachedOnSiege(isBattleOutside);
 
-			bool outsideTown = (defendingHero == visitingHero && garrisonHero);
+				if(nodeSiege == (CBonusSystemNode *)this)
+					cb->garrisonSwapOnSiege(this->id);
 
-			//"borrowing" army from garrison to visiting hero
-			if(!outsideTown && armedGarrison() &&
-				visitingHero && defendingHero == visitingHero)
-			{
-				mergeGarrisonOnSiege();
+				const_cast<CGHeroInstance *>(defendingHero)->inTownGarrison = false; //hack to return visitor from garrison after battle
 			}
-
-			cb->startBattlePrimary(h, defendingArmy, getSightCenter(), h, defendingHero, false, (outsideTown ? nullptr : this));
+			cb->startBattlePrimary(h, defendingArmy, getSightCenter(), h, defendingHero, false, (isBattleOutside ? nullptr : this));
 		}
 		else
 		{
-			cb->setOwner(this, h->tempOwner);
-			if(cb->gameState()->getPlayerStatus(h->getOwner()) == EPlayerStatus::WINNER)
+			auto heroColor = h->getOwner();
+			setOwner(heroColor);
+			if(cb->gameState()->getPlayerStatus(heroColor) == EPlayerStatus::WINNER)
 			{
 				return; //we just won game, we do not need to perform any extra actions
 				//TODO: check how does H3 behave, visiting town on victory can affect campaigns (spells learned, +1 stat building visited)
 			}
-			removeCapitols(h->getOwner());
 			cb->heroVisitCastle(this, h);
 		}
 	}
@@ -1253,7 +1252,8 @@ void CGTownInstance::recreateBuildingsBonuses()
 	for(auto b : bl)
 		removeBonus(b);
 
-	for(auto bid : builtBuildings)
+	auto owner = this->getOwner();
+	for(const auto bid : builtBuildings)
 	{
 		if(vstd::contains(overriddenBuildings, bid)) //tricky! -> checks tavern only if no bratherhood of sword
 			continue;
@@ -1263,10 +1263,15 @@ void CGTownInstance::recreateBuildingsBonuses()
 		if(building->buildingBonuses.empty())
 			continue;
 
-		for(auto bonus : building->buildingBonuses)
+		for(auto & bonus : building->buildingBonuses)
 		{
-			if(bonus->effectRange == Bonus::ONLY_ENEMY_ARMY) //will be added in the 'setupBattle' to Battle node.
+			if(bonus->limiter && bonus->effectRange == Bonus::ONLY_ENEMY_ARMY) //ONLY_ENEMY_ARMY is only mark for OppositeSide limiter to avoid extra dynamic_cast.
+			{
+				auto bCopy = std::make_shared<Bonus>(*bonus); //just a copy of the shared_ptr has been changed and reassigned.
+				bCopy->limiter = std::make_shared<OppositeSideLimiter>(this->getOwner());
+				addNewBonus(bCopy);
 				continue;
+			}
 			if(bonus->propagator != nullptr && bonus->propagator->getPropagatorType() == ALL_CREATURES)
 				VLC->creh->addBonusForAllCreatures(bonus);
 			else
@@ -1277,14 +1282,7 @@ void CGTownInstance::recreateBuildingsBonuses()
 
 void CGTownInstance::setVisitingHero(CGHeroInstance *h)
 {
-	//if (!(!!visitingHero == !h))
-	//{
-	//	logGlobal->warn("Hero visiting town %s is %s ", name, (visitingHero.get() ? visitingHero->name : "NULL"));
-	//	logGlobal->warn("New hero will be %s ", (h ? h->name : "NULL"));
-	//
-	//}
 	assert(!!visitingHero == !h);
-
 	if(h)
 	{
 		PlayerState *p = cb->gameState()->getPlayerState(h->tempOwner);
@@ -1388,7 +1386,7 @@ const CGTownBuilding * CGTownInstance::getBonusingBuilding(BuildingSubID::EBuild
 
 bool CGTownInstance::hasBuiltSomeTradeBuilding() const
 {
-	for (const auto & bid : builtBuildings)
+	for(const auto & bid : builtBuildings)
 	{
 		if(town->buildings.at(bid)->IsTradeBuilding())
 			return true;
@@ -1492,18 +1490,19 @@ void CGTownInstance::addHeroToStructureVisitors(const CGHeroInstance *h, si64 st
 	}
 }
 
-void CGTownInstance::battleFinished(const CGHeroInstance *hero, const BattleResult &result) const
+void CGTownInstance::battleFinished(const CGHeroInstance * hero, const BattleResult & result) const
 {
-	if(result.winner == 0)
+	if(result.winner == BattleSide::ATTACKER)
 	{
+		auto heroColor = hero->getOwner(); //get tempOwner
+
 		clearArmy();
-		removeCapitols(hero->getOwner());
-		cb->setOwner (this, hero->tempOwner); //give control after checkout is done
+		setOwner(heroColor); //give control after checkout is done
 		FoWChange fw;
-		fw.player = hero->tempOwner;
+		fw.player = heroColor;
 		fw.mode = 1;
 		cb->getTilesInRange(fw.tiles, getSightCenter(), getSightRadius(), tempOwner, 1);
-		cb->sendAndApply (&fw);
+		cb->sendAndApply(&fw);
 	}
 }
 

lib/mapObjects/CGTownInstance.h

@@ -274,6 +274,9 @@ public:
 			h & overriddenBuildings;
 		else if(!h.saving)
 			updateTown794();
+
+		if(!h.saving)
+			this->setNodeType(CBonusSystemNode::TOWN);
 	}
 	//////////////////////////////////////////////////////////////////////////
 
@@ -349,11 +352,16 @@ public:
 	void afterAddToMap(CMap * map) override;
 	static void reset();
 
+	STRONG_INLINE bool isBattleOutsideTown(const CGHeroInstance * defendingHero) const
+	{
+		return defendingHero && garrisonHero && defendingHero != garrisonHero;
+	}
 protected:
 	void setPropertyDer(ui8 what, ui32 val) override;
 	void serializeJsonOptions(JsonSerializeFormat & handler) override;
 
 private:
+	void setOwner(const PlayerColor owner) const;
 	int getDwellingBonus(const std::vector<CreatureID>& creatureIds, const std::vector<ConstTransitivePtr<CGDwelling> >& dwellings) const;
 	void updateBonusingBuildings();
 	bool hasBuiltInOldWay(ETownType::ETownType type, BuildingID bid) const;

lib/registerTypes/RegisterTypes.h

@@ -143,6 +143,7 @@ void registerTypesMapObjectTypes(Serializer &s)
 
 	s.template registerType<ILimiter, AnyOfLimiter>();
 	s.template registerType<ILimiter, NoneOfLimiter>();
+	s.template registerType<ILimiter, OppositeSideLimiter>();
 	//new types (other than netpacks) must register here
 	//order of type registration is critical for loading old savegames
 }

server/CGameHandler.cpp

@@ -682,7 +682,7 @@ void CGameHandler::changeSecSkill(const CGHeroInstance * hero, SecondarySkill wh
 	}
 }
 
-void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHeroInstance *hero2)
+void CGameHandler::endBattle(int3 tile, const CGHeroInstance * heroAttacker, const CGHeroInstance * heroDefender)
 {
 	LOG_TRACE(logGlobal);
 
@@ -691,16 +691,16 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHer
 
 	if (battleResult.get()->result == BattleResult::NORMAL) // give 500 exp for defeating hero, unless he escaped
 	{
-		if (hero1)
+		if(heroAttacker)
 			battleResult.data->exp[1] += 500;
-		if (hero2)
+		if(heroDefender)
 			battleResult.data->exp[0] += 500;
 	}
 
-	if (hero1)
-		battleResult.data->exp[0] = hero1->calculateXp(battleResult.data->exp[0]);//scholar skill
-	if (hero2)
-		battleResult.data->exp[1] = hero2->calculateXp(battleResult.data->exp[1]);
+	if(heroAttacker)
+		battleResult.data->exp[0] = heroAttacker->calculateXp(battleResult.data->exp[0]);//scholar skill
+	if(heroDefender)
+		battleResult.data->exp[1] = heroDefender->calculateXp(battleResult.data->exp[1]);
 
 	const CArmedInstance *bEndArmy1 = gs->curB->sides.at(0).armyObject;
 	const CArmedInstance *bEndArmy2 = gs->curB->sides.at(1).armyObject;
@@ -731,9 +731,7 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHer
 	const int queriedPlayers = battleQuery ? (int)boost::count(queries.allQueries(), battleQuery) : 0;
 	finishingBattle = make_unique<FinishingBattleHelper>(battleQuery, queriedPlayers);
 
-
 	CasualtiesAfterBattle cab1(bEndArmy1, gs->curB), cab2(bEndArmy2, gs->curB); //calculate casualties before deleting battle
-
 	ChangeSpells cs; //for Eagle Eye
 
 	if (finishingBattle->winnerHero)
@@ -749,7 +747,6 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHer
 			}
 		}
 	}
-
 	std::vector<const CArtifactInstance *> arts; //display them in window
 
 	if (result == BattleResult::NORMAL && finishingBattle->winnerHero)
@@ -760,6 +757,7 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHer
 			ma->dst = ArtifactLocation(finishingBattle->winnerHero, art->firstAvailableSlot(finishingBattle->winnerHero));
 			sendAndApply(ma);
 		};
+
 		if (finishingBattle->loserHero)
 		{
 			//TODO: wrap it into a function, somehow (boost::variant -_-)
@@ -818,7 +816,6 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHer
 			}
 		}
 	}
-
 	sendAndApply(battleResult.data); //after this point casualties objects are destroyed
 
 	if (arts.size()) //display loot
@@ -876,35 +873,41 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance *hero1, const CGHer
 				iw.text.addReplacement(MetaString::GENERAL_TXT, 141); // " and "
 			iw.components.push_back(Component(Component::SPELL, *it, 0, 0));
 		}
-
 		sendAndApply(&iw);
 		sendAndApply(&cs);
 	}
-
 	cab1.updateArmy(this);
 	cab2.updateArmy(this); //take casualties after battle is deleted
 
-	//if one hero has lost we will erase him
-	if (battleResult.data->winner!=0 && hero1)
+	if(battleResult.data->winner != BattleSide::ATTACKER && heroAttacker) //remove beaten Attacker
 	{
-		RemoveObject ro(hero1->id);
+		RemoveObject ro(heroAttacker->id);
 		sendAndApply(&ro);
 	}
-	if (battleResult.data->winner!=1 && hero2)
+
+	if(battleResult.data->winner != BattleSide::DEFENDER && heroDefender) //remove beaten Defender
 	{
-		auto town = hero2->visitedTown;
-		RemoveObject ro(hero2->id);
+		auto town = heroDefender->visitedTown;
+		RemoveObject ro(heroDefender->id);
 		sendAndApply(&ro);
 
-		if (town && !town->garrisonHero) // TODO: that must be called from CGHeroInstance or CGTownInstance
-			town->battleFinished(hero1, *battleResult.get());
+		if(town && !town->garrisonHero) // TODO: that must be called from CGHeroInstance or CGTownInstance
+			town->battleFinished(heroAttacker, *battleResult.get());
 	}
 
+	if(battleResult.data->winner == BattleSide::DEFENDER 
+		&& heroDefender 
+		&& heroDefender->visitedTown
+		&& !heroDefender->inTownGarrison
+		&& heroDefender->visitedTown->garrisonHero == heroDefender)
+	{
+		garrisonSwapOnSiege(heroDefender->visitedTown->id); //return defending visitor from garrison to its rightful place
+	}
 	//give exp
-	if (battleResult.data->exp[0] && hero1 && battleResult.get()->winner == 0)
-		changePrimSkill(hero1, PrimarySkill::EXPERIENCE, battleResult.data->exp[0]);
-	else if (battleResult.data->exp[1] && hero2 && battleResult.get()->winner == 1)
-		changePrimSkill(hero2, PrimarySkill::EXPERIENCE, battleResult.data->exp[1]);
+	if (battleResult.data->exp[0] && heroAttacker && battleResult.get()->winner == BattleSide::ATTACKER)
+		changePrimSkill(heroAttacker, PrimarySkill::EXPERIENCE, battleResult.data->exp[0]);
+	else if (battleResult.data->exp[1] && heroDefender && battleResult.get()->winner == BattleSide::DEFENDER)
+		changePrimSkill(heroDefender, PrimarySkill::EXPERIENCE, battleResult.data->exp[1]);
 
 	queries.popIfTop(battleQuery);
 
@@ -2504,7 +2507,7 @@ bool CGameHandler::teleportHero(ObjectInstanceID hid, ObjectInstanceID dstid, ui
 	return true;
 }
 
-void CGameHandler::setOwner(const CGObjectInstance * obj, PlayerColor owner)
+void CGameHandler::setOwner(const CGObjectInstance * obj, const PlayerColor owner)
 {
 	PlayerColor oldOwner = getOwner(obj->id);
 	SetObjectProperty sop(obj->id, ObjProperty::OWNER, owner.getNum());
@@ -3489,6 +3492,33 @@ void CGameHandler::moveArmy(const CArmedInstance *src, const CArmedInstance *dst
 	}
 }
 
+bool CGameHandler::garrisonSwapOnSiege(ObjectInstanceID tid)
+{
+	const CGTownInstance * town = getTown(tid);
+
+	if(!town->garrisonHero == !town->visitingHero)
+		return false;
+
+	SetHeroesInTown intown;
+	intown.tid = tid;
+
+	if(town->garrisonHero) //garrison -> vising
+	{
+		intown.garrison = ObjectInstanceID();
+		intown.visiting = town->garrisonHero->id;
+	}
+	else //visiting -> garrison
+	{
+		if(town->armedGarrison())
+			town->mergeGarrisonOnSiege();
+
+		intown.visiting = ObjectInstanceID();
+		intown.garrison = town->visitingHero->id;
+	}
+	sendAndApply(&intown);
+	return true;
+}
+
 bool CGameHandler::garrisonSwap(ObjectInstanceID tid)
 {
 	const CGTownInstance * town = getTown(tid);

server/CGameHandler.h

@@ -242,6 +242,7 @@ public:
 	//void lootArtifacts (TArtHolder source, TArtHolder dest, std::vector<ui32> &arts); //after battle - move al arts to winer
 	bool buySecSkill( const IMarket *m, const CGHeroInstance *h, SecondarySkill skill);
 	bool garrisonSwap(ObjectInstanceID tid);
+	bool garrisonSwapOnSiege(ObjectInstanceID tid);
 	bool upgradeCreature( ObjectInstanceID objid, SlotID pos, CreatureID upgID );
 	bool recruitCreatures(ObjectInstanceID objid, ObjectInstanceID dst, CreatureID crid, ui32 cram, si32 level);
 	bool buildStructure(ObjectInstanceID tid, BuildingID bid, bool force=false);//force - for events: no cost, no checkings

test/mock/mock_IGameCallback.h

@@ -78,6 +78,7 @@ public:
 	void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, int3 tile, bool creatureBank = false) override {}; //if any of armies is hero, hero will be used
 	void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, bool creatureBank = false) override {}; //if any of armies is hero, hero will be used, visitable tile of second obj is place of battle
 	bool moveHero(ObjectInstanceID hid, int3 dst, ui8 teleporting, bool transit = false, PlayerColor asker = PlayerColor::NEUTRAL) override {return false;};
+	bool garrisonSwapOnSiege(ObjectInstanceID tid) override {};
 	void giveHeroBonus(GiveBonus * bonus) override {};
 	void setMovePoints(SetMovePoints * smp) override {};
 	void setManaPoints(ObjectInstanceID hid, int val) override {};