Fix crash with objects belonging players without state

lib/CArtHandler.cpp

@@ -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)

lib/CCreatureHandler.cpp

@@ -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()

lib/CCreatureSet.cpp

@@ -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

lib/CGameState.cpp

@@ -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));
+		}
 	}
 }
 

lib/CStack.cpp

@@ -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);
+		assert(army);
-		attachTo(const_cast<CCreature *>(type));
+		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

lib/HeroBonus.cpp

@@ -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));
+	assert(!vstd::contains(parents, &parent));
-	parents.push_back(parent);
+	parents.push_back(&parent);
 
 	if(!isHypothetic())
 	{
-		if(parent->actsAsBonusSourceOnly())
+		if(parent.actsAsBonusSourceOnly())
-			parent->newRedDescendant(this);
+			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())
+		if(parent.actsAsBonusSourceOnly())
-			parent->removedRedDescendant(this);
+			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));
+	assert(!vstd::contains(children, &child));
-	children.push_back(child);
+	children.push_back(&child);
 }
 
-void CBonusSystemNode::childDetached(CBonusSystemNode *child)
+void CBonusSystemNode::childDetached(CBonusSystemNode & child)
 {
-	if(vstd::contains(children, child))
+	if(vstd::contains(children, &child))
-		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);
 	}
 }
 

lib/HeroBonus.h

@@ -791,21 +791,21 @@ public:
 	static PlayerColor retrieveNodeOwner(const CBonusSystemNode * node);
 	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector & selector);
 
-	void attachTo(CBonusSystemNode *parent);
+	void attachTo(CBonusSystemNode & parent);
-	void detachFrom(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 newChildAttached(CBonusSystemNode & child);
-	void childDetached(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 newRedDescendant(CBonusSystemNode & descendant); //propagation needed
-	void removedRedDescendant(CBonusSystemNode *descendant); //de-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;

lib/NetPacksLib.cpp

@@ -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->detachFrom(gs->globalEffects);
-	h->attachTo(gs->getPlayerState(player));
+	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();
+		CBonusSystemNode & nodeToMove = cai->whatShouldBeAttached();
-		nodeToMove->detachFrom(cai->whereShouldBeAttached(gs));
+		nodeToMove.detachFrom(cai->whereShouldBeAttached(gs));
 		obj->setProperty(what,val);
-		nodeToMove->attachTo(cai->whereShouldBeAttached(gs));
+		nodeToMove.attachTo(cai->whereShouldBeAttached(gs));
 	}
 	else //not an armed instance
 	{

lib/battle/BattleInfo.cpp

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

lib/mapObjects/CArmedInstance.cpp

@@ -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);
+		if(auto * where = gs->getPlayerState(tempOwner))
-	else
+			return *where;
-		return &gs->globalEffects;
+
+	return gs->globalEffects;
 }
 
-CBonusSystemNode * CArmedInstance::whatShouldBeAttached()
+CBonusSystemNode & CArmedInstance::whatShouldBeAttached()
 {
-	return this;
+	return *this;
 }
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CArmedInstance.h

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

lib/mapObjects/CGHeroInstance.cpp

@@ -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);

lib/mapObjects/CGHeroInstance.h

@@ -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;

lib/mapObjects/CGTownInstance.cpp

@@ -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->detachFrom(*p);
-		h->attachTo(&townAndVis);
+		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->detachFrom(townAndVis);
-		visitingHero->attachTo(p);
+		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->detachFrom(*p);
-		h->attachTo(this);
+		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->detachFrom(*this);
-		garrisonHero->attachTo(p);
+		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

lib/mapObjects/CGTownInstance.h

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