Merge pull request #676 from ShubusCorporation/do/fix/mod_system/Patch3

Patch III for Mod system improvement : Negative bonuses: new feature, fixes, town building: Magic Well

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

@@ -559,6 +559,7 @@ int CClient::sendRequest(const CPackForServer * request, PlayerColor player)
 void CClient::battleStarted(const 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 swapGarrisonOnSiege(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

@@ -404,18 +404,18 @@ 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)
+		std::string addInfo = "";
+		for(auto & bonus : * modifierList)
 		{
-			if(elem->val != 0)
+			if(bonus->val)
-				//no bonuses with value 0
+				addInfo += "\n" + bonus->Description();
-				text += "\n" + elem->Description();
 		}
+		text = addInfo.empty() 
+			? text + CGI->generaltexth->arraytxt[noneTxtId] 
+			: text + addInfo;
 	}
-
 	std::string imageName;
 	if (small)
 		imageName = morale ? "IMRL30": "ILCK30";

config/creatures/necropolis.json

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

config/translate.json

@@ -53,7 +53,8 @@
 		"hasNotProduced" : "The %s has not produced anything yet.",
 		"hasProduced" : "The %s produced %d %s this week.",
 		"greetingCustomBonus" : "%s gives you +%d %s%s",
-		"greetingCustomUntil" : " until next battle."
+		"greetingCustomUntil" : " until next battle.",
+		"greetingInTownMagicWell" : "%s has restored your spell points to maximum."
 	},
 	"logicalExpressions" :
 	{

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)
@@ -1322,14 +1315,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 addBonusForTier(int tier, const std::shared_ptr<Bonus> & b); //tier must be <1-7>
-	void addBonusForAllCreatures(std::shared_ptr<Bonus> b);
+	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/CCreatureSet.cpp

@@ -662,16 +662,15 @@ 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));
 
 	_armyObj = ArmyObj;
+
 	if(ArmyObj)
-	{
 		attachTo(const_cast<CArmedInstance*>(_armyObj));
-	}
 }
 
 std::string CStackInstance::getQuantityTXT(bool capitalized) const
@@ -714,6 +713,11 @@ std::string CStackInstance::nodeName() const
 	return oss.str();
 }
 
+PlayerColor CStackInstance::getOwner() const
+{
+	return _armyObj ? _armyObj->getOwner() : PlayerColor::NEUTRAL;
+}
+
 void CStackInstance::deserializationFix()
 {
 	const CCreature *backup = type;

lib/CCreatureSet.h

@@ -99,6 +99,7 @@ public:
 	ArtBearer::ArtBearer bearerType() const override; //from CArtifactSet
 	virtual std::string nodeName() const override; //from CBonusSystemnode
 	void deserializationFix();
+	PlayerColor getOwner() const override;
 };
 
 class DLL_LINKAGE CCommanderInstance : public CStackInstance

lib/CStack.h

@@ -20,6 +20,7 @@
 struct BattleStackAttacked;
 class BattleInfo;
 
+//Represents STACK_BATTLE nodes
 class DLL_LINKAGE CStack : public CBonusSystemNode, public battle::CUnitState, public battle::IUnitEnvironment
 {
 public:
@@ -79,6 +80,11 @@ public:
 
 	void spendMana(ServerCallback * server, const int spellCost) const override;
 
+	PlayerColor getOwner() const override
+	{
+		return this->owner;
+	}
+
 	template <typename Handler> void serialize(Handler & h, const int version)
 	{
 		//this assumes that stack objects is newly created

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,9 +26,7 @@
 #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)
 
 #define BONUS_NAME(x) { #x, Bonus::x },
 	const std::map<std::string, Bonus::BonusType> bonusNameMap = { BONUS_LIST };
@@ -73,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 =
@@ -452,7 +451,6 @@ int BonusList::totalValue() const
 			{
 				vstd::amax(indepMax, b->val);
 			}
-
 			break;
 		case Bonus::INDEPENDENT_MIN:
 			if (!hasIndepMin)
@@ -464,7 +462,6 @@ int BonusList::totalValue() const
 			{
 				vstd::amin(indepMin, b->val);
 			}
-
 			break;
 		}
 	}
@@ -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);
 	}
 }
@@ -595,11 +593,12 @@ void BonusList::insert(BonusList::TInternalContainer::iterator position, BonusLi
 	changed();
 }
 
-CSelector IBonusBearer::anaffectedByMoraleSelector
+CSelector IBonusBearer::anaffectedByMoraleSelector =
-	= Selector::type()(Bonus::NON_LIVING)
+Selector::type()(Bonus::NON_LIVING)
-		.Or(Selector::type()(Bonus::UNDEAD))
+.Or(Selector::type()(Bonus::UNDEAD))
-		.Or(Selector::type()(Bonus::NO_MORALE))
+.Or(Selector::type()(Bonus::SIEGE_WEAPON))
-		.Or(Selector::type()(Bonus::SIEGE_WEAPON));
+.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);
@@ -789,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
 }
 
@@ -830,7 +831,36 @@ 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)
+{
+	return node ? node->getOwner() : PlayerColor::CANNOT_DETERMINE;
+}
+
+std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & selector)
 {
 	auto ret = bonuses.getFirst(selector);
 	if(ret)
@@ -846,7 +876,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);
 }
@@ -869,30 +899,33 @@ 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;
+	for(auto parent : parents)
-	FOREACH_CPARENT(p)
 	{
-		p->getBonusesRec(beforeUpdate, selector, limit);
+		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)
+	TCNodes lparents;
-	{
+	getAllParents(lparents);
-		p->getAllBonusesRec(beforeUpdate);
+
-	}
+	for(auto parent : lparents)
+		parent->bonuses.getAllBonuses(beforeUpdate);
+
 	bonuses.getAllBonuses(beforeUpdate);
 
 	for(auto b : beforeUpdate)
-		out.push_back(update(b));
+	{
+		auto updated = b->updater 
+			? getUpdatedBonus(b, b->updater) 
+			: b;
+		out.push_back(updated);
+	}
 }
 
 TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root, const std::string &cachingStr) const
@@ -981,11 +1014,10 @@ TConstBonusListPtr CBonusSystemNode::getAllBonusesWithoutCaching(const CSelector
 	return ret;
 }
 
-std::shared_ptr<Bonus> CBonusSystemNode::update(const std::shared_ptr<Bonus> & b) const
+std::shared_ptr<Bonus> CBonusSystemNode::getUpdatedBonus(const std::shared_ptr<Bonus> & b, const TUpdaterPtr updater) const
 {
-	if(b->updater)
+	assert(updater);
-		return b->updater->update(b, *this);
+	return updater->createUpdatedBonus(b, * this);
-	return b;
 }
 
 CBonusSystemNode::CBonusSystemNode()
@@ -1089,13 +1121,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();
 }
 
@@ -1175,16 +1214,19 @@ bool CBonusSystemNode::actsAsBonusSourceOnly() const
 	}
 }
 
-void CBonusSystemNode::propagateBonus(std::shared_ptr<Bonus> b)
+void CBonusSystemNode::propagateBonus(std::shared_ptr<Bonus> b, const CBonusSystemNode & source)
 {
 	if(b->propagator->shouldBeAttached(this))
 	{
-		bonuses.push_back(b);
+		auto propagated = b->propagationUpdater 
-		logBonus->trace("#$# %s #propagated to# %s",  b->Description(), nodeName());
+			? source.getUpdatedBonus(b, b->propagationUpdater)
+			: b;
+		bonuses.push_back(propagated);
+		logBonus->trace("#$# %s #propagated to# %s",  propagated->Description(), nodeName());
 	}
 
 	FOREACH_RED_CHILD(child)
-		child->propagateBonus(b);
+		child->propagateBonus(b, source);
 }
 
 void CBonusSystemNode::unpropagateBonus(std::shared_ptr<Bonus> b)
@@ -1207,12 +1249,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());
+		logBonus->error("Error on Detach. Node %s (nodeType=%d) is not a child of %s (nodeType=%d)"
-		throw std::runtime_error("internal error");
+			, child->nodeShortInfo(), child->nodeType, nodeShortInfo(), nodeType);
 	}
 }
 
@@ -1234,13 +1276,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)
 	{
@@ -1278,14 +1330,24 @@ 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);
+			descendant->propagateBonus(b, *this);
+	}
+	TNodes redParents;
+	getRedAncestors(redParents); //get all red parents recursively
 
-	FOREACH_RED_PARENT(parent)
+	for(auto parent : redParents)
-		parent->newRedDescendant(descendant);
+	{
+		for(auto b : parent->exportedBonuses)
+		{
+			if(b->propagator)
+				descendant->propagateBonus(b, *this);
+		}
+	}
 }
 
 void CBonusSystemNode::removedRedDescendant(CBonusSystemNode *descendant)
@@ -1294,15 +1356,26 @@ void CBonusSystemNode::removedRedDescendant(CBonusSystemNode *descendant)
 		if(b->propagator)
 			descendant->unpropagateBonus(b);
 
-	FOREACH_RED_PARENT(parent)
+	TNodes redParents;
-		parent->removedRedDescendant(descendant);
+	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)
@@ -1315,7 +1388,7 @@ void CBonusSystemNode::getRedDescendants(TNodes &out)
 void CBonusSystemNode::exportBonus(std::shared_ptr<Bonus> b)
 {
 	if(b->propagator)
-		propagateBonus(b);
+		propagateBonus(b, *this);
 	else
 		bonuses.push_back(b);
 
@@ -1736,30 +1809,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())
@@ -2161,6 +2210,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)
@@ -2252,11 +2318,45 @@ std::shared_ptr<Bonus> Bonus::addUpdater(TUpdaterPtr Updater)
 	return this->shared_from_this();
 }
 
+// Update ONLY_ENEMY_ARMY bonuses from old saves to make them workable.
+// Also, we should foreseen possible errors in bonus configuration and fix them.
+void Bonus::updateOppositeBonuses()
+{
+	if(effectRange != Bonus::ONLY_ENEMY_ARMY)
+		return;
+
+	if(propagator)
+	{
+		if(propagator->getPropagatorType() != CBonusSystemNode::BATTLE)
+		{
+			logMod->error("Wrong Propagator will be ignored: The 'ONLY_ENEMY_ARMY' effectRange is only compatible with the 'BATTLE_WIDE' propagator.");
+			propagator.reset(new CPropagatorNodeType(CBonusSystemNode::BATTLE));
+		}
+	}
+	else
+	{
+		propagator = std::make_shared<CPropagatorNodeType>(CBonusSystemNode::BATTLE);
+	}
+	if(limiter)
+	{
+		if(!dynamic_cast<OppositeSideLimiter*>(limiter.get()))
+		{
+			logMod->error("Wrong Limiter will be ignored: The 'ONLY_ENEMY_ARMY' effectRange is only compatible with the 'OPPOSITE_SIDE' limiter.");
+			limiter.reset(new OppositeSideLimiter());
+		}
+	}
+	else
+	{
+		limiter = std::make_shared<OppositeSideLimiter>();
+	}
+	propagationUpdater = std::make_shared<OwnerUpdater>();
+}
+
 IUpdater::~IUpdater()
 {
 }
 
-std::shared_ptr<Bonus> IUpdater::update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
+std::shared_ptr<Bonus> IUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
 {
 	return b;
 }
@@ -2279,7 +2379,7 @@ GrowsWithLevelUpdater::GrowsWithLevelUpdater(int valPer20, int stepSize) : valPe
 {
 }
 
-std::shared_ptr<Bonus> GrowsWithLevelUpdater::update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
+std::shared_ptr<Bonus> GrowsWithLevelUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
 {
 	if(context.getNodeType() == CBonusSystemNode::HERO)
 	{
@@ -2316,7 +2416,7 @@ TimesHeroLevelUpdater::TimesHeroLevelUpdater()
 {
 }
 
-std::shared_ptr<Bonus> TimesHeroLevelUpdater::update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
+std::shared_ptr<Bonus> TimesHeroLevelUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
 {
 	if(context.getNodeType() == CBonusSystemNode::HERO)
 	{
@@ -2342,7 +2442,7 @@ TimesStackLevelUpdater::TimesStackLevelUpdater()
 {
 }
 
-std::shared_ptr<Bonus> TimesStackLevelUpdater::update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
+std::shared_ptr<Bonus> TimesStackLevelUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
 {
 	if(context.getNodeType() == CBonusSystemNode::STACK_INSTANCE)
 	{
@@ -2376,3 +2476,30 @@ JsonNode TimesStackLevelUpdater::toJsonNode() const
 {
 	return JsonUtils::stringNode("TIMES_STACK_LEVEL");
 }
+
+OwnerUpdater::OwnerUpdater()
+{
+}
+
+std::string OwnerUpdater::toString() const
+{
+	return "OwnerUpdater";
+}
+
+JsonNode OwnerUpdater::toJsonNode() const
+{
+	return JsonUtils::stringNode("BONUS_OWNER_UPDATER");
+}
+
+std::shared_ptr<Bonus> OwnerUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
+{
+	auto owner = CBonusSystemNode::retrieveNodeOwner(&context);
+
+	if(owner == PlayerColor::UNFLAGGABLE)
+		owner = PlayerColor::NEUTRAL;
+
+	std::shared_ptr<Bonus> updated = std::make_shared<Bonus>(
+		(Bonus::BonusDuration)b->duration, b->type, b->source, b->val, b->sid, b->subtype, b->valType);
+	updated->limiter = std::make_shared<OppositeSideLimiter>(owner);
+	return updated;
+}

lib/HeroBonus.h

@@ -323,7 +323,7 @@ public:
 	BONUS_NAME(GARGOYLE) /* gargoyle is special than NON_LIVING, cannot be rised or healed */ \
 	BONUS_NAME(SPECIAL_ADD_VALUE_ENCHANT) /*specialty spell like Aenin has, increased effect of spell, additionalInfo = value to add*/\
 	BONUS_NAME(SPECIAL_FIXED_VALUE_ENCHANT) /*specialty spell like Melody has, constant spell effect (i.e. 3 luck), additionalInfo = value to fix.*/\
-
+	BONUS_NAME(TOWN_MAGIC_WELL) /*one-time pseudo-bonus to implement Magic Well in the town*/ \
 	/* end of list */
 
 
@@ -417,6 +417,7 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 	TLimiterPtr limiter;
 	TPropagatorPtr propagator;
 	TUpdaterPtr updater;
+	TUpdaterPtr propagationUpdater;
 
 	std::string description;
 
@@ -455,6 +456,14 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 		{
 			h & updater;
 		}
+		if(version >= 801)
+		{
+			h & propagationUpdater;
+		}
+		if(version < 801 && !h.saving) //Opposite Side bonuses are introduced
+		{
+			updateOppositeBonuses();
+		}
 	}
 
 	template <typename Ptr>
@@ -522,6 +531,7 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this<Bonus>
 	std::shared_ptr<Bonus> addLimiter(TLimiterPtr Limiter); //returns this for convenient chain-calls
 	std::shared_ptr<Bonus> addPropagator(TPropagatorPtr Propagator); //returns this for convenient chain-calls
 	std::shared_ptr<Bonus> addUpdater(TUpdaterPtr Updater); //returns this for convenient chain-calls
+	void updateOppositeBonuses();
 };
 
 DLL_LINKAGE std::ostream & operator<<(std::ostream &out, const Bonus &bonus);
@@ -740,7 +750,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)
@@ -764,10 +774,9 @@ 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;
+	std::shared_ptr<Bonus> getUpdatedBonus(const std::shared_ptr<Bonus> & b, const TUpdaterPtr updater) const;
 
 public:
 	explicit CBonusSystemNode();
@@ -780,15 +789,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);
-	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector &selector);
+	void getAllParents(TCNodes & out) const;
+	static PlayerColor retrieveNodeOwner(const CBonusSystemNode * node);
+	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector & selector);
 
 	void attachTo(CBonusSystemNode *parent);
 	void detachFrom(CBonusSystemNode *parent);
@@ -798,7 +810,7 @@ public:
 
 	void newChildAttached(CBonusSystemNode *child);
 	void childDetached(CBonusSystemNode *child);
-	void propagateBonus(std::shared_ptr<Bonus> b);
+	void propagateBonus(std::shared_ptr<Bonus> b, const CBonusSystemNode & source);
 	void unpropagateBonus(std::shared_ptr<Bonus> b);
 	void removeBonus(const std::shared_ptr<Bonus>& b);
 	void removeBonuses(const CSelector & selector);
@@ -812,6 +824,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();
@@ -832,6 +845,11 @@ public:
 
 	int64_t getTreeVersion() const override;
 
+	virtual PlayerColor getOwner() const
+	{
+		return PlayerColor::NEUTRAL;
+	}
+
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 //		h & bonuses;
@@ -1125,6 +1143,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:
@@ -1198,7 +1232,7 @@ class DLL_LINKAGE IUpdater
 public:
 	virtual ~IUpdater();
 
-	virtual std::shared_ptr<Bonus> update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const;
+	virtual std::shared_ptr<Bonus> createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const;
 	virtual std::string toString() const;
 	virtual JsonNode toJsonNode() const;
 
@@ -1223,7 +1257,7 @@ public:
 		h & stepSize;
 	}
 
-	std::shared_ptr<Bonus> update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const override;
+	std::shared_ptr<Bonus> createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const override;
 	virtual std::string toString() const override;
 	virtual JsonNode toJsonNode() const override;
 };
@@ -1238,7 +1272,7 @@ public:
 		h & static_cast<IUpdater &>(*this);
 	}
 
-	std::shared_ptr<Bonus> update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const override;
+	std::shared_ptr<Bonus> createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const override;
 	virtual std::string toString() const override;
 	virtual JsonNode toJsonNode() const override;
 };
@@ -1253,7 +1287,22 @@ public:
 		h & static_cast<IUpdater &>(*this);
 	}
 
-	std::shared_ptr<Bonus> update(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const override;
+	std::shared_ptr<Bonus> createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const override;
+	virtual std::string toString() const override;
+	virtual JsonNode toJsonNode() const override;
+};
+
+class DLL_LINKAGE OwnerUpdater : public IUpdater
+{
+public:
+	OwnerUpdater();
+
+	template <typename Handler> void serialize(Handler& h, const int version)
+	{
+		h & static_cast<IUpdater &>(*this);
+	}
+
+	std::shared_ptr<Bonus> createUpdatedBonus(const std::shared_ptr<Bonus>& b, const CBonusSystemNode& context) const override;
 	virtual std::string toString() const override;
 	virtual JsonNode toJsonNode() const override;
 };

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 swapGarrisonOnSiege(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

@@ -846,7 +846,7 @@ bool JsonUtils::parseBonus(const JsonNode &ability, Bonus *b)
 			break;
 		}
 	}
-
+	b->updateOppositeBonuses();
 	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)
+	if(obj->ID == Obj::HERO) //remove beaten hero
 	{
-		CGHeroInstance *h = static_cast<CGHeroInstance*>(obj);
+		CGHeroInstance * beatenHero = static_cast<CGHeroInstance*>(obj);
-		PlayerState *p = gs->getPlayerState(h->tempOwner);
+		PlayerState * p = gs->getPlayerState(beatenHero->tempOwner);
-		gs->map->heroesOnMap -= h;
+		gs->map->heroesOnMap -= beatenHero;
-		p->heroes -= h;
+		p->heroes -= beatenHero;
-		h->detachFrom(h->whereShouldBeAttached(gs));
+		beatenHero->detachFrom(beatenHero->whereShouldBeAttachedOnSiege(gs));
-		h->tempOwner = PlayerColor::NEUTRAL; //no one owns beaten hero
+		beatenHero->tempOwner = PlayerColor::NEUTRAL; //no one owns beaten hero
-		vstd::erase_if(h->artifactsInBackpack, [](const ArtSlotInfo& asi)
+		vstd::erase_if(beatenHero->artifactsInBackpack, [](const ArtSlotInfo& asi)
 		{
 			return asi.artifact->artType->id == ArtifactID::GRAIL;
 		});
 
-		if(h->visitedTown)
+		if(beatenHero->visitedTown)
 		{
-			if(h->inTownGarrison)
+			if(beatenHero->visitedTown->garrisonHero == beatenHero)
-				h->visitedTown->garrisonHero = nullptr;
+				beatenHero->visitedTown->garrisonHero = nullptr;
 			else
-				h->visitedTown->visitingHero = nullptr;
+				beatenHero->visitedTown->visitingHero = nullptr;
-			h->visitedTown = nullptr;
-		}
 
+			beatenHero->visitedTown = nullptr;
+			beatenHero->inTownGarrison = false;
+		}
 		//return hero to the pool, so he may reappear in tavern
-		gs->hpool.heroesPool[h->subID] = h;
+		gs->hpool.heroesPool[beatenHero->subID] = beatenHero;
-		if(!vstd::contains(gs->hpool.pavailable, h->subID))
+
-			gs->hpool.pavailable[h->subID] = 0xff;
+		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->instanceNames.erase(beatenHero->boat->instanceName);
-			gs->map->objects[h->boat->id.getNum()].dellNull();
+			gs->map->objects[beatenHero->boat->id.getNum()].dellNull();
-			h->boat = nullptr;
+			beatenHero->boat = nullptr;
 		}
-
 		return;
 	}
 

lib/battle/BattleInfo.cpp

@@ -528,7 +528,7 @@ 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));
@@ -555,31 +555,10 @@ 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);
-				}
-			}
-		}
-	}
-
 	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)
@@ -132,7 +137,6 @@ public:
 	const CGHeroInstance * getHero(PlayerColor player) const; //returns fighting hero that belongs to given player
 
 	void localInit();
-
 	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;

lib/mapObjects/CArmedInstance.h

@@ -38,6 +38,11 @@ public:
 	CArmedInstance();
 	CArmedInstance(bool isHypotetic);
 
+	PlayerColor getOwner() const override
+	{
+		return this->tempOwner;
+	}
+
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<CGObjectInstance&>(*this);

lib/mapObjects/CGHeroInstance.cpp

@@ -1069,7 +1069,25 @@ 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)
 	{

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 CGHeroInstance * defendingHero = visitingHero ? visitingHero : garrisonHero;
-			const CArmedInstance *defendingArmy = this;
+			const CArmedInstance * defendingArmy = defendingHero ? (CArmedInstance *)defendingHero : this;
+			const bool isBattleOutside = isBattleOutsideTown(defendingHero);
 
-			if(visitingHero)
+			if(!isBattleOutside && visitingHero && defendingHero == visitingHero)
-				defendingHero = visitingHero;
-			else if(garrisonHero)
-				defendingHero = garrisonHero;
-
-			if(defendingHero)
-				defendingArmy = defendingHero;
-
-			bool outsideTown = (defendingHero == visitingHero && garrisonHero);
-
-			//"borrowing" army from garrison to visiting hero
-			if(!outsideTown && armedGarrison() &&
-				visitingHero && defendingHero == visitingHero)
 			{
-				mergeGarrisonOnSiege();
+				//we have two approaches to merge armies: mergeGarrisonOnSiege() and used in the CGameHandler::garrisonSwap(ObjectInstanceID tid)
-			}
+				auto nodeSiege = defendingHero->whereShouldBeAttachedOnSiege(isBattleOutside);
 
-			cb->startBattlePrimary(h, defendingArmy, getSightCenter(), h, defendingHero, false, (outsideTown ? nullptr : this));
+				if(nodeSiege == (CBonusSystemNode *)this)
+					cb->swapGarrisonOnSiege(this->id);
+
+				const_cast<CGHeroInstance *>(defendingHero)->inTownGarrison = false; //hack to return visitor from garrison after battle
+			}
+			cb->startBattlePrimary(h, defendingArmy, getSightCenter(), h, defendingHero, false, (isBattleOutside ? nullptr : this));
 		}
 		else
 		{
-			cb->setOwner(this, h->tempOwner);
+			auto heroColor = h->getOwner();
-			if(cb->gameState()->getPlayerStatus(h->getOwner()) == EPlayerStatus::WINNER)
+			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);
 		}
 	}
@@ -743,15 +742,6 @@ bool CGTownInstance::townEnvisagesBuilding(BuildingSubID::EBuildingSubID subId)
 	return town->getBuildingType(subId) != BuildingID::NONE;
 }
 
-//it does not check hasBuilt because this check is in the OnHeroVisit handler
-void CGTownInstance::tryAddOnePerWeekBonus(BuildingSubID::EBuildingSubID subID)
-{
-	auto bid = town->getBuildingType(subID);
-
-	if(bid != BuildingID::NONE)
-		bonusingBuildings.push_back(new COPWBonus(bid, subID, this));
-}
-
 void CGTownInstance::initOverriddenBids()
 {
 	for(const auto & bid : builtBuildings)
@@ -763,11 +753,30 @@ void CGTownInstance::initOverriddenBids()
 	}
 }
 
+bool CGTownInstance::isBonusingBuildingAdded(BuildingID::EBuildingID bid) const
+{
+	auto present = std::find_if(bonusingBuildings.begin(), bonusingBuildings.end(), [&](CGTownBuilding* building)
+		{
+			return building->getBuildingType().num == bid;
+		});
+
+	return present != bonusingBuildings.end();
+}
+
+//it does not check hasBuilt because this check is in the OnHeroVisit handler
+void CGTownInstance::tryAddOnePerWeekBonus(BuildingSubID::EBuildingSubID subID)
+{
+	auto bid = town->getBuildingType(subID);
+
+	if(bid != BuildingID::NONE && !isBonusingBuildingAdded(bid))
+		bonusingBuildings.push_back(new COPWBonus(bid, subID, this));
+}
+
 void CGTownInstance::tryAddVisitingBonus(BuildingSubID::EBuildingSubID subID)
 {
 	auto bid = town->getBuildingType(subID);
 
-	if(bid != BuildingID::NONE)
+	if(bid != BuildingID::NONE && !isBonusingBuildingAdded(bid))
 		bonusingBuildings.push_back(new CTownBonus(bid, subID, this));
 }
 
@@ -786,6 +795,28 @@ void CGTownInstance::addTownBonuses()
 	}
 }
 
+void CGTownInstance::fixBonusingDuplicates() //For versions 794-800
+{
+	std::map<BuildingID::EBuildingID, int> bids;
+
+	for(auto i = 0; i != bonusingBuildings.size(); i++)
+	{
+		auto bid = bonusingBuildings[i]->getBuildingType();
+		if(!bids.count(bid))
+			bids.insert({ bid, 0 });
+		else
+			bids[bid]++;
+	}
+	for(auto & pair : bids)
+	{
+		if(!pair.second)
+			continue;
+
+		for(auto i = 0; i < pair.second; i++)
+			deleteTownBonus(pair.first);
+	}
+}
+
 void CGTownInstance::deleteTownBonus(BuildingID::EBuildingID bid)
 {
 	size_t i = 0;
@@ -1253,7 +1284,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,8 +1295,15 @@ void CGTownInstance::recreateBuildingsBonuses()
 		if(building->buildingBonuses.empty())
 			continue;
 
-		for(auto bonus : building->buildingBonuses)
+		for(auto & bonus : building->buildingBonuses)
 		{
+			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
@@ -1275,14 +1314,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);
@@ -1386,7 +1418,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;
@@ -1490,18 +1522,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());
+		setOwner(heroColor); //give control after checkout is done
-		cb->setOwner (this, hero->tempOwner); //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);
 	}
 }
 
@@ -1812,6 +1845,13 @@ void CTownBonus::applyBonuses(CGHeroInstance * h, const BonusList & bonuses) con
 		GiveBonus gb;
 		InfoWindow iw;
 
+		if(bonus->type == Bonus::TOWN_MAGIC_WELL)
+		{
+			if(h->mana >= h->manaLimit())
+				return;
+			cb->setManaPoints(h->id, h->manaLimit());
+			bonus->duration = Bonus::ONE_DAY;
+		}
 		gb.bonus = * bonus;
 		gb.id = h->id.getNum();
 		cb->giveHeroBonus(&gb);
@@ -1895,8 +1935,15 @@ const std::string CGTownBuilding::getVisitingBonusGreeting() const
 	return bonusGreeting;
 }
 
-const std::string CGTownBuilding::getCustomBonusGreeting(const Bonus & bonus)
+const std::string CGTownBuilding::getCustomBonusGreeting(const Bonus & bonus) const
 {
+	if(bonus.type == Bonus::TOWN_MAGIC_WELL)
+	{
+		auto bonusGreeting = std::string(VLC->generaltexth->localizedTexts["townHall"]["greetingInTownMagicWell"].String());
+		auto buildingName = town->town->getSpecialBuilding(bType)->Name();
+		boost::algorithm::replace_first(bonusGreeting, "%s", buildingName);
+		return bonusGreeting;
+	}
 	auto bonusGreeting = std::string(VLC->generaltexth->localizedTexts["townHall"]["greetingCustomBonus"].String()); //"%s gives you +%d %s%s"
 	std::string param = "";
 	std::string until = "";

lib/mapObjects/CGTownInstance.h

@@ -140,7 +140,7 @@ protected:
 	BuildingSubID::EBuildingSubID bType;
 
 	const std::string getVisitingBonusGreeting() const;
-	static const std::string getCustomBonusGreeting(const Bonus & bonus);
+	const std::string getCustomBonusGreeting(const Bonus & bonus) const;
 };
 
 class DLL_LINKAGE COPWBonus : public CGTownBuilding
@@ -274,6 +274,12 @@ public:
 			h & overriddenBuildings;
 		else if(!h.saving)
 			updateTown794();
+
+		if(!h.saving && (version >= 794 && version < 801))
+			fixBonusingDuplicates();
+
+		if(!h.saving)
+			this->setNodeType(CBonusSystemNode::TOWN);
 	}
 	//////////////////////////////////////////////////////////////////////////
 
@@ -349,18 +355,25 @@ public:
 	void afterAddToMap(CMap * map) override;
 	static void reset();
 
+	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;
 	bool townEnvisagesBuilding(BuildingSubID::EBuildingSubID bid) const;
+	bool isBonusingBuildingAdded(BuildingID::EBuildingID bid) const;
 	void tryAddOnePerWeekBonus(BuildingSubID::EBuildingSubID subID);
 	void tryAddVisitingBonus(BuildingSubID::EBuildingSubID subID);
 	void initOverriddenBids();
 	void addTownBonuses();
 	void updateTown794(); //populate overriddenBuildings and vanila bonuses for old saves 
+	void fixBonusingDuplicates(); //For versions 794-800.
 };

lib/mapObjects/CObjectHandler.cpp

@@ -119,14 +119,6 @@ CObjectHandler::CObjectHandler()
 	logGlobal->trace("\t\tDone loading resource prices!");
 }
 
-PlayerColor CGObjectInstance::getOwner() const
-{
-	//if (state)
-	//	return state->owner;
-	//else
-		return tempOwner; //won't have owner
-}
-
 CGObjectInstance::CGObjectInstance():
 	pos(-1,-1,-1),
 	ID(Obj::NO_OBJ),

lib/mapObjects/CObjectHandler.h

@@ -118,12 +118,12 @@ public:
 	Obj ID;
 	/// Subtype of object, depends on type
 	si32 subID;
+	/// Current owner of an object (when below PLAYER_LIMIT)
+	PlayerColor tempOwner;
 	/// Index of object in map's list of objects
 	ObjectInstanceID id;
 	/// Defines appearance of object on map (animation, blocked tiles, blit order, etc)
 	ObjectTemplate appearance;
-	/// Current owner of an object (when below PLAYER_LIMIT)
-	PlayerColor tempOwner;
 	/// If true hero can visit this object only from neighbouring tiles and can't stand on this object
 	bool blockVisit;
 
@@ -140,7 +140,10 @@ public:
 	/// "center" tile from which the sight distance is calculated
 	int3 getSightCenter() const;
 
-	PlayerColor getOwner() const override;
+	PlayerColor getOwner() const override 
+	{
+		return this->tempOwner;
+	}
 	void setOwner(PlayerColor ow);
 
 	/** APPEARANCE ACCESSORS **/
@@ -213,8 +216,8 @@ public:
 
 	///Entry point of Json (de-)serialization
 	void serializeJson(JsonSerializeFormat & handler);
-
 	virtual void updateFrom(const JsonNode & data);
+
 protected:
 	/// virtual method that allows synchronously update object state on server and all clients
 	virtual void setPropertyDer(ui8 what, ui32 val);

lib/registerTypes/RegisterTypes.h

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

lib/serializer/CSerializer.h

@@ -12,7 +12,7 @@
 #include "../ConstTransitivePtr.h"
 #include "../GameConstants.h"
 
-const ui32 SERIALIZATION_VERSION = 800;
+const ui32 SERIALIZATION_VERSION = 801;
 const ui32 MINIMAL_SERIALIZATION_VERSION = 753;
 const std::string SAVEGAME_MAGIC = "VCMISVG";
 

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)
+	if(heroAttacker)
-		battleResult.data->exp[0] = hero1->calculateXp(battleResult.data->exp[0]);//scholar skill
+		battleResult.data->exp[0] = heroAttacker->calculateXp(battleResult.data->exp[0]);//scholar skill
-	if (hero2)
+	if(heroDefender)
-		battleResult.data->exp[1] = hero2->calculateXp(battleResult.data->exp[1]);
+		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 != BattleSide::ATTACKER && heroAttacker) //remove beaten Attacker
-	if (battleResult.data->winner!=0 && hero1)
 	{
-		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;
+		auto town = heroDefender->visitedTown;
-		RemoveObject ro(hero2->id);
+		RemoveObject ro(heroDefender->id);
 		sendAndApply(&ro);
 
-		if (town && !town->garrisonHero) // TODO: that must be called from CGHeroInstance or CGTownInstance
+		if(town && !town->garrisonHero) // TODO: that must be called from CGHeroInstance or CGTownInstance
-			town->battleFinished(hero1, *battleResult.get());
+			town->battleFinished(heroAttacker, *battleResult.get());
 	}
 
+	if(battleResult.data->winner == BattleSide::DEFENDER 
+		&& heroDefender 
+		&& heroDefender->visitedTown
+		&& !heroDefender->inTownGarrison
+		&& heroDefender->visitedTown->garrisonHero == heroDefender)
+	{
+		swapGarrisonOnSiege(heroDefender->visitedTown->id); //return defending visitor from garrison to its rightful place
+	}
 	//give exp
-	if (battleResult.data->exp[0] && hero1 && battleResult.get()->winner == 0)
+	if(battleResult.data->exp[0] && heroAttacker && battleResult.get()->winner == BattleSide::ATTACKER)
-		changePrimSkill(hero1, PrimarySkill::EXPERIENCE, battleResult.data->exp[0]);
+		changePrimSkill(heroAttacker, PrimarySkill::EXPERIENCE, battleResult.data->exp[0]);
-	else if (battleResult.data->exp[1] && hero2 && battleResult.get()->winner == 1)
+	else if(battleResult.data->exp[1] && heroDefender && battleResult.get()->winner == BattleSide::DEFENDER)
-		changePrimSkill(hero2, PrimarySkill::EXPERIENCE, battleResult.data->exp[1]);
+		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::swapGarrisonOnSiege(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 swapGarrisonOnSiege(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 swapGarrisonOnSiege(ObjectInstanceID tid) override {};
 	void giveHeroBonus(GiveBonus * bonus) override {};
 	void setMovePoints(SetMovePoints * smp) override {};
 	void setManaPoints(ObjectInstanceID hid, int val) override {};