Code cleanup, fixes to discovered regressions

include/vcmi/Creature.h

@@ -26,6 +26,7 @@ public:
 	virtual ui32 getMovementRange(int turn) const; //get speed (in moving tiles) of creature with all modificators
 	virtual ui32 getMovementRange() const; //get speed (in moving tiles) of creature with all modificators
 	virtual ui32 getMaxHealth() const; //get max HP of stack with all modifiers
+	virtual int32_t getInitiative(int turn = 0) const;
 };
 
 template <typename IdType>

lib/BasicTypes.cpp

@@ -152,6 +152,19 @@ ui32 ACreature::getMovementRange() const
 	return getBonusBearer()->valOfBonuses(BonusType::STACKS_SPEED);
 }
 
+int32_t ACreature::getInitiative(int turn) const
+{
+	if (turn == 0)
+	{
+		return getBonusBearer()->valOfBonuses(BonusType::STACKS_SPEED);
+	}
+	else
+	{
+		const std::string cachingStrSS = "type_STACKS_SPEED_turns_" + std::to_string(turn);
+		return getBonusBearer()->valOfBonuses(Selector::type()(BonusType::STACKS_SPEED).And(Selector::turns(turn)), cachingStrSS);
+	}
+}
+
 ui32 ACreature::getMovementRange(int turn) const
 {
 	if (turn == 0)

lib/battle/CUnitState.cpp

@@ -335,7 +335,7 @@ CUnitState::CUnitState():
 	shots(this),
 	stackSpeedPerTurn(this, Selector::type()(BonusType::STACKS_SPEED), BonusCacheMode::VALUE),
 	immobilizedPerTurn(this, Selector::type()(BonusType::SIEGE_WEAPON).Or(Selector::type()(BonusType::BIND_EFFECT)), BonusCacheMode::PRESENCE),
-	bonusCache(this, generateBonusSelectors()),
+	bonusCache(this),
 	cloneID(-1)
 {
 
@@ -933,38 +933,6 @@ void CUnitState::onRemoved()
 	ghost = true;
 }
 
-const UnitBonusValuesProxy::SelectorsArray * CUnitState::generateBonusSelectors()
-{
-	static const CSelector additionalAttack = Selector::type()(BonusType::ADDITIONAL_ATTACK);
-	static const CSelector selectorMelee = Selector::effectRange()(BonusLimitEffect::NO_LIMIT).Or(Selector::effectRange()(BonusLimitEffect::ONLY_MELEE_FIGHT));
-	static const CSelector selectorRanged = Selector::effectRange()(BonusLimitEffect::NO_LIMIT).Or(Selector::effectRange()(BonusLimitEffect::ONLY_DISTANCE_FIGHT));
-	static const CSelector minDamage = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageMin));
-	static const CSelector maxDamage = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageMax));
-	static const CSelector attack = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::ATTACK));
-	static const CSelector defence = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::DEFENSE));
-
-	static const UnitBonusValuesProxy::SelectorsArray selectors = {
-		additionalAttack.And(selectorMelee), //TOTAL_ATTACKS_MELEE,
-		additionalAttack.And(selectorRanged), //TOTAL_ATTACKS_RANGED,
-		minDamage.And(selectorMelee), //MIN_DAMAGE_MELEE,
-		minDamage.And(selectorRanged), //MIN_DAMAGE_RANGED,
-		maxDamage.And(selectorMelee), //MAX_DAMAGE_MELEE,
-		maxDamage.And(selectorRanged), //MAX_DAMAGE_RANGED,
-		attack.And(selectorRanged),//ATTACK_MELEE,
-		attack.And(selectorRanged),//ATTACK_RANGED,
-		defence.And(selectorRanged),//DEFENCE_MELEE,
-		defence.And(selectorRanged),//DEFENCE_RANGED,
-		Selector::type()(BonusType::IN_FRENZY),//IN_FRENZY,
-		Selector::type()(BonusType::FORGETFULL),//FORGETFULL,
-		Selector::type()(BonusType::HYPNOTIZED),//HYPNOTIZED,
-		Selector::type()(BonusType::FREE_SHOOTING).Or(Selector::type()(BonusType::SIEGE_WEAPON)),//HAS_FREE_SHOOTING,
-		Selector::type()(BonusType::STACK_HEALTH),//STACK_HEALTH,
-		Selector::type()(BonusType::NONE).And(Selector::source(BonusSource::SPELL_EFFECT, BonusSourceID(SpellID(SpellID::CLONE))))
-	};
-
-	return &selectors;
-}
-
 CUnitStateDetached::CUnitStateDetached(const IUnitInfo * unit_, const IBonusBearer * bonus_):
 	unit(unit_),
 	bonus(bonus_)

lib/battle/CUnitState.h

@@ -260,8 +260,6 @@ public:
 	void onRemoved();
 
 private:
-	static const UnitBonusValuesProxy::SelectorsArray * generateBonusSelectors();
-
 	const IUnitEnvironment * env;
 
 	BonusCachePerTurn immobilizedPerTurn;

lib/battle/Unit.h

@@ -115,8 +115,6 @@ public:
 	virtual BattleHex getPosition() const = 0;
 	virtual void setPosition(BattleHex hex) = 0;
 
-	virtual int32_t getInitiative(int turn = 0) const = 0;
-
 	virtual bool canMove(int turn = 0) const = 0; //if stack can move
 	virtual bool defended(int turn = 0) const = 0;
 	virtual bool moved(int turn = 0) const = 0; //if stack was already moved this turn

lib/bonuses/BonusCache.cpp

@@ -43,7 +43,7 @@ int BonusCacheBase::getBonusValueImpl(BonusCacheEntry & currentValue, const CSel
 	}
 }
 
-BonusValueCache::BonusValueCache(const IBonusBearer * target, const CSelector selector)
+BonusValueCache::BonusValueCache(const IBonusBearer * target, const CSelector & selector)
 	:BonusCacheBase(target),selector(selector)
 {}
 
@@ -177,4 +177,36 @@ int BonusCachePerTurn::getValue(int turns) const
 	}
 }
 
+const UnitBonusValuesProxy::SelectorsArray * UnitBonusValuesProxy::generateSelectors()
+{
+	static const CSelector additionalAttack = Selector::type()(BonusType::ADDITIONAL_ATTACK);
+	static const CSelector selectorMelee = Selector::effectRange()(BonusLimitEffect::NO_LIMIT).Or(Selector::effectRange()(BonusLimitEffect::ONLY_MELEE_FIGHT));
+	static const CSelector selectorRanged = Selector::effectRange()(BonusLimitEffect::NO_LIMIT).Or(Selector::effectRange()(BonusLimitEffect::ONLY_DISTANCE_FIGHT));
+	static const CSelector minDamage = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageMin));
+	static const CSelector maxDamage = Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageMax));
+	static const CSelector attack = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::ATTACK));
+	static const CSelector defence = Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::DEFENSE));
+
+	static const UnitBonusValuesProxy::SelectorsArray selectors = {
+		additionalAttack.And(selectorMelee), //TOTAL_ATTACKS_MELEE,
+		additionalAttack.And(selectorRanged), //TOTAL_ATTACKS_RANGED,
+		minDamage.And(selectorMelee), //MIN_DAMAGE_MELEE,
+		minDamage.And(selectorRanged), //MIN_DAMAGE_RANGED,
+		maxDamage.And(selectorMelee), //MAX_DAMAGE_MELEE,
+		maxDamage.And(selectorRanged), //MAX_DAMAGE_RANGED,
+		attack.And(selectorRanged),//ATTACK_MELEE,
+		attack.And(selectorRanged),//ATTACK_RANGED,
+		defence.And(selectorRanged),//DEFENCE_MELEE,
+		defence.And(selectorRanged),//DEFENCE_RANGED,
+		Selector::type()(BonusType::IN_FRENZY),//IN_FRENZY,
+		Selector::type()(BonusType::FORGETFULL),//FORGETFULL,
+		Selector::type()(BonusType::HYPNOTIZED),//HYPNOTIZED,
+		Selector::type()(BonusType::FREE_SHOOTING).Or(Selector::type()(BonusType::SIEGE_WEAPON)),//HAS_FREE_SHOOTING,
+		Selector::type()(BonusType::STACK_HEALTH),//STACK_HEALTH,
+		Selector::type()(BonusType::NONE).And(Selector::source(BonusSource::SPELL_EFFECT, BonusSourceID(SpellID(SpellID::CLONE))))
+	};
+
+	return &selectors;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/bonuses/BonusCache.h

@@ -45,7 +45,7 @@ class BonusValueCache : public BonusCacheBase
 	CSelector selector;
 	mutable BonusCacheEntry value;
 public:
-	BonusValueCache(const IBonusBearer * target, const CSelector selector);
+	BonusValueCache(const IBonusBearer * target, const CSelector & selector);
 	int getValue() const;
 	bool hasBonus() const;
 };
@@ -112,8 +112,8 @@ public:
 
 	using SelectorsArray = BonusValuesArrayCache<KEYS_COUNT>::SelectorsArray;
 
-	UnitBonusValuesProxy(const IBonusBearer * Target, const SelectorsArray * selectors):
+	UnitBonusValuesProxy(const IBonusBearer * Target):
-		cache(Target, selectors)
+		cache(Target, generateSelectors())
 	{}
 
 	int getBonusValue(ECacheKeys which) const
@@ -129,6 +129,8 @@ public:
 	}
 
 private:
+	const SelectorsArray * generateSelectors();
+
 	BonusValuesArrayCache<KEYS_COUNT> cache;
 };
 

lib/mapObjects/CGHeroInstance.cpp

@@ -82,7 +82,7 @@ ui32 CGHeroInstance::getTileMovementCost(const TerrainTile & dest, const Terrain
 	{
 		ret = from.getRoad()->movementCost;
 	}
-	else if(ti->hasNoTerrainPenalty(from.getTerrainID())) //no special movement bonus
+	else if(!ti->hasNoTerrainPenalty(from.getTerrainID())) //no special movement bonus
 	{
 		ret = VLC->terrainTypeHandler->getById(from.getTerrainID())->moveCost;
 		ret -= ti->getRoughTerrainDiscountValue();
@@ -235,22 +235,19 @@ int CGHeroInstance::movementPointsLimit(bool onLand) const
 
 int CGHeroInstance::getLowestCreatureSpeed() const
 {
-	static const CSelector selectorSTACKS_SPEED = Selector::type()(BonusType::STACKS_SPEED);
-	static const std::string cachingStr = "type_" + std::to_string(static_cast<si32>(BonusType::STACKS_SPEED));
-
 	if(stacksCount() != 0)
 	{
 		int minimalSpeed = std::numeric_limits<int>::max();
 		//TODO? should speed modifiers (eg from artifacts) affect hero movement?
 		for(const auto & slot : Slots())
-			minimalSpeed = std::min(minimalSpeed, slot.second->valOfBonuses(selectorSTACKS_SPEED, cachingStr));
+			minimalSpeed = std::min(minimalSpeed, slot.second->getInitiative());
 
 		return minimalSpeed;
 	}
 	else
 	{
 		if(commander && commander->alive)
-			return commander->valOfBonuses(selectorSTACKS_SPEED, cachingStr);
+			return commander->getInitiative();
 	}
 
 	return 10;
@@ -1348,13 +1345,6 @@ CBonusSystemNode & CGHeroInstance::whereShouldBeAttached(CGameState * gs)
 
 int CGHeroInstance::movementPointsAfterEmbark(int MPsBefore, int basicCost, bool disembark, const TurnInfo * ti) const
 {
-	std::unique_ptr<TurnInfo> turnInfoLocal;
-	if(!ti)
-	{
-		turnInfoLocal = getTurnInfo(0);
-		ti = turnInfoLocal.get();
-	}
-
 	if(!ti->hasFreeShipBoarding())
 		return 0; // take all MPs by default
 	

lib/mapObjects/CGHeroInstance.h

@@ -227,7 +227,7 @@ public:
 	//cached version is much faster, TurnInfo construction is costly
 	int movementPointsLimitCached(bool onLand, const TurnInfo * ti) const;
 
-	int movementPointsAfterEmbark(int MPsBefore, int basicCost, bool disembark = false, const TurnInfo * ti = nullptr) const;
+	int movementPointsAfterEmbark(int MPsBefore, int basicCost, bool disembark, const TurnInfo * ti) const;
 
 	std::unique_ptr<TurnInfo> getTurnInfo(int days) const;
 

lib/pathfinder/CPathfinder.cpp

@@ -572,7 +572,7 @@ void CPathfinderHelper::getNeighbours(
 	CMap * map = gs->map;
 	const TerrainType * sourceTerrain = sourceTile.getTerrain();
 
-	constexpr std::array dirs = {
+	static constexpr std::array dirs = {
 		int3(-1, +1, +0),	int3(0, +1, +0),	int3(+1, +1, +0),
 		int3(-1, +0, +0),	/* source pos */	int3(+1, +0, +0),
 		int3(-1, -1, +0),	int3(0, -1, +0),	int3(+1, -1, +0)
@@ -689,19 +689,6 @@ int CPathfinderHelper::getMovementCost(
 
 		if (pointsLeft < minimalNextMoveCost)
 			return remainingMovePoints;
-
-//		NeighbourTilesVector vec;
-//
-//		getNeighbours(*dt, dst, vec, ct->isLand(), true);
-//		for(const auto & elem : vec)
-//		{
-//			int fcost = getMovementCost(dst, elem, nullptr, nullptr, pointsLeft, false);
-//			if(fcost <= pointsLeft)
-//			{
-//				return movementCost;
-//			}
-//		}
-//		movementCost = remainingMovePoints;
 	}
 
 	return movementCost;

lib/pathfinder/CPathfinder.h

@@ -93,7 +93,6 @@ public:
 	void updateTurnInfo(const int turn = 0);
 	bool isLayerAvailable(const EPathfindingLayer & layer) const;
 	const TurnInfo * getTurnInfo() const;
-	//bool hasBonusOfType(BonusType type) const;
 	int getMaxMovePoints(const EPathfindingLayer & layer) const;
 
 	TeleporterTilesVector getCastleGates(const PathNodeInfo & source) const;

lib/pathfinder/TurnInfo.cpp

@@ -80,8 +80,8 @@ int TurnInfo::getMovePointsLimitWater() const
 }
 
 TurnInfo::TurnInfo(TurnInfoCache * sharedCache, const CGHeroInstance * target, int Turn)
-	: noterrainPenalty(VLC->terrainTypeHandler->size())
+	: target(target)
-	, target(target)
+	, noterrainPenalty(VLC->terrainTypeHandler->size())
 {
 	CSelector daySelector = Selector::days(Turn);
 
@@ -100,27 +100,27 @@ TurnInfo::TurnInfo(TurnInfoCache * sharedCache, const CGHeroInstance * target, i
 	{
 		static const CSelector selector = Selector::type()(BonusType::WATER_WALKING);
 		const auto & bonuses = sharedCache->waterWalking.getBonusList(target, selector);
-		waterWalkingTest = bonuses->getFirst(selector) != nullptr;
+		waterWalkingTest = bonuses->getFirst(daySelector) != nullptr;
-		waterWalkingValue = bonuses->valOfBonuses(selector);
+		waterWalkingValue = bonuses->valOfBonuses(daySelector);
 	}
 
 	{
 		static const CSelector selector = Selector::type()(BonusType::FLYING_MOVEMENT);
 		const auto & bonuses = sharedCache->flyingMovement.getBonusList(target, selector);
-		flyingMovementTest = bonuses->getFirst(selector) != nullptr;
+		flyingMovementTest = bonuses->getFirst(daySelector) != nullptr;
-		flyingMovementValue = bonuses->valOfBonuses(selector);
+		flyingMovementValue = bonuses->valOfBonuses(daySelector);
 	}
 
 	{
 		static const CSelector selector = Selector::type()(BonusType::FREE_SHIP_BOARDING);
 		const auto & bonuses = sharedCache->freeShipBoarding.getBonusList(target, selector);
-		freeShipBoardingTest = bonuses->getFirst(selector) != nullptr;
+		freeShipBoardingTest = bonuses->getFirst(daySelector) != nullptr;
 	}
 
 	{
 		static const CSelector selector = Selector::type()(BonusType::ROUGH_TERRAIN_DISCOUNT);
 		const auto & bonuses = sharedCache->roughTerrainDiscount.getBonusList(target, selector);
-		roughTerrainDiscountValue = bonuses->getFirst(selector) != nullptr;
+		roughTerrainDiscountValue = bonuses->getFirst(daySelector) != nullptr;
 	}
 
 	{
@@ -133,7 +133,7 @@ TurnInfo::TurnInfo(TurnInfoCache * sharedCache, const CGHeroInstance * target, i
 		else
 			baseMovementPointsSea = vectorSea.back().Integer();
 
-		movePointsLimitWater = bonuses->valOfBonuses(selector, baseMovementPointsSea);
+		movePointsLimitWater = bonuses->valOfBonuses(daySelector, baseMovementPointsSea);
 	}
 
 	{
@@ -146,7 +146,7 @@ TurnInfo::TurnInfo(TurnInfoCache * sharedCache, const CGHeroInstance * target, i
 		else
 			baseMovementPointsLand = vectorLand.back().Integer();
 
-		movePointsLimitLand = bonuses->valOfBonuses(selector, baseMovementPointsLand);
+		movePointsLimitLand = bonuses->valOfBonuses(daySelector, baseMovementPointsLand);
 	}
 
 	{
@@ -157,6 +157,13 @@ TurnInfo::TurnInfo(TurnInfoCache * sharedCache, const CGHeroInstance * target, i
 			TerrainId affectedTerrain = bonus->subtype.as<TerrainId>();
 			noterrainPenalty.at(affectedTerrain.num) = true;
 		}
+
+		const auto nativeTerrain = target->getNativeTerrain();
+		if (nativeTerrain.hasValue())
+			noterrainPenalty.at(nativeTerrain.num) = true;
+
+		if (nativeTerrain == ETerrainId::ANY_TERRAIN)
+			boost::range::fill(noterrainPenalty, true);
 	}
 }