1
0
mirror of https://github.com/vcmi/vcmi.git synced 2024-12-24 22:14:36 +02:00

Merge pull request #1110 from Nordsoft91/whereshouldbeattached

#1754 Fix crash with objects belonging players without state
This commit is contained in:
Andrii Danylchenko 2022-11-10 11:43:50 +02:00 committed by GitHub
commit 5222ea8c25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 92 additions and 88 deletions

View File

@ -729,7 +729,7 @@ CArtifactInstance::CArtifactInstance( CArtifact *Art)
void CArtifactInstance::setType( CArtifact *Art )
{
artType = Art;
attachTo(Art);
attachTo(*Art);
}
std::string CArtifactInstance::nodeName() const
@ -852,7 +852,7 @@ void CArtifactInstance::putAt(ArtifactLocation al)
al.getHolderArtSet()->setNewArtSlot(al.slot, this, false);
if(al.slot < GameConstants::BACKPACK_START)
al.getHolderNode()->attachTo(this);
al.getHolderNode()->attachTo(*this);
}
void CArtifactInstance::removeFrom(ArtifactLocation al)
@ -860,7 +860,7 @@ void CArtifactInstance::removeFrom(ArtifactLocation al)
assert(al.getHolderArtSet()->getArt(al.slot) == this);
al.getHolderArtSet()->eraseArtSlot(al.slot);
if(al.slot < GameConstants::BACKPACK_START)
al.getHolderNode()->detachFrom(this);
al.getHolderNode()->detachFrom(*this);
//TODO delete me?
}
@ -1054,7 +1054,7 @@ void CCombinedArtifactInstance::addAsConstituent(CArtifactInstance *art, Artifac
assert(vstd::contains(*artType->constituents, art->artType.get()));
assert(art->getParentNodes().size() == 1 && art->getParentNodes().front() == art->artType);
constituentsInfo.push_back(ConstituentInfo(art, slot));
attachTo(art);
attachTo(*art);
}
void CCombinedArtifactInstance::putAt(ArtifactLocation al)
@ -1143,7 +1143,7 @@ CArtifactInstance * CCombinedArtifactInstance::figureMainConstituent(const Artif
void CCombinedArtifactInstance::deserializationFix()
{
for(ConstituentInfo &ci : constituentsInfo)
attachTo(ci.art);
attachTo(*ci.art);
}
bool CCombinedArtifactInstance::isPart(const CArtifactInstance *supposedPart) const
@ -1350,7 +1350,7 @@ void CArtifactSet::artDeserializationFix(CBonusSystemNode *node)
{
for(auto & elem : artifactsWorn)
if(elem.second.artifact && !elem.second.locked)
node->attachTo(elem.second.artifact);
node->attachTo(*elem.second.artifact);
}
void CArtifactSet::serializeJsonArtifacts(JsonSerializeFormat & handler, const std::string & fieldName, CMap * map)

View File

@ -1345,12 +1345,12 @@ void CCreatureHandler::buildBonusTreeForTiers()
for(CCreature * c : objects)
{
if(vstd::isbetween(c->level, 0, ARRAY_COUNT(creaturesOfLevel)))
c->attachTo(&creaturesOfLevel[c->level]);
c->attachTo(creaturesOfLevel[c->level]);
else
c->attachTo(&creaturesOfLevel[0]);
c->attachTo(creaturesOfLevel[0]);
}
for(CBonusSystemNode &b : creaturesOfLevel)
b.attachTo(&allCreatures);
b.attachTo(allCreatures);
}
void CCreatureHandler::afterLoadFinalization()

View File

@ -773,7 +773,7 @@ void CStackInstance::setType(const CCreature *c)
{
if(type)
{
detachFrom(const_cast<CCreature*>(type));
detachFrom(const_cast<CCreature&>(*type));
if (type->isMyUpgrade(c) && VLC->modh->modules.STACK_EXP)
experience = static_cast<TExpType>(experience * VLC->creh->expAfterUpgrade / 100.0);
}
@ -781,7 +781,7 @@ void CStackInstance::setType(const CCreature *c)
CStackBasicDescriptor::setType(c);
if(type)
attachTo(const_cast<CCreature*>(type));
attachTo(const_cast<CCreature&>(*type));
}
std::string CStackInstance::bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const
{
@ -804,12 +804,12 @@ std::string CStackInstance::bonusToGraphics(const std::shared_ptr<Bonus>& bonus)
void CStackInstance::setArmyObj(const CArmedInstance * ArmyObj)
{
if(_armyObj)
detachFrom(const_cast<CArmedInstance*>(_armyObj));
detachFrom(const_cast<CArmedInstance&>(*_armyObj));
_armyObj = ArmyObj;
if(ArmyObj)
attachTo(const_cast<CArmedInstance*>(_armyObj));
attachTo(const_cast<CArmedInstance&>(*_armyObj));
}
std::string CStackInstance::getQuantityTXT(bool capitalized) const

View File

@ -2724,13 +2724,13 @@ void CGameState::buildGlobalTeamPlayerTree()
for(auto k=teams.begin(); k!=teams.end(); ++k)
{
TeamState *t = &k->second;
t->attachTo(&globalEffects);
t->attachTo(globalEffects);
for(PlayerColor teamMember : k->second.players)
{
PlayerState *p = getPlayerState(teamMember);
assert(p);
p->attachTo(t);
p->attachTo(*t);
}
}
}
@ -2740,7 +2740,9 @@ void CGameState::attachArmedObjects()
for(CGObjectInstance *obj : map->objects)
{
if(CArmedInstance *armed = dynamic_cast<CArmedInstance*>(obj))
armed->whatShouldBeAttached()->attachTo(armed->whereShouldBeAttached(this));
{
armed->whatShouldBeAttached().attachTo(armed->whereShouldBeAttached(this));
}
}
}

View File

@ -82,13 +82,14 @@ void CStack::localInit(BattleInfo * battleInfo)
exportBonuses();
if(base) //stack originating from "real" stack in garrison -> attach to it
{
attachTo(const_cast<CStackInstance *>(base));
attachTo(const_cast<CStackInstance&>(*base));
}
else //attach directly to obj to which stack belongs and creature type
{
CArmedInstance * army = battle->battleGetArmyObject(side);
attachTo(army);
attachTo(const_cast<CCreature *>(type));
assert(army);
attachTo(*army);
attachTo(const_cast<CCreature&>(*type));
}
nativeTerrain = type->getNativeTerrain(); //save nativeTerrain in the variable on the battle start to avoid dead lock
CUnitState::localInit(this); //it causes execution of the CStack::isOnNativeTerrain where nativeTerrain will be considered

View File

@ -1150,53 +1150,53 @@ CBonusSystemNode::~CBonusSystemNode()
if(children.size())
{
while(children.size())
children.front()->detachFrom(this);
children.front()->detachFrom(*this);
}
}
void CBonusSystemNode::attachTo(CBonusSystemNode *parent)
void CBonusSystemNode::attachTo(CBonusSystemNode & parent)
{
assert(!vstd::contains(parents, parent));
parents.push_back(parent);
assert(!vstd::contains(parents, &parent));
parents.push_back(&parent);
if(!isHypothetic())
{
if(parent->actsAsBonusSourceOnly())
parent->newRedDescendant(this);
if(parent.actsAsBonusSourceOnly())
parent.newRedDescendant(*this);
else
newRedDescendant(parent);
parent->newChildAttached(this);
parent.newChildAttached(*this);
}
CBonusSystemNode::treeHasChanged();
}
void CBonusSystemNode::detachFrom(CBonusSystemNode *parent)
void CBonusSystemNode::detachFrom(CBonusSystemNode & parent)
{
assert(vstd::contains(parents, parent));
assert(vstd::contains(parents, &parent));
if(!isHypothetic())
{
if(parent->actsAsBonusSourceOnly())
parent->removedRedDescendant(this);
if(parent.actsAsBonusSourceOnly())
parent.removedRedDescendant(*this);
else
removedRedDescendant(parent);
}
if (vstd::contains(parents, parent))
if (vstd::contains(parents, &parent))
{
parents -= parent;
parents -= &parent;
}
else
{
logBonus->error("Error on Detach. Node %s (nodeType=%d) has not parent %s (nodeType=%d)"
, nodeShortInfo(), nodeType, parent->nodeShortInfo(), parent->nodeType);
, nodeShortInfo(), nodeType, parent.nodeShortInfo(), parent.nodeType);
}
if(!isHypothetic())
{
parent->childDetached(this);
parent.childDetached(*this);
}
CBonusSystemNode::treeHasChanged();
}
@ -1304,27 +1304,27 @@ void CBonusSystemNode::unpropagateBonus(std::shared_ptr<Bonus> b)
child->unpropagateBonus(b);
}
void CBonusSystemNode::newChildAttached(CBonusSystemNode *child)
void CBonusSystemNode::newChildAttached(CBonusSystemNode & child)
{
assert(!vstd::contains(children, child));
children.push_back(child);
assert(!vstd::contains(children, &child));
children.push_back(&child);
}
void CBonusSystemNode::childDetached(CBonusSystemNode *child)
void CBonusSystemNode::childDetached(CBonusSystemNode & child)
{
if(vstd::contains(children, child))
children -= child;
if(vstd::contains(children, &child))
children -= &child;
else
{
logBonus->error("Error on Detach. Node %s (nodeType=%d) is not a child of %s (nodeType=%d)"
, child->nodeShortInfo(), child->nodeType, nodeShortInfo(), nodeType);
, child.nodeShortInfo(), child.nodeType, nodeShortInfo(), nodeType);
}
}
void CBonusSystemNode::detachFromAll()
{
while(parents.size())
detachFrom(parents.front());
detachFrom(*parents.front());
}
bool CBonusSystemNode::isIndependentNode() const
@ -1393,12 +1393,12 @@ void CBonusSystemNode::getRedChildren(TNodes &out)
}
}
void CBonusSystemNode::newRedDescendant(CBonusSystemNode * descendant)
void CBonusSystemNode::newRedDescendant(CBonusSystemNode & descendant)
{
for(auto b : exportedBonuses)
{
if(b->propagator)
descendant->propagateBonus(b, *this);
descendant.propagateBonus(b, *this);
}
TNodes redParents;
getRedAncestors(redParents); //get all red parents recursively
@ -1408,16 +1408,16 @@ void CBonusSystemNode::newRedDescendant(CBonusSystemNode * descendant)
for(auto b : parent->exportedBonuses)
{
if(b->propagator)
descendant->propagateBonus(b, *this);
descendant.propagateBonus(b, *this);
}
}
}
void CBonusSystemNode::removedRedDescendant(CBonusSystemNode *descendant)
void CBonusSystemNode::removedRedDescendant(CBonusSystemNode & descendant)
{
for(auto b : exportedBonuses)
if(b->propagator)
descendant->unpropagateBonus(b);
descendant.unpropagateBonus(b);
TNodes redParents;
getRedAncestors(redParents); //get all red parents recursively
@ -1426,7 +1426,7 @@ void CBonusSystemNode::removedRedDescendant(CBonusSystemNode *descendant)
{
for(auto b : parent->exportedBonuses)
if(b->propagator)
descendant->unpropagateBonus(b);
descendant.unpropagateBonus(b);
}
}

View File

@ -791,21 +791,21 @@ public:
static PlayerColor retrieveNodeOwner(const CBonusSystemNode * node);
std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector & selector);
void attachTo(CBonusSystemNode *parent);
void detachFrom(CBonusSystemNode *parent);
void attachTo(CBonusSystemNode & parent);
void detachFrom(CBonusSystemNode & parent);
void detachFromAll();
virtual void addNewBonus(const std::shared_ptr<Bonus>& b);
void accumulateBonus(const std::shared_ptr<Bonus>& b); //add value of bonus with same type/subtype or create new
void newChildAttached(CBonusSystemNode *child);
void childDetached(CBonusSystemNode *child);
void newChildAttached(CBonusSystemNode & child);
void childDetached(CBonusSystemNode & child);
void propagateBonus(std::shared_ptr<Bonus> b, const CBonusSystemNode & source);
void unpropagateBonus(std::shared_ptr<Bonus> b);
void removeBonus(const std::shared_ptr<Bonus>& b);
void removeBonuses(const CSelector & selector);
void removeBonusesRecursive(const CSelector & s);
void newRedDescendant(CBonusSystemNode *descendant); //propagation needed
void removedRedDescendant(CBonusSystemNode *descendant); //de-propagation needed
void newRedDescendant(CBonusSystemNode & descendant); //propagation needed
void removedRedDescendant(CBonusSystemNode & descendant); //de-propagation needed
bool isIndependentNode() const; //node is independent when it has no parents nor children
bool actsAsBonusSourceOnly() const;

View File

@ -414,7 +414,7 @@ DLL_LINKAGE void RemoveObject::applyGs(CGameState *gs)
PlayerState * p = gs->getPlayerState(beatenHero->tempOwner);
gs->map->heroesOnMap -= beatenHero;
p->heroes -= beatenHero;
beatenHero->detachFrom(beatenHero->whereShouldBeAttachedOnSiege(gs));
beatenHero->detachFrom(*beatenHero->whereShouldBeAttachedOnSiege(gs));
beatenHero->tempOwner = PlayerColor::NEUTRAL; //no one owns beaten hero
vstd::erase_if(beatenHero->artifactsInBackpack, [](const ArtSlotInfo& asi)
{
@ -683,7 +683,7 @@ DLL_LINKAGE void HeroRecruited::applyGs(CGameState *gs)
gs->map->heroesOnMap.push_back(h);
p->heroes.push_back(h);
h->attachTo(p);
h->attachTo(*p);
if(fresh)
{
h->initObj(gs->getRandomGenerator());
@ -701,8 +701,8 @@ DLL_LINKAGE void GiveHero::applyGs(CGameState *gs)
CGHeroInstance *h = gs->getHero(id);
//bonus system
h->detachFrom(&gs->globalEffects);
h->attachTo(gs->getPlayerState(player));
h->detachFrom(gs->globalEffects);
h->attachTo(*gs->getPlayerState(player));
h->appearance = VLC->objtypeh->getHandlerFor(Obj::HERO, h->type->heroClass->getIndex())->getTemplates().front();
gs->map->removeBlockVisTiles(h,true);
@ -1153,7 +1153,7 @@ DLL_LINKAGE void DisassembledArtifact::applyGs(CGameState *gs)
{
ArtifactLocation constituentLoc = al;
constituentLoc.slot = (ci.slot >= 0 ? ci.slot : al.slot); //-1 is slot of main constituent -> it'll replace combined artifact in its pos
disassembled->detachFrom(ci.art);
disassembled->detachFrom(*ci.art);
ci.art->putAt(constituentLoc);
}
@ -1281,10 +1281,10 @@ DLL_LINKAGE void SetObjectProperty::applyGs(CGameState *gs)
}
}
CBonusSystemNode *nodeToMove = cai->whatShouldBeAttached();
nodeToMove->detachFrom(cai->whereShouldBeAttached(gs));
CBonusSystemNode & nodeToMove = cai->whatShouldBeAttached();
nodeToMove.detachFrom(cai->whereShouldBeAttached(gs));
obj->setProperty(what,val);
nodeToMove->attachTo(cai->whereShouldBeAttached(gs));
nodeToMove.attachTo(cai->whereShouldBeAttached(gs));
}
else //not an armed instance
{

View File

@ -84,7 +84,7 @@ void BattleInfo::localInit()
{
auto armyObj = battleGetArmyObject(i);
armyObj->battle = this;
armyObj->attachTo(this);
armyObj->attachTo(*this);
}
for(CStack * s : stacks)

View File

@ -142,17 +142,18 @@ void CArmedInstance::armyChanged()
updateMoraleBonusFromArmy();
}
CBonusSystemNode * CArmedInstance::whereShouldBeAttached(CGameState *gs)
CBonusSystemNode & CArmedInstance::whereShouldBeAttached(CGameState * gs)
{
if(tempOwner < PlayerColor::PLAYER_LIMIT)
return gs->getPlayerState(tempOwner);
else
return &gs->globalEffects;
if(auto * where = gs->getPlayerState(tempOwner))
return *where;
return gs->globalEffects;
}
CBonusSystemNode * CArmedInstance::whatShouldBeAttached()
CBonusSystemNode & CArmedInstance::whatShouldBeAttached()
{
return this;
return *this;
}
VCMI_LIB_NAMESPACE_END

View File

@ -33,8 +33,8 @@ public:
//////////////////////////////////////////////////////////////////////////
// int valOfGlobalBonuses(CSelector selector) const; //used only for castle interface ???
virtual CBonusSystemNode *whereShouldBeAttached(CGameState *gs);
virtual CBonusSystemNode *whatShouldBeAttached();
virtual CBonusSystemNode & whereShouldBeAttached(CGameState * gs);
virtual CBonusSystemNode & whatShouldBeAttached();
//////////////////////////////////////////////////////////////////////////
CArmedInstance();

View File

@ -1062,17 +1062,17 @@ CBonusSystemNode * CGHeroInstance::whereShouldBeAttachedOnSiege(CGameState * gs)
if(visitedTown)
return whereShouldBeAttachedOnSiege(visitedTown->isBattleOutsideTown(this));
return CArmedInstance::whereShouldBeAttached(gs);
return &CArmedInstance::whereShouldBeAttached(gs);
}
CBonusSystemNode * CGHeroInstance::whereShouldBeAttached(CGameState * gs)
CBonusSystemNode & CGHeroInstance::whereShouldBeAttached(CGameState * gs)
{
if(visitedTown)
{
if(inTownGarrison)
return visitedTown;
return *visitedTown;
else
return &visitedTown->townAndVis;
return visitedTown->townAndVis;
}
else
return CArmedInstance::whereShouldBeAttached(gs);

View File

@ -240,7 +240,7 @@ public:
ArtBearer::ArtBearer bearerType() const override;
///IBonusBearer
CBonusSystemNode * whereShouldBeAttached(CGameState *gs) override;
CBonusSystemNode & whereShouldBeAttached(CGameState * gs) override;
std::string nodeName() const override;
CBonusSystemNode * whereShouldBeAttachedOnSiege(const bool isBattleOutsideTown) const;

View File

@ -1146,7 +1146,7 @@ std::string CGTownInstance::nodeName() const
void CGTownInstance::deserializationFix()
{
attachTo(&townAndVis);
attachTo(townAndVis);
//Hero is already handled by CGameState::attachArmedObjects
@ -1216,8 +1216,8 @@ void CGTownInstance::setVisitingHero(CGHeroInstance *h)
{
PlayerState *p = cb->gameState()->getPlayerState(h->tempOwner);
assert(p);
h->detachFrom(p);
h->attachTo(&townAndVis);
h->detachFrom(*p);
h->attachTo(townAndVis);
visitingHero = h;
h->visitedTown = this;
h->inTownGarrison = false;
@ -1226,8 +1226,8 @@ void CGTownInstance::setVisitingHero(CGHeroInstance *h)
{
PlayerState *p = cb->gameState()->getPlayerState(visitingHero->tempOwner);
visitingHero->visitedTown = nullptr;
visitingHero->detachFrom(&townAndVis);
visitingHero->attachTo(p);
visitingHero->detachFrom(townAndVis);
visitingHero->attachTo(*p);
visitingHero = nullptr;
}
}
@ -1239,8 +1239,8 @@ void CGTownInstance::setGarrisonedHero(CGHeroInstance *h)
{
PlayerState *p = cb->gameState()->getPlayerState(h->tempOwner);
assert(p);
h->detachFrom(p);
h->attachTo(this);
h->detachFrom(*p);
h->attachTo(*this);
garrisonHero = h;
h->visitedTown = this;
h->inTownGarrison = true;
@ -1250,8 +1250,8 @@ void CGTownInstance::setGarrisonedHero(CGHeroInstance *h)
PlayerState *p = cb->gameState()->getPlayerState(garrisonHero->tempOwner);
garrisonHero->visitedTown = nullptr;
garrisonHero->inTownGarrison = false;
garrisonHero->detachFrom(this);
garrisonHero->attachTo(p);
garrisonHero->detachFrom(*this);
garrisonHero->attachTo(*p);
garrisonHero = nullptr;
}
updateMoraleBonusFromArmy(); //avoid giving morale bonus for same army twice
@ -1290,9 +1290,9 @@ int CGTownInstance::getTownLevel() const
return level;
}
CBonusSystemNode * CGTownInstance::whatShouldBeAttached()
CBonusSystemNode & CGTownInstance::whatShouldBeAttached()
{
return &townAndVis;
return townAndVis;
}
const CArmedInstance * CGTownInstance::getUpperArmy() const

View File

@ -274,7 +274,7 @@ public:
}
//////////////////////////////////////////////////////////////////////////
CBonusSystemNode *whatShouldBeAttached() override;
CBonusSystemNode & whatShouldBeAttached() override;
std::string nodeName() const override;
void updateMoraleBonusFromArmy() override;
void deserializationFix();