vcmi: configurable teleport v2

1. Redesign wall and teleport penalty using shortest path
This will avoid OH3 exploits with teleport inside walls

2. Teleport is now configurable

client/battle/BattleActionsController.cpp

@@ -583,10 +583,7 @@ bool BattleActionsController::actionIsLegal(PossiblePlayerBattleAction action, B
 			return false;
 
 		case PossiblePlayerBattleAction::TELEPORT:
-		{
-			ui8 skill = getCurrentSpellcaster()->getEffectLevel(SpellID(SpellID::TELEPORT).toSpell());
-			return owner.curInt->cb->battleCanTeleportTo(selectedStack, targetHex, skill);
-		}
+			return selectedStack && isCastingPossibleHere(action.spell().toSpell(), selectedStack, targetHex);
 
 		case PossiblePlayerBattleAction::SACRIFICE: //choose our living stack to sacrifice
 			return targetStack && targetStack != selectedStack && targetStackOwned && targetStack->alive();
@@ -914,6 +911,8 @@ bool BattleActionsController::isCastingPossibleHere(const CSpell * currentSpell,
 	const spells::Mode mode = heroSpellToCast ? spells::Mode::HERO : spells::Mode::CREATURE_ACTIVE;
 
 	spells::Target target;
+	if(targetStack)
+		target.emplace_back(targetStack);
 	target.emplace_back(targetHex);
 
 	spells::BattleCast cast(owner.curInt->cb.get(), caster, mode, currentSpell);

config/spells/other.json

@@ -636,6 +636,21 @@
 					}
 				},
 				"targetModifier":{"smart":true}
+			},
+			"advanced" :{
+				"battleEffects":{
+					"teleport":{
+						"isMoatPassable" : true
+					}
+				}
+			},
+			"expert" : {
+				"battleEffects":{
+					"teleport":{
+						"isWallPassable" : true,
+						"isMoatPassable" : true
+					}
+				}
 			}
 		},
 		"flags" : {

lib/battle/CBattleInfoCallback.cpp

@@ -133,11 +133,13 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastSpell(con
 	return ESpellCastProblem::OK;
 }
 
-bool CBattleInfoCallback::battleHasWallPenalty(const IBonusBearer * shooter, BattleHex shooterPosition, BattleHex destHex) const
+bool CBattleInfoCallback::battleHasPenaltyOnLine(BattleHex from, BattleHex dest, bool checkWall, bool checkMoat) const
 {
 	auto isTileBlocked = [&](BattleHex tile)
 	{
 		EWallPart wallPart = battleHexToWallPart(tile);
+		if (wallPart == EWallPart::INVALID)
+			return false; // there is no wall here
 		if (wallPart == EWallPart::INDESTRUCTIBLE_PART_OF_GATE)
 			return false; // does not blocks ranged attacks
 		if (wallPart == EWallPart::INDESTRUCTIBLE_PART)
@@ -145,35 +147,54 @@ bool CBattleInfoCallback::battleHasWallPenalty(const IBonusBearer * shooter, Bat
 
 		return isWallPartAttackable(wallPart);
 	};
-
-	auto needWallPenalty = [&](BattleHex from, BattleHex dest)
+	// Count wall penalty requirement by shortest path, not by arbitrary line, to avoid various OH3 bugs
+	auto getShortestPath = [](BattleHex from, BattleHex dest) -> std::vector<BattleHex>
 	{
-		// arbitrary selected cell size for virtual grid
-		// any even number can be selected (for division by two)
-		static const int cellSize = 10;
+		//Out early
+		if(from == dest)
+			return {};
 
-		// create line that goes from center of shooter cell to center of target cell
-		Point line1{ from.getX()*cellSize+cellSize/2, from.getY()*cellSize+cellSize/2};
-		Point line2{ dest.getX()*cellSize+cellSize/2, dest.getY()*cellSize+cellSize/2};
+		std::vector<BattleHex> ret;
+		auto next = from;
+		//Not a real direction, only to indicate to which side we should search closest tile
+		auto direction = from.getX() > dest.getX() ? BattleSide::DEFENDER : BattleSide::ATTACKER;
 
-		for (int y = 0; y < GameConstants::BFIELD_HEIGHT; ++y)
+		while (next != dest)
 		{
-			BattleHex obstacle = lineToWallHex(y);
-			if (!isTileBlocked(obstacle))
-				continue;
-
-			// create rect around cell with an obstacle
-			Rect rect {
-				Point(obstacle.getX(), obstacle.getY()) * cellSize,
-				Point( cellSize, cellSize)
-			};
-
-			if ( rect.intersectionTest(line1, line2))
-				return true;
+			auto tiles = next.neighbouringTiles();
+			std::set<BattleHex> possibilities = {tiles.begin(), tiles.end()};
+			next = BattleHex::getClosestTile(direction, dest, possibilities);
+			ret.push_back(next);
 		}
-		return false;
+		assert(!ret.empty());
+		ret.pop_back(); //Remove destination hex
+		return ret;
 	};
 
+	RETURN_IF_NOT_BATTLE(false);
+	auto checkNeeded = !sameSideOfWall(from, dest);
+	bool pathHasWall = false;
+	bool pathHasMoat = false;
+
+	for(const auto & hex : getShortestPath(from, dest))
+	{
+		pathHasWall |= isTileBlocked(hex);
+		if(!checkMoat)
+			continue;
+
+		auto obstacles = battleGetAllObstaclesOnPos(hex, false);
+
+		if(hex != ESiegeHex::GATE_BRIDGE || (battleIsGatePassable()))
+			for(const auto & obst : obstacles)
+				if(obst->obstacleType ==  CObstacleInstance::MOAT)
+					pathHasMoat |= true;
+	}
+
+	return checkNeeded && ( (checkWall && pathHasWall) || (checkMoat && pathHasMoat) );
+}
+
+bool CBattleInfoCallback::battleHasWallPenalty(const IBonusBearer * shooter, BattleHex shooterPosition, BattleHex destHex) const
+{
 	RETURN_IF_NOT_BATTLE(false);
 	if(!battleGetSiegeLevel())
 		return false;
@@ -184,26 +205,9 @@ bool CBattleInfoCallback::battleHasWallPenalty(const IBonusBearer * shooter, Bat
 	if(shooter->hasBonus(selectorNoWallPenalty, cachingStrNoWallPenalty))
 		return false;
 
-	const int wallInStackLine = lineToWallHex(shooterPosition.getY());
-	const bool shooterOutsideWalls = shooterPosition < wallInStackLine;
+	const auto shooterOutsideWalls = shooterPosition < lineToWallHex(shooterPosition.getY());
 
-	return shooterOutsideWalls && needWallPenalty(shooterPosition, destHex);
-}
-
-si8 CBattleInfoCallback::battleCanTeleportTo(const battle::Unit * stack, BattleHex destHex, int telportLevel) const
-{
-	RETURN_IF_NOT_BATTLE(false);
-	if (!getAccesibility(stack).accessible(destHex, stack))
-		return false;
-
-	const ui8 siegeLevel = battleGetSiegeLevel();
-
-	//check for wall
-	//advanced teleport can pass wall of fort|citadel, expert - of castle
-	if ((siegeLevel > CGTownInstance::NONE && telportLevel < 2) || (siegeLevel >= CGTownInstance::CASTLE && telportLevel < 3))
-		return sameSideOfWall(stack->getPosition(), destHex);
-
-	return true;
+	return shooterOutsideWalls && battleHasPenaltyOnLine(shooterPosition, destHex, true, false);
 }
 
 std::vector<PossiblePlayerBattleAction> CBattleInfoCallback::getClientActionsForStack(const CStack * stack, const BattleClientInterfaceData & data)

lib/battle/CBattleInfoCallback.h

@@ -101,6 +101,7 @@ public:
 	DamageEstimation battleEstimateDamage(const battle::Unit * attacker, const battle::Unit * defender, BattleHex attackerPosition, DamageEstimation * retaliationDmg = nullptr) const;
 	DamageEstimation battleEstimateDamage(const battle::Unit * attacker, const battle::Unit * defender, int movementDistance, DamageEstimation * retaliationDmg = nullptr) const;
 
+	bool battleHasPenaltyOnLine(BattleHex from, BattleHex dest, bool checkWall, bool checkMoat) const;
 	bool battleHasDistancePenalty(const IBonusBearer * shooter, BattleHex shooterPosition, BattleHex destHex) const;
 	bool battleHasWallPenalty(const IBonusBearer * shooter, BattleHex shooterPosition, BattleHex destHex) const;
 	bool battleHasShootingPenalty(const battle::Unit * shooter, BattleHex destHex) const;
@@ -120,7 +121,6 @@ public:
 	SpellID getRandomBeneficialSpell(CRandomGenerator & rand, const CStack * subject) const;
 	SpellID getRandomCastedSpell(CRandomGenerator & rand, const CStack * caster) const; //called at the beginning of turn for Faerie Dragon
 
-	si8 battleCanTeleportTo(const battle::Unit * stack, BattleHex destHex, int telportLevel) const; //checks if teleportation of given stack to given position can take place
 	std::vector<PossiblePlayerBattleAction> getClientActionsForStack(const CStack * stack, const BattleClientInterfaceData & data);
 	PossiblePlayerBattleAction getCasterAction(const CSpell * spell, const spells::Caster * caster, spells::Mode mode) const;
 

lib/spells/effects/Teleport.cpp

@@ -14,6 +14,7 @@
 #include "../ISpellMechanics.h"
 #include "../../NetPacks.h"
 #include "../../battle/CBattleInfoCallback.h"
+#include "../../serializer/JsonSerializeFormat.h"
 #include "../../battle/Unit.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
@@ -45,39 +46,34 @@ void Teleport::adjustTargetTypes(std::vector<TargetType> & types) const
 	}
 }
 
-bool Teleport::applicable(Problem & problem, const Mechanics * m) const
+bool Teleport::applicable(Problem & problem, const Mechanics * m, const EffectTarget & target) const
 {
-	return UnitEffect::applicable(problem, m);
+	if(target.size() == 1) //Assume, this is check only for selecting a unit
+		return UnitEffect::applicable(problem, m);
+
+	if(target.size() != 2)
+		return m->adaptProblem(ESpellCastProblem::WRONG_SPELL_TARGET, problem);
+
+	const auto *targetUnit = target[0].unitValue;
+	const auto & targetHex = target[1].hexValue;
+
+	if(!targetUnit)
+		return m->adaptProblem(ESpellCastProblem::WRONG_SPELL_TARGET, problem);
+
+	if(!targetHex.isValid() || !m->battle()->getAccesibility(targetUnit).accessible(targetHex, targetUnit))
+		return m->adaptProblem(ESpellCastProblem::WRONG_SPELL_TARGET, problem);
+
+	if(m->battle()->battleGetSiegeLevel() && !(isWallPassable && isMoatPassable))
+	{
+		return !m->battle()->battleHasPenaltyOnLine(target[0].hexValue, target[1].hexValue, !isWallPassable, !isMoatPassable);
+	}
+	return true;
 }
 
 void Teleport::apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const
 {
-	if(target.size() != 2)
-	{
-		server->complain("Teleport requires 2 destinations.");
-		return;
-	}
-
 	const auto *targetUnit = target[0].unitValue;
-	if(nullptr == targetUnit)
-	{
-		server->complain("No unit to teleport");
-		return;
-	}
-
-	const BattleHex destination = target[1].hexValue;
-	if(!destination.isValid())
-	{
-		server->complain("Invalid teleport destination");
-		return;
-	}
-
-	//TODO: move here all teleport checks
-	if(!m->battle()->battleCanTeleportTo(targetUnit, destination, m->getEffectLevel()))
-	{
-		server->complain("Forbidden teleport.");
-		return;
-	}
+	const auto destination = target[1].hexValue;
 
 	BattleStackMoved pack;
 	pack.distance = 0;
@@ -91,7 +87,8 @@ void Teleport::apply(ServerCallback * server, const Mechanics * m, const EffectT
 
 void Teleport::serializeJsonUnitEffect(JsonSerializeFormat & handler)
 {
-	//TODO: teleport options
+	handler.serializeBool("isWallPassable", isWallPassable);
+	handler.serializeBool("isMoatPassable", isMoatPassable);
 }
 
 EffectTarget Teleport::transformTarget(const Mechanics * m, const Target & aimPoint, const Target & spellTarget) const

lib/spells/effects/Teleport.h

@@ -26,7 +26,7 @@ class Teleport : public UnitEffect
 public:
 	void adjustTargetTypes(std::vector<TargetType> & types) const override;
 
-	bool applicable(Problem & problem, const Mechanics * m) const override;
+	bool applicable(Problem & problem, const Mechanics * m, const EffectTarget & target) const override;
 
 	void apply(ServerCallback * server, const Mechanics * m, const EffectTarget & target) const override;
 
@@ -34,6 +34,10 @@ public:
 
 protected:
 	void serializeJsonUnitEffect(JsonSerializeFormat & handler) override;
+
+private:
+	bool isWallPassable;
+	bool isMoatPassable;
 };
 
 }