vcmi: move CBonusProxy and friends to new file

To decouple HeroBonus.h more

cmake_modules/VCMI_lib.cmake

@@ -27,6 +27,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/battle/SiegeInfo.cpp
 		${MAIN_LIB_DIR}/battle/Unit.cpp
 
+		${MAIN_LIB_DIR}/bonuses/CBonusProxy.cpp
 		${MAIN_LIB_DIR}/bonuses/HeroBonus.cpp
 		${MAIN_LIB_DIR}/bonuses/ILimiter.cpp
 		${MAIN_LIB_DIR}/bonuses/IUpdater.cpp
@@ -303,6 +304,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/battle/SiegeInfo.h
 		${MAIN_LIB_DIR}/battle/Unit.h
 
+		${MAIN_LIB_DIR}/bonuses/CBonusProxy.h
 		${MAIN_LIB_DIR}/bonuses/HeroBonus.h
 		${MAIN_LIB_DIR}/bonuses/ILimiter.h
 		${MAIN_LIB_DIR}/bonuses/IUpdater.h

lib/battle/CUnitState.h

@@ -11,6 +11,7 @@
 #pragma once
 
 #include "Unit.h"
+#include "../bonuses/CBonusProxy.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 

lib/bonuses/CBonusProxy.cpp

@@ -0,0 +1,208 @@
+/*
+ * 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 "CBonusProxy.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()(Bonus::NO_LIMIT).Or(Selector::effectRange()(Bonus::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()(Bonus::NO_LIMIT).Or(Selector::effectRange()(Bonus::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, CSelector Selector):
+	target(Target),
+	selector(std::move(Selector)),
+	cachedLast(0),
+	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);
+		cachedLast = treeVersion;
+	}
+
+	return hasBonus;
+}
+
+VCMI_LIB_NAMESPACE_END

lib/bonuses/CBonusProxy.h

@@ -0,0 +1,89 @@
+/*
+ * 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 "HeroBonus.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);
+	CCheckProxy(const CCheckProxy & other);
+	CCheckProxy& operator= (const CCheckProxy & other) = default;
+
+	bool getHasBonus() const;
+
+private:
+	const IBonusBearer * target;
+	CSelector selector;
+
+	mutable int64_t cachedLast;
+	mutable bool hasBonus;
+};
+
+VCMI_LIB_NAMESPACE_END

lib/bonuses/HeroBonus.cpp

@@ -100,199 +100,6 @@ const std::set<std::string> deprecatedBonusSet = {
 	"SELF_LUCK"
 };
 
-///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()(Bonus::NO_LIMIT).Or(Selector::effectRange()(Bonus::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()(Bonus::NO_LIMIT).Or(Selector::effectRange()(Bonus::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, CSelector Selector):
-	target(Target),
-	selector(std::move(Selector)),
-	cachedLast(0),
-	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);
-		cachedLast = treeVersion;
-	}
-
-	return hasBonus;
-}
-
 //This constructor should be placed here to avoid side effects
 CAddInfo::CAddInfo() = default;
 

lib/bonuses/HeroBonus.h

@@ -76,78 +76,6 @@ public:
 	}
 };
 
-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);
-	CCheckProxy(const CCheckProxy & other);
-	CCheckProxy& operator= (const CCheckProxy & other) = default;
-
-	bool getHasBonus() const;
-
-private:
-	const IBonusBearer * target;
-	CSelector selector;
-
-	mutable int64_t cachedLast;
-	mutable bool hasBonus;
-};
-
 class DLL_LINKAGE CAddInfo : public std::vector<si32>
 {
 public:

lib/mapObjects/CArmedInstance.h

@@ -11,6 +11,7 @@
 
 #include "CObjectHandler.h"
 #include "../CCreatureSet.h"
+#include "../bonuses/CBonusProxy.h"
 
 VCMI_LIB_NAMESPACE_BEGIN