From 34c1d4f3e95d5dc13b8b65711ea014e330e17117 Mon Sep 17 00:00:00 2001 From: Konstantin Date: Sun, 30 Apr 2023 15:49:26 +0300 Subject: [PATCH] vcmi: move CBonusProxy and friends to new file To decouple HeroBonus.h more --- cmake_modules/VCMI_lib.cmake | 2 + lib/battle/CUnitState.h | 1 + lib/bonuses/CBonusProxy.cpp | 208 ++++++++++++++++++++++++++++++++ lib/bonuses/CBonusProxy.h | 89 ++++++++++++++ lib/bonuses/HeroBonus.cpp | 193 ----------------------------- lib/bonuses/HeroBonus.h | 72 ----------- lib/mapObjects/CArmedInstance.h | 1 + 7 files changed, 301 insertions(+), 265 deletions(-) create mode 100644 lib/bonuses/CBonusProxy.cpp create mode 100644 lib/bonuses/CBonusProxy.h diff --git a/cmake_modules/VCMI_lib.cmake b/cmake_modules/VCMI_lib.cmake index 13d68872d..8f8d437c2 100644 --- a/cmake_modules/VCMI_lib.cmake +++ b/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 diff --git a/lib/battle/CUnitState.h b/lib/battle/CUnitState.h index 7bc6878bf..4b0758f0e 100644 --- a/lib/battle/CUnitState.h +++ b/lib/battle/CUnitState.h @@ -11,6 +11,7 @@ #pragma once #include "Unit.h" +#include "../bonuses/CBonusProxy.h" VCMI_LIB_NAMESPACE_BEGIN diff --git a/lib/bonuses/CBonusProxy.cpp b/lib/bonuses/CBonusProxy.cpp new file mode 100644 index 000000000..68ef74275 --- /dev/null +++ b/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 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_guardlock(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 \ No newline at end of file diff --git a/lib/bonuses/CBonusProxy.h b/lib/bonuses/CBonusProxy.h new file mode 100644 index 000000000..804a33f17 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/lib/bonuses/HeroBonus.cpp b/lib/bonuses/HeroBonus.cpp index a053ff127..18983019c 100644 --- a/lib/bonuses/HeroBonus.cpp +++ b/lib/bonuses/HeroBonus.cpp @@ -100,199 +100,6 @@ const std::set 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 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_guardlock(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; diff --git a/lib/bonuses/HeroBonus.h b/lib/bonuses/HeroBonus.h index b1b9e4e29..d22f225df 100644 --- a/lib/bonuses/HeroBonus.h +++ b/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 { public: diff --git a/lib/mapObjects/CArmedInstance.h b/lib/mapObjects/CArmedInstance.h index 28d5141e8..5a99dd80e 100644 --- a/lib/mapObjects/CArmedInstance.h +++ b/lib/mapObjects/CArmedInstance.h @@ -11,6 +11,7 @@ #include "CObjectHandler.h" #include "../CCreatureSet.h" +#include "../bonuses/CBonusProxy.h" VCMI_LIB_NAMESPACE_BEGIN