Merge pull request #5755 from IvanSavenko/misc_fixes

Miscelllaneous fixes

Mods/vcmi/Content/config/english.json

@@ -404,6 +404,9 @@
 	"vcmi.battleWindow.endWithAutocombat" : "Are you sure you wish to end the battle with auto combat?",
 
 	"vcmi.battleResultsWindow.applyResultsLabel" : "Accept battle result?",
+	"vcmi.battleResultsWindow.spellDurationRemaining.0" : "Remaining duration : %d combat rounds",
+	"vcmi.battleResultsWindow.spellDurationRemaining.1" : "Remaining duration : %d combat round",
+	"vcmi.battleResultsWindow.spellDurationRemaining.2" : "Remaining duration : %d combat rounds",
 
 	"vcmi.tutorialWindow.title" : "Touchscreen Introduction",
 	"vcmi.tutorialWindow.decription.RightClick" : "Touch and hold the element on which you want to right-click. Touch the free area to close.",
@@ -740,5 +743,16 @@
 	"spell.core.strongholdMoat.name" : "Wooden Spikes",
 	"spell.core.strongholdMoatTrigger.name" : "Wooden Spikes",
 	"spell.core.summonDemons.name" : "Summon Demons",
-	"spell.core.towerMoat.name" : "Land Mine"
+	"spell.core.towerMoat.name" : "Land Mine",
+
+	"spell.core.stoneGaze.description.none" : "{Stone Gaze}\n\nThe targeted unit is petrified and cannot move for three combat rounds. When attacked it takes 50% damage, and become unfrozen",
+	"spell.core.poison.description.none" : "{Poison}\n\nWhen poisoned, the target stack's maximum health is decreased by 10% for every combat round for three rounds. After three rounds, the creature is no longer poisoned, but the maximum health remains decreased",
+	"spell.core.bind.description.none" : "{Bind}\n\nTargeted unit is bound to the ground, unable to move until the stack that entangled them moves or perishes.",
+	"spell.core.disease.description.none" : "{Disease}\n\nThe living target becomes ill, thus having their attack and defense values reduced by two for a period of three rounds",
+	"spell.core.paralyze.description.none" : "{Paralyze}\n\nThe target is paralyzed and forgo their turn for the remainder of the current combat round and the two combat rounds thereafter, unless they are attacked meanwhile. Paralyzed creatures take full damage from attacks, but retaliate at only a quarter of their strength.",
+	"spell.core.age.description.none" : "{Age}\n\nAging lowers the maximum health of every creature in the target stack down to 50% for three combat rounds",
+	"spell.core.deathCloud.description.none" : "{Death Cloud}\n\nIn addition to normal ranged attack damage in target hex, the death cloud also affects all 6 adjacent hexes around the target hex dealing damage to all living creatures in the radius",
+	"spell.core.thunderbolt.description.none" : "{Thunderbolt}\n\nWhen the stack attacks, the there is a 20% chance that a lightning strike occurs before the enemy has a chance to retaliate. If if occurs, the lightning strike deals damage equal to ten times the number of attacking Thunderbirds",
+	"spell.core.dispelHelpful.description.none" : "{Dispel Helpful Spells}\n\nRemoves all spell effects from the targeted unit.",
+	"spell.core.acidBreath.description.none" : "{Acid breath}\n\nThe breath reduces the target stack's defense by 3, and has 20% chance to cause additional damage amount of 25 points per attacking unit.",
 }

client/battle/BattleActionsController.cpp

@@ -647,12 +647,15 @@ bool BattleActionsController::actionIsLegal(PossiblePlayerBattleAction action, c
 		case PossiblePlayerBattleAction::ATTACK:
 		case PossiblePlayerBattleAction::WALK_AND_ATTACK:
 		case PossiblePlayerBattleAction::ATTACK_AND_RETURN:
-			if(owner.getBattle()->battleCanAttack(owner.stacksController->getActiveStack(), targetStack, targetHex))
 			{
 				if (owner.fieldController->isTileAttackable(targetHex)) // move isTileAttackable to be part of battleCanAttack?
-					return true;
+				{
+					BattleHex attackFromHex = owner.fieldController->fromWhichHexAttack(targetHex);
+					if(owner.getBattle()->battleCanAttack(owner.stacksController->getActiveStack(), targetStack, attackFromHex))
+						return true;
+				}
+				return false;
 			}
-			return false;
 
 		case PossiblePlayerBattleAction::SHOOT:
 			{

client/battle/BattleFieldController.cpp

@@ -377,12 +377,12 @@ BattleHexArray BattleFieldController::getHighlightedHexesForMovementTarget()
 	BattleHexArray availableHexes = owner.getBattle()->battleGetAvailableHexes(stack, false, false, nullptr);
 
 	auto hoveredStack = owner.getBattle()->battleGetStackByPos(hoveredHex, true);
-	if(owner.getBattle()->battleCanAttack(stack, hoveredStack, hoveredHex))
+
+	if(isTileAttackable(hoveredHex))
 	{
-		if(isTileAttackable(hoveredHex))
+		BattleHex attackFromHex = fromWhichHexAttack(hoveredHex);
+		if(owner.getBattle()->battleCanAttack(stack, hoveredStack, attackFromHex))
 		{
-			BattleHex attackFromHex = fromWhichHexAttack(hoveredHex);
-
 			if(stack->doubleWide())
 				return {attackFromHex, stack->occupiedHex(attackFromHex)};
 			else

client/widgets/CComponent.cpp

@@ -182,7 +182,7 @@ size_t CComponent::getIndex() const
 		case ComponentType::MANA:
 			return 5; // for whatever reason, in H3 mana points icon is located in primary skills icons
 		case ComponentType::SEC_SKILL:
-			return data.subType.getNum() * 3 + 3 + data.value.value_or(0) - 1;
+			return data.subType.getNum() * 3 + 3 + data.value.value_or(1) - 1;
 		case ComponentType::RESOURCE:
 		case ComponentType::RESOURCE_PER_DAY:
 			return data.subType.getNum();
@@ -221,7 +221,7 @@ std::string CComponent::getDescription() const
 		case ComponentType::MANA:
 			return LIBRARY->generaltexth->allTexts[149];
 		case ComponentType::SEC_SKILL:
-			return LIBRARY->skillh->getByIndex(data.subType.getNum())->getDescriptionTranslated(data.value.value_or(0));
+			return LIBRARY->skillh->getByIndex(data.subType.getNum())->getDescriptionTranslated(data.value.value_or(1));
 		case ComponentType::RESOURCE:
 		case ComponentType::RESOURCE_PER_DAY:
 			return LIBRARY->generaltexth->allTexts[242];
@@ -280,7 +280,10 @@ std::string CComponent::getSubtitle() const
 		case ComponentType::MANA:
 			return boost::str(boost::format("%+d %s") % data.value.value_or(0) % LIBRARY->generaltexth->allTexts[387]);
 		case ComponentType::SEC_SKILL:
-			return LIBRARY->generaltexth->levels[data.value.value_or(0)-1] + "\n" + LIBRARY->skillh->getById(data.subType.as<SecondarySkill>())->getNameTranslated();
+			if (data.value)
+				return LIBRARY->generaltexth->levels[data.value.value_or(1)-1] + "\n" + LIBRARY->skillh->getById(data.subType.as<SecondarySkill>())->getNameTranslated();
+			else
+				return LIBRARY->skillh->getById(data.subType.as<SecondarySkill>())->getNameTranslated();
 		case ComponentType::RESOURCE:
 			return std::to_string(data.value.value_or(0));
 		case ComponentType::RESOURCE_PER_DAY:

client/windows/CCreatureWindow.cpp

@@ -41,6 +41,7 @@
 #include "../../lib/networkPacks/ArtifactLocation.h"
 #include "../../lib/texts/CGeneralTextHandler.h"
 #include "../../lib/texts/TextOperations.h"
+#include "../../lib/texts/Languages.h"
 
 class CCreatureArtifactInstance;
 class CSelectableSkill;
@@ -224,27 +225,29 @@ CStackWindow::ActiveSpellsSection::ActiveSpellsSection(CStackWindow * owner, int
 	{
 		const spells::Spell * spell = LIBRARY->spells()->getById(effect);
 
-		std::string spellText;
-
 		//not all effects have graphics (for eg. Acid Breath)
 		//for modded spells iconEffect is added to SpellInt.def
 		const bool hasGraphics = (effect < SpellID::THUNDERBOLT) || (effect >= SpellID::AFTER_LAST);
 
 		if (hasGraphics)
 		{
-			spellText = LIBRARY->generaltexth->allTexts[610]; //"%s, duration: %d rounds."
-			boost::replace_first(spellText, "%s", spell->getNameTranslated());
-			//FIXME: support permanent duration
 			auto spellBonuses = battleStack->getBonuses(Selector::source(BonusSource::SPELL_EFFECT, BonusSourceID(effect)));
 			if (spellBonuses->empty())
 				throw std::runtime_error("Failed to find effects for spell " + effect.toSpell()->getJsonKey());
 
 			int duration = spellBonuses->front()->turnsRemain;
-			boost::replace_first(spellText, "%d", std::to_string(duration));
+			std::string preferredLanguage = LIBRARY->generaltexth->getPreferredLanguage();
+
+			MetaString spellText;
+			spellText.appendTextID(spell->getDescriptionTextID(0)); // TODO: select correct mastery level?
+			spellText.appendRawString("\n");
+			spellText.appendTextID(Languages::getPluralFormTextID( preferredLanguage, duration, "vcmi.battleResultsWindow.spellDurationRemaining"));
+			spellText.replaceNumber(duration);
+			std::string spellDescription = spellText.toString();
 
 			spellIcons.push_back(std::make_shared<CAnimImage>(AnimationPath::builtin("SpellInt"), effect + 1, 0, firstPos.x + offset.x * printed, firstPos.y + offset.y * printed));
 			labels.push_back(std::make_shared<CLabel>(firstPos.x + offset.x * printed + 46, firstPos.y + offset.y * printed + 36, EFonts::FONT_TINY, ETextAlignment::BOTTOMRIGHT, Colors::WHITE, std::to_string(duration)));
-			clickableAreas.push_back(std::make_shared<LRClickableAreaWText>(Rect(firstPos + offset * printed, Point(50, 38)), spellText, spellText));
+			clickableAreas.push_back(std::make_shared<LRClickableAreaWText>(Rect(firstPos + offset * printed, Point(50, 38)), spellDescription, spellDescription));
 			if(++printed >= 8) // interface limit reached
 				break;
 		}

lib/battle/BattleInfo.cpp

@@ -342,6 +342,8 @@ std::unique_ptr<BattleInfo> BattleInfo::setupBattle(IGameInfoCallback *cb, const
 
 			if (pos.isValid())
 				currentBattle->generateNewStack(currentBattle->nextUnitId(), *i->second, side, i->first, pos);
+			else
+				logMod->warn("Invalid battlefield layout! Failed to find position for unit %d for %s", k, side == BattleSide::ATTACKER ? "attacker" : "defender");
 		}
 	}
 

lib/battle/CBattleInfoCallback.cpp

@@ -683,6 +683,18 @@ bool CBattleInfoCallback::battleCanAttack(const battle::Unit * stack, const batt
 	if(!battleMatchOwner(stack, target))
 		return false;
 
+	if (stack->getPosition() != dest)
+	{
+		for (const auto & obstacle : battleGetAllObstacles())
+		{
+			if (obstacle->getStoppingTile().contains(dest))
+				return false;
+
+			if (stack->doubleWide() && obstacle->getStoppingTile().contains(stack->occupiedHex(dest)))
+				return false;
+		}
+	}
+
 	auto id = stack->unitType()->getId();
 	if (id == CreatureID::FIRST_AID_TENT || id == CreatureID::CATAPULT)
 		return false;

lib/mapObjects/CRewardableObject.cpp

@@ -85,6 +85,12 @@ void CRewardableObject::battleFinished(IGameEventCallback & gameEvents, const CG
 {
 	if (result.winner == BattleSide::ATTACKER)
 	{
+		while(!stacks.empty())
+		{
+			logMod->warn("Attacker won, but defender still has remaining units in configurable map objects! Make sure that guards configuration is valid!");
+			gameEvents.eraseStack(StackLocation(id, stacks.begin()->first));
+		}
+
 		doHeroVisit(gameEvents, hero);
 	}
 }

lib/rewardable/Reward.cpp

@@ -107,8 +107,11 @@ void Rewardable::Reward::loadComponents(std::vector<Component> & comps, const CG
 		auto skillID = entry.first;
 		int levelsGained = entry.second;
 		int currentLevel = h ? h->getSecSkillLevel(skillID) : 0;
-		int finalLevel = std::clamp<int>(currentLevel + levelsGained, MasteryLevel::BASIC, MasteryLevel::EXPERT);
-		comps.emplace_back(ComponentType::SEC_SKILL, entry.first, finalLevel);
+		int finalLevel = std::clamp<int>(currentLevel + levelsGained, MasteryLevel::NONE, MasteryLevel::EXPERT);
+		if (finalLevel == MasteryLevel::NONE)
+			comps.emplace_back(ComponentType::SEC_SKILL, entry.first);
+		else
+			comps.emplace_back(ComponentType::SEC_SKILL, entry.first, finalLevel);
 	}
 
 	for(const auto & entry : grantedArtifacts)

lib/spells/BattleSpellMechanics.cpp

@@ -283,7 +283,8 @@ void BattleSpellMechanics::cast(ServerCallback * server, const Target & target)
 	sc.tile = target.at(0).hexValue;
 
 	sc.castByHero = mode == Mode::HERO;
-	sc.casterStack = caster->getCasterUnitId();
+	if (mode != Mode::HERO)
+		sc.casterStack = caster->getCasterUnitId();
 	sc.manaGained = 0;
 
 	sc.activeCast = false;

server/battles/BattleActionProcessor.cpp

@@ -143,8 +143,8 @@ bool BattleActionProcessor::doWalkAction(const CBattleInfoCallback & battle, con
 		return false;
 	}
 
-	int walkedTiles = moveStack(battle, ba.stackNumber, target.at(0).hexValue); //move
-	if (!walkedTiles)
+	auto movementResult = moveStack(battle, ba.stackNumber, target.at(0).hexValue); //move
+	if (movementResult.invalidRequest)
 	{
 		gameHandler->complain("Stack failed movement!");
 		return false;
@@ -229,14 +229,20 @@ bool BattleActionProcessor::doAttackAction(const CBattleInfoCallback & battle, c
 	}
 
 	BattleHex startingPos = stack->getPosition();
-	int distance = moveStack(battle, ba.stackNumber, attackPos);
+	const auto movementResult = moveStack(battle, ba.stackNumber, attackPos);
 
 	logGlobal->trace("%s will attack %s", stack->nodeName(), destinationStack->nodeName());
 
-	if(stack->getPosition() != attackPos && !(stack->doubleWide() && (stack->getPosition() == attackPos.cloneInDirection(stack->destShiftDir(), false))) )
+	if (movementResult.invalidRequest)
+	{
+		gameHandler->complain("Stack failed attack - unable to reach target!");
+		return false;
+	}
+
+	if(movementResult.obstacleHit)
 	{
 		// we were not able to reach destination tile, nor occupy specified hex
-		// abort attack attempt, but treat this case as legal - we may have stepped onto a quicksands/mine
+		// abort attack attempt, but treat this case as legal - we have stepped onto a quicksands/mine
 		return true;
 	}
 
@@ -285,7 +291,7 @@ bool BattleActionProcessor::doAttackAction(const CBattleInfoCallback & battle, c
 		//move can cause death, eg. by walking into the moat, first strike can cause death or paralysis/petrification
 		if(stack->alive() && !stack->hasBonusOfType(BonusType::NOT_ACTIVE) && destinationStack->alive())
 		{
-			makeAttack(battle, stack, destinationStack, (i ? 0 : distance), destinationTile, i==0, false, false);//no distance travelled on second attack
+			makeAttack(battle, stack, destinationStack, (i ? 0 : movementResult.distance), destinationTile, i==0, false, false);//no distance travelled on second attack
 
 			if(!ferocityApplied && stack->hasBonusOfType(BonusType::FEROCITY))
 			{
@@ -615,10 +621,8 @@ bool BattleActionProcessor::makeBattleActionImpl(const CBattleInfoCallback & bat
 	return result;
 }
 
-int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int stack, BattleHex dest)
+BattleActionProcessor::MovementResult BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int stack, BattleHex dest)
 {
-	int ret = 0;
-
 	const CStack *curStack = battle.battleGetStackByID(stack);
 	const CStack *stackAtEnd = battle.battleGetStackByPos(dest);
 
@@ -632,7 +636,7 @@ int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int sta
 
 	auto start = curStack->getPosition();
 	if (start == dest)
-		return 0;
+		return { 0, false, false };
 
 	//initing necessary tables
 	auto accessibility = battle.getAccessibility(curStack);
@@ -654,7 +658,7 @@ int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int sta
 	if((stackAtEnd && stackAtEnd!=curStack && stackAtEnd->alive()) || !accessibility.accessible(dest, curStack))
 	{
 		gameHandler->complain("Given destination is not accessible!");
-		return 0;
+		return { 0, false, true };
 	}
 
 	bool canUseGate = false;
@@ -667,8 +671,8 @@ int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int sta
 	}
 
 	std::pair< BattleHexArray, int > path = battle.getPath(start, dest, curStack);
-
-	ret = path.second;
+	int8_t passedHexes = path.second;
+	bool movementSuccess = true;
 
 	int creSpeed = curStack->getMovementRange(0);
 
@@ -805,15 +809,10 @@ int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int sta
 			}
 		}
 
-		bool stackIsMoving = true;
-
-		while(stackIsMoving)
+		while(movementSuccess)
 		{
 			if (v<tilesToMove)
-			{
-				logGlobal->error("Movement terminated abnormally");
-				break;
-			}
+				throw std::runtime_error("Movement terminated abnormally");
 
 			bool gateStateChanging = false;
 			//special handling for opening gate on from starting hex
@@ -865,9 +864,9 @@ int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int sta
 			//we don't handle obstacle at the destination tile -> it's handled separately in the if at the end
 			if (curStack->getPosition() != dest)
 			{
-				if(stackIsMoving && start != curStack->getPosition())
+				if(movementSuccess && start != curStack->getPosition())
 				{
-					stackIsMoving = battle.handleObstacleTriggersForUnit(*gameHandler->spellEnv, *curStack, passed);
+					movementSuccess &= battle.handleObstacleTriggersForUnit(*gameHandler->spellEnv, *curStack, passed);
 					passed.insert(curStack->getPosition());
 					if(curStack->doubleWide())
 						passed.insert(curStack->occupiedHex());
@@ -894,8 +893,10 @@ int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int sta
 				}
 			}
 			else
+			{
 				//movement finished normally: we reached destination
-				stackIsMoving = false;
+				break;
+			}
 		}
 	}
 	//handle last hex separately for deviation
@@ -908,9 +909,9 @@ int BattleActionProcessor::moveStack(const CBattleInfoCallback & battle, int sta
 	if(dest == start) 	//If dest is equal to start, then we should handle obstacles for it anyway
 		passed.clear();	//Just empty passed, obstacles will handled automatically
 	//handling obstacle on the final field (separate, because it affects both flying and walking stacks)
-	battle.handleObstacleTriggersForUnit(*gameHandler->spellEnv, *curStack, passed);
+	movementSuccess &= battle.handleObstacleTriggersForUnit(*gameHandler->spellEnv, *curStack, passed);
 
-	return ret;
+	return { passedHexes, !movementSuccess, false };
 }
 
 void BattleActionProcessor::makeAttack(const CBattleInfoCallback & battle, const CStack * attacker, const CStack * defender, int distance, const BattleHex & targetHex, bool first, bool ranged, bool counter)

server/battles/BattleActionProcessor.h

@@ -36,12 +36,22 @@ class BattleProcessor;
 /// Processes incoming battle action queries and applies requested action(s)
 class BattleActionProcessor : boost::noncopyable
 {
+	struct MovementResult
+	{
+		/// number of tiles unit moved through, unset for flying units
+		int16_t distance;
+		/// Unit failed to complete movement due to stepping into obstacle
+		bool obstacleHit;
+		/// Unit was unable to move to destination, e.g. invalid request
+		bool invalidRequest;
+	};
+
 	using FireShieldInfo = std::vector<std::pair<const CStack *, int64_t>>;
 
 	BattleProcessor * owner;
 	CGameHandler * gameHandler;
 
-	int moveStack(const CBattleInfoCallback & battle, int stack, BattleHex dest); //returned value - travelled distance
+	MovementResult moveStack(const CBattleInfoCallback & battle, int stack, BattleHex dest); //returned value - travelled distance
 	void makeAttack(const CBattleInfoCallback & battle, const CStack * attacker, const CStack * defender, int distance, const BattleHex & targetHex, bool first, bool ranged, bool counter);
 
 	void handleAttackBeforeCasting(const CBattleInfoCallback & battle, bool ranged, const CStack * attacker, const CStack * defender);