diff --git a/client/Client.cpp b/client/Client.cpp index 312b3d52f..0701a5611 100644 --- a/client/Client.cpp +++ b/client/Client.cpp @@ -558,7 +558,6 @@ int CClient::sendRequest(const CPackForServer * request, PlayerColor player) void CClient::battleStarted(const BattleInfo * info) { - BattleInfo::adjustOppositeBonuses(const_cast(info)); setBattle(info); for(auto & battleCb : battleCallbacks) diff --git a/client/Client.h b/client/Client.h index 9754d7672..7f93aa3fe 100644 --- a/client/Client.h +++ b/client/Client.h @@ -202,7 +202,7 @@ public: bool moveStack(const StackLocation & src, const StackLocation & dst, TQuantity count = -1) override {return false;} void removeAfterVisit(const CGObjectInstance * object) override {}; - bool garrisonSwapOnSiege(ObjectInstanceID tid) override {return false;}; + bool swapGarrisonOnSiege(ObjectInstanceID tid) override {return false;}; void giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos) override {}; void giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) override {}; void putArtifact(const ArtifactLocation & al, const CArtifactInstance * a) override {}; diff --git a/client/widgets/MiscWidgets.cpp b/client/widgets/MiscWidgets.cpp index 588b077e0..81cda3d1b 100644 --- a/client/widgets/MiscWidgets.cpp +++ b/client/widgets/MiscWidgets.cpp @@ -406,19 +406,15 @@ void MoraleLuckBox::set(const IBonusBearer * node) } else { - bool isListActual = false; std::string addInfo = ""; - for(auto & bonus : * modifierList) { if(bonus->val) - { addInfo += "\n" + bonus->Description(); - isListActual = true; - } } - text = isListActual ? text + addInfo - : text + CGI->generaltexth->arraytxt[noneTxtId];//no modifiers + text = addInfo.empty() + ? text + CGI->generaltexth->arraytxt[noneTxtId] + : text + addInfo; } std::string imageName; if (small) diff --git a/config/creatures/necropolis.json b/config/creatures/necropolis.json index 5038d06a3..49e0c3cd4 100644 --- a/config/creatures/necropolis.json +++ b/config/creatures/necropolis.json @@ -339,7 +339,7 @@ { "type" : "DRAGON_NATURE" }, - "descreaseMorale" : + "decreaseMorale" : { "type" : "MORALE", "effectRange": "ONLY_ENEMY_ARMY", @@ -372,7 +372,7 @@ { "type" : "DRAGON_NATURE" }, - "descreaseMorale" : + "decreaseMorale" : { "type" : "MORALE", "effectRange": "ONLY_ENEMY_ARMY", diff --git a/lib/CCreatureHandler.cpp b/lib/CCreatureHandler.cpp index 7f9170635..da2824240 100644 --- a/lib/CCreatureHandler.cpp +++ b/lib/CCreatureHandler.cpp @@ -392,20 +392,6 @@ void CCreature::fillWarMachine() warMachine = ArtifactID::NONE; //this creature is not artifact } -void CCreature::updateOppositeBonuses() -{ - auto & bonusList = getExportedBonusList(); - for(auto & bonus : bonusList) - { - if(bonus->effectRange == Bonus::ONLY_ENEMY_ARMY //Opposite Side bonuses should not be exported from CREATURE node. - || (bonus->propagator && bonus->propagator->getPropagatorType() == CBonusSystemNode::BATTLE)) - { - bonus->effectRange == Bonus::ONLY_ENEMY_ARMY; - bonus->propagator.reset(); - } - } -} - CCreatureHandler::CCreatureHandler() : expAfterUpgrade(0) { @@ -877,10 +863,6 @@ void CCreatureHandler::loadCreatureJson(CCreature * creature, const JsonNode & c if (!ability.second.isNull()) { auto b = JsonUtils::parseBonus(ability.second); - - if(b->effectRange == Bonus::ONLY_ENEMY_ARMY) //Opposite Side bonuses should not be exported from CREATURE node. - b->propagator.reset(); - b->source = Bonus::CREATURE_ABILITY; b->sid = creature->getIndex(); b->duration = Bonus::PERMANENT; diff --git a/lib/CCreatureHandler.h b/lib/CCreatureHandler.h index 25c608ab9..dfdcd3e01 100644 --- a/lib/CCreatureHandler.h +++ b/lib/CCreatureHandler.h @@ -223,17 +223,12 @@ public: { fillWarMachine(); } - if(version < 801 && !h.saving) // Opposite bonuses are introduced - { - updateOppositeBonuses(); - } } CCreature(); private: void fillWarMachine(); - void updateOppositeBonuses(); }; class DLL_LINKAGE CCreatureHandler : public CHandlerBase diff --git a/lib/CCreatureSet.cpp b/lib/CCreatureSet.cpp index 7cfd8d2d4..4c9d23d36 100644 --- a/lib/CCreatureSet.cpp +++ b/lib/CCreatureSet.cpp @@ -630,38 +630,10 @@ void CStackInstance::setType(CreatureID creID) setType((const CCreature*)nullptr); } - -void CStackInstance::copyOppositeBonusesFromCreature(const CCreature * creature) -{ - auto owner = _armyObj ? _armyObj->getOwner() : PlayerColor::NEUTRAL; - - if(owner == PlayerColor::UNFLAGGABLE) - owner = PlayerColor::NEUTRAL; - - auto & creatureBonuses = const_cast(type)->getExportedBonusList(); - - for(auto & creatureBonus : creatureBonuses) - { - if(creatureBonus->effectRange == Bonus::ONLY_ENEMY_ARMY) //it has better prformance than dynamic_cast - { - auto bCopy = std::make_shared(*creatureBonus); - bCopy->propagator.reset(new CPropagatorNodeType(CBonusSystemNode::BATTLE)); - bCopy->limiter.reset(new OppositeSideLimiter(owner)); - this->addNewBonus(bCopy); - } - } -} - -void CStackInstance::removeOppositeBonuses() -{ - removeBonuses(Selector::effectRange()(Bonus::ONLY_ENEMY_ARMY)); -} - void CStackInstance::setType(const CCreature *c) { if(type) { - removeOppositeBonuses(); detachFrom(const_cast(type)); if (type->isMyUpgrade(c) && VLC->modh->modules.STACK_EXP) experience = static_cast(experience * VLC->creh->expAfterUpgrade / 100.0); @@ -670,11 +642,7 @@ void CStackInstance::setType(const CCreature *c) CStackBasicDescriptor::setType(c); if(type) - { - auto creature = const_cast(type); - copyOppositeBonusesFromCreature(creature); - attachTo(creature); - } + attachTo(const_cast(type)); } std::string CStackInstance::bonusToString(const std::shared_ptr& bonus, bool description) const { @@ -700,28 +668,9 @@ void CStackInstance::setArmyObj(const CArmedInstance * ArmyObj) detachFrom(const_cast(_armyObj)); _armyObj = ArmyObj; + if(ArmyObj) - { - auto owner = _armyObj->getOwner(); - - if(owner == PlayerColor::UNFLAGGABLE) - owner = PlayerColor::NEUTRAL; - - auto & bonusList = getExportedBonusList(); - - for(auto & bonus : bonusList) - { - if(bonus->effectRange != Bonus::ONLY_ENEMY_ARMY) - continue; - - auto limPtr = bonus->limiter.get(); - if(limPtr) - static_cast(limPtr)->owner = owner; - else - logGlobal->error("setArmyObj. Limiter has been lost. Creature is %s", type ? type->nodeShortInfo() : std::string("No creature")); - } attachTo(const_cast(_armyObj)); - } } std::string CStackInstance::getQuantityTXT(bool capitalized) const @@ -764,6 +713,11 @@ std::string CStackInstance::nodeName() const return oss.str(); } +PlayerColor CStackInstance::getOwner() const +{ + return _armyObj ? _armyObj->getOwner() : PlayerColor::NEUTRAL; +} + void CStackInstance::deserializationFix() { const CCreature *backup = type; diff --git a/lib/CCreatureSet.h b/lib/CCreatureSet.h index cc6557865..e0a1d2a58 100644 --- a/lib/CCreatureSet.h +++ b/lib/CCreatureSet.h @@ -99,10 +99,7 @@ public: ArtBearer::ArtBearer bearerType() const override; //from CArtifactSet virtual std::string nodeName() const override; //from CBonusSystemnode void deserializationFix(); - -private: - void copyOppositeBonusesFromCreature(const CCreature * creature); - inline void removeOppositeBonuses(); + PlayerColor getOwner() const override; }; class DLL_LINKAGE CCommanderInstance : public CStackInstance diff --git a/lib/CStack.h b/lib/CStack.h index 8914aaefc..2171a33b2 100644 --- a/lib/CStack.h +++ b/lib/CStack.h @@ -20,6 +20,7 @@ struct BattleStackAttacked; class BattleInfo; +//Represents STACK_BATTLE nodes class DLL_LINKAGE CStack : public CBonusSystemNode, public battle::CUnitState, public battle::IUnitEnvironment { public: @@ -79,6 +80,11 @@ public: void spendMana(ServerCallback * server, const int spellCost) const override; + PlayerColor getOwner() const override + { + return this->owner; + } + template void serialize(Handler & h, const int version) { //this assumes that stack objects is newly created diff --git a/lib/HeroBonus.cpp b/lib/HeroBonus.cpp index 4c6c99077..1393a6e79 100644 --- a/lib/HeroBonus.cpp +++ b/lib/HeroBonus.cpp @@ -857,18 +857,7 @@ const CStackInstance * retrieveStackInstance(const CBonusSystemNode * node) PlayerColor CBonusSystemNode::retrieveNodeOwner(const CBonusSystemNode * node) { - if(!node) - return PlayerColor::CANNOT_DETERMINE; - - const CStack * stack = retrieveStackBattle(node); - if(stack) - return stack->owner; - - const CStackInstance * csi = retrieveStackInstance(node); - if(csi && csi->armyObj) - return csi->armyObj->getOwner(); - - return PlayerColor::NEUTRAL; + return node ? node->getOwner() : PlayerColor::CANNOT_DETERMINE; } std::shared_ptr CBonusSystemNode::getBonusLocalFirst(const CSelector & selector) @@ -931,7 +920,12 @@ void CBonusSystemNode::getAllBonusesRec(BonusList &out) const bonuses.getAllBonuses(beforeUpdate); for(auto b : beforeUpdate) - out.push_back(update(b)); + { + auto updated = b->updater + ? getUpdatedBonus(b, b->updater) + : b; + out.push_back(updated); + } } TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root, const std::string &cachingStr) const @@ -1020,11 +1014,10 @@ TConstBonusListPtr CBonusSystemNode::getAllBonusesWithoutCaching(const CSelector return ret; } -std::shared_ptr CBonusSystemNode::update(const std::shared_ptr & b) const +std::shared_ptr CBonusSystemNode::getUpdatedBonus(const std::shared_ptr & b, const TUpdaterPtr updater) const { - if(b->updater) - return b->updater->update(b, *this); - return b; + assert(updater); + return updater->createUpdatedBonus(b, * this); } CBonusSystemNode::CBonusSystemNode() @@ -1215,23 +1208,25 @@ bool CBonusSystemNode::actsAsBonusSourceOnly() const case CREATURE: case ARTIFACT: case ARTIFACT_INSTANCE: - case TOWN: return true; default: return false; } } -void CBonusSystemNode::propagateBonus(std::shared_ptr b) +void CBonusSystemNode::propagateBonus(std::shared_ptr b, const CBonusSystemNode & source) { if(b->propagator->shouldBeAttached(this)) { - bonuses.push_back(b); - logBonus->trace("#$# %s #propagated to# %s", b->Description(), nodeName()); + auto propagated = b->propagationUpdater + ? source.getUpdatedBonus(b, b->propagationUpdater) + : b; + bonuses.push_back(propagated); + logBonus->trace("#$# %s #propagated to# %s", propagated->Description(), nodeName()); } FOREACH_RED_CHILD(child) - child->propagateBonus(b); + child->propagateBonus(b, source); } void CBonusSystemNode::unpropagateBonus(std::shared_ptr b) @@ -1340,30 +1335,17 @@ void CBonusSystemNode::newRedDescendant(CBonusSystemNode * descendant) for(auto b : exportedBonuses) { if(b->propagator) - descendant->propagateBonus(b); + descendant->propagateBonus(b, *this); } TNodes redParents; getRedAncestors(redParents); //get all red parents recursively - //it's not in the 'getRedParents' due to infinite recursion loop. - //it's actual only for battle, otherwise, when garrison hero is linking to the town, town's 'children' is already empty - if(descendant->nodeType == BATTLE && nodeType == TOWN) //acts as a pure bonus source, but has bonus bearers. - { - for(auto child : children) - { - for(auto b : child->exportedBonuses) - { - if(b->propagator && b->propagator->getPropagatorType() == BATTLE) - descendant->propagateBonus(b); - } - } - } for(auto parent : redParents) { for(auto b : parent->exportedBonuses) { if(b->propagator) - descendant->propagateBonus(b); + descendant->propagateBonus(b, *this); } } } @@ -1406,7 +1388,7 @@ void CBonusSystemNode::getRedDescendants(TNodes &out) void CBonusSystemNode::exportBonus(std::shared_ptr b) { if(b->propagator) - propagateBonus(b); + propagateBonus(b, *this); else bonuses.push_back(b); @@ -2336,24 +2318,13 @@ std::shared_ptr Bonus::addUpdater(TUpdaterPtr Updater) return this->shared_from_this(); } -void Bonus::createOppositeLimiter() +// Update ONLY_ENEMY_ARMY bonuses from old saves to make them workable. +// Also, we should foreseen possible errors in bonus configuration and fix them. +void Bonus::updateOppositeBonuses() { - if(limiter) - { - if(!dynamic_cast(limiter.get())) - { - logMod->error("Wrong Limiter will be ignored: The 'ONLY_ENEMY_ARMY' effectRange is only compatible with the 'OPPOSITE_SIDE' limiter."); - limiter.reset(new OppositeSideLimiter()); - } - } - else - { - limiter = std::make_shared(); - } -} + if(effectRange != Bonus::ONLY_ENEMY_ARMY) + return; -void Bonus::createBattlePropagator() -{ if(propagator) { if(propagator->getPropagatorType() != CBonusSystemNode::BATTLE) @@ -2366,27 +2337,26 @@ void Bonus::createBattlePropagator() { propagator = std::make_shared(CBonusSystemNode::BATTLE); } -} - -void Bonus::updateOppositeBonuses() -{ - if(effectRange == Bonus::ONLY_ENEMY_ARMY) + if(limiter) { - createBattlePropagator(); - createOppositeLimiter(); + if(!dynamic_cast(limiter.get())) + { + logMod->error("Wrong Limiter will be ignored: The 'ONLY_ENEMY_ARMY' effectRange is only compatible with the 'OPPOSITE_SIDE' limiter."); + limiter.reset(new OppositeSideLimiter()); + } } - else if(limiter && dynamic_cast(limiter.get())) + else { - createBattlePropagator(); - effectRange = Bonus::ONLY_ENEMY_ARMY; + limiter = std::make_shared(); } + propagationUpdater = std::make_shared(); } IUpdater::~IUpdater() { } -std::shared_ptr IUpdater::update(const std::shared_ptr & b, const CBonusSystemNode & context) const +std::shared_ptr IUpdater::createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const { return b; } @@ -2409,7 +2379,7 @@ GrowsWithLevelUpdater::GrowsWithLevelUpdater(int valPer20, int stepSize) : valPe { } -std::shared_ptr GrowsWithLevelUpdater::update(const std::shared_ptr & b, const CBonusSystemNode & context) const +std::shared_ptr GrowsWithLevelUpdater::createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const { if(context.getNodeType() == CBonusSystemNode::HERO) { @@ -2446,7 +2416,7 @@ TimesHeroLevelUpdater::TimesHeroLevelUpdater() { } -std::shared_ptr TimesHeroLevelUpdater::update(const std::shared_ptr & b, const CBonusSystemNode & context) const +std::shared_ptr TimesHeroLevelUpdater::createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const { if(context.getNodeType() == CBonusSystemNode::HERO) { @@ -2472,7 +2442,7 @@ TimesStackLevelUpdater::TimesStackLevelUpdater() { } -std::shared_ptr TimesStackLevelUpdater::update(const std::shared_ptr & b, const CBonusSystemNode & context) const +std::shared_ptr TimesStackLevelUpdater::createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const { if(context.getNodeType() == CBonusSystemNode::STACK_INSTANCE) { @@ -2506,3 +2476,30 @@ JsonNode TimesStackLevelUpdater::toJsonNode() const { return JsonUtils::stringNode("TIMES_STACK_LEVEL"); } + +OwnerUpdater::OwnerUpdater() +{ +} + +std::string OwnerUpdater::toString() const +{ + return "OwnerUpdater"; +} + +JsonNode OwnerUpdater::toJsonNode() const +{ + return JsonUtils::stringNode("BONUS_OWNER_UPDATER"); +} + +std::shared_ptr OwnerUpdater::createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const +{ + auto owner = CBonusSystemNode::retrieveNodeOwner(&context); + + if(owner == PlayerColor::UNFLAGGABLE) + owner = PlayerColor::NEUTRAL; + + std::shared_ptr updated = std::make_shared( + (Bonus::BonusDuration)b->duration, b->type, b->source, b->val, b->sid, b->subtype, b->valType); + updated->limiter = std::make_shared(owner); + return updated; +} diff --git a/lib/HeroBonus.h b/lib/HeroBonus.h index 55c5629e8..9c373139c 100644 --- a/lib/HeroBonus.h +++ b/lib/HeroBonus.h @@ -417,6 +417,7 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this TLimiterPtr limiter; TPropagatorPtr propagator; TUpdaterPtr updater; + TUpdaterPtr propagationUpdater; std::string description; @@ -455,6 +456,10 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this { h & updater; } + if(version >= 801) + { + h & propagationUpdater; + } if(version < 801 && !h.saving) //Opposite Side bonuses are introduced { updateOppositeBonuses(); @@ -526,9 +531,6 @@ struct DLL_LINKAGE Bonus : public std::enable_shared_from_this std::shared_ptr addLimiter(TLimiterPtr Limiter); //returns this for convenient chain-calls std::shared_ptr addPropagator(TPropagatorPtr Propagator); //returns this for convenient chain-calls std::shared_ptr addUpdater(TUpdaterPtr Updater); //returns this for convenient chain-calls - - inline void createOppositeLimiter(); - inline void createBattlePropagator(); void updateOppositeBonuses(); }; @@ -774,7 +776,7 @@ private: void getAllBonusesRec(BonusList &out) const; TConstBonusListPtr getAllBonusesWithoutCaching(const CSelector &selector, const CSelector &limit, const CBonusSystemNode *root = nullptr) const; - std::shared_ptr update(const std::shared_ptr & b) const; + std::shared_ptr getUpdatedBonus(const std::shared_ptr & b, const TUpdaterPtr updater) const; public: explicit CBonusSystemNode(); @@ -808,7 +810,7 @@ public: void newChildAttached(CBonusSystemNode *child); void childDetached(CBonusSystemNode *child); - void propagateBonus(std::shared_ptr b); + void propagateBonus(std::shared_ptr b, const CBonusSystemNode & source); void unpropagateBonus(std::shared_ptr b); void removeBonus(const std::shared_ptr& b); void removeBonuses(const CSelector & selector); @@ -843,6 +845,11 @@ public: int64_t getTreeVersion() const override; + virtual PlayerColor getOwner() const + { + return PlayerColor::NEUTRAL; + } + template void serialize(Handler &h, const int version) { // h & bonuses; @@ -1225,7 +1232,7 @@ class DLL_LINKAGE IUpdater public: virtual ~IUpdater(); - virtual std::shared_ptr update(const std::shared_ptr & b, const CBonusSystemNode & context) const; + virtual std::shared_ptr createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const; virtual std::string toString() const; virtual JsonNode toJsonNode() const; @@ -1250,7 +1257,7 @@ public: h & stepSize; } - std::shared_ptr update(const std::shared_ptr & b, const CBonusSystemNode & context) const override; + std::shared_ptr createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const override; virtual std::string toString() const override; virtual JsonNode toJsonNode() const override; }; @@ -1265,7 +1272,7 @@ public: h & static_cast(*this); } - std::shared_ptr update(const std::shared_ptr & b, const CBonusSystemNode & context) const override; + std::shared_ptr createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const override; virtual std::string toString() const override; virtual JsonNode toJsonNode() const override; }; @@ -1280,7 +1287,22 @@ public: h & static_cast(*this); } - std::shared_ptr update(const std::shared_ptr & b, const CBonusSystemNode & context) const override; + std::shared_ptr createUpdatedBonus(const std::shared_ptr & b, const CBonusSystemNode & context) const override; + virtual std::string toString() const override; + virtual JsonNode toJsonNode() const override; +}; + +class DLL_LINKAGE OwnerUpdater : public IUpdater +{ +public: + OwnerUpdater(); + + template void serialize(Handler& h, const int version) + { + h & static_cast(*this); + } + + std::shared_ptr createUpdatedBonus(const std::shared_ptr& b, const CBonusSystemNode& context) const override; virtual std::string toString() const override; virtual JsonNode toJsonNode() const override; }; diff --git a/lib/IGameCallback.h b/lib/IGameCallback.h index 2cd5bd393..532532dac 100644 --- a/lib/IGameCallback.h +++ b/lib/IGameCallback.h @@ -98,7 +98,7 @@ public: virtual void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, int3 tile, bool creatureBank = false)=0; //if any of armies is hero, hero will be used virtual void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, bool creatureBank = false)=0; //if any of armies is hero, hero will be used, visitable tile of second obj is place of battle virtual bool moveHero(ObjectInstanceID hid, int3 dst, ui8 teleporting, bool transit = false, PlayerColor asker = PlayerColor::NEUTRAL)=0; - virtual bool garrisonSwapOnSiege(ObjectInstanceID tid)=0; + virtual bool swapGarrisonOnSiege(ObjectInstanceID tid)=0; virtual void giveHeroBonus(GiveBonus * bonus)=0; virtual void setMovePoints(SetMovePoints * smp)=0; virtual void setManaPoints(ObjectInstanceID hid, int val)=0; diff --git a/lib/battle/BattleInfo.cpp b/lib/battle/BattleInfo.cpp index 1bb4de8e6..12e347cf7 100644 --- a/lib/battle/BattleInfo.cpp +++ b/lib/battle/BattleInfo.cpp @@ -186,48 +186,6 @@ struct RangeGenerator std::function myRand; }; -void BattleInfo::adjustOppositeBonuses(CBonusSystemNode * node, PlayerColor ownerColor) -{ - auto & bonusList = node->getExportedBonusList(); - if(bonusList.empty()) - return; - - for(const auto & bonus : bonusList) - { - if(bonus->effectRange != Bonus::ONLY_ENEMY_ARMY) - continue; - - auto limPtr = bonus->limiter.get(); - if(limPtr) - static_cast(limPtr)->owner = ownerColor; - else - logGlobal->error("adjustOppositeBonuses. Limiter has been lost. Node is %s", node->nodeShortInfo()); - } -} - -void BattleInfo::adjustOppositeBonuses(BattleInfo * curB) -{ - for(int i = 0; i < 2; i++) - { - TNodes nodes; - auto * army = curB->battleGetArmyObject(i); - auto ownerColor = curB->sides[i].color; - - if(army->getNodeType() == CBonusSystemNode::HERO) - adjustOppositeBonuses(army, ownerColor); - - army->getRedAncestors(nodes); - - for(auto node : nodes) - { - auto currentNodeType = node->getNodeType(); - - if(currentNodeType == CBonusSystemNode::ARTIFACT || currentNodeType == CBonusSystemNode::TOWN) - adjustOppositeBonuses(node, ownerColor); - } - } -} - BattleInfo * BattleInfo::setupBattle(int3 tile, ETerrainType terrain, BFieldType battlefieldType, const CArmedInstance * armies[2], const CGHeroInstance * heroes[2], bool creatureBank, const CGTownInstance * town) { CMP_stack cmpst; @@ -597,7 +555,6 @@ BattleInfo * BattleInfo::setupBattle(int3 tile, ETerrainType terrain, BFieldType else curB->tacticDistance = 0; - adjustOppositeBonuses(curB); return curB; } diff --git a/lib/battle/BattleInfo.h b/lib/battle/BattleInfo.h index 262dda2dd..e1e85a7da 100644 --- a/lib/battle/BattleInfo.h +++ b/lib/battle/BattleInfo.h @@ -137,7 +137,6 @@ public: const CGHeroInstance * getHero(PlayerColor player) const; //returns fighting hero that belongs to given player void localInit(); - static void adjustOppositeBonuses(BattleInfo * curB); static BattleInfo * setupBattle(int3 tile, ETerrainType terrain, BFieldType battlefieldType, const CArmedInstance * armies[2], const CGHeroInstance * heroes[2], bool creatureBank, const CGTownInstance * town); ui8 whatSide(PlayerColor player) const; @@ -146,9 +145,6 @@ public: protected: scripting::Pool * getContextPool() const override; - -private: - inline static void adjustOppositeBonuses(CBonusSystemNode * node, PlayerColor ownerColor); }; diff --git a/lib/mapObjects/CArmedInstance.h b/lib/mapObjects/CArmedInstance.h index 5d8ac28be..a17a3998e 100644 --- a/lib/mapObjects/CArmedInstance.h +++ b/lib/mapObjects/CArmedInstance.h @@ -38,6 +38,11 @@ public: CArmedInstance(); CArmedInstance(bool isHypotetic); + PlayerColor getOwner() const override + { + return this->tempOwner; + } + template void serialize(Handler &h, const int version) { h & static_cast(*this); diff --git a/lib/mapObjects/CGTownInstance.cpp b/lib/mapObjects/CGTownInstance.cpp index 58e4566d1..6f6e0b057 100644 --- a/lib/mapObjects/CGTownInstance.cpp +++ b/lib/mapObjects/CGTownInstance.cpp @@ -665,8 +665,8 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const if(armedGarrison() || visitingHero) { const CGHeroInstance * defendingHero = visitingHero ? visitingHero : garrisonHero; - const CArmedInstance * defendingArmy = defendingHero ? (CArmedInstance *)defendingHero : (CArmedInstance *)this; - const bool isBattleOutside = isBattleOutsideTown(defendingHero); //defendingHero && garrisonHero && defendingHero != garrisonHero; + const CArmedInstance * defendingArmy = defendingHero ? (CArmedInstance *)defendingHero : this; + const bool isBattleOutside = isBattleOutsideTown(defendingHero); if(!isBattleOutside && visitingHero && defendingHero == visitingHero) { @@ -674,7 +674,7 @@ void CGTownInstance::onHeroVisit(const CGHeroInstance * h) const auto nodeSiege = defendingHero->whereShouldBeAttachedOnSiege(isBattleOutside); if(nodeSiege == (CBonusSystemNode *)this) - cb->garrisonSwapOnSiege(this->id); + cb->swapGarrisonOnSiege(this->id); const_cast(defendingHero)->inTownGarrison = false; //hack to return visitor from garrison after battle } @@ -742,15 +742,6 @@ bool CGTownInstance::townEnvisagesBuilding(BuildingSubID::EBuildingSubID subId) return town->getBuildingType(subId) != BuildingID::NONE; } -//it does not check hasBuilt because this check is in the OnHeroVisit handler -void CGTownInstance::tryAddOnePerWeekBonus(BuildingSubID::EBuildingSubID subID) -{ - auto bid = town->getBuildingType(subID); - - if(bid != BuildingID::NONE) - bonusingBuildings.push_back(new COPWBonus(bid, subID, this)); -} - void CGTownInstance::initOverriddenBids() { for(const auto & bid : builtBuildings) @@ -762,11 +753,30 @@ void CGTownInstance::initOverriddenBids() } } +bool CGTownInstance::isBonusingBuildingAdded(BuildingID::EBuildingID bid) const +{ + auto present = std::find_if(bonusingBuildings.begin(), bonusingBuildings.end(), [&](CGTownBuilding* building) + { + return building->getBuildingType().num == bid; + }); + + return present != bonusingBuildings.end(); +} + +//it does not check hasBuilt because this check is in the OnHeroVisit handler +void CGTownInstance::tryAddOnePerWeekBonus(BuildingSubID::EBuildingSubID subID) +{ + auto bid = town->getBuildingType(subID); + + if(bid != BuildingID::NONE && !isBonusingBuildingAdded(bid)) + bonusingBuildings.push_back(new COPWBonus(bid, subID, this)); +} + void CGTownInstance::tryAddVisitingBonus(BuildingSubID::EBuildingSubID subID) { auto bid = town->getBuildingType(subID); - if(bid != BuildingID::NONE) + if(bid != BuildingID::NONE && !isBonusingBuildingAdded(bid)) bonusingBuildings.push_back(new CTownBonus(bid, subID, this)); } @@ -785,6 +795,28 @@ void CGTownInstance::addTownBonuses() } } +void CGTownInstance::fixBonusingDuplicates() //For versions 794-800 +{ + std::map bids; + + for(auto i = 0; i != bonusingBuildings.size(); i++) + { + auto bid = bonusingBuildings[i]->getBuildingType(); + if(!bids.count(bid)) + bids.insert({ bid, 0 }); + else + bids[bid]++; + } + for(auto & pair : bids) + { + if(!pair.second) + continue; + + for(auto i = 0; i < pair.second; i++) + deleteTownBonus(pair.first); + } +} + void CGTownInstance::deleteTownBonus(BuildingID::EBuildingID bid) { size_t i = 0; diff --git a/lib/mapObjects/CGTownInstance.h b/lib/mapObjects/CGTownInstance.h index 4338c5ed6..826f447b1 100644 --- a/lib/mapObjects/CGTownInstance.h +++ b/lib/mapObjects/CGTownInstance.h @@ -275,6 +275,9 @@ public: else if(!h.saving) updateTown794(); + if(!h.saving && (version >= 794 && version < 801)) + fixBonusingDuplicates(); + if(!h.saving) this->setNodeType(CBonusSystemNode::TOWN); } @@ -366,9 +369,11 @@ private: void updateBonusingBuildings(); bool hasBuiltInOldWay(ETownType::ETownType type, BuildingID bid) const; bool townEnvisagesBuilding(BuildingSubID::EBuildingSubID bid) const; + bool isBonusingBuildingAdded(BuildingID::EBuildingID bid) const; void tryAddOnePerWeekBonus(BuildingSubID::EBuildingSubID subID); void tryAddVisitingBonus(BuildingSubID::EBuildingSubID subID); void initOverriddenBids(); void addTownBonuses(); void updateTown794(); //populate overriddenBuildings and vanila bonuses for old saves + void fixBonusingDuplicates(); //For versions 794-800. }; diff --git a/lib/mapObjects/CObjectHandler.cpp b/lib/mapObjects/CObjectHandler.cpp index 8b3dee46a..411296ea9 100644 --- a/lib/mapObjects/CObjectHandler.cpp +++ b/lib/mapObjects/CObjectHandler.cpp @@ -119,14 +119,6 @@ CObjectHandler::CObjectHandler() logGlobal->trace("\t\tDone loading resource prices!"); } -PlayerColor CGObjectInstance::getOwner() const -{ - //if (state) - // return state->owner; - //else - return tempOwner; //won't have owner -} - CGObjectInstance::CGObjectInstance(): pos(-1,-1,-1), ID(Obj::NO_OBJ), diff --git a/lib/mapObjects/CObjectHandler.h b/lib/mapObjects/CObjectHandler.h index 3e3777d91..cbc7f511f 100644 --- a/lib/mapObjects/CObjectHandler.h +++ b/lib/mapObjects/CObjectHandler.h @@ -118,12 +118,12 @@ public: Obj ID; /// Subtype of object, depends on type si32 subID; + /// Current owner of an object (when below PLAYER_LIMIT) + PlayerColor tempOwner; /// Index of object in map's list of objects ObjectInstanceID id; /// Defines appearance of object on map (animation, blocked tiles, blit order, etc) ObjectTemplate appearance; - /// Current owner of an object (when below PLAYER_LIMIT) - PlayerColor tempOwner; /// If true hero can visit this object only from neighbouring tiles and can't stand on this object bool blockVisit; @@ -140,7 +140,10 @@ public: /// "center" tile from which the sight distance is calculated int3 getSightCenter() const; - PlayerColor getOwner() const override; + PlayerColor getOwner() const override + { + return this->tempOwner; + } void setOwner(PlayerColor ow); /** APPEARANCE ACCESSORS **/ @@ -213,8 +216,8 @@ public: ///Entry point of Json (de-)serialization void serializeJson(JsonSerializeFormat & handler); - virtual void updateFrom(const JsonNode & data); + protected: /// virtual method that allows synchronously update object state on server and all clients virtual void setPropertyDer(ui8 what, ui32 val); diff --git a/lib/registerTypes/RegisterTypes.h b/lib/registerTypes/RegisterTypes.h index f51200ec5..9178c38cc 100644 --- a/lib/registerTypes/RegisterTypes.h +++ b/lib/registerTypes/RegisterTypes.h @@ -144,6 +144,7 @@ void registerTypesMapObjectTypes(Serializer &s) s.template registerType(); s.template registerType(); s.template registerType(); + s.template registerType(); //new types (other than netpacks) must register here //order of type registration is critical for loading old savegames } diff --git a/server/CGameHandler.cpp b/server/CGameHandler.cpp index 4df8e5bd0..6aa11f7cc 100644 --- a/server/CGameHandler.cpp +++ b/server/CGameHandler.cpp @@ -901,7 +901,7 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance * heroAttacker, con && !heroDefender->inTownGarrison && heroDefender->visitedTown->garrisonHero == heroDefender) { - garrisonSwapOnSiege(heroDefender->visitedTown->id); //return defending visitor from garrison to its rightful place + swapGarrisonOnSiege(heroDefender->visitedTown->id); //return defending visitor from garrison to its rightful place } //give exp if(battleResult.data->exp[0] && heroAttacker && battleResult.get()->winner == BattleSide::ATTACKER) @@ -3492,7 +3492,7 @@ void CGameHandler::moveArmy(const CArmedInstance *src, const CArmedInstance *dst } } -bool CGameHandler::garrisonSwapOnSiege(ObjectInstanceID tid) +bool CGameHandler::swapGarrisonOnSiege(ObjectInstanceID tid) { const CGTownInstance * town = getTown(tid); diff --git a/server/CGameHandler.h b/server/CGameHandler.h index 57cbbb355..547f425f7 100644 --- a/server/CGameHandler.h +++ b/server/CGameHandler.h @@ -242,7 +242,7 @@ public: //void lootArtifacts (TArtHolder source, TArtHolder dest, std::vector &arts); //after battle - move al arts to winer bool buySecSkill( const IMarket *m, const CGHeroInstance *h, SecondarySkill skill); bool garrisonSwap(ObjectInstanceID tid); - bool garrisonSwapOnSiege(ObjectInstanceID tid); + bool swapGarrisonOnSiege(ObjectInstanceID tid); bool upgradeCreature( ObjectInstanceID objid, SlotID pos, CreatureID upgID ); bool recruitCreatures(ObjectInstanceID objid, ObjectInstanceID dst, CreatureID crid, ui32 cram, si32 level); bool buildStructure(ObjectInstanceID tid, BuildingID bid, bool force=false);//force - for events: no cost, no checkings diff --git a/test/mock/mock_IGameCallback.h b/test/mock/mock_IGameCallback.h index af5cf8c78..76a83fbb3 100644 --- a/test/mock/mock_IGameCallback.h +++ b/test/mock/mock_IGameCallback.h @@ -78,7 +78,7 @@ public: void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, int3 tile, bool creatureBank = false) override {}; //if any of armies is hero, hero will be used void startBattleI(const CArmedInstance *army1, const CArmedInstance *army2, bool creatureBank = false) override {}; //if any of armies is hero, hero will be used, visitable tile of second obj is place of battle bool moveHero(ObjectInstanceID hid, int3 dst, ui8 teleporting, bool transit = false, PlayerColor asker = PlayerColor::NEUTRAL) override {return false;}; - bool garrisonSwapOnSiege(ObjectInstanceID tid) override {}; + bool swapGarrisonOnSiege(ObjectInstanceID tid) override {}; void giveHeroBonus(GiveBonus * bonus) override {}; void setMovePoints(SetMovePoints * smp) override {}; void setManaPoints(ObjectInstanceID hid, int val) override {};