Merge pull request #5158 from IvanSavenko/bonus_caching

[1.6.3] Bonus caching improvements

AI/BattleAI/BattleExchangeVariant.cpp

@@ -857,15 +857,6 @@ BattleScore BattleExchangeEvaluator::calculateExchange(
 		exchangeBattle->nextRound();
 	}
 
-	// avoid blocking path for stronger stack by weaker stack
-	// the method checks if all stacks can be placed around enemy
-	std::map<BattleHex, battle::Units> reachabilityMap;
-
-	auto hexes = ap.attack.defender->getSurroundingHexes();
-
-	for(auto hex : hexes)
-		reachabilityMap[hex] = getOneTurnReachableUnits(turn, hex);
-
 	auto score = v.getScore();
 
 	if(simulationTurnsCount < totalTurnsCount)

AI/Nullkiller/Pathfinding/Actors.cpp

@@ -47,11 +47,10 @@ ChainActor::ChainActor(const CGHeroInstance * hero, HeroRole heroRole, uint64_t
 	initialTurn = 0;
 	armyValue = getHeroArmyStrengthWithCommander(hero, hero);
 	heroFightingStrength = hero->getHeroStrength();
-	tiCache.reset(new TurnInfo(hero));
 }
 
 ChainActor::ChainActor(const ChainActor * carrier, const ChainActor * other, const CCreatureSet * heroArmy)
-	:hero(carrier->hero), tiCache(carrier->tiCache), heroRole(carrier->heroRole), isMovable(true), creatureSet(heroArmy), chainMask(carrier->chainMask | other->chainMask),
+	:hero(carrier->hero), heroRole(carrier->heroRole), isMovable(true), creatureSet(heroArmy), chainMask(carrier->chainMask | other->chainMask),
 	baseActor(this), carrierParent(carrier), otherParent(other), heroFightingStrength(carrier->heroFightingStrength),
 	actorExchangeCount(carrier->actorExchangeCount + other->actorExchangeCount), armyCost(carrier->armyCost + other->armyCost), actorAction()
 {
@@ -75,7 +74,7 @@ int ChainActor::maxMovePoints(CGPathNode::ELayer layer)
 		throw std::logic_error("Asking movement points for static actor");
 #endif
 
-	return hero->movementPointsLimitCached(layer, tiCache.get());
+	return hero->movementPointsLimit(layer);
 }
 
 std::string ChainActor::toString() const
@@ -133,7 +132,6 @@ void ChainActor::setBaseActor(HeroActor * base)
 	heroFightingStrength = base->heroFightingStrength;
 	armyCost = base->armyCost;
 	actorAction = base->actorAction;
-	tiCache = base->tiCache;
 	actorExchangeCount = base->actorExchangeCount;
 }
 

AI/Nullkiller/Pathfinding/Actors.h

@@ -73,7 +73,6 @@ public:
 	float heroFightingStrength;
 	uint8_t actorExchangeCount;
 	TResources armyCost;
-	std::shared_ptr<TurnInfo> tiCache;
 
 	ChainActor() = default;
 	virtual ~ChainActor() = default;

client/NetPacksClient.cpp

@@ -821,7 +821,7 @@ void ApplyClientNetPackVisitor::visitBattleSetActiveStack(BattleSetActiveStack &
 
 	const CStack *activated = gs.getBattle(pack.battleID)->battleGetStackByID(pack.stack);
 	PlayerColor playerToCall; //pack.player that will move activated stack
-	if(activated->hasBonusOfType(BonusType::HYPNOTIZED))
+	if(activated->isHypnotized())
 	{
 		playerToCall = gs.getBattle(pack.battleID)->getSide(BattleSide::ATTACKER).color == activated->unitOwner()
 			? gs.getBattle(pack.battleID)->getSide(BattleSide::DEFENDER).color

config/gameConfig.json

@@ -305,7 +305,11 @@
 			// if heroes are invitable in tavern
 			"tavernInvite"            : false,
 			// minimal primary skills for heroes
-			"minimalPrimarySkills": [ 0, 0, 1, 1]
+			"minimalPrimarySkills": [ 0, 0, 1, 1],
+			/// movement points hero can get on start of the turn when on land, depending on speed of slowest creature (0-based list)
+			"movementPointsLand" : [ 1500, 1500, 1500, 1500, 1560, 1630, 1700, 1760, 1830, 1900, 1960, 2000 ],
+			/// movement points hero can get on start of the turn when on sea, depending on speed of slowest creature (0-based list)
+			"movementPointsSea" : [ 1500 ]
 		},
 
 		"towns":
@@ -560,29 +564,6 @@
 					"type" : "MANA_PER_KNOWLEDGE_PERCENTAGE", //1000% mana per knowledge
 					"val" : 1000,
 					"valueType" : "BASE_NUMBER"
-				},
-				"landMovement" :
-				{
-					"type" : "MOVEMENT", //Basic land movement
-					"subtype" : "heroMovementLand",
-					"val" : 1300,
-					"valueType" : "BASE_NUMBER",
-					"updater" : {
-						"type" : "ARMY_MOVEMENT", //Enable army movement bonus
-						"parameters" : [
-							20, // Movement points for lowest speed numerator
-							3,  // Movement points for lowest speed denominator
-							10, // Resulting value, rounded down, will be multiplied by this number
-							700 // All army movement bonus cannot be higher than this value (so, max movement will be 1300 + 700 for this settings)
-						]
-					}
-				},
-				"seaMovement" :
-				{
-					"type" : "MOVEMENT", //Basic sea movement
-					"subtype" : "heroMovementSea",
-					"val" : 1500,
-					"valueType" : "BASE_NUMBER"
 				}
 			}
 		},

include/vcmi/Creature.h

@@ -23,9 +23,10 @@ class DLL_LINKAGE ACreature: public AFactionMember
 {
 public:
 	bool isLiving() const; //non-undead, non-non living or alive
-	ui32 getMovementRange(int turn) const; //get speed (in moving tiles) of creature with all modificators
+	virtual ui32 getMovementRange(int turn) const; //get speed (in moving tiles) of creature with all modificators
-	ui32 getMovementRange() 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>

include/vcmi/FactionMember.h

@@ -44,10 +44,6 @@ public:
 	 Returns defence of creature or hero.
 	*/
 	virtual int getDefense(bool ranged) const;
-	/**
-	 Returns primskill of creature or hero.
-	*/
-	int getPrimSkillLevel(PrimarySkill id) const;
 	/**
 	 Returns morale of creature or hero. Taking absolute bonuses into account.
 	 For now, uses range from EGameSettings

lib/BasicTypes.cpp

@@ -69,14 +69,6 @@ int AFactionMember::getMaxDamage(bool ranged) const
 	return getBonusBearer()->valOfBonuses(selector, cachingStr);
 }
 
-int AFactionMember::getPrimSkillLevel(PrimarySkill id) const
-{
-	auto allSkills = getBonusBearer()->getBonusesOfType(BonusType::PRIMARY_SKILL);
-	int ret = allSkills->valOfBonuses(Selector::subtype()(BonusSubtypeID(id)));
-	int minSkillValue = VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, id.getNum());
-	return std::max(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect, sp=0 works in old saves
-}
-
 int AFactionMember::moraleValAndBonusList(TConstBonusListPtr & bonusList) const
 {
 	int32_t maxGoodMorale = VLC->engineSettings()->getVector(EGameSettings::COMBAT_GOOD_MORALE_DICE).size();
@@ -160,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/CMakeLists.txt

@@ -65,12 +65,12 @@ set(lib_MAIN_SRCS
 	battle/Unit.cpp
 
 	bonuses/Bonus.cpp
+	bonuses/BonusCache.cpp
 	bonuses/BonusEnum.cpp
 	bonuses/BonusList.cpp
 	bonuses/BonusParams.cpp
 	bonuses/BonusSelector.cpp
 	bonuses/BonusCustomTypes.cpp
-	bonuses/CBonusProxy.cpp
 	bonuses/CBonusSystemNode.cpp
 	bonuses/IBonusBearer.cpp
 	bonuses/Limiters.cpp
@@ -435,12 +435,12 @@ set(lib_MAIN_HEADERS
 	battle/Unit.h
 
 	bonuses/Bonus.h
+	bonuses/BonusCache.h
 	bonuses/BonusEnum.h
 	bonuses/BonusList.h
 	bonuses/BonusParams.h
 	bonuses/BonusSelector.h
 	bonuses/BonusCustomTypes.h
-	bonuses/CBonusProxy.h
 	bonuses/CBonusSystemNode.h
 	bonuses/IBonusBearer.h
 	bonuses/Limiters.h

lib/GameSettings.cpp

@@ -76,6 +76,8 @@ const std::vector<GameSettings::SettingOption> GameSettings::settingProperties =
 		{EGameSettings::HEROES_RETREAT_ON_WIN_WITHOUT_TROOPS,             "heroes",    "retreatOnWinWithoutTroops"            },
 		{EGameSettings::HEROES_STARTING_STACKS_CHANCES,                   "heroes",    "startingStackChances"                 },
 		{EGameSettings::HEROES_TAVERN_INVITE,                             "heroes",    "tavernInvite"                         },
+		{EGameSettings::HEROES_MOVEMENT_POINTS_LAND,                      "heroes",    "movementPointsLand"                   },
+		{EGameSettings::HEROES_MOVEMENT_POINTS_SEA,                       "heroes",    "movementPointsSea"                    },
 		{EGameSettings::MAP_FORMAT_ARMAGEDDONS_BLADE,                     "mapFormat", "armageddonsBlade"                     },
 		{EGameSettings::MAP_FORMAT_CHRONICLES,                            "mapFormat", "chronicles"                           },
 		{EGameSettings::MAP_FORMAT_HORN_OF_THE_ABYSS,                     "mapFormat", "hornOfTheAbyss"                       },

lib/IGameSettings.h

@@ -49,6 +49,8 @@ enum class EGameSettings
 	HEROES_RETREAT_ON_WIN_WITHOUT_TROOPS,
 	HEROES_STARTING_STACKS_CHANCES,
 	HEROES_TAVERN_INVITE,
+	HEROES_MOVEMENT_POINTS_LAND,
+	HEROES_MOVEMENT_POINTS_SEA,
 	MAP_FORMAT_ARMAGEDDONS_BLADE,
 	MAP_FORMAT_CHRONICLES,
 	MAP_FORMAT_HORN_OF_THE_ABYSS,

lib/TerrainHandler.cpp

@@ -151,36 +151,6 @@ std::vector<JsonNode> TerrainTypeHandler::loadLegacyData()
 	return result;
 }
 
-bool TerrainType::isLand() const
-{
-	return !isWater();
-}
-
-bool TerrainType::isWater() const
-{
-	return passabilityType & PassabilityType::WATER;
-}
-
-bool TerrainType::isRock() const
-{
-	return passabilityType & PassabilityType::ROCK;
-}
-
-bool TerrainType::isPassable() const
-{
-	return !isRock();
-}
-
-bool TerrainType::isSurface() const
-{
-	return passabilityType & PassabilityType::SURFACE;
-}
-
-bool TerrainType::isUnderground() const
-{
-	return passabilityType & PassabilityType::SUBTERRANEAN;
-}
-
 bool TerrainType::isTransitionRequired() const
 {
 	return transitionRequired;

lib/TerrainHandler.h

@@ -83,14 +83,13 @@ public:
 
 	TerrainType() = default;
 
-	bool isLand() const;
+	inline bool isLand() const;
-	bool isWater() const;
+	inline bool isWater() const;
-	bool isRock() const;
+	inline bool isRock() const;
+	inline bool isPassable() const;
+	inline bool isSurface() const;
+	inline bool isUnderground() const;
 
-	bool isPassable() const;
-
-	bool isSurface() const;
-	bool isUnderground() const;
 	bool isTransitionRequired() const;
 };
 
@@ -112,4 +111,34 @@ public:
 	std::vector<JsonNode> loadLegacyData() override;
 };
 
+inline bool TerrainType::isLand() const
+{
+	return !isWater();
+}
+
+inline bool TerrainType::isWater() const
+{
+	return passabilityType & PassabilityType::WATER;
+}
+
+inline bool TerrainType::isRock() const
+{
+	return passabilityType & PassabilityType::ROCK;
+}
+
+inline bool TerrainType::isPassable() const
+{
+	return !isRock();
+}
+
+inline bool TerrainType::isSurface() const
+{
+	return passabilityType & PassabilityType::SURFACE;
+}
+
+inline bool TerrainType::isUnderground() const
+{
+	return passabilityType & PassabilityType::SUBTERRANEAN;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/battle/CBattleInfoCallback.cpp

@@ -714,18 +714,7 @@ bool CBattleInfoCallback::battleCanShoot(const battle::Unit * attacker) const
 	if (!attacker->canShoot())
 		return false;
 
-	//forgetfulness
+	return attacker->canShootBlocked() || !battleIsUnitBlocked(attacker);
-	TConstBonusListPtr forgetfulList = attacker->getBonusesOfType(BonusType::FORGETFULL);
-	if(!forgetfulList->empty())
-	{
-		int forgetful = forgetfulList->totalValue();
-
-		//advanced+ level
-		if(forgetful > 1)
-			return false;
-	}
-
-	return !battleIsUnitBlocked(attacker) || attacker->hasBonusOfType(BonusType::FREE_SHOOTING);
 }
 
 bool CBattleInfoCallback::battleCanTargetEmptyHex(const battle::Unit * attacker) const
@@ -1732,9 +1721,6 @@ bool CBattleInfoCallback::battleIsUnitBlocked(const battle::Unit * unit) const
 {
 	RETURN_IF_NOT_BATTLE(false);
 
-	if(unit->hasBonusOfType(BonusType::SIEGE_WEAPON)) //siege weapons cannot be blocked
-		return false;
-
 	for(const auto * adjacent : battleAdjacentUnits(unit))
 	{
 		if(adjacent->unitOwner() != unit->unitOwner()) //blocked by enemy stack

lib/battle/CBattleInfoEssentials.cpp

@@ -404,7 +404,7 @@ PlayerColor CBattleInfoEssentials::battleGetOwner(const battle::Unit * unit) con
 
 	PlayerColor initialOwner = getBattle()->getSidePlayer(unit->unitSide());
 
-	if(unit->hasBonusOfType(BonusType::HYPNOTIZED))
+	if(unit->isHypnotized())
 		return otherPlayer(initialOwner);
 	else
 		return initialOwner;

lib/battle/CUnitState.cpp

@@ -26,7 +26,7 @@ namespace battle
 CAmmo::CAmmo(const battle::Unit * Owner, CSelector totalSelector):
 	used(0),
 	owner(Owner),
-	totalProxy(Owner, std::move(totalSelector))
+	totalProxy(Owner, totalSelector)
 {
 	reset();
 }
@@ -34,7 +34,6 @@ CAmmo::CAmmo(const battle::Unit * Owner, CSelector totalSelector):
 CAmmo & CAmmo::operator= (const CAmmo & other)
 {
 	used = other.used;
-	totalProxy = other.totalProxy;
 	return *this;
 }
 
@@ -60,7 +59,7 @@ void CAmmo::reset()
 
 int32_t CAmmo::total() const
 {
-	return totalProxy->totalValue();
+	return totalProxy.getValue();
 }
 
 void CAmmo::use(int32_t amount)
@@ -85,20 +84,13 @@ void CAmmo::serializeJson(JsonSerializeFormat & handler)
 ///CShots
 CShots::CShots(const battle::Unit * Owner)
 	: CAmmo(Owner, Selector::type()(BonusType::SHOTS)),
-	shooter(Owner, BonusType::SHOOTER)
+	shooter(Owner, Selector::type()(BonusType::SHOOTER))
 {
 }
 
-CShots & CShots::operator=(const CShots & other)
-{
-	CAmmo::operator=(other);
-	shooter = other.shooter;
-	return *this;
-}
-
 bool CShots::isLimited() const
 {
-	return !shooter.getHasBonus() || !env->unitHasAmmoCart(owner);
+	return !shooter.hasBonus() || !env->unitHasAmmoCart(owner);
 }
 
 void CShots::setEnv(const IUnitEnvironment * env_)
@@ -108,7 +100,7 @@ void CShots::setEnv(const IUnitEnvironment * env_)
 
 int32_t CShots::total() const
 {
-	if(shooter.getHasBonus())
+	if(shooter.hasBonus())
 		return CAmmo::total();
 	else
 		return 0;
@@ -124,23 +116,23 @@ CCasts::CCasts(const battle::Unit * Owner):
 CRetaliations::CRetaliations(const battle::Unit * Owner)
 	: CAmmo(Owner, Selector::type()(BonusType::ADDITIONAL_RETALIATION)),
 	totalCache(0),
-	noRetaliation(Owner, Selector::type()(BonusType::SIEGE_WEAPON).Or(Selector::type()(BonusType::HYPNOTIZED)).Or(Selector::type()(BonusType::NO_RETALIATION)), "CRetaliations::noRetaliation"),
+	noRetaliation(Owner, Selector::type()(BonusType::SIEGE_WEAPON).Or(Selector::type()(BonusType::HYPNOTIZED)).Or(Selector::type()(BonusType::NO_RETALIATION))),
-	unlimited(Owner, BonusType::UNLIMITED_RETALIATIONS)
+	unlimited(Owner, Selector::type()(BonusType::UNLIMITED_RETALIATIONS))
 {
 }
 
 bool CRetaliations::isLimited() const
 {
-	return !unlimited.getHasBonus() || noRetaliation.getHasBonus();
+	return !unlimited.hasBonus() || noRetaliation.hasBonus();
 }
 
 int32_t CRetaliations::total() const
 {
-	if(noRetaliation.getHasBonus())
+	if(noRetaliation.hasBonus())
 		return 0;
 
 	//after dispel bonus should remain during current round
-	int32_t val = 1 + totalProxy->totalValue();
+	int32_t val = 1 + totalProxy.getValue();
 	vstd::amax(totalCache, val);
 	return totalCache;
 }
@@ -341,13 +333,9 @@ CUnitState::CUnitState():
 	counterAttacks(this),
 	health(this),
 	shots(this),
-	totalAttacks(this, Selector::type()(BonusType::ADDITIONAL_ATTACK), 1),
+	stackSpeedPerTurn(this, Selector::type()(BonusType::STACKS_SPEED), BonusCacheMode::VALUE),
-	minDamage(this, Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageMin)), 0),
+	immobilizedPerTurn(this, Selector::type()(BonusType::SIEGE_WEAPON).Or(Selector::type()(BonusType::BIND_EFFECT)), BonusCacheMode::PRESENCE),
-	maxDamage(this, Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageBoth).Or(Selector::typeSubtype(BonusType::CREATURE_DAMAGE, BonusCustomSubtype::creatureDamageMax)), 0),
+	bonusCache(this),
-	attack(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::ATTACK)), 0),
-	defence(this, Selector::typeSubtype(BonusType::PRIMARY_SKILL, BonusSubtypeID(PrimarySkill::DEFENSE)), 0),
-	inFrenzy(this, Selector::type()(BonusType::IN_FRENZY)),
-	cloneLifetimeMarker(this, Selector::type()(BonusType::NONE).And(Selector::source(BonusSource::SPELL_EFFECT, BonusSourceID(SpellID(SpellID::CLONE)))), "CUnitState::cloneLifetimeMarker"),
 	cloneID(-1)
 {
 
@@ -371,16 +359,7 @@ CUnitState & CUnitState::operator=(const CUnitState & other)
 	waiting = other.waiting;
 	waitedThisTurn = other.waitedThisTurn;
 	casts = other.casts;
-	counterAttacks = other.counterAttacks;
 	health = other.health;
-	shots = other.shots;
-	totalAttacks = other.totalAttacks;
-	minDamage = other.minDamage;
-	maxDamage = other.maxDamage;
-	attack = other.attack;
-	defence = other.defence;
-	inFrenzy = other.inFrenzy;
-	cloneLifetimeMarker = other.cloneLifetimeMarker;
 	cloneID = other.cloneID;
 	position = other.position;
 	return *this;
@@ -542,9 +521,16 @@ bool CUnitState::isCaster() const
 	return casts.total() > 0;//do not check specific cast abilities here
 }
 
+bool CUnitState::canShootBlocked() const
+{
+	return bonusCache.getBonusValue(UnitBonusValuesProxy::HAS_FREE_SHOOTING);
+}
+
 bool CUnitState::canShoot() const
 {
-	return shots.canUse(1);
+	return
+		shots.canUse(1) &&
+		bonusCache.getBonusValue(UnitBonusValuesProxy::FORGETFULL) <= 1; //advanced+ level
 }
 
 bool CUnitState::isShooter() const
@@ -579,6 +565,11 @@ int64_t CUnitState::getTotalHealth() const
 	return health.total();
 }
 
+uint32_t CUnitState::getMaxHealth() const
+{
+	return std::max(1, bonusCache.getBonusValue(UnitBonusValuesProxy::STACK_HEALTH));
+}
+
 BattleHex CUnitState::getPosition() const
 {
 	return position;
@@ -591,11 +582,20 @@ void CUnitState::setPosition(BattleHex hex)
 
 int32_t CUnitState::getInitiative(int turn) const
 {
-	if (turn == 0)
+	return stackSpeedPerTurn.getValue(turn);
-		return valOfBonuses(BonusType::STACKS_SPEED);
+}
 
-	std::string cachingStr = "type_STACKS_SPEED_turns_" + std::to_string(turn);
+ui32 CUnitState::getMovementRange(int turn) const
-	return valOfBonuses(Selector::type()(BonusType::STACKS_SPEED).And(Selector::turns(turn)), cachingStr);
+{
+	if (immobilizedPerTurn.getValue(0) != 0)
+		return 0;
+
+	return stackSpeedPerTurn.getValue(0);
+}
+
+ui32 CUnitState::getMovementRange() const
+{
+	return getMovementRange(0);
 }
 
 uint8_t CUnitState::getRangedFullDamageDistance() const
@@ -693,47 +693,69 @@ BattlePhases::Type CUnitState::battleQueuePhase(int turn) const
 	}
 }
 
+bool CUnitState::isHypnotized() const
+{
+	return bonusCache.getBonusValue(UnitBonusValuesProxy::HYPNOTIZED);
+}
+
 int CUnitState::getTotalAttacks(bool ranged) const
 {
-	return ranged ? totalAttacks.getRangedValue() : totalAttacks.getMeleeValue();
+	return 1 + (ranged ?
+		bonusCache.getBonusValue(UnitBonusValuesProxy::TOTAL_ATTACKS_RANGED):
+		bonusCache.getBonusValue(UnitBonusValuesProxy::TOTAL_ATTACKS_MELEE));
 }
 
 int CUnitState::getMinDamage(bool ranged) const
 {
-	return ranged ? minDamage.getRangedValue() : minDamage.getMeleeValue();
+	return ranged ?
+		bonusCache.getBonusValue(UnitBonusValuesProxy::MIN_DAMAGE_RANGED):
+		bonusCache.getBonusValue(UnitBonusValuesProxy::MIN_DAMAGE_MELEE);
+
 }
 
 int CUnitState::getMaxDamage(bool ranged) const
 {
-	return ranged ? maxDamage.getRangedValue() : maxDamage.getMeleeValue();
+	return ranged ?
+		bonusCache.getBonusValue(UnitBonusValuesProxy::MAX_DAMAGE_RANGED):
+		bonusCache.getBonusValue(UnitBonusValuesProxy::MAX_DAMAGE_MELEE);
 }
 
 int CUnitState::getAttack(bool ranged) const
 {
-	int ret = ranged ? attack.getRangedValue() : attack.getMeleeValue();
+	int attack = ranged ?
+		bonusCache.getBonusValue(UnitBonusValuesProxy::ATTACK_RANGED):
+		bonusCache.getBonusValue(UnitBonusValuesProxy::ATTACK_MELEE);
 
-	if(!inFrenzy->empty())
+	int frenzy = bonusCache.getBonusValue(UnitBonusValuesProxy::IN_FRENZY);
+	if(frenzy != 0)
 	{
-		double frenzyPower = static_cast<double>(inFrenzy->totalValue()) / 100;
+		int defence = ranged ?
-		frenzyPower *= static_cast<double>(ranged ? defence.getRangedValue() : defence.getMeleeValue());
+			bonusCache.getBonusValue(UnitBonusValuesProxy::DEFENCE_RANGED):
-		ret += static_cast<int>(frenzyPower);
+			bonusCache.getBonusValue(UnitBonusValuesProxy::DEFENCE_MELEE);
+
+		int frenzyBonus = frenzy * defence / 100;
+		attack += frenzyBonus;
 	}
 
-	vstd::amax(ret, 0);
+	vstd::amax(attack, 0);
-	return ret;
+	return attack;
 }
 
 int CUnitState::getDefense(bool ranged) const
 {
-	if(!inFrenzy->empty())
+	int frenzy = bonusCache.getBonusValue(UnitBonusValuesProxy::IN_FRENZY);
+
+	if(frenzy != 0)
 	{
 		return 0;
 	}
 	else
 	{
-		int ret = ranged ? defence.getRangedValue() : defence.getMeleeValue();
+		int defence = ranged ?
-		vstd::amax(ret, 0);
+						  bonusCache.getBonusValue(UnitBonusValuesProxy::DEFENCE_RANGED):
-		return ret;
+						  bonusCache.getBonusValue(UnitBonusValuesProxy::DEFENCE_MELEE);
+		vstd::amax(defence, 0);
+		return defence;
 	}
 }
 
@@ -886,7 +908,7 @@ void CUnitState::afterNewRound()
 
 	if(alive() && isClone())
 	{
-		if(!cloneLifetimeMarker.getHasBonus())
+		if(!bonusCache.hasBonus(UnitBonusValuesProxy::CLONE_MARKER))
 			makeGhost();
 	}
 }

lib/battle/CUnitState.h

@@ -11,7 +11,7 @@
 #pragma once
 
 #include "Unit.h"
-#include "../bonuses/CBonusProxy.h"
+#include "../bonuses/BonusCache.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -32,10 +32,6 @@ class DLL_LINKAGE CAmmo
 public:
 	explicit CAmmo(const battle::Unit * Owner, CSelector totalSelector);
 
-	//only copy construction is allowed for acquire(), serializeJson should be used for any other "assignment"
-	CAmmo(const CAmmo & other) = default;
-	CAmmo(CAmmo && other) = delete;
-
 	CAmmo & operator=(const CAmmo & other);
 	CAmmo & operator=(CAmmo && other) = delete;
 
@@ -50,15 +46,14 @@ public:
 protected:
 	int32_t used;
 	const battle::Unit * owner;
-	CBonusProxy totalProxy;
+	BonusValueCache totalProxy;
 };
 
 class DLL_LINKAGE CShots : public CAmmo
 {
 public:
 	explicit CShots(const battle::Unit * Owner);
-	CShots(const CShots & other) = default;
+
-	CShots & operator=(const CShots & other);
 	bool isLimited() const override;
 	int32_t total() const override;
 
@@ -66,23 +61,20 @@ public:
 private:
 	const IUnitEnvironment * env;
 
-	CCheckProxy shooter;
+	BonusValueCache shooter;
 };
 
 class DLL_LINKAGE CCasts : public CAmmo
 {
 public:
 	explicit CCasts(const battle::Unit * Owner);
-	CCasts(const CCasts & other) = default;
-	CCasts & operator=(const CCasts & other) = default;
 };
 
 class DLL_LINKAGE CRetaliations : public CAmmo
 {
 public:
 	explicit CRetaliations(const battle::Unit * Owner);
-	CRetaliations(const CRetaliations & other) = default;
+
-	CRetaliations & operator=(const CRetaliations & other) = default;
 	bool isLimited() const override;
 	int32_t total() const override;
 	void reset() override;
@@ -91,8 +83,8 @@ public:
 private:
 	mutable int32_t totalCache;
 
-	CCheckProxy noRetaliation;
+	BonusValueCache noRetaliation;
-	CCheckProxy unlimited;
+	BonusValueCache unlimited;
 };
 
 class DLL_LINKAGE CHealth
@@ -154,11 +146,6 @@ public:
 	CHealth health;
 	CShots shots;
 
-	CTotalsProxy totalAttacks;
-
-	CTotalsProxy minDamage;
-	CTotalsProxy maxDamage;
-
 	///id of alive clone of this stack clone if any
 	si32 cloneID;
 
@@ -205,11 +192,14 @@ public:
 	bool isFrozen() const override;
 	bool isValidTarget(bool allowDead = false) const override;
 
+	bool isHypnotized() const override;
+
 	bool isClone() const override;
 	bool hasClone() const override;
 
 	bool canCast() const override;
 	bool isCaster() const override;
+	bool canShootBlocked() const override;
 	bool canShoot() const override;
 	bool isShooter() const override;
 
@@ -218,6 +208,7 @@ public:
 	int32_t getFirstHPleft() const override;
 	int64_t getAvailableHealth() const override;
 	int64_t getTotalHealth() const override;
+	uint32_t getMaxHealth() const override;
 
 	BattleHex getPosition() const override;
 	void setPosition(BattleHex hex) override;
@@ -225,6 +216,9 @@ public:
 	uint8_t getRangedFullDamageDistance() const;
 	uint8_t getShootingRangeDistance() const;
 
+	ui32 getMovementRange(int turn) const override;
+	ui32 getMovementRange() const override;
+
 	bool canMove(int turn = 0) const override;
 	bool defended(int turn = 0) const override;
 	bool moved(int turn = 0) const override;
@@ -268,11 +262,9 @@ public:
 private:
 	const IUnitEnvironment * env;
 
-	CTotalsProxy attack;
+	BonusCachePerTurn immobilizedPerTurn;
-	CTotalsProxy defence;
+	BonusCachePerTurn stackSpeedPerTurn;
-	CBonusProxy inFrenzy;
+	UnitBonusValuesProxy bonusCache;
-
-	CCheckProxy cloneLifetimeMarker;
 
 	void reset();
 };
@@ -282,13 +274,12 @@ class DLL_LINKAGE CUnitStateDetached : public CUnitState
 public:
 	explicit CUnitStateDetached(const IUnitInfo * unit_, const IBonusBearer * bonus_);
 
-	TConstBonusListPtr getAllBonuses(const CSelector & selector, const CSelector & limit,
+	CUnitStateDetached & operator= (const CUnitState & other);
-		const std::string & cachingStr = "") const override;
+
+	TConstBonusListPtr getAllBonuses(const CSelector & selector, const CSelector & limit, const std::string & cachingStr = "") const override;
 
 	int64_t getTreeVersion() const override;
 
-	CUnitStateDetached & operator= (const CUnitState & other);
-
 	uint32_t unitId() const override;
 	BattleSide unitSide() const override;
 
@@ -297,7 +288,6 @@ public:
 
 	SlotID unitSlot() const override;
 
-
 	int32_t unitBaseAmount() const override;
 
 	void spendMana(ServerCallback * server, const int spellCost) const override;

lib/battle/Unit.h

@@ -84,11 +84,14 @@ public:
 	bool isTurret() const;
 	virtual bool isValidTarget(bool allowDead = false) const = 0; //non-turret non-ghost stacks (can be attacked or be object of magic effect)
 
+	virtual bool isHypnotized() const = 0;
+
 	virtual bool isClone() const = 0;
 	virtual bool hasClone() const = 0;
 
 	virtual bool canCast() const = 0;
 	virtual bool isCaster() const = 0;
+	virtual bool canShootBlocked() const = 0;
 	virtual bool canShoot() const = 0;
 	virtual bool isShooter() const = 0;
 
@@ -112,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

@@ -0,0 +1,212 @@
+/*
+ * BonusCache.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 "BonusCache.h"
+#include "IBonusBearer.h"
+
+#include "BonusSelector.h"
+#include "BonusList.h"
+
+#include "../VCMI_Lib.h"
+#include "../IGameSettings.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+int BonusCacheBase::getBonusValueImpl(BonusCacheEntry & currentValue, const CSelector & selector, BonusCacheMode mode) const
+{
+	if (target->getTreeVersion() == currentValue.version)
+	{
+		return currentValue.value;
+	}
+	else
+	{
+		// NOTE: following code theoretically can fail if bonus tree was changed by another thread between two following lines
+		// However, this situation should not be possible - gamestate modification should only happen in single-treaded mode with locked gamestate mutex
+		int newValue;
+
+		if (mode == BonusCacheMode::VALUE)
+			newValue = target->valOfBonuses(selector);
+		else
+			newValue = target->hasBonus(selector);
+		currentValue.value = newValue;
+		currentValue.version = target->getTreeVersion();
+
+		return newValue;
+	}
+}
+
+BonusValueCache::BonusValueCache(const IBonusBearer * target, const CSelector & selector)
+	:BonusCacheBase(target),selector(selector)
+{}
+
+int BonusValueCache::getValue() const
+{
+	return getBonusValueImpl(value, selector, BonusCacheMode::VALUE);
+}
+
+bool BonusValueCache::hasBonus() const
+{
+	return getBonusValueImpl(value, selector, BonusCacheMode::PRESENCE);
+}
+
+MagicSchoolMasteryCache::MagicSchoolMasteryCache(const IBonusBearer * target)
+	:target(target)
+{}
+
+void MagicSchoolMasteryCache::update() const
+{
+	static const CSelector allBonusesSelector = Selector::type()(BonusType::MAGIC_SCHOOL_SKILL);
+	static const std::array schoolsSelector = {
+		Selector::subtype()(SpellSchool::ANY),
+		Selector::subtype()(SpellSchool::AIR),
+		Selector::subtype()(SpellSchool::FIRE),
+		Selector::subtype()(SpellSchool::WATER),
+		Selector::subtype()(SpellSchool::EARTH),
+	};
+
+	auto list = target->getBonuses(allBonusesSelector);
+	for (int i = 0; i < schoolsSelector.size(); ++i)
+		schools[i] = list->valOfBonuses(schoolsSelector[i]);
+
+	version = target->getTreeVersion();
+}
+
+int32_t MagicSchoolMasteryCache::getMastery(const SpellSchool & school) const
+{
+	if (target->getTreeVersion() != version)
+		update();
+	return schools[school.num + 1];
+}
+
+PrimarySkillsCache::PrimarySkillsCache(const IBonusBearer * target)
+	:target(target)
+{}
+
+void PrimarySkillsCache::update() const
+{
+	static const CSelector primarySkillsSelector = Selector::type()(BonusType::PRIMARY_SKILL);
+	static const CSelector attackSelector = Selector::subtype()(PrimarySkill::ATTACK);
+	static const CSelector defenceSelector = Selector::subtype()(PrimarySkill::DEFENSE);
+	static const CSelector spellPowerSelector = Selector::subtype()(PrimarySkill::SPELL_POWER);
+	static const CSelector knowledgeSelector = Selector::subtype()(PrimarySkill::KNOWLEDGE);
+
+	std::array<int, 4> minValues = {
+		VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, PrimarySkill::ATTACK),
+		VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, PrimarySkill::DEFENSE),
+		VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, PrimarySkill::SPELL_POWER),
+		VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, PrimarySkill::KNOWLEDGE)
+	};
+
+	auto list = target->getBonuses(primarySkillsSelector);
+	skills[PrimarySkill::ATTACK] = std::max(minValues[PrimarySkill::ATTACK], list->valOfBonuses(attackSelector));
+	skills[PrimarySkill::DEFENSE] = std::max(minValues[PrimarySkill::DEFENSE], list->valOfBonuses(defenceSelector));
+	skills[PrimarySkill::SPELL_POWER] = std::max(minValues[PrimarySkill::SPELL_POWER], list->valOfBonuses(spellPowerSelector));
+	skills[PrimarySkill::KNOWLEDGE] = std::max(minValues[PrimarySkill::KNOWLEDGE], list->valOfBonuses(knowledgeSelector));
+
+	version = target->getTreeVersion();
+}
+
+const std::array<std::atomic<int32_t>, 4> & PrimarySkillsCache::getSkills() const
+{
+	if (target->getTreeVersion() != version)
+		update();
+	return skills;
+}
+
+int BonusCachePerTurn::getValueUncached(int turns) const
+{
+	std::lock_guard lock(bonusListMutex);
+
+	int nodeTreeVersion = target->getTreeVersion();
+
+	if (bonusListVersion != nodeTreeVersion)
+	{
+		bonusList = target->getBonuses(selector);
+		bonusListVersion = nodeTreeVersion;
+	}
+
+	if (mode == BonusCacheMode::VALUE)
+	{
+		if (turns != 0)
+			return bonusList->valOfBonuses(Selector::turns(turns));
+		else
+			return bonusList->totalValue();
+	}
+	else
+	{
+		if (turns != 0)
+			return bonusList->getFirst(Selector::turns(turns)) != nullptr;
+		else
+			return !bonusList->empty();
+	}
+}
+
+int BonusCachePerTurn::getValue(int turns) const
+{
+	int nodeTreeVersion = target->getTreeVersion();
+
+	if (turns < cachedTurns)
+	{
+		auto & entry = cache[turns];
+		if (entry.version == nodeTreeVersion)
+		{
+			// best case: value is in cache and up-to-date
+			return entry.value;
+		}
+		else
+		{
+			// else - compute value and update it in the cache
+			int newValue = getValueUncached(turns);
+			entry.value = newValue;
+			entry.version = nodeTreeVersion;
+			return newValue;
+		}
+	}
+	else
+	{
+		// non-cacheable value - compute and return (should be 0 / close to 0 calls)
+		return getValueUncached(turns);
+	}
+}
+
+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

@@ -0,0 +1,188 @@
+/*
+ * BonusCache.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 "BonusSelector.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+enum class BonusCacheMode : int8_t
+{
+	VALUE, // total value of bonus will be cached
+	PRESENCE, // presence of bonus will be cached
+};
+
+/// Internal base class with no own cache
+class BonusCacheBase
+{
+protected:
+	const IBonusBearer * target;
+
+	explicit BonusCacheBase(const IBonusBearer * target):
+		target(target)
+	{}
+
+	struct BonusCacheEntry
+	{
+		std::atomic<int64_t> version = 0;
+		std::atomic<int64_t> value = 0;
+	};
+
+	int getBonusValueImpl(BonusCacheEntry & currentValue, const CSelector & selector, BonusCacheMode) const;
+};
+
+/// Cache that tracks a single query to bonus system
+class BonusValueCache : public BonusCacheBase
+{
+	CSelector selector;
+	mutable BonusCacheEntry value;
+public:
+	BonusValueCache(const IBonusBearer * target, const CSelector & selector);
+	int getValue() const;
+	bool hasBonus() const;
+};
+
+/// Cache that can track a list of queries to bonus system
+template<size_t SIZE>
+class BonusValuesArrayCache : public BonusCacheBase
+{
+public:
+	using SelectorsArray = std::array<const CSelector, SIZE>;
+
+	BonusValuesArrayCache(const IBonusBearer * target, const SelectorsArray * selectors)
+		: BonusCacheBase(target)
+		, selectors(selectors)
+	{}
+
+	int getBonusValue(int index) const
+	{
+		return getBonusValueImpl(cache[index], (*selectors)[index], BonusCacheMode::VALUE);
+	}
+
+	int hasBonus(int index) const
+	{
+		return getBonusValueImpl(cache[index], (*selectors)[index], BonusCacheMode::PRESENCE);
+	}
+
+private:
+	using CacheArray = std::array<BonusCacheEntry, SIZE>;
+
+	const SelectorsArray * selectors;
+	mutable CacheArray cache;
+};
+
+class UnitBonusValuesProxy
+{
+public:
+	enum ECacheKeys : int8_t
+	{
+		TOTAL_ATTACKS_MELEE,
+		TOTAL_ATTACKS_RANGED,
+
+		MIN_DAMAGE_MELEE,
+		MIN_DAMAGE_RANGED,
+		MAX_DAMAGE_MELEE,
+		MAX_DAMAGE_RANGED,
+
+		ATTACK_MELEE,
+		ATTACK_RANGED,
+
+		DEFENCE_MELEE,
+		DEFENCE_RANGED,
+
+		IN_FRENZY,
+		HYPNOTIZED,
+		FORGETFULL,
+		HAS_FREE_SHOOTING,
+		STACK_HEALTH,
+
+		CLONE_MARKER,
+
+		TOTAL_KEYS,
+	};
+	static constexpr size_t KEYS_COUNT = static_cast<size_t>(ECacheKeys::TOTAL_KEYS);
+
+	using SelectorsArray = BonusValuesArrayCache<KEYS_COUNT>::SelectorsArray;
+
+	UnitBonusValuesProxy(const IBonusBearer * Target):
+		cache(Target, generateSelectors())
+	{}
+
+	int getBonusValue(ECacheKeys which) const
+	{
+		auto index = static_cast<size_t>(which);
+		return cache.getBonusValue(index);
+	}
+
+	int hasBonus(ECacheKeys which) const
+	{
+		auto index = static_cast<size_t>(which);
+		return cache.hasBonus(index);
+	}
+
+private:
+	const SelectorsArray * generateSelectors();
+
+	BonusValuesArrayCache<KEYS_COUNT> cache;
+};
+
+/// Cache that tracks values of primary skill values in bonus system
+class PrimarySkillsCache
+{
+	const IBonusBearer * target;
+	mutable std::atomic<int64_t> version = 0;
+	mutable std::array<std::atomic<int32_t>, 4> skills;
+
+	void update() const;
+public:
+	PrimarySkillsCache(const IBonusBearer * target);
+
+	const std::array<std::atomic<int32_t>, 4> & getSkills() const;
+};
+
+/// Cache that tracks values of spell school mastery in bonus system
+class MagicSchoolMasteryCache
+{
+	const IBonusBearer * target;
+	mutable std::atomic<int64_t> version = 0;
+	mutable std::array<std::atomic<int32_t>, 4+1> schools;
+
+	void update() const;
+public:
+	MagicSchoolMasteryCache(const IBonusBearer * target);
+
+	int32_t getMastery(const SpellSchool & school) const;
+};
+
+/// Cache that tracks values for different values of bonus duration
+class BonusCachePerTurn : public BonusCacheBase
+{
+	static constexpr int cachedTurns = 8;
+
+	const CSelector selector;
+	mutable TConstBonusListPtr bonusList;
+	mutable std::mutex bonusListMutex;
+	mutable std::atomic<int64_t> bonusListVersion = 0;
+	mutable std::array<BonusCacheEntry, cachedTurns> cache;
+	const BonusCacheMode mode;
+
+	int getValueUncached(int turns) const;
+public:
+	BonusCachePerTurn(const IBonusBearer * target, const CSelector & selector, BonusCacheMode mode)
+		: BonusCacheBase(target)
+		, selector(selector)
+		, mode(mode)
+	{}
+
+	int getValue(int turns) const;
+};
+
+VCMI_LIB_NAMESPACE_END

lib/bonuses/BonusList.cpp

@@ -83,10 +83,10 @@ void BonusList::stackBonuses()
 	}
 }
 
-int BonusList::totalValue() const
+int BonusList::totalValue(int baseValue) const
 {
 	if (bonuses.empty())
-		return 0;
+		return baseValue;
 
 	struct BonusCollection
 	{
@@ -104,6 +104,7 @@ int BonusList::totalValue() const
 	};
 
 	BonusCollection accumulated;
+	accumulated.base = baseValue;
 	int indexMaxCount = 0;
 	int indexMinCount = 0;
 
@@ -208,12 +209,12 @@ void BonusList::getAllBonuses(BonusList &out) const
 		out.push_back(b);
 }
 
-int BonusList::valOfBonuses(const CSelector &select) const
+int BonusList::valOfBonuses(const CSelector &select, int baseValue) const
 {
 	BonusList ret;
 	CSelector limit = nullptr;
 	getBonuses(ret, select, limit);
-	return ret.totalValue();
+	return ret.totalValue(baseValue);
 }
 
 JsonNode BonusList::toJsonNode() const

lib/bonuses/BonusList.h

@@ -58,14 +58,14 @@ public:
 
 	// BonusList functions
 	void stackBonuses();
-	int totalValue() const;
+	int totalValue(int baseValue = 0) const;
 	void getBonuses(BonusList &out, const CSelector &selector, const CSelector &limit = nullptr) const;
 	void getAllBonuses(BonusList &out) const;
 
 	//special find functions
 	std::shared_ptr<Bonus> getFirst(const CSelector &select);
 	std::shared_ptr<const Bonus> getFirst(const CSelector &select) const;
-	int valOfBonuses(const CSelector &select) const;
+	int valOfBonuses(const CSelector &select, int baseValue = 0) const;
 
 	// conversion / output
 	JsonNode toJsonNode() const;

lib/bonuses/CBonusProxy.cpp

@@ -1,221 +0,0 @@
-/*
- * CBonusProxy.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 "BonusList.h"
-#include "CBonusProxy.h"
-#include "IBonusBearer.h"
-
-VCMI_LIB_NAMESPACE_BEGIN
-
-///CBonusProxy
-CBonusProxy::CBonusProxy(const IBonusBearer * Target, CSelector Selector):
-	bonusListCachedLast(0),
-	target(Target),
-	selector(std::move(Selector)),
-	currentBonusListIndex(0)
-{
-}
-
-CBonusProxy::CBonusProxy(const CBonusProxy & other):
-	bonusListCachedLast(other.bonusListCachedLast),
-	target(other.target),
-	selector(other.selector),
-	currentBonusListIndex(other.currentBonusListIndex)
-{
-	bonusList[currentBonusListIndex] = other.bonusList[currentBonusListIndex];
-}
-
-CBonusProxy::CBonusProxy(CBonusProxy && other) noexcept:
-	bonusListCachedLast(0),
-	target(other.target),
-	currentBonusListIndex(0)
-{
-	std::swap(bonusListCachedLast, other.bonusListCachedLast);
-	std::swap(selector, other.selector);
-	std::swap(bonusList, other.bonusList);
-	std::swap(currentBonusListIndex, other.currentBonusListIndex);
-}
-
-CBonusProxy & CBonusProxy::operator=(const CBonusProxy & other)
-{
-	boost::lock_guard<boost::mutex> lock(swapGuard);
-
-	selector = other.selector;
-	swapBonusList(other.bonusList[other.currentBonusListIndex]);
-	bonusListCachedLast = other.bonusListCachedLast;
-
-	return *this;
-}
-
-CBonusProxy & CBonusProxy::operator=(CBonusProxy && other) noexcept
-{
-	std::swap(bonusListCachedLast, other.bonusListCachedLast);
-	std::swap(selector, other.selector);
-	std::swap(bonusList, other.bonusList);
-	std::swap(currentBonusListIndex, other.currentBonusListIndex);
-
-	return *this;
-}
-
-void CBonusProxy::swapBonusList(TConstBonusListPtr other) const
-{
-	// The idea here is to avoid changing active bonusList while it can be read by a different thread.
-	// Because such use of shared ptr is not thread safe
-	// So to avoid this we change the second offline instance and swap active index
-	auto newCurrent = 1 - currentBonusListIndex;
-	bonusList[newCurrent] = std::move(other);
-	currentBonusListIndex = newCurrent;
-}
-
-TConstBonusListPtr CBonusProxy::getBonusList() const
-{
-	auto needUpdateBonusList = [&]() -> bool
-	{
-		return target->getTreeVersion() != bonusListCachedLast || !bonusList[currentBonusListIndex];
-	};
-
-	// avoid locking if everything is up-to-date
-	if(needUpdateBonusList())
-	{
-		boost::lock_guard<boost::mutex>lock(swapGuard);
-
-		if(needUpdateBonusList())
-		{
-			//TODO: support limiters
-			swapBonusList(target->getAllBonuses(selector, Selector::all));
-			bonusListCachedLast = target->getTreeVersion();
-		}
-	}
-
-	return bonusList[currentBonusListIndex];
-}
-
-const BonusList * CBonusProxy::operator->() const
-{
-	return getBonusList().get();
-}
-
-CTotalsProxy::CTotalsProxy(const IBonusBearer * Target, CSelector Selector, int InitialValue):
-	CBonusProxy(Target, std::move(Selector)),
-	initialValue(InitialValue),
-	meleeCachedLast(0),
-	meleeValue(0),
-	rangedCachedLast(0),
-	rangedValue(0)
-{
-}
-
-CTotalsProxy::CTotalsProxy(const CTotalsProxy & other)
-	: CBonusProxy(other),
-	initialValue(other.initialValue),
-	meleeCachedLast(other.meleeCachedLast),
-	meleeValue(other.meleeValue),
-	rangedCachedLast(other.rangedCachedLast),
-	rangedValue(other.rangedValue)
-{
-}
-
-int CTotalsProxy::getValue() const
-{
-	const auto treeVersion = target->getTreeVersion();
-
-	if(treeVersion != valueCachedLast)
-	{
-		auto bonuses = getBonusList();
-
-		value = initialValue + bonuses->totalValue();
-		valueCachedLast = treeVersion;
-	}
-	return value;
-}
-
-int CTotalsProxy::getValueAndList(TConstBonusListPtr & outBonusList) const
-{
-	const auto treeVersion = target->getTreeVersion();
-	outBonusList = getBonusList();
-
-	if(treeVersion != valueCachedLast)
-	{
-		value = initialValue + outBonusList->totalValue();
-		valueCachedLast = treeVersion;
-	}
-	return value;
-}
-
-int CTotalsProxy::getMeleeValue() const
-{
-	static const auto limit = Selector::effectRange()(BonusLimitEffect::NO_LIMIT).Or(Selector::effectRange()(BonusLimitEffect::ONLY_MELEE_FIGHT));
-
-	const auto treeVersion = target->getTreeVersion();
-
-	if(treeVersion != meleeCachedLast)
-	{
-		auto bonuses = target->getBonuses(selector, limit);
-		meleeValue = initialValue + bonuses->totalValue();
-		meleeCachedLast = treeVersion;
-	}
-
-	return meleeValue;
-}
-
-int CTotalsProxy::getRangedValue() const
-{
-	static const auto limit = Selector::effectRange()(BonusLimitEffect::NO_LIMIT).Or(Selector::effectRange()(BonusLimitEffect::ONLY_DISTANCE_FIGHT));
-
-	const auto treeVersion = target->getTreeVersion();
-
-	if(treeVersion != rangedCachedLast)
-	{
-		auto bonuses = target->getBonuses(selector, limit);
-		rangedValue = initialValue + bonuses->totalValue();
-		rangedCachedLast = treeVersion;
-	}
-
-	return rangedValue;
-}
-
-///CCheckProxy
-CCheckProxy::CCheckProxy(const IBonusBearer * Target, BonusType bonusType):
-	target(Target),
-	selector(Selector::type()(bonusType)),
-	cachingStr("type_" + std::to_string(static_cast<int>(bonusType))),
-	cachedLast(0),
-	hasBonus(false)
-{
-
-}
-
-CCheckProxy::CCheckProxy(const IBonusBearer * Target, CSelector Selector, const std::string & cachingStr):
-	target(Target),
-	selector(std::move(Selector)),
-	cachedLast(0),
-	cachingStr(cachingStr),
-	hasBonus(false)
-{
-}
-
-//This constructor should be placed here to avoid side effects
-CCheckProxy::CCheckProxy(const CCheckProxy & other) = default;
-
-bool CCheckProxy::getHasBonus() const
-{
-	const auto treeVersion = target->getTreeVersion();
-
-	if(treeVersion != cachedLast)
-	{
-		hasBonus = target->hasBonus(selector, cachingStr);
-		cachedLast = treeVersion;
-	}
-
-	return hasBonus;
-}
-
-VCMI_LIB_NAMESPACE_END

lib/bonuses/CBonusProxy.h

@@ -1,92 +0,0 @@
-/*
- * CBonusProxy.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 "Bonus.h"
-#include "BonusSelector.h"
-
-VCMI_LIB_NAMESPACE_BEGIN
-
-class DLL_LINKAGE CBonusProxy
-{
-public:
-	CBonusProxy(const IBonusBearer * Target, CSelector Selector);
-	CBonusProxy(const CBonusProxy & other);
-	CBonusProxy(CBonusProxy && other) noexcept;
-
-	CBonusProxy & operator=(CBonusProxy && other) noexcept;
-	CBonusProxy & operator=(const CBonusProxy & other);
-	const BonusList * operator->() const;
-	TConstBonusListPtr getBonusList() const;
-
-protected:
-	CSelector selector;
-	const IBonusBearer * target;
-	mutable int64_t bonusListCachedLast;
-	mutable TConstBonusListPtr bonusList[2];
-	mutable int currentBonusListIndex;
-	mutable boost::mutex swapGuard;
-	void swapBonusList(TConstBonusListPtr other) const;
-};
-
-class DLL_LINKAGE CTotalsProxy : public CBonusProxy
-{
-public:
-	CTotalsProxy(const IBonusBearer * Target, CSelector Selector, int InitialValue);
-	CTotalsProxy(const CTotalsProxy & other);
-	CTotalsProxy(CTotalsProxy && other) = delete;
-
-	CTotalsProxy & operator=(const CTotalsProxy & other) = default;
-	CTotalsProxy & operator=(CTotalsProxy && other) = delete;
-
-	int getMeleeValue() const;
-	int getRangedValue() const;
-	int getValue() const;
-	/**
-	Returns total value of all selected bonuses and sets bonusList as a pointer to the list of selected bonuses
-	@param bonusList is the out list of all selected bonuses
-	@return total value of all selected bonuses and 0 otherwise
-	*/
-	int getValueAndList(TConstBonusListPtr & bonusList) const;
-
-private:
-	int initialValue;
-
-	mutable int64_t valueCachedLast = 0;
-	mutable int value = 0;
-
-	mutable int64_t meleeCachedLast;
-	mutable int meleeValue;
-
-	mutable int64_t rangedCachedLast;
-	mutable int rangedValue;
-};
-
-class DLL_LINKAGE CCheckProxy
-{
-public:
-	CCheckProxy(const IBonusBearer * Target, CSelector Selector, const std::string & cachingStr);
-	CCheckProxy(const IBonusBearer * Target, BonusType bonusType);
-	CCheckProxy(const CCheckProxy & other);
-	CCheckProxy& operator= (const CCheckProxy & other) = default;
-
-	bool getHasBonus() const;
-
-private:
-	const IBonusBearer * target;
-	std::string cachingStr;
-	CSelector selector;
-
-	mutable int64_t cachedLast;
-	mutable bool hasBonus;
-};
-
-VCMI_LIB_NAMESPACE_END

lib/bonuses/IBonusBearer.cpp

@@ -105,7 +105,7 @@ bool IBonusBearer::hasBonusOfType(BonusType type, BonusSubtypeID subtype) const
 
 bool IBonusBearer::hasBonusFrom(BonusSource source, BonusSourceID sourceID) const
 {
-	std::string cachingStr = "source_" + std::to_string(static_cast<int>(source)) + "_" + sourceID.toString();
+	std::string cachingStr = "source_" + std::to_string(static_cast<int>(source)) + "_" + std::to_string(sourceID.getNum());
 	return hasBonus(Selector::source(source,sourceID), cachingStr);
 }
 

lib/bonuses/Updaters.cpp

@@ -111,18 +111,6 @@ ArmyMovementUpdater::ArmyMovementUpdater(int base, int divider, int multiplier,
 
 std::shared_ptr<Bonus> ArmyMovementUpdater::createUpdatedBonus(const std::shared_ptr<Bonus> & b, const CBonusSystemNode & context) const
 {
-	if(b->type == BonusType::MOVEMENT && context.getNodeType() == CBonusSystemNode::HERO)
-	{
-		auto speed = static_cast<const CGHeroInstance &>(context).getLowestCreatureSpeed();
-		si32 armySpeed = speed * base / divider;
-		auto counted = armySpeed * multiplier;
-		auto newBonus = std::make_shared<Bonus>(*b);
-		newBonus->source = BonusSource::ARMY;
-		newBonus->val += vstd::amin(counted, max);
-		return newBonus;
-	}
-	if(b->type != BonusType::MOVEMENT)
-		logGlobal->error("ArmyMovementUpdater should only be used for MOVEMENT bonus!");
 	return b;
 }
 

lib/mapObjects/CArmedInstance.cpp

@@ -46,7 +46,7 @@ CArmedInstance::CArmedInstance(IGameCallback *cb)
 CArmedInstance::CArmedInstance(IGameCallback *cb, bool isHypothetic):
 	CGObjectInstance(cb),
 	CBonusSystemNode(isHypothetic),
-	nonEvilAlignmentMix(this, BonusType::NONEVIL_ALIGNMENT_MIX), // Take Angelic Alliance troop-mixing freedom of non-evil units into account.
+	nonEvilAlignmentMix(this, Selector::type()(BonusType::NONEVIL_ALIGNMENT_MIX)), // Take Angelic Alliance troop-mixing freedom of non-evil units into account.
 	battle(nullptr)
 {
 }
@@ -86,7 +86,7 @@ void CArmedInstance::updateMoraleBonusFromArmy()
 
 	size_t factionsInArmy = factions.size(); //town garrison seems to take both sets into account
 
-	if (nonEvilAlignmentMix.getHasBonus())
+	if (nonEvilAlignmentMix.hasBonus())
 	{
 		size_t mixableFactions = 0;
 

lib/mapObjects/CArmedInstance.h

@@ -11,8 +11,8 @@
 
 #include "CGObjectInstance.h"
 #include "../CCreatureSet.h"
-#include "../bonuses/CBonusProxy.h"
 #include "../bonuses/CBonusSystemNode.h"
+#include "../bonuses/BonusCache.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -23,8 +23,7 @@ class JsonSerializeFormat;
 class DLL_LINKAGE CArmedInstance: public CGObjectInstance, public CBonusSystemNode, public CCreatureSet, public IConstBonusProvider
 {
 private:
-	CCheckProxy nonEvilAlignmentMix;
+	BonusValueCache nonEvilAlignmentMix;
-	static CSelector nonEvilAlignmentMixSelector;
 
 public:
 	BattleInfo *battle; //set to the current battle, if engaged

lib/mapObjects/CGHeroInstance.cpp

@@ -73,31 +73,6 @@ void CGHeroPlaceholder::serializeJsonOptions(JsonSerializeFormat & handler)
 		handler.serializeInt("powerRank", powerRank.value());
 }
 
-static int lowestSpeed(const CGHeroInstance * chi)
-{
-	static const CSelector selectorSTACKS_SPEED = Selector::type()(BonusType::STACKS_SPEED);
-	static const std::string keySTACKS_SPEED = "type_" + std::to_string(static_cast<si32>(BonusType::STACKS_SPEED));
-
-	if(!chi->stacksCount())
-	{
-		if(chi->commander && chi->commander->alive)
-		{
-			return chi->commander->valOfBonuses(selectorSTACKS_SPEED, keySTACKS_SPEED);
-		}
-
-		logGlobal->error("Hero %d (%s) has no army!", chi->id.getNum(), chi->getNameTranslated());
-		return 20;
-	}
-
-	auto i = chi->Slots().begin();
-	//TODO? should speed modifiers (eg from artifacts) affect hero movement?
-
-	int ret = (i++)->second->valOfBonuses(selectorSTACKS_SPEED, keySTACKS_SPEED);
-	for(; i != chi->Slots().end(); i++)
-		ret = std::min(ret, i->second->valOfBonuses(selectorSTACKS_SPEED, keySTACKS_SPEED));
-	return ret;
-}
-
 ui32 CGHeroInstance::getTileMovementCost(const TerrainTile & dest, const TerrainTile & from, const TurnInfo * ti) const
 {
 	int64_t ret = GameConstants::BASE_MOVEMENT_COST;
@@ -107,13 +82,10 @@ ui32 CGHeroInstance::getTileMovementCost(const TerrainTile & dest, const Terrain
 	{
 		ret = from.getRoad()->movementCost;
 	}
-	else if(ti->nativeTerrain != from.getTerrainID() &&//the terrain is not native
+	else if(!ti->hasNoTerrainPenalty(from.getTerrainID())) //no special movement bonus
-			ti->nativeTerrain != ETerrainId::ANY_TERRAIN && //no special creature bonus
-			!ti->hasBonusOfType(BonusType::NO_TERRAIN_PENALTY, BonusSubtypeID(from.getTerrainID()))) //no special movement bonus
 	{
-
 		ret = VLC->terrainTypeHandler->getById(from.getTerrainID())->moveCost;
-		ret -= ti->valOfBonuses(BonusType::ROUGH_TERRAIN_DISCOUNT);
+		ret -= ti->getRoughTerrainDiscountValue();
 		if(ret < GameConstants::BASE_MOVEMENT_COST)
 			ret = GameConstants::BASE_MOVEMENT_COST;
 	}
@@ -257,30 +229,41 @@ void CGHeroInstance::setMovementPoints(int points)
 
 int CGHeroInstance::movementPointsLimit(bool onLand) const
 {
-	return valOfBonuses(BonusType::MOVEMENT, onLand ? BonusCustomSubtype::heroMovementLand : BonusCustomSubtype::heroMovementSea);
+	auto ti = getTurnInfo(0);
+	return onLand ? ti->getMovePointsLimitLand() : ti->getMovePointsLimitWater();
 }
 
 int CGHeroInstance::getLowestCreatureSpeed() const
 {
-	return lowestCreatureSpeed;
+	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->getInitiative());
+
+		return minimalSpeed;
+	}
+	else
+	{
+		if(commander && commander->alive)
+			return commander->getInitiative();
+	}
+
+	return 10;
 }
 
-void CGHeroInstance::updateArmyMovementBonus(bool onLand, const TurnInfo * ti) const
+std::unique_ptr<TurnInfo> CGHeroInstance::getTurnInfo(int days) const
 {
-	auto realLowestSpeed = lowestSpeed(this);
+	return std::make_unique<TurnInfo>(turnInfoCache.get(), this, days);
-	if(lowestCreatureSpeed != realLowestSpeed)
-	{
-		lowestCreatureSpeed = realLowestSpeed;
-		//Let updaters run again
-		treeHasChanged();
-		ti->updateHeroBonuses(BonusType::MOVEMENT);
-	}
 }
 
 int CGHeroInstance::movementPointsLimitCached(bool onLand, const TurnInfo * ti) const
 {
-	updateArmyMovementBonus(onLand, ti);
+	if (onLand)
-	return ti->valOfBonuses(BonusType::MOVEMENT, onLand ? BonusCustomSubtype::heroMovementLand : BonusCustomSubtype::heroMovementSea);
+		return ti->getMovePointsLimitLand();
+	else
+		return ti->getMovePointsLimitWater();
 }
 
 CGHeroInstance::CGHeroInstance(IGameCallback * cb)
@@ -293,7 +276,10 @@ CGHeroInstance::CGHeroInstance(IGameCallback * cb)
 	level(1),
 	exp(UNINITIALIZED_EXPERIENCE),
 	gender(EHeroGender::DEFAULT),
-	lowestCreatureSpeed(0)
+	primarySkills(this),
+	magicSchoolMastery(this),
+	turnInfoCache(std::make_unique<TurnInfoCache>(this)),
+	manaPerKnowledgeCached(this, Selector::type()(BonusType::MANA_PER_KNOWLEDGE_PERCENTAGE))
 {
 	setNodeType(HERO);
 	ID = Obj::HERO;
@@ -704,40 +690,20 @@ void CGHeroInstance::setPropertyDer(ObjProperty what, ObjPropertyID identifier)
 		setStackCount(SlotID(0), identifier.getNum());
 }
 
-std::array<int, 4> CGHeroInstance::getPrimarySkills() const
+int CGHeroInstance::getPrimSkillLevel(PrimarySkill id) const
 {
-	std::array<int, 4> result;
+	return primarySkills.getSkills()[id];
-
-	auto allSkills = getBonusBearer()->getBonusesOfType(BonusType::PRIMARY_SKILL);
-	for (auto skill : PrimarySkill::ALL_SKILLS())
-	{
-		int ret = allSkills->valOfBonuses(Selector::subtype()(BonusSubtypeID(skill)));
-		int minSkillValue = VLC->engineSettings()->getVectorValue(EGameSettings::HEROES_MINIMAL_PRIMARY_SKILLS, skill.getNum());
-		result[skill] = std::max(ret, minSkillValue); //otherwise, some artifacts may cause negative skill value effect
-	}
-
-	return result;
 }
 
 double CGHeroInstance::getFightingStrength() const
 {
-	const auto & primarySkills = getPrimarySkills();
+	const auto & skillValues = primarySkills.getSkills();
-	return getFightingStrengthImpl(primarySkills);
+	return sqrt((1.0 + 0.05*skillValues[PrimarySkill::ATTACK]) * (1.0 + 0.05*skillValues[PrimarySkill::DEFENSE]));
-}
-
-double CGHeroInstance::getFightingStrengthImpl(const std::array<int, 4> & primarySkills) const
-{
-	return sqrt((1.0 + 0.05*primarySkills[PrimarySkill::ATTACK]) * (1.0 + 0.05*primarySkills[PrimarySkill::DEFENSE]));
 }
 
 double CGHeroInstance::getMagicStrength() const
 {
-	const auto & primarySkills = getPrimarySkills();
+	const auto & skillValues = primarySkills.getSkills();
-	return getMagicStrengthImpl(primarySkills);
-}
-
-double CGHeroInstance::getMagicStrengthImpl(const std::array<int, 4> & primarySkills) const
-{
 	if (!hasSpellbook())
 		return 1;
 	bool atLeastOneCombatSpell = false;
@@ -751,13 +717,12 @@ double CGHeroInstance::getMagicStrengthImpl(const std::array<int, 4> & primarySk
 	}
 	if (!atLeastOneCombatSpell)
 		return 1;
-	return sqrt((1.0 + 0.05*primarySkills[PrimarySkill::KNOWLEDGE] * mana / manaLimit()) * (1.0 + 0.05*primarySkills[PrimarySkill::SPELL_POWER] * mana / manaLimit()));
+	return sqrt((1.0 + 0.05*skillValues[PrimarySkill::KNOWLEDGE] * mana / manaLimit()) * (1.0 + 0.05*skillValues[PrimarySkill::SPELL_POWER] * mana / manaLimit()));
 }
 
 double CGHeroInstance::getHeroStrength() const
 {
-	const auto & primarySkills = getPrimarySkills();
+	return getFightingStrength() * getMagicStrength();
-	return getFightingStrengthImpl(primarySkills) * getMagicStrengthImpl(primarySkills);
 }
 
 uint64_t CGHeroInstance::getValueForDiplomacy() const
@@ -809,7 +774,7 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, SpellSc
 
 	spell->forEachSchool([&, this](const SpellSchool & cnf, bool & stop)
 	{
-		int32_t thisSchool = valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, BonusSubtypeID(cnf)); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
+		int32_t thisSchool = magicSchoolMastery.getMastery(cnf); //FIXME: Bonus shouldn't be additive (Witchking Artifacts : Crown of Skies)
 		if(thisSchool > skill)
 		{
 			skill = thisSchool;
@@ -818,7 +783,7 @@ int32_t CGHeroInstance::getSpellSchoolLevel(const spells::Spell * spell, SpellSc
 		}
 	});
 
-	vstd::amax(skill, valOfBonuses(BonusType::MAGIC_SCHOOL_SKILL, BonusSubtypeID(SpellSchool::ANY))); //any school bonus
+	vstd::amax(skill, magicSchoolMastery.getMastery(SpellSchool::ANY)); //any school bonus
 	vstd::amax(skill, valOfBonuses(BonusType::SPELL, BonusSubtypeID(spell->getId()))); //given by artifact or other effect
 
 	vstd::amax(skill, 0); //in case we don't know any school
@@ -1207,8 +1172,7 @@ std::string CGHeroInstance::nodeName() const
 
 si32 CGHeroInstance::manaLimit() const
 {
-	return si32(getPrimSkillLevel(PrimarySkill::KNOWLEDGE)
+	return getPrimSkillLevel(PrimarySkill::KNOWLEDGE) * manaPerKnowledgeCached.getValue() / 100;
-		* (valOfBonuses(BonusType::MANA_PER_KNOWLEDGE_PERCENTAGE))) / 100;
 }
 
 HeroTypeID CGHeroInstance::getPortraitSource() const
@@ -1381,14 +1345,7 @@ 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->hasFreeShipBoarding())
-	if(!ti)
-	{
-		turnInfoLocal = std::make_unique<TurnInfo>(this);
-		ti = turnInfoLocal.get();
-	}
-
-	if(!ti->hasBonusOfType(BonusType::FREE_SHIP_BOARDING))
 		return 0; // take all MPs by default
 	
 	auto boatLayer = boat ? boat->layer : EPathfindingLayer::SAIL;

lib/mapObjects/CGHeroInstance.h

@@ -14,6 +14,7 @@
 #include "CArmedInstance.h"
 #include "IOwnableObject.h"
 
+#include "../bonuses/BonusCache.h"
 #include "../entities/hero/EHeroGender.h"
 #include "../CArtHandler.h" // For CArtifactSet
 
@@ -24,8 +25,10 @@ class CGBoat;
 class CGTownInstance;
 class CMap;
 class UpgradeInfo;
+class TurnInfo;
+
 struct TerrainTile;
-struct TurnInfo;
+struct TurnInfoCache;
 
 class DLL_LINKAGE CGHeroPlaceholder : public CGObjectInstance
 {
@@ -58,12 +61,13 @@ class DLL_LINKAGE CGHeroInstance : public CArmedInstance, public IBoatGenerator,
 	friend class CMapFormatJson;
 
 private:
-	std::set<SpellID> spells; //known spells (spell IDs)
+	PrimarySkillsCache primarySkills;
-	mutable int lowestCreatureSpeed;
+	MagicSchoolMasteryCache magicSchoolMastery;
-	ui32 movement; //remaining movement points
+	BonusValueCache manaPerKnowledgeCached;
+	std::unique_ptr<TurnInfoCache> turnInfoCache;
 
-	double getFightingStrengthImpl(const std::array<int, 4> & primarySkills) const;
+	std::set<SpellID> spells; //known spells (spell IDs)
-	double getMagicStrengthImpl(const std::array<int, 4> & primarySkills) const;
+	ui32 movement; //remaining movement points
 
 public:
 
@@ -204,7 +208,7 @@ public:
 	std::vector<SecondarySkill> getLevelUpProposedSecondarySkills(vstd::RNG & rand) const;
 
 	ui8 getSecSkillLevel(const SecondarySkill & skill) const; //0 - no skill
-	std::array<int, 4> getPrimarySkills() const;
+	int getPrimSkillLevel(PrimarySkill id) const;
 
 	/// Returns true if hero has free secondary skill slot.
 	bool canLearnSkill() const;
@@ -222,10 +226,10 @@ public:
 	int movementPointsLimit(bool onLand) const;
 	//cached version is much faster, TurnInfo construction is costly
 	int movementPointsLimitCached(bool onLand, const TurnInfo * ti) const;
-	//update army movement bonus
-	void updateArmyMovementBonus(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;
 
 	double getFightingStrength() const; // takes attack / defense skill into account
 	double getMagicStrength() const; // takes knowledge / spell power skill but also current mana, whether the hero owns a spell-book and whether that books contains anything into account

lib/mapping/CMap.cpp

@@ -143,17 +143,6 @@ TerrainTile::TerrainTile():
 {
 }
 
-bool TerrainTile::entrableTerrain(const TerrainTile * from) const
-{
-	return entrableTerrain(from ? from->isLand() : true, from ? from->isWater() : true);
-}
-
-bool TerrainTile::entrableTerrain(bool allowLand, bool allowSea) const
-{
-	return getTerrain()->isPassable()
-			&& ((allowSea && isWater())  ||  (allowLand && isLand()));
-}
-
 bool TerrainTile::isClear(const TerrainTile * from) const
 {
 	return entrableTerrain(from) && !blocked();
@@ -187,72 +176,6 @@ EDiggingStatus TerrainTile::getDiggingStatus(const bool excludeTop) const
 		return EDiggingStatus::CAN_DIG;
 }
 
-bool TerrainTile::hasFavorableWinds() const
-{
-	return extTileFlags & 128;
-}
-
-bool TerrainTile::isWater() const
-{
-	return getTerrain()->isWater();
-}
-
-bool TerrainTile::isLand() const
-{
-	return getTerrain()->isLand();
-}
-
-bool TerrainTile::visitable() const
-{
-	return !visitableObjects.empty();
-}
-
-bool TerrainTile::blocked() const
-{
-	return !blockingObjects.empty();
-}
-
-bool TerrainTile::hasRiver() const
-{
-	return getRiverID() != RiverId::NO_RIVER;
-}
-
-bool TerrainTile::hasRoad() const
-{
-	return getRoadID() != RoadId::NO_ROAD;
-}
-
-const TerrainType * TerrainTile::getTerrain() const
-{
-	return terrainType.toEntity(VLC);
-}
-
-const RiverType * TerrainTile::getRiver() const
-{
-	return riverType.toEntity(VLC);
-}
-
-const RoadType * TerrainTile::getRoad() const
-{
-	return roadType.toEntity(VLC);
-}
-
-TerrainId TerrainTile::getTerrainID() const
-{
-	return terrainType;
-}
-
-RiverId TerrainTile::getRiverID() const
-{
-	return riverType;
-}
-
-RoadId TerrainTile::getRoadID() const
-{
-	return roadType;
-}
-
-
 CMap::CMap(IGameCallback * cb)
 	: GameCallbackHolder(cb)
 	, checksum(0)
@@ -365,7 +288,7 @@ bool CMap::isCoastalTile(const int3 & pos) const
 		return false;
 	}
 
-	if(isWaterTile(pos))
+	if(getTile(pos).isWater())
 		return false;
 
 	for(const auto & dir : dirs)
@@ -382,22 +305,6 @@ bool CMap::isCoastalTile(const int3 & pos) const
 	return false;
 }
 
-TerrainTile & CMap::getTile(const int3 & tile)
-{
-	assert(isInTheMap(tile));
-	return terrain[tile.z][tile.x][tile.y];
-}
-
-const TerrainTile & CMap::getTile(const int3 & tile) const
-{
-	assert(isInTheMap(tile));
-	return terrain[tile.z][tile.x][tile.y];
-}
-
-bool CMap::isWaterTile(const int3 &pos) const
-{
-	return isInTheMap(pos) && getTile(pos).isWater();
-}
 bool CMap::canMoveBetween(const int3 &src, const int3 &dst) const
 {
 	const TerrainTile * dstTile = &getTile(dst);

lib/mapping/CMap.h

@@ -86,18 +86,10 @@ public:
 	void initTerrain();
 
 	CMapEditManager * getEditManager();
-	TerrainTile & getTile(const int3 & tile);
+	inline TerrainTile & getTile(const int3 & tile);
-	const TerrainTile & getTile(const int3 & tile) const;
+	inline const TerrainTile & getTile(const int3 & tile) const;
 	bool isCoastalTile(const int3 & pos) const;
-	bool isWaterTile(const int3 & pos) const;
+	inline bool isInTheMap(const int3 & pos) const;
-	inline bool isInTheMap(const int3 & pos) const
-	{
-		// Check whether coord < 0 is done implicitly. Negative signed int overflows to unsigned number larger than all signed ints.
-		return
-			static_cast<uint32_t>(pos.x) < static_cast<uint32_t>(width) &&
-			static_cast<uint32_t>(pos.y) < static_cast<uint32_t>(height) &&
-			static_cast<uint32_t>(pos.z) <= (twoLevel ? 1 : 0);
-	}
 
 	bool canMoveBetween(const int3 &src, const int3 &dst) const;
 	bool checkForVisitableDir(const int3 & src, const TerrainTile * pom, const int3 & dst) const;
@@ -250,4 +242,25 @@ public:
 	}
 };
 
+inline bool CMap::isInTheMap(const int3 & pos) const
+{
+	// Check whether coord < 0 is done implicitly. Negative signed int overflows to unsigned number larger than all signed ints.
+	return
+		static_cast<uint32_t>(pos.x) < static_cast<uint32_t>(width) &&
+		static_cast<uint32_t>(pos.y) < static_cast<uint32_t>(height) &&
+		static_cast<uint32_t>(pos.z) <= (twoLevel ? 1 : 0);
+}
+
+inline TerrainTile & CMap::getTile(const int3 & tile)
+{
+	assert(isInTheMap(tile));
+	return terrain[tile.z][tile.x][tile.y];
+}
+
+inline const TerrainTile & CMap::getTile(const int3 & tile) const
+{
+	assert(isInTheMap(tile));
+	return terrain[tile.z][tile.x][tile.y];
+}
+
 VCMI_LIB_NAMESPACE_END

lib/mapping/CMapDefines.h

@@ -12,7 +12,8 @@
 
 #include "../ResourceSet.h"
 #include "../texts/MetaString.h"
-#include "../int3.h"
+#include "../VCMI_Lib.h"
+#include "../TerrainHandler.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -103,31 +104,32 @@ struct DLL_LINKAGE TerrainTile
 	TerrainTile();
 
 	/// Gets true if the terrain is not a rock. If from is water/land, same type is also required.
-	bool entrableTerrain(const TerrainTile * from = nullptr) const;
+	inline bool entrableTerrain() const;
-	bool entrableTerrain(bool allowLand, bool allowSea) const;
+	inline bool entrableTerrain(const TerrainTile * from) const;
+	inline bool entrableTerrain(bool allowLand, bool allowSea) const;
 	/// Checks for blocking objects and terraint type (water / land).
 	bool isClear(const TerrainTile * from = nullptr) const;
 	/// Gets the ID of the top visitable object or -1 if there is none.
 	Obj topVisitableId(bool excludeTop = false) const;
 	CGObjectInstance * topVisitableObj(bool excludeTop = false) const;
-	bool isWater() const;
+	inline bool isWater() const;
-	bool isLand() const;
+	inline bool isLand() const;
 	EDiggingStatus getDiggingStatus(bool excludeTop = true) const;
-	bool hasFavorableWinds() const;
+	inline bool hasFavorableWinds() const;
 
-	bool visitable() const;
+	inline bool visitable() const;
-	bool blocked() const;
+	inline bool blocked() const;
 
-	const TerrainType * getTerrain() const;
+	inline const TerrainType * getTerrain() const;
-	const RiverType * getRiver() const;
+	inline const RiverType * getRiver() const;
-	const RoadType * getRoad() const;
+	inline const RoadType * getRoad() const;
 
-	TerrainId getTerrainID() const;
+	inline TerrainId getTerrainID() const;
-	RiverId getRiverID() const;
+	inline RiverId getRiverID() const;
-	RoadId getRoadID() const;
+	inline RoadId getRoadID() const;
 
-	bool hasRiver() const;
+	inline bool hasRiver() const;
-	bool hasRoad() const;
+	inline bool hasRoad() const;
 
 	TerrainId terrainType;
 	RiverId riverType;
@@ -193,4 +195,86 @@ struct DLL_LINKAGE TerrainTile
 	}
 };
 
+inline bool TerrainTile::hasFavorableWinds() const
+{
+	return extTileFlags & 128;
+}
+
+inline bool TerrainTile::isWater() const
+{
+	return getTerrain()->isWater();
+}
+
+inline bool TerrainTile::isLand() const
+{
+	return getTerrain()->isLand();
+}
+
+inline bool TerrainTile::visitable() const
+{
+	return !visitableObjects.empty();
+}
+
+inline bool TerrainTile::blocked() const
+{
+	return !blockingObjects.empty();
+}
+
+inline bool TerrainTile::hasRiver() const
+{
+	return getRiverID() != RiverId::NO_RIVER;
+}
+
+inline bool TerrainTile::hasRoad() const
+{
+	return getRoadID() != RoadId::NO_ROAD;
+}
+
+inline const TerrainType * TerrainTile::getTerrain() const
+{
+	return terrainType.toEntity(VLC);
+}
+
+inline const RiverType * TerrainTile::getRiver() const
+{
+	return riverType.toEntity(VLC);
+}
+
+inline const RoadType * TerrainTile::getRoad() const
+{
+	return roadType.toEntity(VLC);
+}
+
+inline TerrainId TerrainTile::getTerrainID() const
+{
+	return terrainType;
+}
+
+inline RiverId TerrainTile::getRiverID() const
+{
+	return riverType;
+}
+
+inline RoadId TerrainTile::getRoadID() const
+{
+	return roadType;
+}
+
+inline bool TerrainTile::entrableTerrain() const
+{
+	return entrableTerrain(true, true);
+}
+
+inline bool TerrainTile::entrableTerrain(const TerrainTile * from) const
+{
+	const TerrainType * terrainFrom = from->getTerrain();
+	return entrableTerrain(terrainFrom->isLand(), terrainFrom->isWater());
+}
+
+inline bool TerrainTile::entrableTerrain(bool allowLand, bool allowSea) const
+{
+	const TerrainType * terrain = getTerrain();
+	return terrain->isPassable() && ((allowSea && terrain->isWater()) || (allowLand && terrain->isLand()));
+}
+
 VCMI_LIB_NAMESPACE_END

lib/pathfinder/CPathfinder.cpp

@@ -514,11 +514,7 @@ CPathfinderHelper::CPathfinderHelper(CGameState * gs, const CGHeroInstance * Her
 	canCastWaterWalk = Hero->canCastThisSpell(waterWalk.toSpell());
 }
 
-CPathfinderHelper::~CPathfinderHelper()
+CPathfinderHelper::~CPathfinderHelper() = default;
-{
-	for(auto * ti : turnsInfo)
-		delete ti;
-}
 
 void CPathfinderHelper::updateTurnInfo(const int Turn)
 {
@@ -526,10 +522,7 @@ void CPathfinderHelper::updateTurnInfo(const int Turn)
 	{
 		turn = Turn;
 		if(turn >= turnsInfo.size())
-		{
+			turnsInfo.push_back(hero->getTurnInfo(turn));
-			auto * ti = new TurnInfo(hero, turn);
-			turnsInfo.push_back(ti);
-		}
 	}
 }
 
@@ -561,12 +554,7 @@ bool CPathfinderHelper::isLayerAvailable(const EPathfindingLayer & layer) const
 
 const TurnInfo * CPathfinderHelper::getTurnInfo() const
 {
-	return turnsInfo[turn];
+	return turnsInfo[turn].get();
-}
-
-bool CPathfinderHelper::hasBonusOfType(const BonusType type) const
-{
-	return turnsInfo[turn]->hasBonusOfType(type);
 }
 
 int CPathfinderHelper::getMaxMovePoints(const EPathfindingLayer & layer) const
@@ -575,15 +563,16 @@ int CPathfinderHelper::getMaxMovePoints(const EPathfindingLayer & layer) const
 }
 
 void CPathfinderHelper::getNeighbours(
-	const TerrainTile & srcTile,
+	const TerrainTile & sourceTile,
 	const int3 & srcCoord,
 	NeighbourTilesVector & vec,
 	const boost::logic::tribool & onLand,
 	const bool limitCoastSailing) const
 {
 	CMap * map = gs->map;
+	const TerrainType * sourceTerrain = sourceTile.getTerrain();
 
-	static const int3 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)
@@ -596,12 +585,12 @@ void CPathfinderHelper::getNeighbours(
 			continue;
 
 		const TerrainTile & destTile = map->getTile(destCoord);
-		const TerrainType* terrain = destTile.getTerrain();
+		const TerrainType * destTerrain = destTile.getTerrain();
-		if(!terrain->isPassable())
+		if(!destTerrain->isPassable())
 			continue;
 
 		/// Following condition let us avoid diagonal movement over coast when sailing
-		if(srcTile.isWater() && limitCoastSailing && terrain->isWater() && dir.x && dir.y) //diagonal move through water
+		if(sourceTerrain->isWater() && limitCoastSailing && destTerrain->isWater() && dir.x && dir.y) //diagonal move through water
 		{
 			const int3 horizontalNeighbour = srcCoord + int3{dir.x, 0, 0};
 			const int3 verticalNeighbour = srcCoord + int3{0, dir.y, 0};
@@ -609,7 +598,7 @@ void CPathfinderHelper::getNeighbours(
 				continue;
 		}
 
-		if(indeterminate(onLand) || onLand == terrain->isLand())
+		if(indeterminate(onLand) || onLand == destTerrain->isLand())
 		{
 			vec.push_back(destCoord);
 		}
@@ -663,54 +652,46 @@ int CPathfinderHelper::getMovementCost(
 
 	bool isWaterLayer;
 	if(indeterminate(isDstWaterLayer))
-		isWaterLayer = ((hero->boat && hero->boat->layer == EPathfindingLayer::WATER) || ti->hasBonusOfType(BonusType::WATER_WALKING)) && dt->isWater();
+		isWaterLayer = ((hero->boat && hero->boat->layer == EPathfindingLayer::WATER) || ti->hasWaterWalking()) && dt->isWater();
 	else
 		isWaterLayer = static_cast<bool>(isDstWaterLayer);
 	
-	bool isAirLayer = (hero->boat && hero->boat->layer == EPathfindingLayer::AIR) || ti->hasBonusOfType(BonusType::FLYING_MOVEMENT);
+	bool isAirLayer = (hero->boat && hero->boat->layer == EPathfindingLayer::AIR) || ti->hasFlyingMovement();
 
-	int ret = hero->getTileMovementCost(*dt, *ct, ti);
+	int movementCost = hero->getTileMovementCost(*dt, *ct, ti);
 	if(isSailLayer)
 	{
 		if(ct->hasFavorableWinds())
-			ret = static_cast<int>(ret * 2.0 / 3);
+			movementCost = static_cast<int>(movementCost * 2.0 / 3);
 	}
 	else if(isAirLayer)
-		vstd::amin(ret, GameConstants::BASE_MOVEMENT_COST + ti->valOfBonuses(BonusType::FLYING_MOVEMENT));
+		vstd::amin(movementCost, GameConstants::BASE_MOVEMENT_COST + ti->getFlyingMovementValue());
-	else if(isWaterLayer && ti->hasBonusOfType(BonusType::WATER_WALKING))
+	else if(isWaterLayer && ti->hasWaterWalking())
-		ret = static_cast<int>(ret * (100.0 + ti->valOfBonuses(BonusType::WATER_WALKING)) / 100.0);
+		movementCost = static_cast<int>(movementCost * (100.0 + ti->getWaterWalkingValue()) / 100.0);
 
 	if(src.x != dst.x && src.y != dst.y) //it's diagonal move
 	{
-		int old = ret;
+		int old = movementCost;
-		ret = static_cast<int>(ret * M_SQRT2);
+		movementCost = static_cast<int>(movementCost * M_SQRT2);
 		//diagonal move costs too much but normal move is possible - allow diagonal move for remaining move points
 		// https://heroes.thelazy.net/index.php/Movement#Diagonal_move_exception
-		if(ret > remainingMovePoints && remainingMovePoints >= old)
+		if(movementCost > remainingMovePoints && remainingMovePoints >= old)
 		{
 			return remainingMovePoints;
 		}
 	}
 
-	const int left = remainingMovePoints - ret;
+	//it might be the last tile - if no further move possible we take all move points
-	constexpr auto maxCostOfOneStep = static_cast<int>(175 * M_SQRT2); // diagonal move on Swamp - 247 MP
+	const int pointsLeft = remainingMovePoints - movementCost;
-	if(checkLast && left > 0 && left <= maxCostOfOneStep) //it might be the last tile - if no further move possible we take all move points
+	if(checkLast && pointsLeft > 0)
 	{
-		NeighbourTilesVector vec;
+		int minimalNextMoveCost = hero->getTileMovementCost(*dt, *ct, ti);
 
-		getNeighbours(*dt, dst, vec, ct->isLand(), true);
+		if (pointsLeft < minimalNextMoveCost)
-		for(const auto & elem : vec)
+			return remainingMovePoints;
-		{
-			int fcost = getMovementCost(dst, elem, nullptr, nullptr, left, false);
-			if(fcost <= left)
-			{
-				return ret;
-			}
-		}
-		ret = remainingMovePoints;
 	}
 
-	return ret;
+	return movementCost;
 }
 
 VCMI_LIB_NAMESPACE_END

lib/pathfinder/CPathfinder.h

@@ -16,7 +16,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 class CGWhirlpool;
-struct TurnInfo;
+class TurnInfo;
 struct PathfinderOptions;
 
 // Optimized storage - tile can have 0-8 neighbour tiles
@@ -78,7 +78,7 @@ public:
 	int turn;
 	PlayerColor owner;
 	const CGHeroInstance * hero;
-	std::vector<TurnInfo *> turnsInfo;
+	std::vector<std::unique_ptr<TurnInfo>> turnsInfo;
 	const PathfinderOptions & options;
 	bool canCastFly;
 	bool canCastWaterWalk;
@@ -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

@@ -10,40 +10,161 @@
 #include "StdInc.h"
 #include "TurnInfo.h"
 
+#include "../IGameCallback.h"
+#include "../IGameSettings.h"
 #include "../TerrainHandler.h"
 #include "../VCMI_Lib.h"
 #include "../bonuses/BonusList.h"
+#include "../json/JsonNode.h"
 #include "../mapObjects/CGHeroInstance.h"
 #include "../mapObjects/MiscObjects.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-TurnInfo::BonusCache::BonusCache(const TConstBonusListPtr & bl)
+TConstBonusListPtr TurnInfoBonusList::getBonusList(const CGHeroInstance * target, const CSelector & bonusSelector)
 {
-	for(const auto & terrain : VLC->terrainTypeHandler->objects)
+	std::lock_guard guard(bonusListMutex);
-	{
-		auto selector = Selector::typeSubtype(BonusType::NO_TERRAIN_PENALTY, BonusSubtypeID(terrain->getId()));
-		if (bl->getFirst(selector))
-			noTerrainPenalty.insert(terrain->getId());
-	}
 
-	freeShipBoarding = static_cast<bool>(bl->getFirst(Selector::type()(BonusType::FREE_SHIP_BOARDING)));
+	if (target->getTreeVersion() == bonusListVersion)
-	flyingMovement = static_cast<bool>(bl->getFirst(Selector::type()(BonusType::FLYING_MOVEMENT)));
+		return bonusList;
-	flyingMovementVal = bl->valOfBonuses(Selector::type()(BonusType::FLYING_MOVEMENT));
+
-	waterWalking = static_cast<bool>(bl->getFirst(Selector::type()(BonusType::WATER_WALKING)));
+	bonusList = target->getBonuses(bonusSelector);
-	waterWalkingVal = bl->valOfBonuses(Selector::type()(BonusType::WATER_WALKING));
+	bonusListVersion = target->getTreeVersion();
-	pathfindingVal = bl->valOfBonuses(Selector::type()(BonusType::ROUGH_TERRAIN_DISCOUNT));
+
+	return bonusList;
 }
 
-TurnInfo::TurnInfo(const CGHeroInstance * Hero, const int turn):
+int TurnInfo::hasWaterWalking() const
-	hero(Hero),
-	maxMovePointsLand(-1),
-	maxMovePointsWater(-1),
-	turn(turn)
 {
-	bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, "all_days" + std::to_string(turn));
+	return waterWalkingTest;
-	bonusCache = std::make_unique<BonusCache>(bonuses);
+}
-	nativeTerrain = hero->getNativeTerrain();
+
+int TurnInfo::hasFlyingMovement() const
+{
+	return flyingMovementTest;
+}
+
+int TurnInfo::hasNoTerrainPenalty(const TerrainId &terrain) const
+{
+	return noterrainPenalty[terrain.num];
+}
+
+int TurnInfo::hasFreeShipBoarding() const
+{
+	return freeShipBoardingTest;
+}
+
+int TurnInfo::getFlyingMovementValue() const
+{
+	return flyingMovementValue;
+}
+
+int TurnInfo::getWaterWalkingValue() const
+{
+	return waterWalkingValue;
+}
+
+int TurnInfo::getRoughTerrainDiscountValue() const
+{
+	return roughTerrainDiscountValue;
+}
+
+int TurnInfo::getMovePointsLimitLand() const
+{
+	return movePointsLimitLand;
+}
+
+int TurnInfo::getMovePointsLimitWater() const
+{
+	return movePointsLimitWater;
+}
+
+TurnInfo::TurnInfo(TurnInfoCache * sharedCache, const CGHeroInstance * target, int Turn)
+	: target(target)
+	, noterrainPenalty(VLC->terrainTypeHandler->size())
+{
+	CSelector daySelector = Selector::days(Turn);
+
+	int lowestSpeed;
+	if (target->getTreeVersion() == sharedCache->heroLowestSpeedVersion)
+	{
+		lowestSpeed = sharedCache->heroLowestSpeedValue;
+	}
+	else
+	{
+		lowestSpeed = target->getLowestCreatureSpeed();
+		sharedCache->heroLowestSpeedValue = lowestSpeed;
+		sharedCache->heroLowestSpeedVersion = target->getTreeVersion();
+	}
+
+	{
+		static const CSelector selector = Selector::type()(BonusType::WATER_WALKING);
+		const auto & bonuses = sharedCache->waterWalking.getBonusList(target, selector);
+		waterWalkingTest = bonuses->getFirst(daySelector) != nullptr;
+		waterWalkingValue = bonuses->valOfBonuses(daySelector);
+	}
+
+	{
+		static const CSelector selector = Selector::type()(BonusType::FLYING_MOVEMENT);
+		const auto & bonuses = sharedCache->flyingMovement.getBonusList(target, selector);
+		flyingMovementTest = bonuses->getFirst(daySelector) != nullptr;
+		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(daySelector) != nullptr;
+	}
+
+	{
+		static const CSelector selector = Selector::type()(BonusType::ROUGH_TERRAIN_DISCOUNT);
+		const auto & bonuses = sharedCache->roughTerrainDiscount.getBonusList(target, selector);
+		roughTerrainDiscountValue = bonuses->getFirst(daySelector) != nullptr;
+	}
+
+	{
+		static const CSelector selector = Selector::typeSubtype(BonusType::MOVEMENT, BonusCustomSubtype::heroMovementSea);
+		const auto & vectorSea = target->cb->getSettings().getValue(EGameSettings::HEROES_MOVEMENT_POINTS_SEA).Vector();
+		const auto & bonuses = sharedCache->movementPointsLimitWater.getBonusList(target, selector);
+		int baseMovementPointsSea;
+		if (lowestSpeed < vectorSea.size())
+			baseMovementPointsSea = vectorSea[lowestSpeed].Integer();
+		else
+			baseMovementPointsSea = vectorSea.back().Integer();
+
+		movePointsLimitWater = bonuses->valOfBonuses(daySelector, baseMovementPointsSea);
+	}
+
+	{
+		static const CSelector selector = Selector::typeSubtype(BonusType::MOVEMENT, BonusCustomSubtype::heroMovementSea);
+		const auto & vectorLand = target->cb->getSettings().getValue(EGameSettings::HEROES_MOVEMENT_POINTS_LAND).Vector();
+		const auto & bonuses = sharedCache->movementPointsLimitLand.getBonusList(target, selector);
+		int baseMovementPointsLand;
+		if (lowestSpeed < vectorLand.size())
+			baseMovementPointsLand = vectorLand[lowestSpeed].Integer();
+		else
+			baseMovementPointsLand = vectorLand.back().Integer();
+
+		movePointsLimitLand = bonuses->valOfBonuses(daySelector, baseMovementPointsLand);
+	}
+
+	{
+		static const CSelector selector = Selector::type()(BonusType::NO_TERRAIN_PENALTY);
+		const auto & bonuses = sharedCache->noTerrainPenalty.getBonusList(target, selector);
+		for (const auto & bonus : *bonuses)
+		{
+			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);
+	}
 }
 
 bool TurnInfo::isLayerAvailable(const EPathfindingLayer & layer) const
@@ -51,19 +172,19 @@ bool TurnInfo::isLayerAvailable(const EPathfindingLayer & layer) const
 	switch(layer.toEnum())
 	{
 	case EPathfindingLayer::AIR:
-		if(hero && hero->boat && hero->boat->layer == EPathfindingLayer::AIR)
+		if(target && target->boat && target->boat->layer == EPathfindingLayer::AIR)
 			break;
 
-		if(!hasBonusOfType(BonusType::FLYING_MOVEMENT))
+		if(!hasFlyingMovement())
 			return false;
 
 		break;
 
 	case EPathfindingLayer::WATER:
-		if(hero && hero->boat && hero->boat->layer == EPathfindingLayer::WATER)
+		if(target && target->boat && target->boat->layer == EPathfindingLayer::WATER)
 			break;
 
-		if(!hasBonusOfType(BonusType::WATER_WALKING))
+		if(!hasWaterWalking())
 			return false;
 
 		break;
@@ -72,80 +193,9 @@ bool TurnInfo::isLayerAvailable(const EPathfindingLayer & layer) const
 	return true;
 }
 
-bool TurnInfo::hasBonusOfType(BonusType type) const
-{
-	return hasBonusOfType(type, BonusSubtypeID());
-}
-
-bool TurnInfo::hasBonusOfType(BonusType type, BonusSubtypeID subtype) const
-{
-	switch(type)
-	{
-	case BonusType::FREE_SHIP_BOARDING:
-		return bonusCache->freeShipBoarding;
-	case BonusType::FLYING_MOVEMENT:
-		return bonusCache->flyingMovement;
-	case BonusType::WATER_WALKING:
-		return bonusCache->waterWalking;
-	case BonusType::NO_TERRAIN_PENALTY:
-		return bonusCache->noTerrainPenalty.count(subtype.as<TerrainId>());
-	}
-
-	return static_cast<bool>(
-			bonuses->getFirst(Selector::type()(type).And(Selector::subtype()(subtype))));
-}
-
-int TurnInfo::valOfBonuses(BonusType type) const
-{
-	return valOfBonuses(type, BonusSubtypeID());
-}
-
-int TurnInfo::valOfBonuses(BonusType type, BonusSubtypeID subtype) const
-{
-	switch(type)
-	{
-	case BonusType::FLYING_MOVEMENT:
-		return bonusCache->flyingMovementVal;
-	case BonusType::WATER_WALKING:
-		return bonusCache->waterWalkingVal;
-	case BonusType::ROUGH_TERRAIN_DISCOUNT:
-		return bonusCache->pathfindingVal;
-	}
-
-	return bonuses->valOfBonuses(Selector::type()(type).And(Selector::subtype()(subtype)));
-}
-
 int TurnInfo::getMaxMovePoints(const EPathfindingLayer & layer) const
 {
-	if(maxMovePointsLand == -1)
+	return layer == EPathfindingLayer::SAIL ? getMovePointsLimitWater() : getMovePointsLimitLand();
-		maxMovePointsLand = hero->movementPointsLimitCached(true, this);
-	if(maxMovePointsWater == -1)
-		maxMovePointsWater = hero->movementPointsLimitCached(false, this);
-
-	return layer == EPathfindingLayer::SAIL ? maxMovePointsWater : maxMovePointsLand;
-}
-
-void TurnInfo::updateHeroBonuses(BonusType type) const
-{
-	switch(type)
-	{
-	case BonusType::FREE_SHIP_BOARDING:
-		bonusCache->freeShipBoarding = static_cast<bool>(bonuses->getFirst(Selector::type()(BonusType::FREE_SHIP_BOARDING)));
-		break;
-	case BonusType::FLYING_MOVEMENT:
-		bonusCache->flyingMovement = static_cast<bool>(bonuses->getFirst(Selector::type()(BonusType::FLYING_MOVEMENT)));
-		bonusCache->flyingMovementVal = bonuses->valOfBonuses(Selector::type()(BonusType::FLYING_MOVEMENT));
-		break;
-	case BonusType::WATER_WALKING:
-		bonusCache->waterWalking = static_cast<bool>(bonuses->getFirst(Selector::type()(BonusType::WATER_WALKING)));
-		bonusCache->waterWalkingVal = bonuses->valOfBonuses(Selector::type()(BonusType::WATER_WALKING));
-		break;
-	case BonusType::ROUGH_TERRAIN_DISCOUNT:
-		bonusCache->pathfindingVal = bonuses->valOfBonuses(Selector::type()(BonusType::ROUGH_TERRAIN_DISCOUNT));
-		break;
-	default:
-		bonuses = hero->getAllBonuses(Selector::days(turn), Selector::all, "all_days" + std::to_string(turn));
-	}
 }
 
 VCMI_LIB_NAMESPACE_END

lib/pathfinder/TurnInfo.h

@@ -10,43 +10,74 @@
 #pragma once
 
 #include "../bonuses/Bonus.h"
-#include "../GameConstants.h"
+#include "../bonuses/BonusSelector.h"
+#include "../bonuses/BonusCache.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
 class CGHeroInstance;
 
-struct DLL_LINKAGE TurnInfo
+class TurnInfoBonusList
 {
-	/// This is certainly not the best design ever and certainly can be improved
+	TConstBonusListPtr bonusList;
-	/// Unfortunately for pathfinder that do hundreds of thousands calls onus system add too big overhead
+	std::mutex bonusListMutex;
-	struct BonusCache {
+	std::atomic<int64_t> bonusListVersion = 0;
-		std::set<TerrainId> noTerrainPenalty;
+public:
-		bool freeShipBoarding;
+	TConstBonusListPtr getBonusList(const CGHeroInstance * target, const CSelector & bonusSelector);
-		bool flyingMovement;
+};
-		int flyingMovementVal;
-		bool waterWalking;
-		int waterWalkingVal;
-		int pathfindingVal;
 
-		BonusCache(const TConstBonusListPtr & bonusList);
+struct TurnInfoCache
-	};
+{
-	std::unique_ptr<BonusCache> bonusCache;
+	TurnInfoBonusList waterWalking;
+	TurnInfoBonusList flyingMovement;
+	TurnInfoBonusList noTerrainPenalty;
+	TurnInfoBonusList freeShipBoarding;
+	TurnInfoBonusList roughTerrainDiscount;
+	TurnInfoBonusList movementPointsLimitLand;
+	TurnInfoBonusList movementPointsLimitWater;
 
-	const CGHeroInstance * hero;
+	const CGHeroInstance * target;
-	mutable TConstBonusListPtr bonuses;
-	mutable int maxMovePointsLand;
-	mutable int maxMovePointsWater;
-	TerrainId nativeTerrain;
-	int turn;
 
-	TurnInfo(const CGHeroInstance * Hero, const int Turn = 0);
+	mutable std::atomic<int64_t> heroLowestSpeedVersion = 0;
+	mutable std::atomic<int64_t> heroLowestSpeedValue = 0;
+
+	explicit TurnInfoCache(const CGHeroInstance * target):
+		target(target)
+	{}
+};
+
+class DLL_LINKAGE TurnInfo
+{
+private:
+	const CGHeroInstance * target;
+
+	// stores cached values per each terrain
+	std::vector<bool> noterrainPenalty;
+
+	int flyingMovementValue;
+	int waterWalkingValue;
+	int roughTerrainDiscountValue;
+	int movePointsLimitLand;
+	int movePointsLimitWater;
+
+	bool waterWalkingTest;
+	bool flyingMovementTest;
+	bool freeShipBoardingTest;
+
+public:
+	int hasWaterWalking() const;
+	int hasFlyingMovement() const;
+	int hasNoTerrainPenalty(const TerrainId & terrain) const;
+	int hasFreeShipBoarding() const;
+
+	int getFlyingMovementValue() const;
+	int getWaterWalkingValue() const;
+	int getRoughTerrainDiscountValue() const;
+	int getMovePointsLimitLand() const;
+	int getMovePointsLimitWater() const;
+
+	TurnInfo(TurnInfoCache * sharedCache, const CGHeroInstance * target, int Turn);
 	bool isLayerAvailable(const EPathfindingLayer & layer) const;
-	bool hasBonusOfType(const BonusType type) const;
-	bool hasBonusOfType(const BonusType type, const BonusSubtypeID subtype) const;
-	int valOfBonuses(const BonusType type) const;
-	int valOfBonuses(const BonusType type, const BonusSubtypeID subtype) const;
-	void updateHeroBonuses(BonusType type) const;
 	int getMaxMovePoints(const EPathfindingLayer & layer) const;
 };
 

server/CGameHandler.cpp

@@ -846,8 +846,8 @@ bool CGameHandler::moveHero(ObjectInstanceID hid, int3 dst, EMovementMode moveme
 	auto pathfinderHelper = std::make_unique<CPathfinderHelper>(gs, h, PathfinderOptions(this));
 	auto ti = pathfinderHelper->getTurnInfo();
 
-	const bool canFly = pathfinderHelper->hasBonusOfType(BonusType::FLYING_MOVEMENT) || (h->boat && h->boat->layer == EPathfindingLayer::AIR);
+	const bool canFly = ti->hasFlyingMovement() || (h->boat && h->boat->layer == EPathfindingLayer::AIR);
-	const bool canWalkOnSea = pathfinderHelper->hasBonusOfType(BonusType::WATER_WALKING) || (h->boat && h->boat->layer == EPathfindingLayer::WATER);
+	const bool canWalkOnSea = ti->hasWaterWalking() || (h->boat && h->boat->layer == EPathfindingLayer::WATER);
 	const int cost = pathfinderHelper->getMovementCost(h->visitablePos(), hmpos, nullptr, nullptr, h->movementPointsRemaining());
 
 	const bool movingOntoObstacle = t.blocked() && !t.visitable();

server/battles/BattleActionProcessor.cpp

@@ -257,7 +257,7 @@ bool BattleActionProcessor::doAttackAction(const CBattleInfoCallback & battle, c
 	}
 
 	//attack
-	int totalAttacks = stack->totalAttacks.getMeleeValue();
+	int totalAttacks = stack->getTotalAttacks(false);
 
 	//TODO: move to CUnitState
 	const auto * attackingHero = battle.battleGetFightingHero(ba.side);
@@ -378,7 +378,7 @@ bool BattleActionProcessor::doShootAction(const CBattleInfoCallback & battle, co
 	}
 	//allow more than one additional attack
 
-	int totalRangedAttacks = stack->totalAttacks.getRangedValue();
+	int totalRangedAttacks = stack->getTotalAttacks(true);
 
 	//TODO: move to CUnitState
 	const auto * attackingHero = battle.battleGetFightingHero(ba.side);

server/processors/NewTurnProcessor.cpp

@@ -584,7 +584,7 @@ std::vector<SetMovePoints> NewTurnProcessor::updateHeroesMovementPoints()
 	{
 		for (CGHeroInstance *h : elem.second.getHeroes())
 		{
-			auto ti = std::make_unique<TurnInfo>(h, 1);
+			auto ti = h->getTurnInfo(1);
 			// NOTE: this code executed when bonuses of previous day not yet updated (this happen in NewTurn::applyGs). See issue 2356
 			int32_t newMovementPoints = h->movementPointsLimitCached(gameHandler->gameState()->map->getTile(h->visitablePos()).isLand(), ti.get());
 

test/mock/mock_battle_Unit.h

@@ -57,10 +57,12 @@ public:
 	MOCK_CONST_METHOD0(isFrozen, bool());
 	MOCK_CONST_METHOD1(isValidTarget, bool(bool));
 
+	MOCK_CONST_METHOD0(isHypnotized, bool());
 	MOCK_CONST_METHOD0(isClone, bool());
 	MOCK_CONST_METHOD0(hasClone, bool());
 	MOCK_CONST_METHOD0(canCast, bool());
 	MOCK_CONST_METHOD0(isCaster, bool());
+	MOCK_CONST_METHOD0(canShootBlocked, bool());
 	MOCK_CONST_METHOD0(canShoot, bool());
 	MOCK_CONST_METHOD0(isShooter, bool());