Merge pull request #1706 from rilian-la-te/moats-landmines

Proper moats: mechanincs

AI/BattleAI/BattleAI.cpp

@@ -19,6 +19,7 @@
 #include "../../lib/spells/CSpellHandler.h"
 #include "../../lib/spells/ISpellMechanics.h"
 #include "../../lib/battle/BattleStateInfoForRetreat.h"
+#include "../../lib/battle/CObstacleInstance.h"
 #include "../../lib/CStack.h" // TODO: remove
                               // Eventually only IBattleInfoCallback and battle::Unit should be used,
                               // CUnitState should be private and CStack should be removed completely
@@ -309,25 +310,37 @@ BattleAction CBattleAI::goTowardsNearest(const CStack * stack, std::vector<Battl
 
 	if(stack->hasBonusOfType(Bonus::FLYING))
 	{
-		std::set<BattleHex> moatHexes;
+		std::set<BattleHex> obstacleHexes;
 
-		if(hb.battleGetSiegeLevel() >= BuildingID::CITADEL)
-		{
-			auto townMoat = hb.getDefendedTown()->town->moatHexes;
+		auto insertAffected = [](const CObstacleInstance & spellObst, std::set<BattleHex> obstacleHexes) {
+			auto affectedHexes = spellObst.getAffectedTiles();
+			obstacleHexes.insert(affectedHexes.cbegin(), affectedHexes.cend());
+		};
+
+		const auto & obstacles = hb.battleGetAllObstacles();
+
+		for (const auto & obst: obstacles) {
 
-			moatHexes = std::set<BattleHex>(townMoat.begin(), townMoat.end());
+			if(obst->triggersEffects())
+			{
+				auto triggerAbility =  VLC->spells()->getById(obst->getTrigger());
+				auto triggerIsNegative = triggerAbility->isNegative() || triggerAbility->isDamage();
+
+				if(triggerIsNegative)
+					insertAffected(*obst, obstacleHexes);
+			}
 		}
 		// Flying stack doesn't go hex by hex, so we can't backtrack using predecessors.
 		// We just check all available hexes and pick the one closest to the target.
 		auto nearestAvailableHex = vstd::minElementByFun(avHexes, [&](BattleHex hex) -> int
 		{
-			const int MOAT_PENALTY = 100; // avoid landing on moat
+			const int NEGATIVE_OBSTACLE_PENALTY = 100; // avoid landing on negative obstacle (moat, fire wall, etc)
 			const int BLOCKED_STACK_PENALTY = 100; // avoid landing on moat
 
 			auto distance = BattleHex::getDistance(bestNeighbor, hex);
 
-			if(vstd::contains(moatHexes, hex))
-				distance += MOAT_PENALTY;
+			if(vstd::contains(obstacleHexes, hex))
+				distance += NEGATIVE_OBSTACLE_PENALTY;
 
 			return scoreEvaluator.checkPositionBlocksOurStacks(hb, stack, hex) ? BLOCKED_STACK_PENALTY + distance : distance;
 		});

Mods/vcmi/config/vcmi/russian.json

@@ -143,6 +143,16 @@
 	"mapObject.core.pyramid.pyramid.name" : "Пирамида",
 	"mapObject.core.shipwreck.shipwreck.name" : "Кораблекрушение",
 
+	"spell.core.landMineTrigger.name" : "Мина",
+	"spell.core.fireWallTrigger.name" : "Стена огня",
+	"spell.core.castleMoatTrigger.name" : "Ров",
+	"spell.core.rampartMoatTrigger.name" : "Колючий куст",
+	"spell.core.infernoMoatTrigger.name" : "Лава",
+	"spell.core.necropolisMoatTrigger.name" : "Груда костей",
+	"spell.core.dungeonMoatTrigger.name" : "Кипящее масло",
+	"spell.core.strongholdMoatTrigger.name" : "Стена шипов",
+	"spell.core.fortressMoatTrigger.name" : "Кипящая смола",
+
 	// few strings from WoG used by vcmi
 	"vcmi.stackExperience.description" : "» О п ы т   с у щ е с т в «\n\nТип существа ................... : %s\nРанг опыта ................. : %s (%i)\nОчки опыта ............... : %i\nДо следующего .. : %i\nМаксимум за битву ... : %i%% (%i)\nЧисло в отряде .... : %i\nМаксимум новичков\n без потери ранга .... : %i\nМножитель опыта ........... : %.2f\nМножитель улучшения .......... : %.2f\nОпыт после 10 ранга ........ : %i\nМаксимум новичков для сохранения\n ранга 10 при максимальном опыте : %i",
 	"vcmi.stackExperience.rank.1" : "Рекрут",

client/CPlayerInterface.cpp

@@ -759,6 +759,7 @@ void CPlayerInterface::battleObstaclesChanged(const std::vector<ObstacleChanges>
 	BATTLE_EVENT_POSSIBLE_RETURN;
 
 	std::vector<std::shared_ptr<const CObstacleInstance>> newObstacles;
+	std::vector<ObstacleChanges> removedObstacles;
 
 	for(auto & change : obstacles)
 	{
@@ -770,11 +771,16 @@ void CPlayerInterface::battleObstaclesChanged(const std::vector<ObstacleChanges>
 			else
 				logNetwork->error("Invalid obstacle instance %d", change.id);
 		}
+		if(change.operation == BattleChanges::EOperation::REMOVE)
+			removedObstacles.push_back(change); //Obstacles are already removed, so, show animation based on json struct
 	}
 
 	if (!newObstacles.empty())
 		battleInt->obstaclePlaced(newObstacles);
 
+	if (!removedObstacles.empty())
+		battleInt->obstacleRemoved(removedObstacles);
+
 	battleInt->fieldController->redrawBackgroundWithHexes();
 }
 

client/NetPacksClient.cpp

@@ -324,13 +324,13 @@ void ApplyClientNetPackVisitor::visitGiveBonus(GiveBonus & pack)
 	cl.invalidatePaths();
 	switch(pack.who)
 	{
-	case GiveBonus::HERO:
+	case GiveBonus::ETarget::HERO:
 		{
 			const CGHeroInstance *h = gs.getHero(ObjectInstanceID(pack.id));
 			callInterfaceIfPresent(cl, h->tempOwner, &IGameEventsReceiver::heroBonusChanged, h, *h->getBonusList().back(), true);
 		}
 		break;
-	case GiveBonus::PLAYER:
+	case GiveBonus::ETarget::PLAYER:
 		{
 			const PlayerState *p = gs.getPlayerState(PlayerColor(pack.id));
 			callInterfaceIfPresent(cl, PlayerColor(pack.id), &IGameEventsReceiver::playerBonusChanged, *p->getBonusList().back(), true);
@@ -399,13 +399,13 @@ void ApplyClientNetPackVisitor::visitRemoveBonus(RemoveBonus & pack)
 	cl.invalidatePaths();
 	switch(pack.who)
 	{
-	case RemoveBonus::HERO:
+	case GiveBonus::ETarget::HERO:
 		{
 			const CGHeroInstance *h = gs.getHero(ObjectInstanceID(pack.id));
 			callInterfaceIfPresent(cl, h->tempOwner, &IGameEventsReceiver::heroBonusChanged, h, pack.bonus, false);
 		}
 		break;
-	case RemoveBonus::PLAYER:
+	case GiveBonus::ETarget::PLAYER:
 		{
 			//const PlayerState *p = gs.getPlayerState(pack.id);
 			callInterfaceIfPresent(cl, PlayerColor(pack.id), &IGameEventsReceiver::playerBonusChanged, pack.bonus, false);

client/battle/BattleAnimationClasses.cpp

@@ -871,42 +871,43 @@ uint32_t CastAnimation::getAttackClimaxFrame() const
 	return maxFrames / 2;
 }
 
-EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, int effects):
+EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, int effects, bool reversed):
 	BattleAnimation(owner),
 	animation(std::make_shared<CAnimation>(animationName)),
 	effectFlags(effects),
-	effectFinished(false)
+	effectFinished(false),
+	reversed(reversed)
 {
 	logAnim->debug("CPointEffectAnimation::init: effect %s", animationName);
 }
 
-EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<BattleHex> hex, int effects):
-	EffectAnimation(owner, animationName, effects)
+EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<BattleHex> hex, int effects, bool reversed):
+	EffectAnimation(owner, animationName, effects, reversed)
 {
 	battlehexes = hex;
 }
 
-EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, BattleHex hex, int effects):
-	EffectAnimation(owner, animationName, effects)
+EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, BattleHex hex, int effects, bool reversed):
+	EffectAnimation(owner, animationName, effects, reversed)
 {
 	assert(hex.isValid());
 	battlehexes.push_back(hex);
 }
 
-EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<Point> pos, int effects):
-	EffectAnimation(owner, animationName, effects)
+EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<Point> pos, int effects, bool reversed):
+	EffectAnimation(owner, animationName, effects, reversed)
 {
 	positions = pos;
 }
 
-EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, Point pos, int effects):
-	EffectAnimation(owner, animationName, effects)
+EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, Point pos, int effects, bool reversed):
+	EffectAnimation(owner, animationName, effects, reversed)
 {
 	positions.push_back(pos);
 }
 
-EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, Point pos, BattleHex hex,   int effects):
-	EffectAnimation(owner, animationName, effects)
+EffectAnimation::EffectAnimation(BattleInterface & owner, std::string animationName, Point pos, BattleHex hex, int effects, bool reversed):
+	EffectAnimation(owner, animationName, effects, reversed)
 {
 	assert(hex.isValid());
 	battlehexes.push_back(hex);
@@ -924,6 +925,13 @@ bool EffectAnimation::init()
 		return false;
 	}
 
+	for (size_t i = 0; i < animation->size(size_t(BattleEffect::AnimType::DEFAULT)); ++i)
+	{
+		size_t current = animation->size(size_t(BattleEffect::AnimType::DEFAULT)) - 1 - i;
+
+		animation->duplicateImage(size_t(BattleEffect::AnimType::DEFAULT), current, size_t(BattleEffect::AnimType::REVERSE));
+	}
+
 	if (screenFill())
 	{
 		for(int i=0; i * first->width() < owner.fieldController->pos.w ; ++i)
@@ -935,6 +943,7 @@ bool EffectAnimation::init()
 	be.effectID = ID;
 	be.animation = animation;
 	be.currentFrame = 0;
+	be.type = reversed ? BattleEffect::AnimType::REVERSE : BattleEffect::AnimType::DEFAULT;
 
 	for (size_t i = 0; i < std::max(battlehexes.size(), positions.size()); ++i)
 	{

client/battle/BattleAnimationClasses.h

@@ -310,6 +310,7 @@ class EffectAnimation : public BattleAnimation
 {
 	std::string soundName;
 	bool effectFinished;
+	bool reversed;
 	int effectFlags;
 
 	std::shared_ptr<CAnimation>	animation;
@@ -334,17 +335,17 @@ public:
 	};
 
 	/// Create animation with screen-wide effect
-	EffectAnimation(BattleInterface & owner, std::string animationName, int effects = 0);
+	EffectAnimation(BattleInterface & owner, std::string animationName, int effects = 0, bool reversed = false);
 
 	/// Create animation positioned at point(s). Note that positions must be are absolute, including battleint position offset
-	EffectAnimation(BattleInterface & owner, std::string animationName, Point pos                 , int effects = 0);
-	EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<Point> pos    , int effects = 0);
+	EffectAnimation(BattleInterface & owner, std::string animationName, Point pos                 , int effects = 0, bool reversed = false);
+	EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<Point> pos    , int effects = 0, bool reversed = false);
 
 	/// Create animation positioned at certain hex(es)
-	EffectAnimation(BattleInterface & owner, std::string animationName, BattleHex hex             , int effects = 0);
-	EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<BattleHex> hex, int effects = 0);
+	EffectAnimation(BattleInterface & owner, std::string animationName, BattleHex hex             , int effects = 0, bool reversed = false);
+	EffectAnimation(BattleInterface & owner, std::string animationName, std::vector<BattleHex> hex, int effects = 0, bool reversed = false);
 
-	EffectAnimation(BattleInterface & owner, std::string animationName, Point pos, BattleHex hex,   int effects = 0);
+	EffectAnimation(BattleInterface & owner, std::string animationName, Point pos, BattleHex hex,   int effects = 0, bool reversed = false);
 	 ~EffectAnimation();
 
 	bool init() override;

client/battle/BattleEffectsController.cpp

@@ -122,7 +122,7 @@ void BattleEffectsController::collectRenderableObjects(BattleRenderer & renderer
 			int currentFrame = static_cast<int>(floor(elem.currentFrame));
 			currentFrame %= elem.animation->size();
 
-			auto img = elem.animation->getImage(currentFrame);
+			auto img = elem.animation->getImage(currentFrame, static_cast<size_t>(elem.type));
 
 			canvas.draw(img, elem.pos);
 		});

client/battle/BattleEffectsController.h

@@ -30,6 +30,13 @@ class EffectAnimation;
 /// Struct for battle effect animation e.g. morale, prayer, armageddon, bless,...
 struct BattleEffect
 {
+	enum class AnimType : ui8 
+	{
+		DEFAULT = 0, //If we have such animation
+		REVERSE = 1 //Reverse DEFAULT will be used
+	};
+
+	AnimType type;
 	Point pos; //position on the screen
 	float currentFrame;
 	std::shared_ptr<CAnimation> animation;

client/battle/BattleInterface.cpp

@@ -653,6 +653,11 @@ void BattleInterface::obstaclePlaced(const std::vector<std::shared_ptr<const COb
 	obstacleController->obstaclePlaced(oi);
 }
 
+void BattleInterface::obstacleRemoved(const std::vector<ObstacleChanges> & obstacles)
+{
+	obstacleController->obstacleRemoved(obstacles);
+}
+
 const CGHeroInstance *BattleInterface::currentHero() const
 {
 	if (attackingHeroInstance && attackingHeroInstance->tempOwner == curInt->playerID)

client/battle/BattleInterface.h

@@ -29,6 +29,7 @@ struct CatapultAttack;
 struct BattleTriggerEffect;
 struct BattleHex;
 struct InfoAboutHero;
+class ObstacleChanges;
 
 VCMI_LIB_NAMESPACE_END
 
@@ -214,6 +215,7 @@ public:
 	void endAction(const BattleAction* action);
 
 	void obstaclePlaced(const std::vector<std::shared_ptr<const CObstacleInstance>> oi);
+	void obstacleRemoved(const std::vector<ObstacleChanges> & obstacles);
 
 	void gateStateChanged(const EGateState state);
 

client/battle/BattleObstacleController.cpp

@@ -42,17 +42,7 @@ BattleObstacleController::BattleObstacleController(BattleInterface & owner):
 
 void BattleObstacleController::loadObstacleImage(const CObstacleInstance & oi)
 {
-	std::string animationName;
-
-	if (auto spellObstacle = dynamic_cast<const SpellCreatedObstacle*>(&oi))
-	{
-		animationName = spellObstacle->animation;
-	}
-	else
-	{
-		assert( oi.obstacleType == CObstacleInstance::USUAL || oi.obstacleType == CObstacleInstance::ABSOLUTE_OBSTACLE);
-		animationName = oi.getInfo().animation;
-	}
+	std::string animationName = oi.getAnimation();
 
 	if (animationsCache.count(animationName) == 0)
 	{
@@ -74,24 +64,47 @@ void BattleObstacleController::loadObstacleImage(const CObstacleInstance & oi)
 	obstacleAnimations[oi.uniqueID] = animationsCache[animationName];
 }
 
-void BattleObstacleController::obstaclePlaced(const std::vector<std::shared_ptr<const CObstacleInstance>> & obstacles)
+void BattleObstacleController::obstacleRemoved(const std::vector<ObstacleChanges> & obstacles)
 {
 	for (auto const & oi : obstacles)
 	{
-		auto side = owner.curInt->cb->playerToSide(owner.curInt->playerID);
+		auto & obstacle = oi.data["obstacle"];
 
-		if(!oi->visibleForSide(side.get(),owner.curInt->cb->battleHasNativeStack(side.get())))
+		if (!obstacle.isStruct())
+		{
+			logGlobal->error("I don't know how to animate removal of this obstacle");
 			continue;
+		}
 
-		auto spellObstacle = dynamic_cast<const SpellCreatedObstacle*>(oi.get());
+		auto animation = std::make_shared<CAnimation>(obstacle["appearAnimation"].String());
+		animation->preload();
 
-		if (!spellObstacle)
-		{
-			logGlobal->error("I don't know how to animate appearing obstacle of type %d", (int)oi->obstacleType);
+		auto first = animation->getImage(0, 0);
+		if(!first)
+			continue;
+
+		//we assume here that effect graphics have the same size as the usual obstacle image
+		// -> if we know how to blit obstacle, let's blit the effect in the same place
+		Point whereTo = getObstaclePosition(first, obstacle);
+		//AFAIK, in H3 there is no sound of obstacle removal
+		owner.stacksController->addNewAnim(new EffectAnimation(owner, obstacle["appearAnimation"].String(), whereTo, obstacle["position"].Integer(), 0, true));
+
+		obstacleAnimations.erase(oi.id);
+		//so when multiple obstacles are removed, they show up one after another
+		owner.waitForAnimations();
+	}
+}
+
+void BattleObstacleController::obstaclePlaced(const std::vector<std::shared_ptr<const CObstacleInstance>> & obstacles)
+{
+	for (auto const & oi : obstacles)
+	{
+		auto side = owner.curInt->cb->playerToSide(owner.curInt->playerID);
+
+		if(!oi->visibleForSide(side.get(),owner.curInt->cb->battleHasNativeStack(side.get())))
 			continue;
-		}
 
-		auto animation = std::make_shared<CAnimation>(spellObstacle->appearAnimation);
+		auto animation = std::make_shared<CAnimation>(oi->getAppearAnimation());
 		animation->preload();
 
 		auto first = animation->getImage(0, 0);
@@ -101,13 +114,13 @@ void BattleObstacleController::obstaclePlaced(const std::vector<std::shared_ptr<
 		//we assume here that effect graphics have the same size as the usual obstacle image
 		// -> if we know how to blit obstacle, let's blit the effect in the same place
 		Point whereTo = getObstaclePosition(first, *oi);
-		CCS->soundh->playSound( spellObstacle->appearSound );
-		owner.stacksController->addNewAnim(new EffectAnimation(owner, spellObstacle->appearAnimation, whereTo, oi->pos));
+		CCS->soundh->playSound( oi->getAppearSound() );
+		owner.stacksController->addNewAnim(new EffectAnimation(owner, oi->getAppearAnimation(), whereTo, oi->pos));
 
 		//so when multiple obstacles are added, they show up one after another
 		owner.waitForAnimations();
 
-		loadObstacleImage(*spellObstacle);
+		loadObstacleImage(*oi);
 	}
 }
 
@@ -180,3 +193,19 @@ Point BattleObstacleController::getObstaclePosition(std::shared_ptr<IImage> imag
 
 	return r.topLeft();
 }
+
+Point BattleObstacleController::getObstaclePosition(std::shared_ptr<IImage> image, const JsonNode & obstacle)
+{
+	auto animationYOffset = obstacle["animationYOffset"].Integer();
+	auto offset = image->height() % 42;
+
+	if(obstacle["needAnimationOffsetFix"].Bool() && offset > 37)
+		animationYOffset -= 42;
+
+	offset += animationYOffset;
+
+	Rect r = owner.fieldController->hexPositionLocal(obstacle["position"].Integer());
+	r.y += 42 - image->height() + offset;
+
+	return r.topLeft();
+}

client/battle/BattleObstacleController.h

@@ -13,6 +13,8 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 struct BattleHex;
 struct CObstacleInstance;
+class JsonNode;
+class ObstacleChanges;
 class Point;
 
 VCMI_LIB_NAMESPACE_END
@@ -42,6 +44,7 @@ class BattleObstacleController
 
 	std::shared_ptr<IImage> getObstacleImage(const CObstacleInstance & oi);
 	Point getObstaclePosition(std::shared_ptr<IImage> image, const CObstacleInstance & obstacle);
+	Point getObstaclePosition(std::shared_ptr<IImage> image, const JsonNode & obstacle);
 
 public:
 	BattleObstacleController(BattleInterface & owner);
@@ -52,6 +55,9 @@ public:
 	/// call-in from network pack, add newly placed obstacles with any required animations
 	void obstaclePlaced(const std::vector<std::shared_ptr<const CObstacleInstance>> & oi);
 
+	/// call-in from network pack, remove required obstacles with any required animations
+	void obstacleRemoved(const std::vector<ObstacleChanges> & obstacles);
+
 	/// renders all "absolute" obstacles
 	void showAbsoluteObstacles(Canvas & canvas);
 

client/battle/BattleSiegeController.cpp

@@ -118,7 +118,7 @@ void BattleSiegeController::showWallPiece(Canvas & canvas, EWallVisual::EWallVis
 	auto & ci = town->town->clientInfo;
 	auto const & pos = ci.siegePositions[what];
 
-	if ( wallPieceImages[what])
+	if ( wallPieceImages[what] && pos.isValid())
 		canvas.draw(wallPieceImages[what], Point(pos.x, pos.y));
 }
 
@@ -135,8 +135,8 @@ bool BattleSiegeController::getWallPieceExistance(EWallVisual::EWallVisual what)
 
 	switch (what)
 	{
-	case EWallVisual::MOAT:              return town->hasBuilt(BuildingID::CITADEL) && town->town->faction->getIndex() != ETownType::TOWER;
-	case EWallVisual::MOAT_BANK:         return town->hasBuilt(BuildingID::CITADEL) && town->town->faction->getIndex() != ETownType::TOWER && town->town->faction->getIndex() != ETownType::NECROPOLIS;
+	case EWallVisual::MOAT:              return town->hasBuilt(BuildingID::CITADEL) && town->town->clientInfo.siegePositions.at(EWallVisual::MOAT).isValid();
+	case EWallVisual::MOAT_BANK:         return town->hasBuilt(BuildingID::CITADEL) && town->town->clientInfo.siegePositions.at(EWallVisual::MOAT_BANK).isValid();
 	case EWallVisual::KEEP_BATTLEMENT:   return town->hasBuilt(BuildingID::CITADEL) && owner.curInt->cb->battleGetWallState(EWallPart::KEEP) != EWallState::DESTROYED;
 	case EWallVisual::UPPER_BATTLEMENT:  return town->hasBuilt(BuildingID::CASTLE) && owner.curInt->cb->battleGetWallState(EWallPart::UPPER_TOWER) != EWallState::DESTROYED;
 	case EWallVisual::BOTTOM_BATTLEMENT: return town->hasBuilt(BuildingID::CASTLE) && owner.curInt->cb->battleGetWallState(EWallPart::BOTTOM_TOWER) != EWallState::DESTROYED;

client/renderSDL/CursorSoftware.cpp

@@ -53,8 +53,6 @@ void CursorSoftware::createTexture(const Point & dimensions)
 
 void CursorSoftware::updateTexture()
 {
-	Point dimensions(-1, -1);
-
 	if (!cursorSurface ||  Point(cursorSurface->w, cursorSurface->h) != cursorImage->dimensions())
 		createTexture(cursorImage->dimensions());
 

cmake_modules/VCMI_lib.cmake

@@ -133,6 +133,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/spells/BonusCaster.cpp
 		${MAIN_LIB_DIR}/spells/CSpellHandler.cpp
 		${MAIN_LIB_DIR}/spells/ISpellMechanics.cpp
+		${MAIN_LIB_DIR}/spells/ObstacleCasterProxy.cpp
 		${MAIN_LIB_DIR}/spells/Problem.cpp
 		${MAIN_LIB_DIR}/spells/ProxyCaster.cpp
 		${MAIN_LIB_DIR}/spells/TargetCondition.cpp
@@ -147,6 +148,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/spells/effects/Effects.cpp
 		${MAIN_LIB_DIR}/spells/effects/Heal.cpp
 		${MAIN_LIB_DIR}/spells/effects/LocationEffect.cpp
+		${MAIN_LIB_DIR}/spells/effects/Moat.cpp
 		${MAIN_LIB_DIR}/spells/effects/Obstacle.cpp
 		${MAIN_LIB_DIR}/spells/effects/Registry.cpp
 		${MAIN_LIB_DIR}/spells/effects/UnitEffect.cpp

config/creatures/conflux.json

@@ -237,7 +237,7 @@
 			"fireWallVulnerablity" :
 			{
 				"type" : "MORE_DAMAGE_FROM_SPELL",
-				"subtype" : "spell.fireWall",
+				"subtype" : "spell.fireWallTrigger",
 				"val" : 100
 			},
 			"armageddonVulnerablity" :
@@ -431,7 +431,7 @@
 			"fireWallVulnerablity" :
 			{
 				"type" : "MORE_DAMAGE_FROM_SPELL",
-				"subtype" : "spell.fireWall",
+				"subtype" : "spell.fireWallTrigger",
 				"val" : 100
 			},
 			"armageddonVulnerablity" :

config/factions/castle.json

@@ -147,8 +147,7 @@
 			"horde" : [ 2, -1 ],
 			"mageGuild" : 4,
 			"warMachine" : "ballista",
-			"moatDamage" : 70,
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.castleMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :

config/factions/conflux.json

@@ -152,8 +152,7 @@
 			"mageGuild" : 5,
 			"primaryResource" : "mercury",
 			"warMachine" : "ballista",
-			"moatDamage" : 70,
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.castleMoat",
 
 			"buildings" :
 			{

config/factions/dungeon.json

@@ -148,8 +148,8 @@
 			"mageGuild" : 5,
 			"primaryResource" : "sulfur",
 			"warMachine" : "ballista",
-			"moatDamage" : 90,
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.dungeonMoat",
+
 
 			"buildings" :
 			{

config/factions/fortress.json

@@ -147,8 +147,7 @@
 			"horde" : [ 0, -1 ],
 			"mageGuild" : 3,
 			"warMachine" : "firstAidTent",
-			"moatDamage" : 90,
-			"moatHexes" : [ 10, 11, 27, 28, 43, 44, 60, 61, 76, 77, 94, 110, 111, 128, 129, 145, 146, 163, 164, 180, 181 ],
+			"moatAbility" : "core:spell.fortressMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :

config/factions/inferno.json

@@ -149,8 +149,7 @@
 			"mageGuild" : 5,
 			"primaryResource" : "mercury",
 			"warMachine" : "ammoCart",
-			"moatDamage" : 90,
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.infernoMoat",
 
 			"buildings" :
 			{

config/factions/necropolis.json

@@ -152,8 +152,7 @@
 			"horde" : [ 0, -1 ],
 			"mageGuild" : 5,
 			"warMachine" : "firstAidTent",
-			"moatDamage" : 70,
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.necropolisMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :
@@ -222,7 +221,6 @@
 				},
 				"moat" :
 				{
-					"bank" : { "x" : -1, "y" : -1 }, // Should not be present
 					"moat" : { "x" : 406, "y" : 77 }
 				},
 				"static" :

config/factions/rampart.json

@@ -152,8 +152,7 @@
 			"mageGuild" : 5,
 			"primaryResource" : "crystal",
 			"warMachine" : "firstAidTent",
-			"moatDamage" : 70,
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.rampartMoat",
 
 			"buildings" :
 			{

config/factions/stronghold.json

@@ -145,8 +145,7 @@
 			"horde" : [ 0, -1 ],
 			"mageGuild" : 3,
 			"warMachine" : "ammoCart",
-			"moatDamage" : 70,
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.strongholdMoat",
 			// primaryResource not specified so town get both Wood and Ore for resource bonus
 
 			"buildings" :

config/factions/tower.json

@@ -147,8 +147,7 @@
 			"primaryResource" : "gems",
 			"mageGuild" : 5,
 			"warMachine" : "ammoCart",
-			"moatDamage" : 0, //TODO: minefield
-			"moatHexes" : [ 11, 28, 44, 61, 77, 111, 129, 146, 164, 181 ],
+			"moatAbility" : "core:spell.towerMoat",
 
 			"buildings" :
 			{
@@ -210,8 +209,6 @@
 				},
 				"moat" :
 				{
-					"bank" : { "x" : 410, "y" : 80 },
-					"moat" : { "x" : 410, "y" : 90 }
 				},
 				"static" :
 				{

config/gameConfig.json

@@ -80,6 +80,7 @@
 		"config/spells/timed.json",
 		"config/spells/ability.json",
 		"config/spells/vcmiAbility.json"
+		"config/spells/moats.json"
 	],
 	"skills" :
 	[
@@ -164,7 +165,9 @@
 			// every 1 defense point damage influence in battle when defense points > attack points during creature attack
 			"defensePointDamageFactor": 0.025, 
 			// limit of damage reduction that can be achieved by overpowering defense points
-			"defensePointDamageFactorCap": 0.7
+			"defensePointDamageFactorCap": 0.7,
+			// If set to true, double-wide creatures will trigger obstacle effect when moving one tile forward or backwards
+			"oneHexTriggersObstacles": false
 		},	
 
 		"creatures":

config/heroes/conflux.json

@@ -269,7 +269,7 @@
 		"specialty" : {
 			"bonuses" : {
 				"fireWall" : {
-					"subtype" : "spell.fireWall",
+					"subtype" : "spell.fireWallTrigger",
 					"type" : "SPECIFIC_SPELL_DAMAGE",
 					"val" : 100,
 					"valueType" : "BASE_NUMBER"

config/schemas/faction.json

@@ -109,7 +109,7 @@
 			"additionalProperties" : false,
 			"required" : [
 				"mapObject", "buildingsIcons", "buildings", "creatures", "guildWindow", "names",
-				"hallBackground", "hallSlots", "horde", "mageGuild", "moatDamage", "defaultTavern", "tavernVideo", "guildBackground", "musicTheme", "siege", "structures", "townBackground", "warMachine"
+				"hallBackground", "hallSlots", "horde", "mageGuild", "moatAbility", "defaultTavern", "tavernVideo", "guildBackground", "musicTheme", "siege", "structures", "townBackground", "warMachine"
 			],
 			"description": "town",
 			"properties":{
@@ -230,14 +230,9 @@
 					"type":"number",
 					"description": "Maximal level of mage guild"
 				},
-				"moatDamage": {
-					"type":"number",
-					"description": "Damage dealt to creature that entered town moat during siege"
-				},
-				"moatHexes": {
-					"type" : "array",
-					"description" : "Numbers of battlefield hexes affected by moat during siege",
-					"items" : { "type" : "number" }
+				"moatAbility": {
+					"type":"string",
+					"description": "Identifier of ability to use as town moat during siege"
 				},
 				"musicTheme": {
 					"type":"string",

config/schemas/spell.json

@@ -233,7 +233,11 @@
 						"special":{
 								"type": "boolean",
 								"description": "Special spell. Can be given only by Bonus::SPELL"
-						}
+						},
+						"nonMagical":{
+							"type": "boolean",
+							"description": "Non-magical ability. Usually used by some creatures. Should not be affected by sorcery and generic magic resistance. School resistances apply. Examples: dendroid bind, efreet fire shield."
+					}
 				}
 		},
 		"immunity":{

config/spells/ability.json

@@ -119,6 +119,7 @@
 			}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"indifferent": true
 		}
 	},

config/spells/moats.json

@@ -0,0 +1,732 @@
+{
+    "castleMoatTrigger" :
+    {
+        "targetType" : "CREATURE",
+        "type": "ability",
+        "name": "Moat",
+        "school": {},
+        "level": 0,
+        "power": 0,
+        "gainChance": {},
+        "levels" : {
+            "base": {
+                "power" : 0,
+                "range" : "0",
+                "description" : "", //For validation
+                "cost" : 0, //For validation
+                "aiValue" : 0, //For validation
+                "battleEffects" : {
+                    "directDamage" : {
+                        "type":"core:damage"
+                    }
+                },
+                "targetModifier":{"smart":false}
+            },
+            "none" : {
+            },
+            "basic" : {
+            },
+            "advanced" : {
+            },
+            "expert" : {
+            }
+        },
+        "flags" : {
+            "damage": true,
+            "negative": true,
+            "nonMagical" : true,
+            "special": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "castleMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Moat",
+        "school" : {},
+        "level": 0,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : false,
+                        "trap" : true,
+                        "triggerAbility" : "core:castleMoatTrigger",
+                        "dispellable" : false,
+                        "removeOnTrigger" : false,
+                        "moatDamage" : 70,
+                        "moatHexes" : [[11, 28, 44, 61, 77, 111, 129, 146, 164, 181]],
+                        "defender" :{
+                        },
+                        "bonus" :{
+                            "primarySkill" : {
+                                "val" : -3,
+                                "type" : "PRIMARY_SKILL",
+                                "subtype" : "primSkill.defence",
+                                "valueType" : "ADDITIVE_VALUE"
+                            }
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "rampartMoatTrigger" :
+    {
+        "targetType" : "CREATURE",
+        "type": "ability",
+        "name": "Brambles",
+        "school": {},
+        "level": 0,
+        "power": 0,
+        "gainChance": {},
+        "levels" : {
+            "base": {
+                "power" : 0,
+                "range" : "0",
+                "description" : "", //For validation
+                "cost" : 0, //For validation
+                "aiValue" : 0, //For validation
+                "battleEffects" : {
+                    "directDamage" : {
+                        "type":"core:damage"
+                    }
+                },
+                "targetModifier":{"smart":false}
+            },
+            "none" : {
+            },
+            "basic" : {
+            },
+            "advanced" : {
+            },
+            "expert" : {
+            }
+        },
+        "flags" : {
+            "damage": true,
+            "negative": true,
+            "nonMagical" : true,
+            "special": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "rampartMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Brambles",
+        "school" : {},
+        "level": 0,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : false,
+                        "trap" : true,
+                        "triggerAbility" : "core:rampartMoatTrigger",
+                        "dispellable" : false,
+                        "removeOnTrigger" : false,
+                        "moatDamage" : 70,
+                        "moatHexes" : [[11, 28, 44, 61, 77, 111, 129, 146, 164, 181]],
+                        "defender" :{
+                        },
+                        "bonus" :{
+                            "primarySkill" : {
+                                "val" : -3,
+                                "type" : "PRIMARY_SKILL",
+                                "subtype" : "primSkill.defence",
+                                "valueType" : "ADDITIVE_VALUE"
+                            }
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "towerMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Land Mine",
+        "school" : {},
+        "level": 3,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : true,
+                        "trap" : false,
+                        "triggerAbility" : "core:landMineTrigger",
+                        "dispellable" : true,
+                        "removeOnTrigger" : true,
+                        "moatDamage" : 150,
+                        "moatHexes" : [[11], [28], [44], [61], [77], [111], [129], [146], [164], [181]],
+                        "defender" :{
+                            "animation" : "C09SPF1",
+                            "appearAnimation" : "C09SPF0",
+                            "appearSound" : "LANDMINE"
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "infernoMoatTrigger" :
+    {
+        "targetType" : "CREATURE",
+        "type": "ability",
+        "name": "Lava",
+        "school": {},
+        "level": 0,
+        "power": 0,
+        "gainChance": {},
+        "levels" : {
+            "base": {
+                "power" : 0,
+                "range" : "0",
+                "description" : "", //For validation
+                "cost" : 0, //For validation
+                "aiValue" : 0, //For validation
+                "battleEffects" : {
+                    "directDamage" : {
+                        "type":"core:damage"
+                    }
+                },
+                "targetModifier":{"smart":false}
+            },
+            "none" : {
+            },
+            "basic" : {
+            },
+            "advanced" : {
+            },
+            "expert" : {
+            }
+        },
+        "flags" : {
+            "damage": true,
+            "negative": true,
+            "nonMagical" : true,
+            "special": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "infernoMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Lava",
+        "school" : {},
+        "level": 0,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : false,
+                        "trap" : true,
+                        "triggerAbility" : "core:infernoMoatTrigger",
+                        "dispellable" : false,
+                        "removeOnTrigger" : false,
+                        "moatDamage" : 90,
+                        "moatHexes" : [[11, 28, 44, 61, 77, 111, 129, 146, 164, 181]],
+                        "defender" :{
+                        },
+                        "bonus" :{
+                            "primarySkill" : {
+                                "val" : -3,
+                                "type" : "PRIMARY_SKILL",
+                                "subtype" : "primSkill.defence",
+                                "valueType" : "ADDITIVE_VALUE"
+                            }
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "necropolisMoatTrigger" :
+    {
+        "targetType" : "CREATURE",
+        "type": "ability",
+        "name": "Boneyard",
+        "school": {},
+        "level": 0,
+        "power": 0,
+        "gainChance": {},
+        "levels" : {
+            "base": {
+                "power" : 0,
+                "range" : "0",
+                "description" : "", //For validation
+                "cost" : 0, //For validation
+                "aiValue" : 0, //For validation
+                "battleEffects" : {
+                    "directDamage" : {
+                        "type":"core:damage"
+                    }
+                },
+                "targetModifier":{"smart":false}
+            },
+            "none" : {
+            },
+            "basic" : {
+            },
+            "advanced" : {
+            },
+            "expert" : {
+            }
+        },
+        "flags" : {
+            "damage": true,
+            "negative": true,
+            "nonMagical" : true,
+            "special": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "necropolisMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Boneyard",
+        "school" : {},
+        "level": 0,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : false,
+                        "trap" : true,
+                        "triggerAbility" : "core:necropolisMoatTrigger",
+                        "dispellable" : false,
+                        "removeOnTrigger" : false,
+                        "moatDamage" : 70,
+                        "moatHexes" : [[11, 28, 44, 61, 77, 111, 129, 146, 164, 181]],
+                        "defender" :{
+                        },
+                        "bonus" :{
+                            "primarySkill" : {
+                                "val" : -3,
+                                "type" : "PRIMARY_SKILL",
+                                "subtype" : "primSkill.defence",
+                                "valueType" : "ADDITIVE_VALUE"
+                            }
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "dungeonMoatTrigger" :
+    {
+        "targetType" : "CREATURE",
+        "type": "ability",
+        "name": "Boiling Oil",
+        "school": {},
+        "level": 0,
+        "power": 0,
+        "gainChance": {},
+        "levels" : {
+            "base": {
+                "power" : 0,
+                "range" : "0",
+                "description" : "", //For validation
+                "cost" : 0, //For validation
+                "aiValue" : 0, //For validation
+                "battleEffects" : {
+                    "directDamage" : {
+                        "type":"core:damage"
+                    }
+                },
+                "targetModifier":{"smart":false}
+            },
+            "none" : {
+            },
+            "basic" : {
+            },
+            "advanced" : {
+            },
+            "expert" : {
+            }
+        },
+        "flags" : {
+            "damage": true,
+            "negative": true,
+            "nonMagical" : true,
+            "special": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "dungeonMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Boiling Oil",
+        "school" : {},
+        "level": 0,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : false,
+                        "trap" : true,
+                        "triggerAbility" : "core:dungeonMoatTrigger",
+                        "dispellable" : false,
+                        "removeOnTrigger" : false,
+                        "moatDamage" : 90,
+                        "moatHexes" : [[11, 28, 44, 61, 77, 111, 129, 146, 164, 181]],
+                        "defender" :{
+                        },
+                        "bonus" :{
+                            "primarySkill" : {
+                                "val" : -3,
+                                "type" : "PRIMARY_SKILL",
+                                "subtype" : "primSkill.defence",
+                                "valueType" : "ADDITIVE_VALUE"
+                            }
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "strongholdMoatTrigger" :
+    {
+        "targetType" : "CREATURE",
+        "type": "ability",
+        "name": "Wooden Spikes",
+        "school": {},
+        "level": 0,
+        "power": 0,
+        "gainChance": {},
+        "levels" : {
+            "base": {
+                "power" : 0,
+                "range" : "0",
+                "description" : "", //For validation
+                "cost" : 0, //For validation
+                "aiValue" : 0, //For validation
+                "battleEffects" : {
+                    "directDamage" : {
+                        "type":"core:damage"
+                    }
+                },
+                "targetModifier":{"smart":false}
+            },
+            "none" : {
+            },
+            "basic" : {
+            },
+            "advanced" : {
+            },
+            "expert" : {
+            }
+        },
+        "flags" : {
+            "damage": true,
+            "negative": true,
+            "nonMagical" : true,
+            "special": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "strongholdMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Wooden Spikes",
+        "school" : {},
+        "level": 0,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : false,
+                        "trap" : true,
+                        "triggerAbility" : "core:strongholdMoatTrigger",
+                        "dispellable" : false,
+                        "removeOnTrigger" : false,
+                        "moatDamage" : 70,
+                        "moatHexes" : [[11, 28, 44, 61, 77, 111, 129, 146, 164, 181]],
+                        "defender" :{
+                        },
+                        "bonus" :{
+                            "primarySkill" : {
+                                "val" : -3,
+                                "type" : "PRIMARY_SKILL",
+                                "subtype" : "primSkill.defence",
+                                "valueType" : "ADDITIVE_VALUE"
+                            }
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "fortressMoatTrigger" :
+    {
+        "targetType" : "CREATURE",
+        "type": "ability",
+        "name": "Boiling Tar",
+        "school": {},
+        "level": 0,
+        "power": 0,
+        "gainChance": {},
+        "levels" : {
+            "base": {
+                "power" : 0,
+                "range" : "0",
+                "description" : "", //For validation
+                "cost" : 0, //For validation
+                "aiValue" : 0, //For validation
+                "battleEffects" : {
+                    "directDamage" : {
+                        "type":"core:damage"
+                    }
+                },
+                "targetModifier":{"smart":false}
+            },
+            "none" : {
+            },
+            "basic" : {
+            },
+            "advanced" : {
+            },
+            "expert" : {
+            }
+        },
+        "flags" : {
+            "damage": true,
+            "negative": true,
+            "nonMagical" : true,
+            "special": true
+        },
+        "targetCondition" : {
+        }
+    },
+    "fortressMoat": {
+        "targetType" : "NO_TARGET",
+        "type": "ability",
+        "name": "Boiling Tar",
+        "school" : {},
+        "level": 0,
+        "power": 0,
+        "defaultGainChance": 0,
+        "gainChance": {},
+        "levels" : {
+            "base":{
+                "description" : "",
+                "aiValue" : 0,
+                "power" : 0,
+                "cost" : 0,
+                "targetModifier":{"smart":false},
+                "battleEffects":{
+                    "moat":{
+                        "type":"core:moat",
+                        "hidden" : false,
+                        "trap" : true,
+                        "triggerAbility" : "core:fortressMoatTrigger",
+                        "dispellable" : false,
+                        "removeOnTrigger" : false,
+                        "moatDamage" : 90,
+                        "moatHexes" : [[10, 11, 27, 28, 43, 44, 60, 61, 76, 77, 94, 110, 111, 128, 129, 145, 146, 163, 164, 180, 181]],
+                        "defender" :{
+                        },
+                        "bonus" :{
+                            "primarySkill" : {
+                                "val" : -3,
+                                "type" : "PRIMARY_SKILL",
+                                "subtype" : "primSkill.defence",
+                                "valueType" : "ADDITIVE_VALUE"
+                            }
+                        }
+                    }
+                },
+                "range" : "X"
+            },
+            "none" :{
+            },
+            "basic" :{
+            },
+            "advanced" :{
+            },
+            "expert" :{
+            }
+        },
+        "flags" : {
+            "nonMagical" : true,
+            "indifferent": true
+        },
+        "targetCondition" : {
+        }
+    }
+}

config/spells/other.json

@@ -15,7 +15,6 @@
 						"hidden" : true,
 						"passable" : true,
 						"trap" : true,
-						"trigger" : false,
 						"patchCount" : 4,
 						"turnsRemaining" : -1,
 						"attacker" :{
@@ -52,6 +51,65 @@
 			"indifferent": true
 		}
 	},
+	"landMineTrigger" :
+	{
+		"targetType" : "CREATURE",
+		"type": "combat",
+		"name": "Land Mine",
+		"school":
+		{
+			"air": false,
+			"earth": false,
+			"fire": true,
+			"water": false
+		},
+		"level": 3,
+		"power": 10,
+		"gainChance": {},
+		"animation" : {
+			"hit" : ["C09SPF3"]
+		},
+		"sounds" : {
+		   "cast" : "LANDKILL"
+		},
+		"levels" : {
+			"base": {
+				"power" : 25,
+				"range" : "0",
+				"description" : "", //For validation
+				"cost" : 0, //For validation
+				"aiValue" : 0, //For validation
+				"battleEffects" : {
+					"directDamage" : {
+						"type":"core:damage"
+					}
+				},
+				"targetModifier":{"smart":false}
+			},
+			"none" : {
+				"power" : 25
+			},
+			"basic" : {
+				"power" : 25
+			},
+			"advanced" : {
+				"power" : 50
+			},
+			"expert" : {
+				"power" : 100
+			}
+		},
+		"flags" : {
+			"damage": true,
+			"negative": true,
+			"special": true
+		},
+		"targetCondition" : {
+			"noneOf" : {
+				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
+			}
+		}
+	},
 	"landMine" : {
 		"index" : 11,
 		"targetType" : "NO_TARGET",
@@ -68,23 +126,19 @@
 						"hidden" : true,
 						"passable" : true,
 						"trap" : false,
-						"trigger" : true,
+						"triggerAbility" : "core:landMineTrigger",
 						"removeOnTrigger" : true,
 						"patchCount" : 4,
 						"turnsRemaining" : -1,
 						"attacker" :{
 							"animation" : "C09SPF1",
 							"appearAnimation" : "C09SPF0",
-							"appearSound" : "LANDMINE",
-							"triggerAnimation" : "C09SPF3",
-							"triggerSound" : "LANDKILL"
+							"appearSound" : "LANDMINE"
 						},
 						"defender" :{
 							"animation" : "C09SPF1",
 							"appearAnimation" : "C09SPF0",
-							"appearSound" : "LANDMINE",
-							"triggerAnimation" : "C09SPF3",
-							"triggerSound" : "LANDKILL"
+							"appearSound" : "LANDMINE"
 						}
 					},
 					"damage":{
@@ -138,8 +192,6 @@
 						"hidden" : false,
 						"passable" : false,
 						"trap" : false,
-						"trigger" : false,
-						"patchCount" : 1,
 						"turnsRemaining" : 2,
 						"attacker" :{
 							"range" : [[""]],
@@ -193,6 +245,58 @@
 			"indifferent": true
 		}
 	},
+	"fireWallTrigger" : {
+		"targetType" : "CREATURE",
+		"type": "combat",
+		"name": "Fire Wall",
+		"school":
+		{
+			"air": false,
+			"earth": false,
+			"fire": true,
+			"water": false
+		},
+		"level": 2,
+		"power": 10,
+		"gainChance": {},
+		"levels" : {
+			"base": {
+				"power" : 10,
+				"range" : "0",
+				"description" : "", //For validation
+				"cost" : 0, //For validation
+				"aiValue" : 0, //For validation
+				"battleEffects" : {
+					"directDamage" : {
+						"type":"core:damage"
+					}
+				},
+				"targetModifier":{"smart":false}
+			},
+			"none" : {
+				"power" : 10
+			},
+			"basic" : {
+				"power" : 10
+			},
+			"advanced" : {
+				"power" : 20
+			},
+			"expert" : {
+				"power" : 50
+			}
+		},
+		"flags" : {
+			"damage": true,
+			"negative": true,
+			"special": true
+		},
+		"targetCondition" : {
+			"noneOf" : {
+				"bonus.DIRECT_DAMAGE_IMMUNITY" : "normal"
+			}
+		}
+	},
 	"fireWall" : {
 		"index" : 13,
 		"targetType" : "LOCATION",
@@ -212,8 +316,7 @@
 						"hidden" : false,
 						"passable" : true,
 						"trap" : false,
-						"trigger" : true,
-						"patchCount" : 1,
+						"triggerAbility" : "core:fireWallTrigger",
 						"turnsRemaining" : 2,
 						"attacker" :{
 							"shape" : [[""]],
@@ -227,11 +330,6 @@
 							"animation" : "C07SPF61",
 							"appearAnimation" : "C07SPF60"
 						}
-					},
-					"damage":{
-						"type":"core:damage",
-						"optional":false,
-						"indirect":true
 					}
 				}
 			},

config/spells/vcmiAbility.json

@@ -93,10 +93,10 @@
 			}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"positive": true
 		},
 		"targetCondition" : {
-			"nonMagical" : true,
 			"noneOf" : {
 				"bonus.SIEGE_WEAPON" : "absolute"
 			}
@@ -179,10 +179,8 @@
 			}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"indifferent": true
-		},
-		"targetCondition" : {
-			"nonMagical" : true
 		}
 	},
 	"cyclopsShot" : {
@@ -227,10 +225,8 @@
 			"expert" : {}
 		},
 		"flags" : {
+			"nonMagical" : true,
 			"indifferent": true
-		},
-		"targetCondition" : {
-			"nonMagical" : true
 		}
 	}
 }

include/vcmi/spells/Spell.h

@@ -41,6 +41,7 @@ public:
 	virtual bool isDamage() const = 0;
 	virtual bool isOffensive() const = 0;
 	virtual bool isSpecial() const = 0;
+	virtual bool isMagical() const = 0; //Should this spell considered as magical effect or as ability (like dendroid's bind)
 
 	virtual void forEachSchool(const SchoolCallback & cb) const = 0;
 	virtual const std::string & getCastSound() const = 0;

lib/CTownHandler.cpp

@@ -173,7 +173,7 @@ void CFaction::serializeJson(JsonSerializeFormat & handler)
 
 
 CTown::CTown()
-	: faction(nullptr), mageLevel(0), primaryRes(0), moatDamage(0), defaultTavernChance(0)
+	: faction(nullptr), mageLevel(0), primaryRes(0), moatAbility(SpellID::NONE), defaultTavernChance(0)
 {
 }
 
@@ -769,6 +769,9 @@ void CTownHandler::loadTownHall(CTown &town, const JsonNode & source) const
 
 Point JsonToPoint(const JsonNode & node)
 {
+	if(!node.isStruct())
+		return Point::makeInvalid();
+
 	Point ret;
 	ret.x = static_cast<si32>(node["x"].Float());
 	ret.y = static_cast<si32>(node["y"].Float());
@@ -873,9 +876,6 @@ void CTownHandler::loadTown(CTown * town, const JsonNode & source)
 
 	warMachinesToLoad[town] = source["warMachine"];
 
-	town->moatDamage = static_cast<si32>(source["moatDamage"].Float());
-	town->moatHexes = source["moatHexes"].convertTo<std::vector<BattleHex> >();
-
 	town->mageLevel = static_cast<ui32>(source["mageGuild"].Float());
 
 	town->namesCount = 0;
@@ -885,6 +885,11 @@ void CTownHandler::loadTown(CTown * town, const JsonNode & source)
 		town->namesCount += 1;
 	}
 
+	VLC->modh->identifiers.requestIdentifier(source["moatAbility"], [=](si32 ability)
+	{
+		town->moatAbility = SpellID(ability);
+	});
+
 	//  Horde building creature level
 	for(const JsonNode &node : source["horde"].Vector())
 		town->hordeLvl[static_cast<int>(town->hordeLvl.size())] = static_cast<int>(node.Float());

lib/CTownHandler.h

@@ -271,8 +271,7 @@ public:
 	ui32 mageLevel; //max available mage guild level
 	ui16 primaryRes;
 	ArtifactID warMachine;
-	si32 moatDamage;
-	std::vector<BattleHex> moatHexes;
+	SpellID moatAbility;
 	// default chance for hero of specific class to appear in tavern, if field "tavern" was not set
 	// resulting chance = sqrt(town.chance * heroClass.chance)
 	ui32 defaultTavernChance;
@@ -339,8 +338,7 @@ public:
 		h & primaryRes;
 		h & warMachine;
 		h & clientInfo;
-		h & moatDamage;
-		h & moatHexes;
+		h & moatAbility;
 		h & defaultTavernChance;
 	}
 	

lib/GameSettings.cpp

@@ -58,6 +58,7 @@ void GameSettings::load(const JsonNode & input)
 		{EGameSettings::COMBAT_DEFENSE_POINT_DAMAGE_FACTOR_CAP, "combat",    "defensePointDamageFactorCap"},
 		{EGameSettings::COMBAT_GOOD_LUCK_DICE,                  "combat",    "goodLuckDice"               },
 		{EGameSettings::COMBAT_GOOD_MORALE_DICE,                "combat",    "goodMoraleDice"             },
+		{EGameSettings::COMBAT_ONE_HEX_TRIGGERS_OBSTACLES,      "combat",    "oneHexTriggersObstacles"   },
 		{EGameSettings::CREATURES_ALLOW_ALL_FOR_DOUBLE_MONTH,   "creatures", "allowAllForDoubleMonth"     },
 		{EGameSettings::CREATURES_ALLOW_RANDOM_SPECIAL_WEEKS,   "creatures", "allowRandomSpecialWeeks"    },
 		{EGameSettings::CREATURES_DAILY_STACK_EXPERIENCE,       "creatures", "dailyStackExperience"       },

lib/GameSettings.h

@@ -54,6 +54,7 @@ enum class EGameSettings
 	TEXTS_TERRAIN,
 	TOWNS_BUILDINGS_PER_TURN_CAP,
 	TOWNS_STARTING_DWELLING_CHANCES,
+	COMBAT_ONE_HEX_TRIGGERS_OBSTACLES,
 
 	OPTIONS_COUNT
 };

lib/HeroBonus.cpp

@@ -76,7 +76,8 @@ const std::map<std::string, TLimiterPtr> bonusLimiterMap =
 	{"IS_UNDEAD", std::make_shared<HasAnotherBonusLimiter>(Bonus::UNDEAD)},
 	{"CREATURE_NATIVE_TERRAIN", std::make_shared<CreatureTerrainLimiter>()},
 	{"CREATURE_FACTION", std::make_shared<CreatureFactionLimiter>()},
-	{"OPPOSITE_SIDE", std::make_shared<OppositeSideLimiter>()}
+	{"OPPOSITE_SIDE", std::make_shared<OppositeSideLimiter>()},
+	{"UNIT_ON_HEXES", std::make_shared<UnitOnHexLimiter>()}
 };
 
 const std::map<std::string, TPropagatorPtr> bonusPropagatorMap =
@@ -369,8 +370,8 @@ JsonNode CAddInfo::toJsonNode() const
 	}
 }
 
-std::atomic<int32_t> CBonusSystemNode::treeChanged(1);
-const bool CBonusSystemNode::cachingEnabled = true;
+std::atomic<int64_t> CBonusSystemNode::treeChanged(1);
+constexpr bool CBonusSystemNode::cachingEnabled = true;
 
 BonusList::BonusList(bool BelongsToTree) : belongsToTree(BelongsToTree)
 {
@@ -1499,20 +1500,20 @@ void CBonusSystemNode::limitBonuses(const BonusList &allBonuses, BonusList &out)
 		{
 			auto b = undecided[i];
 			BonusLimitationContext context = {b, *this, out, undecided};
-			int decision = b->limiter ? b->limiter->limit(context) : ILimiter::ACCEPT; //bonuses without limiters will be accepted by default
-			if(decision == ILimiter::DISCARD)
+			auto decision = b->limiter ? b->limiter->limit(context) : ILimiter::EDecision::ACCEPT; //bonuses without limiters will be accepted by default
+			if(decision == ILimiter::EDecision::DISCARD)
 			{
 				undecided.erase(i);
 				i--; continue;
 			}
-			else if(decision == ILimiter::ACCEPT)
+			else if(decision == ILimiter::EDecision::ACCEPT)
 			{
 				accepted.push_back(b);
 				undecided.erase(i);
 				i--; continue;
 			}
 			else
-				assert(decision == ILimiter::NOT_SURE);
+				assert(decision == ILimiter::EDecision::NOT_SURE);
 		}
 
 		if(undecided.size() == undecidedCount) //we haven't moved a single bonus -> limiters reached a stable state
@@ -1534,22 +1535,7 @@ void CBonusSystemNode::treeHasChanged()
 
 int64_t CBonusSystemNode::getTreeVersion() const
 {
-	int64_t ret = treeChanged;
-	return ret << 32;
-}
-
-int NBonus::valOf(const CBonusSystemNode *obj, Bonus::BonusType type, int subtype)
-{
-	if(obj)
-		return obj->valOfBonuses(type, subtype);
-	return 0;
-}
-
-bool NBonus::hasOfType(const CBonusSystemNode *obj, Bonus::BonusType type, int subtype)
-{
-	if(obj)
-		return obj->hasBonusOfType(type, subtype);
-	return false;
+	return treeChanged;
 }
 
 std::string Bonus::Description(boost::optional<si32> customValue) const
@@ -2070,21 +2056,6 @@ namespace Selector
 
 	DLL_LINKAGE CSelector all([](const Bonus * b){return true;});
 	DLL_LINKAGE CSelector none([](const Bonus * b){return false;});
-
-	bool DLL_LINKAGE matchesType(const CSelector &sel, Bonus::BonusType type)
-	{
-		Bonus dummy;
-		dummy.type = type;
-		return sel(&dummy);
-	}
-
-	bool DLL_LINKAGE matchesTypeSubtype(const CSelector &sel, Bonus::BonusType type, TBonusSubtype subtype)
-	{
-		Bonus dummy;
-		dummy.type = type;
-		dummy.subtype = subtype;
-		return sel(&dummy);
-	}
 }
 
 const CCreature * retrieveCreature(const CBonusSystemNode *node)
@@ -2165,9 +2136,9 @@ std::shared_ptr<Bonus> Bonus::addLimiter(const TLimiterPtr & Limiter)
 	return this->shared_from_this();
 }
 
-int ILimiter::limit(const BonusLimitationContext &context) const /*return true to drop the bonus */
+ILimiter::EDecision ILimiter::limit(const BonusLimitationContext &context) const /*return true to drop the bonus */
 {
-	return false;
+	return ILimiter::EDecision::ACCEPT;
 }
 
 std::string ILimiter::toString() const
@@ -2182,12 +2153,14 @@ JsonNode ILimiter::toJsonNode() const
 	return root;
 }
 
-int CCreatureTypeLimiter::limit(const BonusLimitationContext &context) const
+ILimiter::EDecision CCreatureTypeLimiter::limit(const BonusLimitationContext &context) const
 {
 	const CCreature *c = retrieveCreature(&context.node);
 	if(!c)
-		return true;
-	return c->getId() != creature->getId() && (!includeUpgrades || !creature->isMyUpgrade(c));
+		return ILimiter::EDecision::DISCARD;
+	
+	auto accept =  c->getId() == creature->getId() || (includeUpgrades && creature->isMyUpgrade(c));
+	return accept ? ILimiter::EDecision::ACCEPT : ILimiter::EDecision::DISCARD;
 	//drop bonus if it's not our creature and (we don`t check upgrades or its not our upgrade)
 }
 
@@ -2239,7 +2212,7 @@ HasAnotherBonusLimiter::HasAnotherBonusLimiter(Bonus::BonusType bonus, TBonusSub
 {
 }
 
-int HasAnotherBonusLimiter::limit(const BonusLimitationContext &context) const
+ILimiter::EDecision HasAnotherBonusLimiter::limit(const BonusLimitationContext &context) const
 {
 	//TODO: proper selector config with parsing of JSON
 	auto mySelector = Selector::type()(type);
@@ -2253,14 +2226,14 @@ int HasAnotherBonusLimiter::limit(const BonusLimitationContext &context) const
 
 	//if we have a bonus of required type accepted, limiter should accept also this bonus
 	if(context.alreadyAccepted.getFirst(mySelector))
-		return ACCEPT;
+		return ILimiter::EDecision::ACCEPT;
 
 	//if there are no matching bonuses pending, we can (and must) reject right away
 	if(!context.stillUndecided.getFirst(mySelector))
-		return DISCARD;
+		return ILimiter::EDecision::DISCARD;
 
 	//do not accept for now but it may change if more bonuses gets included
-	return NOT_SURE;
+	return ILimiter::EDecision::NOT_SURE;
 }
 
 std::string HasAnotherBonusLimiter::toString() const
@@ -2296,6 +2269,35 @@ JsonNode HasAnotherBonusLimiter::toJsonNode() const
 	return root;
 }
 
+ILimiter::EDecision UnitOnHexLimiter::limit(const BonusLimitationContext &context) const
+{
+	const auto * stack = retrieveStackBattle(&context.node);
+	if(!stack)
+		return ILimiter::EDecision::DISCARD;
+
+	auto accept = false;
+	for (const auto & hex : stack->getHexes())
+		accept |= !!applicableHexes.count(hex);
+
+	return accept ? ILimiter::EDecision::ACCEPT : ILimiter::EDecision::DISCARD;
+}
+
+UnitOnHexLimiter::UnitOnHexLimiter(const std::set<BattleHex> & applicableHexes):
+	applicableHexes(applicableHexes)
+{
+}
+
+JsonNode UnitOnHexLimiter::toJsonNode() const
+{
+	JsonNode root(JsonNode::JsonType::DATA_STRUCT);
+
+	root["type"].String() = "UNIT_ON_HEXES";
+	for(const auto & hex : applicableHexes)
+		root["parameters"].Vector().push_back(JsonUtils::intNode(hex));
+
+	return root;
+}
+
 bool IPropagator::shouldBeAttached(CBonusSystemNode *dest)
 {
 	return false;
@@ -2337,21 +2339,19 @@ CreatureTerrainLimiter::CreatureTerrainLimiter(TerrainId terrain):
 {
 }
 
-int CreatureTerrainLimiter::limit(const BonusLimitationContext &context) const
+ILimiter::EDecision CreatureTerrainLimiter::limit(const BonusLimitationContext &context) const
 {
 	const CStack *stack = retrieveStackBattle(&context.node);
 	if(stack)
 	{
-		if (terrainType == ETerrainId::NATIVE_TERRAIN)//terrainType not specified = native
-		{
-			return !stack->isOnNativeTerrain();
-		}
-		else
-		{
-			return !stack->isOnTerrain(terrainType);
-		}
+		if (terrainType == ETerrainId::NATIVE_TERRAIN && stack->isOnNativeTerrain())//terrainType not specified = native
+			return ILimiter::EDecision::ACCEPT;
+
+		if(terrainType != ETerrainId::NATIVE_TERRAIN && stack->isOnTerrain(terrainType))
+			return ILimiter::EDecision::ACCEPT;
+
 	}
-	return true;
+	return ILimiter::EDecision::DISCARD;
 	//TODO neutral creatues
 }
 
@@ -2384,10 +2384,11 @@ CreatureFactionLimiter::CreatureFactionLimiter():
 {
 }
 
-int CreatureFactionLimiter::limit(const BonusLimitationContext &context) const
+ILimiter::EDecision CreatureFactionLimiter::limit(const BonusLimitationContext &context) const
 {
 	const CCreature *c = retrieveCreature(&context.node);
-	return !c || c->faction != faction; //drop bonus for non-creatures or non-native residents
+	auto accept = c && c->faction == faction;
+	return accept ? ILimiter::EDecision::ACCEPT : ILimiter::EDecision::DISCARD; //drop bonus for non-creatures or non-native residents
 }
 
 std::string CreatureFactionLimiter::toString() const
@@ -2417,23 +2418,19 @@ CreatureAlignmentLimiter::CreatureAlignmentLimiter(si8 Alignment)
 {
 }
 
-int CreatureAlignmentLimiter::limit(const BonusLimitationContext &context) const
+ILimiter::EDecision CreatureAlignmentLimiter::limit(const BonusLimitationContext &context) const
 {
-	const CCreature *c = retrieveCreature(&context.node);
-	if(!c)
-		return true;
-	switch(alignment)
-	{
-	case EAlignment::GOOD:
-		return !c->isGood(); //if not good -> return true (drop bonus)
-	case EAlignment::NEUTRAL:
-		return c->isEvil() || c->isGood();
-	case EAlignment::EVIL:
-		return !c->isEvil();
-	default:
-		logBonus->warn("Warning: illegal alignment in limiter!");
-		return true;
+	const auto * c = retrieveCreature(&context.node);
+	if(c) {
+		if(alignment == EAlignment::GOOD && c->isGood())
+			return ILimiter::EDecision::ACCEPT;
+		if(alignment == EAlignment::EVIL && c->isEvil())
+			return ILimiter::EDecision::ACCEPT;
+		if(alignment == EAlignment::NEUTRAL && !c->isEvil() && !c->isGood())
+			return ILimiter::EDecision::ACCEPT;
 	}
+
+	return ILimiter::EDecision::DISCARD;
 }
 
 std::string CreatureAlignmentLimiter::toString() const
@@ -2463,28 +2460,29 @@ RankRangeLimiter::RankRangeLimiter()
 	minRank = maxRank = -1;
 }
 
-int RankRangeLimiter::limit(const BonusLimitationContext &context) const
+ILimiter::EDecision RankRangeLimiter::limit(const BonusLimitationContext &context) const
 {
 	const CStackInstance * csi = retrieveStackInstance(&context.node);
 	if(csi)
 	{
 		if (csi->getNodeType() == CBonusSystemNode::COMMANDER) //no stack exp bonuses for commander creatures
-			return true;
-		return csi->getExpRank() < minRank || csi->getExpRank() > maxRank;
+			return ILimiter::EDecision::DISCARD;
+		if (csi->getExpRank() > minRank && csi->getExpRank() < maxRank)
+			return ILimiter::EDecision::ACCEPT;
 	}
-	return true;
+	return ILimiter::EDecision::DISCARD;
 }
 
-int StackOwnerLimiter::limit(const BonusLimitationContext &context) const
+ILimiter::EDecision StackOwnerLimiter::limit(const BonusLimitationContext &context) const
 {
 	const CStack * s = retrieveStackBattle(&context.node);
-	if(s)
-		return s->owner != owner;
+	if(s && s->owner == owner)
+		return ILimiter::EDecision::ACCEPT;
 
 	const CStackInstance * csi = retrieveStackInstance(&context.node);
-	if(csi && csi->armyObj)
-		return csi->armyObj->tempOwner != owner;
-	return true;
+	if(csi && csi->armyObj && csi->armyObj->tempOwner == owner)
+		return ILimiter::EDecision::ACCEPT;
+	return ILimiter::EDecision::DISCARD;
 }
 
 StackOwnerLimiter::StackOwnerLimiter()
@@ -2507,10 +2505,10 @@ OppositeSideLimiter::OppositeSideLimiter(const PlayerColor & Owner):
 {
 }
 
-int OppositeSideLimiter::limit(const BonusLimitationContext & context) const
+ILimiter::EDecision OppositeSideLimiter::limit(const BonusLimitationContext & context) const
 {
 	auto contextOwner = CBonusSystemNode::retrieveNodeOwner(& context.node);
-	auto decision = (owner == contextOwner || owner == PlayerColor::CANNOT_DETERMINE) ? ILimiter::DISCARD : ILimiter::ACCEPT;
+	auto decision = (owner == contextOwner || owner == PlayerColor::CANNOT_DETERMINE) ? ILimiter::EDecision::DISCARD : ILimiter::EDecision::ACCEPT;
 	return decision;
 }
 
@@ -2537,20 +2535,20 @@ const std::string & AllOfLimiter::getAggregator() const
 	return aggregator;
 }
 
-int AllOfLimiter::limit(const BonusLimitationContext & context) const
+ILimiter::EDecision AllOfLimiter::limit(const BonusLimitationContext & context) const
 {
 	bool wasntSure = false;
 
 	for(const auto & limiter : limiters)
 	{
 		auto result = limiter->limit(context);
-		if(result == ILimiter::DISCARD)
+		if(result == ILimiter::EDecision::DISCARD)
 			return result;
-		if(result == ILimiter::NOT_SURE)
+		if(result == ILimiter::EDecision::NOT_SURE)
 			wasntSure = true;
 	}
 
-	return wasntSure ? ILimiter::NOT_SURE : ILimiter::ACCEPT;
+	return wasntSure ? ILimiter::EDecision::NOT_SURE : ILimiter::EDecision::ACCEPT;
 }
 
 const std::string AnyOfLimiter::aggregator = "anyOf";
@@ -2559,20 +2557,20 @@ const std::string & AnyOfLimiter::getAggregator() const
 	return aggregator;
 }
 
-int AnyOfLimiter::limit(const BonusLimitationContext & context) const
+ILimiter::EDecision AnyOfLimiter::limit(const BonusLimitationContext & context) const
 {
 	bool wasntSure = false;
 
 	for(const auto & limiter : limiters)
 	{
 		auto result = limiter->limit(context);
-		if(result == ILimiter::ACCEPT)
+		if(result == ILimiter::EDecision::ACCEPT)
 			return result;
-		if(result == ILimiter::NOT_SURE)
+		if(result == ILimiter::EDecision::NOT_SURE)
 			wasntSure = true;
 	}
 
-	return wasntSure ? ILimiter::NOT_SURE : ILimiter::DISCARD;
+	return wasntSure ? ILimiter::EDecision::NOT_SURE : ILimiter::EDecision::DISCARD;
 }
 
 const std::string NoneOfLimiter::aggregator = "noneOf";
@@ -2581,20 +2579,20 @@ const std::string & NoneOfLimiter::getAggregator() const
 	return aggregator;
 }
 
-int NoneOfLimiter::limit(const BonusLimitationContext & context) const
+ILimiter::EDecision NoneOfLimiter::limit(const BonusLimitationContext & context) const
 {
 	bool wasntSure = false;
 
 	for(const auto & limiter : limiters)
 	{
 		auto result = limiter->limit(context);
-		if(result == ILimiter::ACCEPT)
-			return ILimiter::DISCARD;
-		if(result == ILimiter::NOT_SURE)
+		if(result == ILimiter::EDecision::ACCEPT)
+			return ILimiter::EDecision::DISCARD;
+		if(result == ILimiter::EDecision::NOT_SURE)
 			wasntSure = true;
 	}
 
-	return wasntSure ? ILimiter::NOT_SURE : ILimiter::ACCEPT;
+	return wasntSure ? ILimiter::EDecision::NOT_SURE : ILimiter::EDecision::ACCEPT;
 }
 
 // Updaters

lib/HeroBonus.h

@@ -11,6 +11,7 @@
 
 #include "GameConstants.h"
 #include "JsonNode.h"
+#include "battle/BattleHex.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -34,9 +35,9 @@ typedef std::vector<CBonusSystemNode *> TNodesVector;
 
 class CSelector : std::function<bool(const Bonus*)>
 {
-	typedef std::function<bool(const Bonus*)> TBase;
+	using TBase = std::function<bool(const Bonus*)>;
 public:
-	CSelector() {}
+	CSelector() = default;
 	template<typename T>
 	CSelector(const T &t,	//SFINAE trick -> include this c-tor in overload resolution only if parameter is class
 							//(includes functors, lambdas) or function. Without that VC is going mad about ambiguities.
@@ -684,11 +685,11 @@ struct BonusLimitationContext
 class DLL_LINKAGE ILimiter
 {
 public:
-	enum EDecision {ACCEPT, DISCARD, NOT_SURE};
+	enum class EDecision : uint8_t {ACCEPT, DISCARD, NOT_SURE};
 
 	virtual ~ILimiter() = default;
 
-	virtual int limit(const BonusLimitationContext &context) const; //0 - accept bonus; 1 - drop bonus; 2 - delay (drops eventually)
+	virtual EDecision limit(const BonusLimitationContext &context) const; //0 - accept bonus; 1 - drop bonus; 2 - delay (drops eventually)
 	virtual std::string toString() const;
 	virtual JsonNode toJsonNode() const;
 
@@ -781,7 +782,7 @@ private:
 	static const bool cachingEnabled;
 	mutable BonusList cachedBonuses;
 	mutable int64_t cachedLast;
-	static std::atomic<int32_t> treeChanged;
+	static std::atomic<int64_t> treeChanged;
 
 	// Setting a value to cachingStr before getting any bonuses caches the result for later requests.
 	// This string needs to be unique, that's why it has to be setted in the following manner:
@@ -905,13 +906,6 @@ public:
 	}
 };
 
-namespace NBonus
-{
-	//set of methods that may be safely called with nullptr objs
-	DLL_LINKAGE int valOf(const CBonusSystemNode *obj, Bonus::BonusType type, int subtype = -1); //subtype -> subtype of bonus, if -1 then any
-	DLL_LINKAGE bool hasOfType(const CBonusSystemNode *obj, Bonus::BonusType type, int subtype = -1);//determines if hero has a bonus of given type (and optionally subtype)
-}
-
 template<typename T>
 class CSelectFieldEqual
 {
@@ -1019,7 +1013,7 @@ protected:
 	const std::string & getAggregator() const override;
 public:
 	static const std::string aggregator;
-	int limit(const BonusLimitationContext & context) const override;
+	EDecision limit(const BonusLimitationContext & context) const override;
 };
 
 class DLL_LINKAGE AnyOfLimiter : public AggregateLimiter
@@ -1028,7 +1022,7 @@ protected:
 	const std::string & getAggregator() const override;
 public:
 	static const std::string aggregator;
-	int limit(const BonusLimitationContext & context) const override;
+	EDecision limit(const BonusLimitationContext & context) const override;
 };
 
 class DLL_LINKAGE NoneOfLimiter : public AggregateLimiter
@@ -1037,7 +1031,7 @@ protected:
 	const std::string & getAggregator() const override;
 public:
 	static const std::string aggregator;
-	int limit(const BonusLimitationContext & context) const override;
+	EDecision limit(const BonusLimitationContext & context) const override;
 };
 
 class DLL_LINKAGE CCreatureTypeLimiter : public ILimiter //affect only stacks of given creature (and optionally it's upgrades)
@@ -1050,9 +1044,9 @@ public:
 	CCreatureTypeLimiter(const CCreature & creature_, bool IncludeUpgrades);
 	void setCreature(const CreatureID & id);
 
-	int limit(const BonusLimitationContext &context) const override;
-	virtual std::string toString() const override;
-	virtual JsonNode toJsonNode() const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
+	std::string toString() const override;
+	JsonNode toJsonNode() const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1078,9 +1072,9 @@ public:
 	HasAnotherBonusLimiter(Bonus::BonusType bonus, Bonus::BonusSource src);
 	HasAnotherBonusLimiter(Bonus::BonusType bonus, TBonusSubtype _subtype, Bonus::BonusSource src);
 
-	int limit(const BonusLimitationContext &context) const override;
-	virtual std::string toString() const override;
-	virtual JsonNode toJsonNode() const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
+	std::string toString() const override;
+	JsonNode toJsonNode() const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1102,9 +1096,9 @@ public:
 	CreatureTerrainLimiter();
 	CreatureTerrainLimiter(TerrainId terrain);
 
-	int limit(const BonusLimitationContext &context) const override;
-	virtual std::string toString() const override;
-	virtual JsonNode toJsonNode() const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
+	std::string toString() const override;
+	JsonNode toJsonNode() const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1120,9 +1114,9 @@ public:
 	CreatureFactionLimiter();
 	CreatureFactionLimiter(TFaction faction);
 
-	int limit(const BonusLimitationContext &context) const override;
-	virtual std::string toString() const override;
-	virtual JsonNode toJsonNode() const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
+	std::string toString() const override;
+	JsonNode toJsonNode() const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1138,9 +1132,9 @@ public:
 	CreatureAlignmentLimiter();
 	CreatureAlignmentLimiter(si8 Alignment);
 
-	int limit(const BonusLimitationContext &context) const override;
-	virtual std::string toString() const override;
-	virtual JsonNode toJsonNode() const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
+	std::string toString() const override;
+	JsonNode toJsonNode() const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1156,7 +1150,7 @@ public:
 	StackOwnerLimiter();
 	StackOwnerLimiter(const PlayerColor & Owner);
 
-	int limit(const BonusLimitationContext &context) const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1172,7 +1166,7 @@ public:
 	OppositeSideLimiter();
 	OppositeSideLimiter(const PlayerColor & Owner);
 
-	int limit(const BonusLimitationContext &context) const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1188,7 +1182,7 @@ public:
 
 	RankRangeLimiter();
 	RankRangeLimiter(ui8 Min, ui8 Max = 255);
-	int limit(const BonusLimitationContext &context) const override;
+	EDecision limit(const BonusLimitationContext &context) const override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -1198,6 +1192,22 @@ public:
 	}
 };
 
+class DLL_LINKAGE UnitOnHexLimiter : public ILimiter //works only on selected hexes
+{
+public:
+	std::set<BattleHex> applicableHexes;
+
+	UnitOnHexLimiter(const std::set<BattleHex> & applicableHexes = {});
+	EDecision limit(const BonusLimitationContext &context) const override;
+	JsonNode toJsonNode() const override;
+
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & static_cast<ILimiter&>(*this);
+		h & applicableHexes;
+	}
+};
+
 namespace Selector
 {
 	extern DLL_LINKAGE CSelectFieldEqual<Bonus::BonusType> & type();
@@ -1226,9 +1236,6 @@ namespace Selector
 	 * Usage example: Selector::none.Or(<functor>).Or(<functor>)...)
 	 */
 	extern DLL_LINKAGE CSelector none;
-
-	bool DLL_LINKAGE matchesType(const CSelector &sel, Bonus::BonusType type);
-	bool DLL_LINKAGE matchesTypeSubtype(const CSelector &sel, Bonus::BonusType type, TBonusSubtype subtype);
 }
 
 extern DLL_LINKAGE const std::map<std::string, Bonus::BonusType> bonusNameMap;

lib/JsonNode.cpp

@@ -20,6 +20,7 @@
 #include "CGeneralTextHandler.h"
 #include "JsonDetail.h"
 #include "StringConstants.h"
+#include "battle/BattleHex.h"
 
 namespace
 {
@@ -770,6 +771,17 @@ std::shared_ptr<ILimiter> JsonUtils::parseLimiter(const JsonNode & limiter)
 				}
 				return terrainLimiter;
 			}
+			else if(limiterType == "UNIT_ON_HEXES") {
+				auto hexLimiter = std::make_shared<UnitOnHexLimiter>();
+				if(!parameters.empty())
+				{
+					for (const auto & parameter: parameters){
+						if(parameter.isNumber())
+							hexLimiter->applicableHexes.insert(BattleHex(parameter.Integer()));
+					}
+				}
+				return hexLimiter;
+			}
 			else
 			{
 				logMod->error("Error: invalid customizable limiter type %s.", limiterType);

lib/NetPacks.h

@@ -349,15 +349,16 @@ struct DLL_LINKAGE SetAvailableHeroes : public CPackForClient
 
 struct DLL_LINKAGE GiveBonus : public CPackForClient
 {
-	GiveBonus(ui8 Who = 0)
+	enum class ETarget : ui8 { HERO, PLAYER, TOWN, BATTLE };
+	
+	GiveBonus(ETarget Who = ETarget::HERO)
 		:who(Who)
 	{
 	}
 
 	void applyGs(CGameState * gs);
 
-	enum { HERO, PLAYER, TOWN };
-	ui8 who = 0; //who receives bonus, uses enum above
+	ETarget who = ETarget::HERO; //who receives bonus
 	si32 id = 0; //hero. town or player id - whoever receives it
 	Bonus bonus;
 	MetaString bdescr;
@@ -424,15 +425,14 @@ struct DLL_LINKAGE PlayerReinitInterface : public CPackForClient
 
 struct DLL_LINKAGE RemoveBonus : public CPackForClient
 {
-	RemoveBonus(ui8 Who = 0)
+	RemoveBonus(GiveBonus::ETarget Who = GiveBonus::ETarget::HERO)
 		:who(Who)
 	{
 	}
 
 	void applyGs(CGameState * gs);
 
-	enum { HERO, PLAYER, TOWN };
-	ui8 who; //who receives bonus, uses enum above
+	GiveBonus::ETarget who; //who receives bonus
 	ui32 whoID = 0; //hero, town or player id - whoever loses bonus
 
 	//vars to identify bonus: its source

lib/NetPacksLib.cpp

@@ -952,15 +952,19 @@ void GiveBonus::applyGs(CGameState *gs)
 	CBonusSystemNode *cbsn = nullptr;
 	switch(who)
 	{
-	case HERO:
+	case ETarget::HERO:
 		cbsn = gs->getHero(ObjectInstanceID(id));
 		break;
-	case PLAYER:
+	case ETarget::PLAYER:
 		cbsn = gs->getPlayerState(PlayerColor(id));
 		break;
-	case TOWN:
+	case ETarget::TOWN:
 		cbsn = gs->getTown(ObjectInstanceID(id));
 		break;
+	case ETarget::BATTLE:
+		assert(Bonus::OneBattle(&bonus));
+		cbsn = dynamic_cast<CBonusSystemNode*>(gs->curB.get());
+		break;
 	}
 
 	assert(cbsn);
@@ -1106,7 +1110,7 @@ void PlayerReinitInterface::applyGs(CGameState *gs)
 void RemoveBonus::applyGs(CGameState *gs)
 {
 	CBonusSystemNode * node = nullptr;
-	if (who == HERO)
+	if (who == GiveBonus::ETarget::HERO)
 		node = gs->getHero(ObjectInstanceID(whoID));
 	else
 		node = gs->getPlayerState(PlayerColor(whoID));

lib/ObstacleHandler.h

@@ -31,7 +31,7 @@ public:
 	Obstacle obstacle;
 	si32 iconIndex;
 	std::string identifier;
-	std::string appearSound, appearAnimation, triggerAnimation, triggerSound, animation;
+	std::string appearSound, appearAnimation, animation;
 	std::vector<TerrainId> allowedTerrains;
 	std::vector<std::string> allowedSpecialBfields;
 	
@@ -62,12 +62,7 @@ public:
 		h & identifier;
 		h & animation;
 		h & appearAnimation;
-		h & triggerAnimation;
-		if (version > 806)
-		{
-			h & appearSound;
-			h & triggerSound;
-		}
+		h & appearSound;
 		h & allowedTerrains;
 		h & allowedSpecialBfields;
 		h & isAbsoluteObstacle;

lib/Point.h

@@ -19,44 +19,48 @@ public:
 	int x, y;
 
 	//constructors
-	Point()
+	constexpr Point() : x(0), y(0)
 	{
-		x = y = 0;
 	}
 
-	Point(int X, int Y)
+	constexpr Point(int X, int Y)
 		: x(X)
 		, y(Y)
 	{
 	}
 
+	constexpr static Point makeInvalid()
+	{
+		return Point(std::numeric_limits<int>::min(), std::numeric_limits<int>::min());
+	}
+
 	explicit DLL_LINKAGE Point(const int3 &a);
 
 	template<typename T>
-	Point operator+(const T &b) const
+	constexpr Point operator+(const T &b) const
 	{
 		return Point(x+b.x,y+b.y);
 	}
 
 	template<typename T>
-	Point operator/(const T &div) const
+	constexpr Point operator/(const T &div) const
 	{
 		return Point(x/div, y/div);
 	}
 
 	template<typename T>
-	Point operator*(const T &mul) const
+	constexpr Point operator*(const T &mul) const
 	{
 		return Point(x*mul, y*mul);
 	}
 
-	Point operator*(const Point &b) const
+	constexpr Point operator*(const Point &b) const
 	{
 		return Point(x*b.x,y*b.y);
 	}
 
 	template<typename T>
-	Point& operator+=(const T &b)
+	constexpr Point& operator+=(const T &b)
 	{
 		x += b.x;
 		y += b.y;
@@ -64,34 +68,39 @@ public:
 	}
 
 	template<typename T>
-	Point operator-(const T &b) const
+	constexpr Point operator-(const T &b) const
 	{
 		return Point(x - b.x, y - b.y);
 	}
 
 	template<typename T>
-	Point& operator-=(const T &b)
+	constexpr Point& operator-=(const T &b)
 	{
 		x -= b.x;
 		y -= b.y;
 		return *this;
 	}
 
-	template<typename T> Point& operator=(const T &t)
+	template<typename T> constexpr Point& operator=(const T &t)
 	{
 		x = t.x;
 		y = t.y;
 		return *this;
 	}
-	template<typename T> bool operator==(const T &t) const
+	template<typename T> constexpr bool operator==(const T &t) const
 	{
 		return x == t.x  &&  y == t.y;
 	}
-	template<typename T> bool operator!=(const T &t) const
+	template<typename T> constexpr bool operator!=(const T &t) const
 	{
 		return !(*this == t);
 	}
 
+	constexpr bool isValid() const
+	{
+		return x > std::numeric_limits<int>::min() && y > std::numeric_limits<int>::min();
+	}
+
 	template <typename Handler>
 	void serialize(Handler &h, const int version)
 	{

lib/battle/BattleInfo.cpp

@@ -451,12 +451,7 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 			curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(CreatureID::ARROW_TOWERS, 1), 1, SlotID::ARROW_TOWERS_SLOT, BattleHex::CASTLE_BOTTOM_TOWER);
 		}
 
-		//moat
-		auto moat = std::make_shared<MoatObstacle>();
-		moat->ID = curB->town->subID;
-		moat->obstacleType = CObstacleInstance::MOAT;
-		moat->uniqueID = static_cast<si32>(curB->obstacles.size());
-		curB->obstacles.push_back(moat);
+		//Moat generating is done on server
 	}
 
 	std::stable_sort(stacks.begin(),stacks.end(),cmpst);
@@ -740,6 +735,9 @@ void BattleInfo::moveUnit(uint32_t id, BattleHex destination)
 		return;
 	}
 	sta->position = destination;
+	//Bonuses can be limited by unit placement, so, change tree version 
+	//to force updating a bonus. TODO: update version only when such bonuses are present
+	CBonusSystemNode::treeHasChanged();
 }
 
 void BattleInfo::setUnitState(uint32_t id, const JsonNode & data, int64_t healthDelta)

lib/battle/CObstacleInstance.cpp

@@ -60,6 +60,21 @@ bool CObstacleInstance::visibleForSide(ui8 side, bool hasNativeStack) const
 	return true;
 }
 
+const std::string & CObstacleInstance::getAnimation() const
+{
+	return getInfo().animation;
+}
+
+const std::string & CObstacleInstance::getAppearAnimation() const
+{
+	return getInfo().appearAnimation;
+}
+
+const std::string & CObstacleInstance::getAppearSound() const
+{
+	return getInfo().appearSound;
+}
+
 int CObstacleInstance::getAnimationYOffset(int imageHeight) const
 {
 	int offset = imageHeight % 42;
@@ -83,7 +98,43 @@ bool CObstacleInstance::blocksTiles() const
 
 bool CObstacleInstance::triggersEffects() const
 {
-	return false;
+	return getTrigger() != SpellID::NONE;
+}
+
+SpellID CObstacleInstance::getTrigger() const
+{
+	return SpellID::NONE;
+}
+
+void CObstacleInstance::serializeJson(JsonSerializeFormat & handler)
+{
+	auto obstacleInfo = getInfo();
+	auto hidden = false;
+	auto needAnimationOffsetFix = obstacleType == CObstacleInstance::USUAL;
+	int animationYOffset = 0;
+		
+	if(getInfo().blockedTiles.front() < 0) //TODO: holy ground ID=62,65,63
+		animationYOffset -= 42;
+
+	//We need only a subset of obstacle info for correct render
+	handler.serializeInt("position", pos);
+	handler.serializeString("appearSound", obstacleInfo.appearSound);
+	handler.serializeString("appearAnimation", obstacleInfo.appearAnimation);
+	handler.serializeString("animation", obstacleInfo.animation);
+	handler.serializeInt("animationYOffset", animationYOffset);
+
+	handler.serializeBool("hidden", hidden);
+	handler.serializeBool("needAnimationOffsetFix", needAnimationOffsetFix);
+}
+
+void CObstacleInstance::toInfo(ObstacleChanges & info, BattleChanges::EOperation operation)
+{
+	info.id = uniqueID;
+	info.operation = operation;
+
+	info.data.clear();
+	JsonSerializer ser(nullptr, info.data);
+	ser.serializeStruct("obstacle", *this);
 }
 
 SpellCreatedObstacle::SpellCreatedObstacle()
@@ -97,7 +148,9 @@ SpellCreatedObstacle::SpellCreatedObstacle()
 	trap(false),
 	removeOnTrigger(false),
 	revealed(false),
-	animationYOffset(0)
+	animationYOffset(0),
+	nativeVisible(true),
+	minimalDamage(0)
 {
 	obstacleType = SPELL_CREATED;
 }
@@ -107,8 +160,11 @@ bool SpellCreatedObstacle::visibleForSide(ui8 side, bool hasNativeStack) const
 	//we hide mines and not discovered quicksands
 	//quicksands are visible to the caster or if owned unit stepped into that particular patch
 	//additionally if side has a native unit, mines/quicksands will be visible
+	//but it is not a case for a moat, so, hasNativeStack should not work for moats
 
-	return casterSide == side || !hidden || revealed || hasNativeStack;
+	auto nativeVis = hasNativeStack && nativeVisible;
+
+	return casterSide == side || !hidden || revealed || nativeVis;
 }
 
 bool SpellCreatedObstacle::blocksTiles() const
@@ -121,21 +177,11 @@ bool SpellCreatedObstacle::stopsMovement() const
 	return trap;
 }
 
-bool SpellCreatedObstacle::triggersEffects() const
+SpellID SpellCreatedObstacle::getTrigger() const
 {
 	return trigger;
 }
 
-void SpellCreatedObstacle::toInfo(ObstacleChanges & info)
-{
-	info.id = uniqueID;
-	info.operation = ObstacleChanges::EOperation::ADD;
-
-	info.data.clear();
-	JsonSerializer ser(nullptr, info.data);
-	ser.serializeStruct("obstacle", *this);
-}
-
 void SpellCreatedObstacle::fromInfo(const ObstacleChanges & info)
 {
 	uniqueID = info.id;
@@ -156,18 +202,19 @@ void SpellCreatedObstacle::serializeJson(JsonSerializeFormat & handler)
 	handler.serializeInt("casterSpellPower", casterSpellPower);
 	handler.serializeInt("spellLevel", spellLevel);
 	handler.serializeInt("casterSide", casterSide);
+	handler.serializeInt("minimalDamage", minimalDamage);
+	handler.serializeInt("type", obstacleType);
 
 	handler.serializeBool("hidden", hidden);
 	handler.serializeBool("revealed", revealed);
 	handler.serializeBool("passable", passable);
-	handler.serializeBool("trigger", trigger);
+	handler.serializeId("trigger", trigger, SpellID::NONE);
 	handler.serializeBool("trap", trap);
 	handler.serializeBool("removeOnTrigger", removeOnTrigger);
+	handler.serializeBool("nativeVisible", nativeVisible);
 
 	handler.serializeString("appearSound", appearSound);
 	handler.serializeString("appearAnimation", appearAnimation);
-	handler.serializeString("triggerSound", triggerSound);
-	handler.serializeString("triggerAnimation", triggerAnimation);
 	handler.serializeString("animation", animation);
 
 	handler.serializeInt("animationYOffset", animationYOffset);
@@ -192,11 +239,26 @@ void SpellCreatedObstacle::battleTurnPassed()
 		turnsRemaining--;
 }
 
+const std::string & SpellCreatedObstacle::getAnimation() const
+{
+	return animation;
+}
+
+const std::string & SpellCreatedObstacle::getAppearAnimation() const
+{
+	return appearAnimation;
+}
+
+const std::string & SpellCreatedObstacle::getAppearSound() const
+{
+	return appearSound;
+}
+
 int SpellCreatedObstacle::getAnimationYOffset(int imageHeight) const
 {
 	int offset = imageHeight % 42;
 
-	if(obstacleType == CObstacleInstance::SPELL_CREATED)
+	if(obstacleType == CObstacleInstance::SPELL_CREATED || obstacleType == CObstacleInstance::MOAT)
 	{
 		offset += animationYOffset;
 	}
@@ -204,9 +266,4 @@ int SpellCreatedObstacle::getAnimationYOffset(int imageHeight) const
 	return offset;
 }
 
-std::vector<BattleHex> MoatObstacle::getAffectedTiles() const
-{
-	return (*VLC->townh)[ID]->town->moatHexes;
-}
-
 VCMI_LIB_NAMESPACE_END

lib/battle/CObstacleInstance.h

@@ -9,12 +9,14 @@
  */
 #pragma once
 #include "BattleHex.h"
+#include "NetPacksBase.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
 class ObstacleInfo;
 class ObstacleChanges;
 class JsonSerializeFormat;
+class SpellID;
 
 struct DLL_LINKAGE CObstacleInstance
 {
@@ -41,14 +43,24 @@ struct DLL_LINKAGE CObstacleInstance
 	virtual bool blocksTiles() const;
 	virtual bool stopsMovement() const; //if unit stepping onto obstacle, can't continue movement (in general, doesn't checks for the side)
 	virtual bool triggersEffects() const;
+	virtual SpellID getTrigger() const;
 
 	virtual std::vector<BattleHex> getAffectedTiles() const;
 	virtual bool visibleForSide(ui8 side, bool hasNativeStack) const; //0 attacker
 
 	virtual void battleTurnPassed(){};
 
+	//Client helper functions, make it easier to render animations
+	virtual const std::string & getAnimation() const;
+	virtual const std::string & getAppearAnimation() const;
+	virtual const std::string & getAppearSound() const;
+
 	virtual int getAnimationYOffset(int imageHeight) const;
 
+	void toInfo(ObstacleChanges & info, BattleChanges::EOperation operation = BattleChanges::EOperation::ADD);
+	
+	virtual void serializeJson(JsonSerializeFormat & handler);
+
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & ID;
@@ -58,30 +70,25 @@ struct DLL_LINKAGE CObstacleInstance
 	}
 };
 
-struct DLL_LINKAGE MoatObstacle : CObstacleInstance
-{
-	std::vector<BattleHex> getAffectedTiles() const override; //for special effects (not blocking)
-};
-
 struct DLL_LINKAGE SpellCreatedObstacle : CObstacleInstance
 {
 	int32_t turnsRemaining;
 	int32_t casterSpellPower;
 	int32_t spellLevel;
+	int32_t minimalDamage; //How many damage should it do regardless of power and level of caster
 	si8 casterSide; //0 - obstacle created by attacker; 1 - by defender
 
+	SpellID trigger;
+
 	bool hidden;
 	bool passable;
-	bool trigger;
 	bool trap;
 	bool removeOnTrigger;
-
 	bool revealed;
+	bool nativeVisible; //Should native terrain creatures reveal obstacle
 
 	std::string appearSound;
 	std::string appearAnimation;
-	std::string triggerSound;
-	std::string triggerAnimation;
 	std::string animation;
 
 	int animationYOffset;
@@ -95,16 +102,19 @@ struct DLL_LINKAGE SpellCreatedObstacle : CObstacleInstance
 
 	bool blocksTiles() const override;
 	bool stopsMovement() const override;
-	bool triggersEffects() const override;
+	SpellID getTrigger() const override;
 
 	void battleTurnPassed() override;
 
+	//Client helper functions, make it easier to render animations
+	const std::string & getAnimation() const override;
+	const std::string & getAppearAnimation() const override;
+	const std::string & getAppearSound() const override;
 	int getAnimationYOffset(int imageHeight) const override;
 
-	void toInfo(ObstacleChanges & info);
 	void fromInfo(const ObstacleChanges & info);
 
-	void serializeJson(JsonSerializeFormat & handler);
+	void serializeJson(JsonSerializeFormat & handler) override;
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
@@ -115,8 +125,10 @@ struct DLL_LINKAGE SpellCreatedObstacle : CObstacleInstance
 		h & casterSide;
 
 		h & hidden;
+		h & nativeVisible;
 		h & passable;
 		h & trigger;
+		h & minimalDamage;
 		h & trap;
 
 		h & customSize;

lib/mapObjects/CGHeroInstance.cpp

@@ -600,7 +600,9 @@ int64_t CGHeroInstance::getSpellBonus(const spells::Spell * spell, int64_t base,
 {
 	//applying sorcery secondary skill
 
-	base = static_cast<int64_t>(base * (valOfBonuses(Bonus::SPELL_DAMAGE)) / 100.0);
+	if(spell->isMagical())
+		base = static_cast<int64_t>(base * (valOfBonuses(Bonus::SPELL_DAMAGE)) / 100.0);
+
 	base = static_cast<int64_t>(base * (100 + valOfBonuses(Bonus::SPECIFIC_SPELL_DAMAGE, spell->getIndex())) / 100.0);
 
 	int maxSchoolBonus = 0;

lib/mapObjects/CRewardableObject.cpp

@@ -318,7 +318,7 @@ void CRewardableObject::grantRewardAfterLevelup(const CRewardVisitInfo & info, c
 		assert(bonus.source == Bonus::OBJECT);
 		assert(bonus.sid == ID);
 		GiveBonus gb;
-		gb.who = GiveBonus::HERO;
+		gb.who = GiveBonus::ETarget::HERO;
 		gb.bonus = bonus;
 		gb.id = hero->id.getNum();
 		cb->giveHeroBonus(&gb);

lib/mapObjects/MiscObjects.cpp

@@ -2142,7 +2142,7 @@ void CGLighthouse::onHeroVisit( const CGHeroInstance * h ) const
 
 		if(oldOwner < PlayerColor::PLAYER_LIMIT) //remove bonus from old owner
 		{
-			RemoveBonus rb(RemoveBonus::PLAYER);
+			RemoveBonus rb(GiveBonus::ETarget::PLAYER);
 			rb.whoID = oldOwner.getNum();
 			rb.source = Bonus::OBJECT;
 			rb.id = id.getNum();
@@ -2162,7 +2162,7 @@ void CGLighthouse::initObj(CRandomGenerator & rand)
 
 void CGLighthouse::giveBonusTo(const PlayerColor & player, bool onInit) const
 {
-	GiveBonus gb(GiveBonus::PLAYER);
+	GiveBonus gb(GiveBonus::ETarget::PLAYER);
 	gb.bonus.type = Bonus::MOVEMENT;
 	gb.bonus.val = 500;
 	gb.id = player.getNum();

lib/registerTypes/RegisterTypes.h

@@ -183,6 +183,7 @@ void registerTypesMapObjects2(Serializer &s)
 	s.template registerType<ILimiter, CreatureAlignmentLimiter>();
 	s.template registerType<ILimiter, RankRangeLimiter>();
 	s.template registerType<ILimiter, StackOwnerLimiter>();
+	s.template registerType<ILimiter, UnitOnHexLimiter>();
 
 //	s.template registerType<CBonusSystemNode>();
 	s.template registerType<CBonusSystemNode, CArtifact>();
@@ -202,7 +203,6 @@ void registerTypesMapObjects2(Serializer &s)
 	s.template registerType<CArtifactInstance, CCombinedArtifactInstance>();
 
 	//s.template registerType<CObstacleInstance>();
-		s.template registerType<CObstacleInstance, MoatObstacle>();
 		s.template registerType<CObstacleInstance, SpellCreatedObstacle>();
 }
 template<typename Serializer>

lib/serializer/CSerializer.h

@@ -14,8 +14,8 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-const ui32 SERIALIZATION_VERSION = 820;
-const ui32 MINIMAL_SERIALIZATION_VERSION = 820;
+const ui32 SERIALIZATION_VERSION = 821;
+const ui32 MINIMAL_SERIALIZATION_VERSION = 821;
 const std::string SAVEGAME_MAGIC = "VCMISVG";
 
 class CHero;

lib/spells/AbilityCaster.cpp

@@ -22,7 +22,6 @@ namespace spells
 
 AbilityCaster::AbilityCaster(const battle::Unit * actualCaster_, int32_t baseSpellLevel_)
 	: ProxyCaster(actualCaster_),
-	actualCaster(actualCaster_),
 	baseSpellLevel(baseSpellLevel_)
 {
 }
@@ -32,10 +31,11 @@ AbilityCaster::~AbilityCaster() = default;
 int32_t AbilityCaster::getSpellSchoolLevel(const Spell * spell, int32_t * outSelectedSchool) const
 {
 	auto skill = baseSpellLevel;
+	const auto * unit = dynamic_cast<const battle::Unit*>(actualCaster);
 
 	if(spell->getLevel() > 0)
 	{
-		vstd::amax(skill, actualCaster->valOfBonuses(Bonus::MAGIC_SCHOOL_SKILL, 0));
+		vstd::amax(skill, unit->valOfBonuses(Bonus::MAGIC_SCHOOL_SKILL, 0));
 	}
 
 	vstd::amax(skill, 0);

lib/spells/AbilityCaster.h

@@ -29,7 +29,6 @@ public:
 	void spendMana(ServerCallback * server, const int32_t spellCost) const override;
 
 private:
-	const battle::Unit * actualCaster;
 	int32_t baseSpellLevel;
 };
 

lib/spells/BattleSpellMechanics.cpp

@@ -360,7 +360,7 @@ void BattleSpellMechanics::beforeCast(BattleSpellCast & sc, vstd::RNG & rng, con
 
 	auto filterResisted = [&, this](const battle::Unit * unit) -> bool
 	{
-		if(isNegativeSpell())
+		if(isNegativeSpell() && isMagicalEffect())
 		{
 			//magic resistance
 			const int prob = std::min(unit->magicResistance(), 100); //probability of resistance in %

lib/spells/BonusCaster.cpp

@@ -24,10 +24,8 @@ namespace spells
 
 BonusCaster::BonusCaster(const Caster * actualCaster_, std::shared_ptr<Bonus> bonus_):
 	ProxyCaster(actualCaster_),
-	actualCaster(actualCaster_),
 	bonus(std::move(bonus_))
 {
-
 }
 
 BonusCaster::~BonusCaster() = default;

lib/spells/BonusCaster.h

@@ -30,7 +30,6 @@ public:
 	void spendMana(ServerCallback * server, const int spellCost) const override;
 
 private:
-	const Caster * actualCaster;
 	std::shared_ptr<Bonus> bonus;
 };
 

lib/spells/CSpellHandler.cpp

@@ -97,6 +97,7 @@ CSpell::CSpell():
 	damage(false),
 	offensive(false),
 	special(true),
+	nonMagical(false),
 	targetType(spells::AimType::NO_TARGET)
 {
 	levels.resize(GameConstants::SPELL_SCHOOL_LEVELS);
@@ -236,6 +237,11 @@ bool CSpell::isCreatureAbility() const
 	return creatureAbility;
 }
 
+bool CSpell::isMagical() const
+{
+	return !nonMagical;
+}
+
 bool CSpell::isPositive() const
 {
 	return positiveness == POSITIVE;
@@ -397,8 +403,8 @@ int64_t CSpell::adjustRawDamage(const spells::Caster * caster, const battle::Uni
 
 		CSelector selector = Selector::type()(Bonus::SPELL_DAMAGE_REDUCTION).And(Selector::subtype()(-1));
 
-		//general spell dmg reduction
-		if(bearer->hasBonus(selector))
+		//general spell dmg reduction, works only on magical effects
+		if(bearer->hasBonus(selector) && isMagical())
 		{
 			ret *= 100 - bearer->valOfBonuses(selector);
 			ret /= 100;
@@ -751,6 +757,8 @@ CSpell * CSpellHandler::loadFromJson(const std::string & scope, const JsonNode &
 
 	spell->damage = flags["damage"].Bool(); //do this before "offensive"
 
+	spell->nonMagical = flags["nonMagical"].Bool();
+
 	if(flags["offensive"].Bool())
 	{
 		spell->setIsOffensive(true);

lib/spells/CSpellHandler.h

@@ -246,6 +246,7 @@ public:
 	bool isPositive() const override;
 	bool isNegative() const override;
 	bool isNeutral() const override;
+	bool isMagical() const override;
 
 	bool isDamage() const override;
 	bool isOffensive() const override;
@@ -297,6 +298,7 @@ public:
 		h & levels;
 		h & school;
 		h & animationInfo;
+		h & nonMagical;
 	}
 	friend class CSpellHandler;
 	friend class Graphics;
@@ -338,6 +340,7 @@ private:
 	bool damage;
 	bool offensive;
 	bool special;
+	bool nonMagical; //For creature abilities like bind
 
 	std::string attributes; //reference only attributes //todo: remove or include in configuration format, currently unused
 

lib/spells/ISpellMechanics.cpp

@@ -617,6 +617,11 @@ bool BaseMechanics::isPositiveSpell() const
 	return owner->isPositive();
 }
 
+bool BaseMechanics::isMagicalEffect() const
+{
+	return owner->isMagical();
+}
+
 int64_t BaseMechanics::adjustEffectValue(const battle::Unit * target) const
 {
 	return owner->adjustRawDamage(caster, target, getEffectValue());

lib/spells/ISpellMechanics.h

@@ -228,6 +228,7 @@ public:
 
 	virtual bool isNegativeSpell() const = 0;
 	virtual bool isPositiveSpell() const = 0;
+	virtual bool isMagicalEffect() const = 0;
 
 	virtual int64_t adjustEffectValue(const battle::Unit * target) const = 0;
 	virtual int64_t applySpellBonus(int64_t value, const battle::Unit * target) const = 0;
@@ -288,6 +289,7 @@ public:
 
 	bool isNegativeSpell() const override;
 	bool isPositiveSpell() const override;
+	bool isMagicalEffect() const override;
 
 	int64_t adjustEffectValue(const battle::Unit * target) const override;
 	int64_t applySpellBonus(int64_t value, const battle::Unit * target) const override;

lib/spells/ObstacleCasterProxy.cpp

@@ -0,0 +1,91 @@
+/*
+ * ObstacleCasterProxy.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#include "ObstacleCasterProxy.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+namespace spells
+{
+
+ObstacleCasterProxy::ObstacleCasterProxy(PlayerColor owner_, const Caster * hero_, const SpellCreatedObstacle & obs_):
+	SilentCaster(owner_, hero_),
+	obs(obs_)
+{
+}
+
+int32_t ObstacleCasterProxy::getSpellSchoolLevel(const Spell * spell, int32_t * outSelectedSchool) const
+{
+	return obs.spellLevel;
+}
+
+int32_t ObstacleCasterProxy::getEffectLevel(const Spell * spell) const
+{
+	return obs.spellLevel;
+}
+
+int64_t ObstacleCasterProxy::getSpellBonus(const Spell * spell, int64_t base, const battle::Unit * affectedStack) const
+{
+	if(actualCaster)
+		return std::max<int64_t>(actualCaster->getSpellBonus(spell, base, affectedStack), obs.minimalDamage);
+
+	return std::max<int64_t>(base, obs.minimalDamage);
+}
+
+int32_t ObstacleCasterProxy::getEffectPower(const Spell * spell) const
+{
+	return obs.casterSpellPower;
+}
+
+int32_t ObstacleCasterProxy::getEnchantPower(const Spell * spell) const
+{
+	return obs.casterSpellPower;
+}
+
+int64_t ObstacleCasterProxy::getEffectValue(const Spell * spell) const
+{
+	if(actualCaster)
+		return std::max(static_cast<int64_t>(obs.minimalDamage), actualCaster->getEffectValue(spell));
+	else
+		return obs.minimalDamage;
+}
+
+SilentCaster::SilentCaster(PlayerColor owner_, const Caster * hero_):
+	ProxyCaster(hero_),
+	owner(std::move(owner_))
+{
+}
+
+void SilentCaster::getCasterName(MetaString & text) const
+{
+	logGlobal->error("Unexpected call to SilentCaster::getCasterName");
+}
+
+void SilentCaster::getCastDescription(const Spell * spell, const std::vector<const battle::Unit *> & attacked, MetaString & text) const
+{
+		//do nothing
+}
+
+void SilentCaster::spendMana(ServerCallback * server, const int spellCost) const
+{
+		//do nothing
+}
+
+PlayerColor SilentCaster::getCasterOwner() const
+{
+	if(actualCaster)
+		return actualCaster->getCasterOwner();
+
+	return owner;
+}
+
+
+}
+VCMI_LIB_NAMESPACE_END

lib/spells/ObstacleCasterProxy.h

@@ -0,0 +1,50 @@
+/*
+ * ObstacleCasterProxy.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#include "ProxyCaster.h"
+#include "../lib/NetPacksBase.h"
+#include "../battle/BattleHex.h"
+#include "../battle/CObstacleInstance.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+namespace spells
+{
+
+class DLL_LINKAGE SilentCaster : public ProxyCaster
+{
+protected:
+	const PlayerColor owner;
+public:
+	SilentCaster(PlayerColor owner_, const Caster * caster);
+
+	void getCasterName(MetaString & text) const override;
+	void getCastDescription(const Spell * spell, const std::vector<const battle::Unit *> & attacked, MetaString & text) const override;
+	void spendMana(ServerCallback * server, const int spellCost) const override;
+	PlayerColor getCasterOwner() const override;
+};
+
+class DLL_LINKAGE ObstacleCasterProxy : public SilentCaster
+{
+public:
+	ObstacleCasterProxy(PlayerColor owner_, const Caster * hero_, const SpellCreatedObstacle & obs_);
+
+	int32_t getSpellSchoolLevel(const Spell * spell, int32_t * outSelectedSchool = nullptr) const override;
+	int32_t getEffectLevel(const Spell * spell) const override;
+	int64_t getSpellBonus(const Spell * spell, int64_t base, const battle::Unit * affectedStack) const override;
+	int32_t getEffectPower(const Spell * spell) const override;
+	int32_t getEnchantPower(const Spell * spell) const override;
+	int64_t getEffectValue(const Spell * spell) const override;
+
+private:
+	const SpellCreatedObstacle & obs;
+};
+
+}//
+VCMI_LIB_NAMESPACE_END

lib/spells/ProxyCaster.cpp

@@ -13,6 +13,8 @@
 
 #include "../GameConstants.h"
 
+#include <vcmi/spells/Spell.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 namespace spells
@@ -28,62 +30,92 @@ ProxyCaster::~ProxyCaster() = default;
 
 int32_t ProxyCaster::getCasterUnitId() const
 {
-	return actualCaster->getCasterUnitId();
+	if(actualCaster)
+		return actualCaster->getCasterUnitId();
+
+	return -1;
 }
 
 int32_t ProxyCaster::getSpellSchoolLevel(const Spell * spell, int32_t * outSelectedSchool) const
 {
-	return actualCaster->getSpellSchoolLevel(spell, outSelectedSchool);
+	if(actualCaster)
+		return actualCaster->getSpellSchoolLevel(spell, outSelectedSchool);
+
+	return 0;
 }
 
 int32_t ProxyCaster::getEffectLevel(const Spell * spell) const
 {
-	return actualCaster->getEffectLevel(spell);
+	if(actualCaster)
+		return actualCaster->getEffectLevel(spell);
+
+	return 0;
 }
 
 int64_t ProxyCaster::getSpellBonus(const Spell * spell, int64_t base, const battle::Unit * affectedStack) const
 {
-	return actualCaster->getSpellBonus(spell, base, affectedStack);
+	if(actualCaster)
+		return actualCaster->getSpellBonus(spell, base, affectedStack);
+
+	return base;
 }
 
 int64_t ProxyCaster::getSpecificSpellBonus(const Spell * spell, int64_t base) const
 {
-	return actualCaster->getSpecificSpellBonus(spell, base);
+	if(actualCaster)
+		return actualCaster->getSpecificSpellBonus(spell, base);
+
+	return base;
 }
 
 int32_t ProxyCaster::getEffectPower(const Spell * spell) const
 {
-	return actualCaster->getEffectPower(spell);
+	if(actualCaster)
+		return actualCaster->getEffectPower(spell);
+
+	return spell->getLevelPower(getEffectLevel(spell));
 }
 
 int32_t ProxyCaster::getEnchantPower(const Spell * spell) const
 {
-	return actualCaster->getEnchantPower(spell);
+	if(actualCaster)
+		return actualCaster->getEnchantPower(spell);
+
+	return spell->getLevelPower(getEffectLevel(spell));
 }
 
 int64_t ProxyCaster::getEffectValue(const Spell * spell) const
 {
-	return actualCaster->getEffectValue(spell);
+	if(actualCaster)
+		return actualCaster->getEffectValue(spell);
+
+	return 0;
 }
 
 PlayerColor ProxyCaster::getCasterOwner() const
 {
-	return actualCaster->getCasterOwner();
+	if(actualCaster)
+		return actualCaster->getCasterOwner();
+
+	return PlayerColor::CANNOT_DETERMINE;
 }
 
 void ProxyCaster::getCasterName(MetaString & text) const
 {
-	return actualCaster->getCasterName(text);
+	if(actualCaster)
+		actualCaster->getCasterName(text);
 }
 
 void ProxyCaster::getCastDescription(const Spell * spell, const std::vector<const battle::Unit*> & attacked, MetaString & text) const
 {
-	actualCaster->getCastDescription(spell, attacked, text);
+	if(actualCaster)
+		actualCaster->getCastDescription(spell, attacked, text);
 }
 
 void ProxyCaster::spendMana(ServerCallback * server, const int32_t spellCost) const
 {
-	actualCaster->spendMana(server, spellCost);
+	if(actualCaster)
+		actualCaster->spendMana(server, spellCost);
 }
 
 }

lib/spells/ProxyCaster.h

@@ -36,7 +36,7 @@ public:
 	void getCastDescription(const Spell * spell, const std::vector<const battle::Unit *> & attacked, MetaString & text) const override;
 	void spendMana(ServerCallback * server, const int32_t spellCost) const override;
 
-private:
+protected:
 	const Caster * actualCaster;
 };
 

lib/spells/TargetCondition.cpp

@@ -114,6 +114,10 @@ public:
 protected:
 	bool check(const Mechanics * m, const battle::Unit * target) const override
 	{
+
+		if(!m->isMagicalEffect()) //Always pass on non-magical
+			return true;
+
 		std::stringstream cachingStr;
 		cachingStr << "type_" << Bonus::LEVEL_SPELL_IMMUNITY << "addInfo_1";
 
@@ -189,6 +193,8 @@ public:
 protected:
 	bool check(const Mechanics * m, const battle::Unit * target) const override
 	{
+		if(!m->isMagicalEffect()) //Always pass on non-magical
+			return true;
 		TConstBonusListPtr levelImmunities = target->getBonuses(Selector::type()(Bonus::LEVEL_SPELL_IMMUNITY));
 		return levelImmunities->size() == 0 ||
 		levelImmunities->totalValue() < m->getSpellLevel() ||
@@ -426,7 +432,6 @@ bool TargetCondition::isReceptive(const Mechanics * m, const battle::Unit * targ
 
 void TargetCondition::serializeJson(JsonSerializeFormat & handler, const ItemFactory * itemFactory)
 {
-	bool isNonMagical = false;
 	if(handler.saving)
 	{
 		logGlobal->error("Spell target condition saving is not supported");
@@ -438,17 +443,12 @@ void TargetCondition::serializeJson(JsonSerializeFormat & handler, const ItemFac
 	negation.clear();
 
 	absolute.push_back(itemFactory->createAbsoluteSpell());
-
-	handler.serializeBool("nonMagical", isNonMagical);
-	if(!isNonMagical)
-	{
-		absolute.push_back(itemFactory->createAbsoluteLevel());
-		normal.push_back(itemFactory->createElemental());
-		normal.push_back(itemFactory->createNormalLevel());
-		normal.push_back(itemFactory->createNormalSpell());
-		negation.push_back(itemFactory->createReceptiveFeature());
-		negation.push_back(itemFactory->createImmunityNegation());
-	}
+	absolute.push_back(itemFactory->createAbsoluteLevel());
+	normal.push_back(itemFactory->createElemental());
+	normal.push_back(itemFactory->createNormalLevel());
+	normal.push_back(itemFactory->createNormalSpell());
+	negation.push_back(itemFactory->createReceptiveFeature());
+	negation.push_back(itemFactory->createImmunityNegation());
 
 	{
 		auto anyOf = handler.enterStruct("anyOf");

lib/spells/effects/Moat.cpp

@@ -0,0 +1,178 @@
+/*
+ * Moat.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#include "StdInc.h"
+
+#include "Moat.h"
+
+#include "Registry.h"
+#include "../ISpellMechanics.h"
+
+#include "../../NetPacks.h"
+#include "../../mapObjects/CGTownInstance.h"
+#include "../../battle/IBattleState.h"
+#include "../../battle/CBattleInfoCallback.h"
+#include "../../serializer/JsonSerializeFormat.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+static const std::string EFFECT_NAME = "core:moat";
+
+namespace spells
+{
+namespace effects
+{
+
+VCMI_REGISTER_SPELL_EFFECT(Moat, EFFECT_NAME);
+
+static void serializeMoatHexes(JsonSerializeFormat & handler, const std::string & fieldName, std::vector<std::vector<BattleHex>> & moatHexes)
+{
+	{
+		JsonArraySerializer outer = handler.enterArray(fieldName);
+		outer.syncSize(moatHexes, JsonNode::JsonType::DATA_VECTOR);
+
+		for(size_t outerIndex = 0; outerIndex < outer.size(); outerIndex++)
+		{
+			JsonArraySerializer inner = outer.enterArray(outerIndex);
+			inner.syncSize(moatHexes.at(outerIndex), JsonNode::JsonType::DATA_INTEGER);
+
+			for(size_t innerIndex = 0; innerIndex < inner.size(); innerIndex++)
+				inner.serializeInt(innerIndex, moatHexes.at(outerIndex).at(innerIndex));
+		}
+	}
+}
+
+void Moat::serializeJsonEffect(JsonSerializeFormat & handler)
+{
+	handler.serializeBool("hidden", hidden);
+	handler.serializeBool("trap", trap);
+	handler.serializeBool("removeOnTrigger", removeOnTrigger);
+	handler.serializeBool("dispellable", dispellable);
+	handler.serializeInt("moatDamage", moatDamage);
+	serializeMoatHexes(handler, "moatHexes", moatHexes);
+	handler.serializeId("triggerAbility", triggerAbility, SpellID::NONE);
+	handler.serializeStruct("defender", sideOptions); //Moats are defender only
+
+	assert(!handler.saving);
+	{
+		auto guard = handler.enterStruct("bonus");
+		const JsonNode & data = handler.getCurrent();
+
+		for(const auto & p : data.Struct())
+		{
+			//TODO: support JsonSerializeFormat in Bonus
+			auto guard = handler.enterStruct(p.first);
+			const JsonNode & bonusNode = handler.getCurrent();
+			auto b = JsonUtils::parseBonus(bonusNode);
+			bonus.push_back(b);
+		}
+	}
+}
+
+void Moat::convertBonus(const Mechanics * m, std::vector<Bonus> & converted) const
+{
+
+	for(const auto & b : bonus)
+	{
+		Bonus nb(*b);
+
+		//Moat battlefield effect is always permanent
+		nb.duration = Bonus::ONE_BATTLE;
+
+		if(m->battle()->battleGetDefendedTown() && m->battle()->battleGetSiegeLevel() >= CGTownInstance::CITADEL)
+		{
+			nb.sid = Bonus::getSid32(m->battle()->battleGetDefendedTown()->town->faction->getIndex(), BuildingID::CITADEL);
+			nb.source = Bonus::TOWN_STRUCTURE;
+		}
+		else
+		{
+			nb.sid = m->getSpellIndex(); //for all
+			nb.source = Bonus::SPELL_EFFECT;//for all
+		}
+		std::set<BattleHex> flatMoatHexes;
+
+		for(const auto & moatPatch : moatHexes)
+			flatMoatHexes.insert(moatPatch.begin(), moatPatch.end());
+
+		nb.limiter = std::make_shared<UnitOnHexLimiter>(std::move(flatMoatHexes));
+		converted.push_back(nb);
+	}
+}
+
+void Moat::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
+{
+	assert(m->isMassive());
+	assert(m->battle()->battleGetDefendedTown());
+	if(m->isMassive() && m->battle()->battleGetSiegeLevel() >= CGTownInstance::CITADEL)
+	{
+		EffectTarget moat;
+		placeObstacles(server, m, moat);
+
+		std::vector<Bonus> converted;
+		convertBonus(m, converted);
+		for(auto & b : converted)
+		{
+			GiveBonus gb(GiveBonus::ETarget::BATTLE);
+			gb.bonus = b;
+			server->apply(&gb);
+		}
+	}
+}
+
+void Moat::placeObstacles(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
+{
+	assert(m->battle()->battleGetDefendedTown());
+	assert(m->casterSide == BattleSide::DEFENDER); // Moats are always cast by defender
+	assert(turnsRemaining < 0); // Moats should lasts infininte number of turns
+
+	BattleObstaclesChanged pack;
+
+	auto all = m->battle()->battleGetAllObstacles(BattlePerspective::ALL_KNOWING);
+
+	int obstacleIdToGive = 1;
+	for(auto & one : all)
+		if(one->uniqueID >= obstacleIdToGive)
+			obstacleIdToGive = one->uniqueID + 1;
+
+	for(const auto & destination : moatHexes)  //Moat hexes can be different obstacles
+	{
+		SpellCreatedObstacle obstacle;
+		obstacle.uniqueID = obstacleIdToGive++;
+		obstacle.pos = destination.at(0);
+		obstacle.obstacleType = dispellable ? CObstacleInstance::SPELL_CREATED : CObstacleInstance::MOAT;
+		obstacle.ID = m->getSpellIndex();
+
+		obstacle.turnsRemaining = -1; //Moat cannot be expired
+		obstacle.casterSpellPower = m->getEffectPower();
+		obstacle.spellLevel = m->getEffectLevel(); //todo: level of indirect effect should be also configurable
+		obstacle.casterSide = BattleSide::DEFENDER; // Moats are always cast by defender
+		obstacle.minimalDamage = moatDamage; // Minimal moat damage
+		obstacle.hidden = hidden;
+		obstacle.passable = true; //Moats always passable
+		obstacle.trigger = triggerAbility;
+		obstacle.trap = trap;
+		obstacle.removeOnTrigger = removeOnTrigger;
+		obstacle.nativeVisible = false; //Moats is invisible for native terrain
+		obstacle.appearSound = sideOptions.appearSound; //For dispellable moats
+		obstacle.appearAnimation = sideOptions.appearAnimation; //For dispellable moats
+		obstacle.animation = sideOptions.animation;
+		obstacle.customSize.insert(obstacle.customSize.end(),destination.cbegin(), destination.cend());
+		obstacle.animationYOffset = sideOptions.offsetY;
+		pack.changes.emplace_back();
+		obstacle.toInfo(pack.changes.back());
+	}
+
+	if(!pack.changes.empty())
+		server->apply(&pack);
+}
+
+}
+}
+
+VCMI_LIB_NAMESPACE_END

lib/spells/effects/Moat.h

@@ -0,0 +1,41 @@
+/*
+ * Moat.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#pragma once
+
+#include "Obstacle.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+namespace spells
+{
+namespace effects
+{
+
+class Moat : public Obstacle
+{
+private:
+	ObstacleSideOptions sideOptions; //Defender only
+	std::vector<std::vector<BattleHex>> moatHexes; //Determine number of moat patches and hexes
+	std::vector<std::shared_ptr<Bonus>> bonus; //For battle-wide bonuses
+	bool dispellable; //For Tower landmines
+	int moatDamage; // Minimal moat damage
+public:
+	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
+protected:
+	void serializeJsonEffect(JsonSerializeFormat & handler) override;
+	void placeObstacles(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
+	void convertBonus(const Mechanics * m, std::vector<Bonus> & converted) const;
+};
+
+}
+}
+
+VCMI_LIB_NAMESPACE_END

lib/spells/effects/Obstacle.cpp

@@ -91,8 +91,6 @@ void ObstacleSideOptions::serializeJson(JsonSerializeFormat & handler)
 
 	handler.serializeString("appearSound", appearSound);
 	handler.serializeString("appearAnimation", appearAnimation);
-	handler.serializeString("triggerSound", triggerSound);
-	handler.serializeString("triggerAnimation", triggerAnimation);
 	handler.serializeString("animation", animation);
 
 	handler.serializeInt("offsetY", offsetY);
@@ -121,7 +119,7 @@ void Obstacle::adjustAffectedHexes(std::set<BattleHex> & hexes, const Mechanics
 
 bool Obstacle::applicable(Problem & problem, const Mechanics * m) const
 {
-	if(hidden && m->battle()->battleHasNativeStack(m->battle()->otherSide(m->casterSide)))
+	if(hidden && !hideNative && m->battle()->battleHasNativeStack(m->battle()->otherSide(m->casterSide)))
 		return m->adaptProblem(ESpellCastProblem::NO_APPROPRIATE_TARGET, problem);
 
 	return LocationEffect::applicable(problem, m);
@@ -181,42 +179,46 @@ EffectTarget Obstacle::transformTarget(const Mechanics * m, const Target & aimPo
 
 void Obstacle::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
-	if(m->isMassive())
+	if(patchCount > 0)
 	{
 		std::vector<BattleHex> availableTiles;
-		for(int i = 0; i < GameConstants::BFIELD_SIZE; i++)
+		auto insertAvailable = [&m](const BattleHex & hex, std::vector<BattleHex> & availableTiles)
 		{
-			BattleHex hex = i;
 			if(isHexAvailable(m->battle(), hex, true))
 				availableTiles.push_back(hex);
-		}
-		RandomGeneratorUtil::randomShuffle(availableTiles, *server->getRNG());
+		};
 
-		const int patchesToPut = std::min(patchCount, static_cast<int>(availableTiles.size()));
+		if(m->isMassive())
+			for(int i = 0; i < GameConstants::BFIELD_SIZE; i++)
+				insertAvailable(BattleHex(i), availableTiles);
+		else
+			for(const auto & destination : target)
+				insertAvailable(destination.hexValue, availableTiles);
 
+		RandomGeneratorUtil::randomShuffle(availableTiles, *server->getRNG());
+		const int patchesToPut = std::min(patchCount, static_cast<int>(availableTiles.size()));
 		EffectTarget randomTarget;
 		randomTarget.reserve(patchesToPut);
 		for(int i = 0; i < patchesToPut; i++)
 			randomTarget.emplace_back(availableTiles.at(i));
-
 		placeObstacles(server, m, randomTarget);
+		return;
 	}
-	else
-	{
-		placeObstacles(server, m, target);
-	}
+
+	placeObstacles(server, m, target);
 }
 
 void Obstacle::serializeJsonEffect(JsonSerializeFormat & handler)
 {
 	handler.serializeBool("hidden", hidden);
 	handler.serializeBool("passable", passable);
-	handler.serializeBool("trigger", trigger);
 	handler.serializeBool("trap", trap);
-    handler.serializeBool("removeOnTrigger", removeOnTrigger);
+	handler.serializeBool("removeOnTrigger", removeOnTrigger);
+	handler.serializeBool("hideNative", hideNative);
 
 	handler.serializeInt("patchCount", patchCount);
 	handler.serializeInt("turnsRemaining", turnsRemaining, -1);
+	handler.serializeId("triggerAbility", triggerAbility, SpellID::NONE);
 
 	handler.serializeStruct("attacker", sideOptions.at(BattleSide::ATTACKER));
 	handler.serializeStruct("defender", sideOptions.at(BattleSide::DEFENDER));
@@ -285,7 +287,7 @@ void Obstacle::placeObstacles(ServerCallback * server, const Mechanics * m, cons
 		SpellCreatedObstacle obstacle;
 		obstacle.uniqueID = obstacleIdToGive++;
 		obstacle.pos = destination.hexValue;
-		obstacle.obstacleType = CObstacleInstance::USUAL;
+		obstacle.obstacleType = CObstacleInstance::SPELL_CREATED;
 		obstacle.ID = m->getSpellIndex();
 
 		obstacle.turnsRemaining = turnsRemaining;
@@ -293,16 +295,15 @@ void Obstacle::placeObstacles(ServerCallback * server, const Mechanics * m, cons
 		obstacle.spellLevel = m->getEffectLevel();//todo: level of indirect effect should be also configurable
 		obstacle.casterSide = m->casterSide;
 
+		obstacle.nativeVisible = !hideNative;
 		obstacle.hidden = hidden;
 		obstacle.passable = passable;
-		obstacle.trigger = trigger;
+		obstacle.trigger = triggerAbility;
 		obstacle.trap = trap;
 		obstacle.removeOnTrigger = removeOnTrigger;
 
 		obstacle.appearSound = options.appearSound;
 		obstacle.appearAnimation = options.appearAnimation;
-		obstacle.triggerSound = options.triggerSound;
-		obstacle.triggerAnimation = options.triggerAnimation;
 		obstacle.animation = options.animation;
 
 		obstacle.animationYOffset = options.offsetY;
@@ -328,7 +329,6 @@ void Obstacle::placeObstacles(ServerCallback * server, const Mechanics * m, cons
 		server->apply(&pack);
 }
 
-
 }
 }
 

lib/spells/effects/Obstacle.h

@@ -11,6 +11,7 @@
 #pragma once
 
 #include "LocationEffect.h"
+#include "../../GameConstants.h"
 #include "../../battle/BattleHex.h"
 #include "../../battle/CObstacleInstance.h"
 
@@ -31,8 +32,6 @@ public:
 
 	std::string appearSound;
 	std::string appearAnimation;
-	std::string triggerSound;
-	std::string triggerAnimation;
 	std::string animation;
 
 	int offsetY = 0;
@@ -54,22 +53,23 @@ public:
 
 protected:
 	void serializeJsonEffect(JsonSerializeFormat & handler) override;
+	virtual void placeObstacles(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const;
 
-private:
 	bool hidden = false;
-	bool passable = false;
 	bool trigger = false;
 	bool trap = false;
 	bool removeOnTrigger = false;
-	int32_t patchCount = 1;//random patches to place, only for massive spells
+	bool hideNative = false;
+	SpellID triggerAbility;
+private:
+	int32_t patchCount = 0; //random patches to place, for massive spells should be >= 1, for non-massive ones if >= 1, then place only this number inside a target (like H5 landMine)
+	bool passable = false;
 	int32_t turnsRemaining = -1;
 
 	std::array<ObstacleSideOptions, 2> sideOptions;
 
 	static bool isHexAvailable(const CBattleInfoCallback * cb, const BattleHex & hex, const bool mustBeClear);
 	static bool noRoomToPlace(Problem & problem, const Mechanics * m);
-
-	void placeObstacles(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const;
 };
 
 }

lib/spells/effects/RemoveObstacle.cpp

@@ -50,7 +50,11 @@ void RemoveObstacle::apply(ServerCallback * server, const Mechanics * m, const E
 	BattleObstaclesChanged pack;
 
 	for(const auto & obstacle : getTargets(m, target, false))
+	{
+		auto * serializable = const_cast<CObstacleInstance*>(obstacle); //Workaround
 		pack.changes.emplace_back(obstacle->uniqueID, BattleChanges::EOperation::REMOVE);
+		serializable->toInfo(pack.changes.back(), BattleChanges::EOperation::REMOVE);
+	}
 
 	if(!pack.changes.empty())
 		server->apply(&pack);

server/CGameHandler.cpp

@@ -22,6 +22,7 @@
 #include "../lib/spells/BonusCaster.h"
 #include "../lib/spells/CSpellHandler.h"
 #include "../lib/spells/ISpellMechanics.h"
+#include "../lib/spells/ObstacleCasterProxy.h"
 #include "../lib/spells/Problem.h"
 #include "../lib/CGeneralTextHandler.h"
 #include "../lib/CTownHandler.h"
@@ -93,102 +94,6 @@ private:
 	CGameHandler * gh;
 };
 
-VCMI_LIB_NAMESPACE_BEGIN
-namespace spells
-{
-
-class ObstacleCasterProxy : public Caster
-{
-public:
-	ObstacleCasterProxy(const PlayerColor owner_, const CGHeroInstance * hero_, const SpellCreatedObstacle * obs_)
-		: owner(owner_),
-		hero(hero_),
-		obs(obs_)
-	{
-	};
-
-	~ObstacleCasterProxy() = default;
-
-	int32_t getCasterUnitId() const override
-	{
-		if(hero)
-			return hero->getCasterUnitId();
-		else
-			return -1;
-	}
-
-	int32_t getSpellSchoolLevel(const Spell * spell, int32_t * outSelectedSchool = nullptr) const override
-	{
-		return obs->spellLevel;
-	}
-
-	int32_t getEffectLevel(const Spell * spell) const override
-	{
-		return obs->spellLevel;
-	}
-
-	int64_t getSpellBonus(const Spell * spell, int64_t base, const battle::Unit * affectedStack) const override
-	{
-		if(hero)
-			return hero->getSpellBonus(spell, base, affectedStack);
-		else
-			return base;
-	}
-
-	int64_t getSpecificSpellBonus(const Spell * spell, int64_t base) const override
-	{
-		if(hero)
-			return hero->getSpecificSpellBonus(spell, base);
-		else
-			return base;
-	}
-
-	int32_t getEffectPower(const Spell * spell) const override
-	{
-		return obs->casterSpellPower;
-	}
-
-	int32_t getEnchantPower(const Spell * spell) const override
-	{
-		return obs->casterSpellPower;
-	}
-
-	int64_t getEffectValue(const Spell * spell) const override
-	{
-		if(hero)
-			return hero->getEffectValue(spell);
-		else
-			return 0;
-	}
-
-	PlayerColor getCasterOwner() const override
-	{
-		return owner;
-	}
-
-	void getCasterName(MetaString & text) const override
-	{
-		logGlobal->error("Unexpected call to ObstacleCasterProxy::getCasterName");
-	}
-
-	void getCastDescription(const Spell * spell, const std::vector<const battle::Unit *> & attacked, MetaString & text) const override
-	{
-		logGlobal->error("Unexpected call to ObstacleCasterProxy::getCastDescription");
-	}
-
-	void spendMana(ServerCallback * server, const int spellCost) const override
-	{
-		logGlobal->error("Unexpected call to ObstacleCasterProxy::spendMana");
-	}
-
-private:
-	const CGHeroInstance * hero;
-	const PlayerColor owner;
-	const SpellCreatedObstacle * obs;
-};
-
-}//
-VCMI_LIB_NAMESPACE_END
 
 CondSh<bool> battleMadeAction(false);
 CondSh<BattleResult *> battleResult(nullptr);
@@ -1436,10 +1341,14 @@ int CGameHandler::moveStack(int stack, BattleHex dest)
 	ret = path.second;
 
 	int creSpeed = gs->curB->tacticDistance ? GameConstants::BFIELD_SIZE : curStack->Speed(0, true);
+	bool hasWideMoat = vstd::contains_if(battleGetAllObstaclesOnPos(BattleHex(ESiegeHex::GATE_BRIDGE), false), [](const std::shared_ptr<const CObstacleInstance> & obst)
+	{
+		return obst->obstacleType == CObstacleInstance::MOAT;
+	});
 
 	auto isGateDrawbridgeHex = [&](BattleHex hex) -> bool
 	{
-		if (gs->curB->town->subID == ETownType::FORTRESS && hex == ESiegeHex::GATE_BRIDGE)
+		if (hasWideMoat && hex == ESiegeHex::GATE_BRIDGE)
 			return true;
 		if (hex == ESiegeHex::GATE_OUTER)
 			return true;
@@ -1502,7 +1411,7 @@ int CGameHandler::moveStack(int stack, BattleHex dest)
 			{
 				auto needOpenGates = [&](BattleHex hex) -> bool
 				{
-					if (gs->curB->town->subID == ETownType::FORTRESS && hex == ESiegeHex::GATE_BRIDGE)
+					if (hasWideMoat && hex == ESiegeHex::GATE_BRIDGE)
 						return true;
 					if (hex == ESiegeHex::GATE_BRIDGE && i-1 >= 0 && path.first[i-1] == ESiegeHex::GATE_OUTER)
 						return true;
@@ -1538,7 +1447,7 @@ int CGameHandler::moveStack(int stack, BattleHex dest)
 					{
 						gateMayCloseAtHex = path.first[i-1];
 					}
-					if (gs->curB->town->subID == ETownType::FORTRESS)
+					if (hasWideMoat)
 					{
 						if (hex == ESiegeHex::GATE_BRIDGE && i-1 >= 0 && path.first[i-1] != ESiegeHex::GATE_OUTER)
 						{
@@ -1650,7 +1559,13 @@ int CGameHandler::moveStack(int stack, BattleHex dest)
 				stackIsMoving = false;
 		}
 	}
-
+	//handle last hex separately for deviation
+	if (VLC->settings()->getBoolean(EGameSettings::COMBAT_ONE_HEX_TRIGGERS_OBSTACLES))
+	{
+		if (dest == battle::Unit::occupiedHex(start, curStack->doubleWide(), curStack->side)
+			|| start == battle::Unit::occupiedHex(dest, curStack->doubleWide(), curStack->side))
+			passed.clear(); //Just empty passed, obstacles will handled automatically
+	}
 	//handling obstacle on the final field (separate, because it affects both flying and walking stacks)
 	handleDamageFromObstacle(curStack, false, passed);
 
@@ -4513,7 +4428,10 @@ void CGameHandler::updateGateState()
 	bool hasStackAtGateInner   = gs->curB->battleGetStackByPos(BattleHex(ESiegeHex::GATE_INNER), false) != nullptr;
 	bool hasStackAtGateOuter   = gs->curB->battleGetStackByPos(BattleHex(ESiegeHex::GATE_OUTER), false) != nullptr;
 	bool hasStackAtGateBridge  = gs->curB->battleGetStackByPos(BattleHex(ESiegeHex::GATE_BRIDGE), false) != nullptr;
-	bool hasLongBridge         = gs->curB->town->subID == ETownType::FORTRESS;
+	bool hasWideMoat         = vstd::contains_if(battleGetAllObstaclesOnPos(BattleHex(ESiegeHex::GATE_BRIDGE), false), [](const std::shared_ptr<const CObstacleInstance> & obst)
+	{
+		return obst->obstacleType == CObstacleInstance::MOAT;
+	});
 
 	BattleUpdateGateState db;
 	db.state = gs->curB->si.gateState;
@@ -4523,7 +4441,7 @@ void CGameHandler::updateGateState()
 	}
 	else if (db.state == EGateState::OPENED)
 	{
-		bool hasStackOnLongBridge = hasStackAtGateBridge && hasLongBridge;
+		bool hasStackOnLongBridge = hasStackAtGateBridge && hasWideMoat;
 		bool gateCanClose = !hasStackAtGateInner && !hasStackAtGateOuter && !hasStackOnLongBridge;
 
 		if (gateCanClose)
@@ -5317,77 +5235,50 @@ bool CGameHandler::handleDamageFromObstacle(const CStack * curStack, bool stackI
 {
 	if(!curStack->alive())
 		return false;
-	bool containDamageFromMoat = false;
 	bool movementStopped = false;
 	for(auto & obstacle : getAllAffectedObstaclesByStack(curStack, passed))
 	{
-		if(obstacle->obstacleType == CObstacleInstance::SPELL_CREATED)
-		{
-			//helper info
-			const SpellCreatedObstacle * spellObstacle = dynamic_cast<const SpellCreatedObstacle *>(obstacle.get());
-			const ui8 side = curStack->side;
+		//helper info
+		const SpellCreatedObstacle * spellObstacle = dynamic_cast<const SpellCreatedObstacle *>(obstacle.get());
 
-			if(!spellObstacle)
-				COMPLAIN_RET("Invalid obstacle instance");
-
-			if(spellObstacle->triggersEffects())
-			{
-				const bool oneTimeObstacle = spellObstacle->removeOnTrigger;
-
-				//hidden obstacle triggers effects until revealed
-				if(!(spellObstacle->hidden && gs->curB->battleIsObstacleVisibleForSide(*obstacle, (BattlePerspective::BattlePerspective)side)))
-				{
-					const CGHeroInstance * hero = gs->curB->battleGetFightingHero(spellObstacle->casterSide);
-					spells::ObstacleCasterProxy caster(gs->curB->sides.at(spellObstacle->casterSide).color, hero, spellObstacle);
-
-					const CSpell * sp = SpellID(spellObstacle->ID).toSpell();
-					if(!sp)
-						COMPLAIN_RET("Invalid obstacle instance");
-
-					// For the hidden spell created obstacles, e.g. QuickSand, it should be revealed after taking damage
-					ObstacleChanges changeInfo;
-					changeInfo.id = spellObstacle->uniqueID;
-					if (oneTimeObstacle)
-						changeInfo.operation = ObstacleChanges::EOperation::REMOVE;
-					else
-						changeInfo.operation = ObstacleChanges::EOperation::UPDATE;
-
-					SpellCreatedObstacle changedObstacle;
-					changedObstacle.uniqueID = spellObstacle->uniqueID;
-					changedObstacle.revealed = true;
-
-					changeInfo.data.clear();
-					JsonSerializer ser(nullptr, changeInfo.data);
-					ser.serializeStruct("obstacle", changedObstacle);
-
-					BattleObstaclesChanged bocp;
-					bocp.changes.emplace_back(changeInfo);
-					sendAndApply(&bocp);
-
-					spells::BattleCast battleCast(gs->curB, &caster, spells::Mode::HERO, sp);
-					battleCast.applyEffects(spellEnv, spells::Target(1, spells::Destination(curStack)), true);
-				}
-			}
-		}
-		else if(obstacle->obstacleType == CObstacleInstance::MOAT)
+		if(spellObstacle)
 		{
-			auto town = gs->curB->town;
-			int damage = (town == nullptr) ? 0 : town->town->moatDamage;
-			if(!containDamageFromMoat)
+			auto revealObstacles = [&](const SpellCreatedObstacle & spellObstacle) -> void
 			{
-				containDamageFromMoat = true;
+				// For the hidden spell created obstacles, e.g. QuickSand, it should be revealed after taking damage
+				auto operation = ObstacleChanges::EOperation::UPDATE;
+				if (spellObstacle.removeOnTrigger)
+					operation = ObstacleChanges::EOperation::REMOVE;
 
-				BattleStackAttacked bsa;
-				bsa.damageAmount = damage;
-				bsa.stackAttacked = curStack->ID;
-				bsa.attackerID = -1;
-				curStack->prepareAttacked(bsa, getRandomGenerator());
+				SpellCreatedObstacle changedObstacle;
+				changedObstacle.uniqueID = spellObstacle.uniqueID;
+				changedObstacle.revealed = true;
 
-				StacksInjured si;
-				si.stacks.push_back(bsa);
-				sendAndApply(&si);
-				sendGenericKilledLog(curStack, bsa.killedAmount, false);
+				BattleObstaclesChanged bocp;
+				bocp.changes.emplace_back(spellObstacle.uniqueID, operation);
+				changedObstacle.toInfo(bocp.changes.back(), operation);
+				sendAndApply(&bocp);
+			};
+			const auto side = curStack->side;
+			auto shouldReveal = !spellObstacle->hidden || !gs->curB->battleIsObstacleVisibleForSide(*obstacle, (BattlePerspective::BattlePerspective)side);
+			const auto * hero = gs->curB->battleGetFightingHero(spellObstacle->casterSide);
+			auto caster = spells::ObstacleCasterProxy(gs->curB->getSidePlayer(spellObstacle->casterSide), hero, *spellObstacle);
+			const auto * sp = obstacle->getTrigger().toSpell();
+			if(obstacle->triggersEffects() && sp)
+			{
+				auto cast = spells::BattleCast(gs->curB, &caster, spells::Mode::PASSIVE, sp);
+				spells::detail::ProblemImpl ignored;
+				auto target = spells::Target(1, spells::Destination(curStack));
+				if(sp->battleMechanics(&cast)->canBeCastAt(target, ignored)) // Obstacles should not be revealed by immune creatures
+				{
+					if(shouldReveal) { //hidden obstacle triggers effects after revealed
+						revealObstacles(*spellObstacle);
+						cast.cast(spellEnv, target);
+					}
+				}
 			}
+			else if(shouldReveal)
+				revealObstacles(*spellObstacle);
 		}
 
 		if(!curStack->alive())
@@ -6435,6 +6326,17 @@ void CGameHandler::runBattle()
 	assert(gs->curB);
 	//TODO: pre-tactic stuff, call scripts etc.
 
+	//Moat should be initialized here, because only here we can use spellcasting
+	if (gs->curB->town && gs->curB->town->fortLevel() >= CGTownInstance::CITADEL)
+	{
+		const auto * h = gs->curB->battleGetFightingHero(BattleSide::DEFENDER);
+		const auto * actualCaster = h ? static_cast<const spells::Caster*>(h) : nullptr;
+		auto moatCaster = spells::SilentCaster(gs->curB->getSidePlayer(BattleSide::DEFENDER), actualCaster);
+		auto cast = spells::BattleCast(gs->curB, &moatCaster, spells::Mode::PASSIVE, gs->curB->town->town->moatAbility.toSpell());
+		auto target = spells::Target();
+		cast.cast(spellEnv, target);
+	}
+
 	//tactic round
 	{
 		while (gs->curB->tacticDistance && !battleResult.get())
@@ -6943,7 +6845,7 @@ void CGameHandler::handleCheatCode(std::string & cheat, PlayerColor player, cons
 			giveHeroNewArtifact(hero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
 
 		///Give all spells with bonus (to allow banned spells)
-		GiveBonus giveBonus(GiveBonus::HERO);
+		GiveBonus giveBonus(GiveBonus::ETarget::HERO);
 		giveBonus.id = hero->id.getNum();
 		giveBonus.bonus = Bonus(Bonus::PERMANENT, Bonus::SPELLS_OF_LEVEL, Bonus::OTHER, 0, 0);
 		//start with level 0 to skip abilities
@@ -7089,7 +6991,7 @@ void CGameHandler::handleCheatCode(std::string & cheat, PlayerColor player, cons
 		smp.val = 1000000;
 		sendAndApply(&smp);
 
-		GiveBonus gb(GiveBonus::HERO);
+		GiveBonus gb(GiveBonus::ETarget::HERO);
 		gb.bonus.type = Bonus::FREE_SHIP_BOARDING;
 		gb.bonus.duration = Bonus::ONE_DAY;
 		gb.bonus.source = Bonus::OTHER;