Attempt to make constant bonus system nodes (CCreature / CArtifact)

fully constant

lib/CArtifactInstance.cpp

@@ -121,7 +121,7 @@ CArtifactInstance::CArtifactInstance()
 void CArtifactInstance::setType(const CArtifact * art)
 {
 	artType = art;
-	attachTo(const_cast<CArtifact&>(*art));
+	attachToSource(*art);
 }
 
 std::string CArtifactInstance::nodeName() const

lib/CCreatureSet.cpp

@@ -758,7 +758,7 @@ void CStackInstance::setType(const CCreature *c)
 {
 	if(type)
 	{
-		detachFrom(const_cast<CCreature&>(*type));
+		detachFromSource(*type);
 		if (type->isMyUpgrade(c) && VLC->settings()->getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE))
 			experience = static_cast<TExpType>(experience * VLC->creh->expAfterUpgrade / 100.0);
 	}
@@ -766,7 +766,7 @@ void CStackInstance::setType(const CCreature *c)
 	CStackBasicDescriptor::setType(c);
 
 	if(type)
-		attachTo(const_cast<CCreature&>(*type));
+		attachToSource(*type);
 }
 std::string CStackInstance::bonusToString(const std::shared_ptr<Bonus>& bonus, bool description) const
 {

lib/CStack.cpp

@@ -72,7 +72,7 @@ void CStack::localInit(BattleInfo * battleInfo)
 		CArmedInstance * army = battle->battleGetArmyObject(side);
 		assert(army);
 		attachTo(*army);
-		attachTo(const_cast<CCreature&>(*type));
+		attachToSource(*type);
 	}
 	nativeTerrain = 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/bonuses/CBonusSystemNode.cpp

@@ -21,14 +21,22 @@ std::atomic<int64_t> CBonusSystemNode::treeChanged(1);
 constexpr bool CBonusSystemNode::cachingEnabled = true;
 
 std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & selector)
+{
+	auto ret = bonuses.getFirst(selector);
+	if(ret)
+		return ret;
+	return nullptr;
+}
+
+std::shared_ptr<const Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & selector) const
 {
 	auto ret = bonuses.getFirst(selector);
 	if(ret)
 		return ret;
 
-	TNodes lparents;
+	TCNodes lparents;
 	getParents(lparents);
-	for(CBonusSystemNode *pname : lparents)
+	for(const CBonusSystemNode *pname : lparents)
 	{
 		ret = pname->getBonusLocalFirst(selector);
 		if (ret)
@@ -38,28 +46,15 @@ std::shared_ptr<Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & se
 	return nullptr;
 }
 
-std::shared_ptr<const Bonus> CBonusSystemNode::getBonusLocalFirst(const CSelector & selector) const
-{
-	return (const_cast<CBonusSystemNode*>(this))->getBonusLocalFirst(selector);
-}
-
 void CBonusSystemNode::getParents(TCNodes & out) const /*retrieves list of parent nodes (nodes to inherit bonuses from) */
 {
-	for(const auto * elem : parents)
+	for(const auto * elem : parentsToInherit)
 		out.insert(elem);
 }
 
-void CBonusSystemNode::getParents(TNodes &out)
-{
-	for (auto * elem : parents)
-	{
-		out.insert(elem);
-	}
-}
-
 void CBonusSystemNode::getAllParents(TCNodes & out) const //retrieves list of parent nodes (nodes to inherit bonuses from)
 {
-	for(auto * parent : parents)
+	for(auto * parent : parentsToInherit)
 	{
 		out.insert(parent);
 		parent->getAllParents(out);
@@ -239,14 +234,14 @@ CBonusSystemNode::~CBonusSystemNode()
 
 void CBonusSystemNode::attachTo(CBonusSystemNode & parent)
 {
-	assert(!vstd::contains(parents, &parent));
-	parents.push_back(&parent);
+	assert(!vstd::contains(parentsToPropagate, &parent));
+	parentsToPropagate.push_back(&parent);
+
+	attachToSource(parent);
 
 	if(!isHypothetic())
 	{
-		if(parent.actsAsBonusSourceOnly())
-			parent.newRedDescendant(*this);
-		else
+		if(!parent.actsAsBonusSourceOnly())
 			newRedDescendant(parent);
 
 		parent.newChildAttached(*this);
@@ -255,21 +250,35 @@ void CBonusSystemNode::attachTo(CBonusSystemNode & parent)
 	CBonusSystemNode::treeHasChanged();
 }
 
-void CBonusSystemNode::detachFrom(CBonusSystemNode & parent)
+void CBonusSystemNode::attachToSource(const CBonusSystemNode & parent)
 {
-	assert(vstd::contains(parents, &parent));
+	assert(!vstd::contains(parentsToInherit, &parent));
+	parentsToInherit.push_back(&parent);
 
 	if(!isHypothetic())
 	{
 		if(parent.actsAsBonusSourceOnly())
-			parent.removedRedDescendant(*this);
-		else
+			parent.newRedDescendant(*this);
+	}
+
+	CBonusSystemNode::treeHasChanged();
+}
+
+void CBonusSystemNode::detachFrom(CBonusSystemNode & parent)
+{
+	assert(vstd::contains(parentsToPropagate, &parent));
+
+	if(!isHypothetic())
+	{
+		if(!parent.actsAsBonusSourceOnly())
 			removedRedDescendant(parent);
 	}
 
-	if (vstd::contains(parents, &parent))
+	detachFromSource(parent);
+
+	if (vstd::contains(parentsToPropagate, &parent))
 	{
-		parents -= &parent;
+		parentsToPropagate -= &parent;
 	}
 	else
 	{
@@ -284,6 +293,30 @@ void CBonusSystemNode::detachFrom(CBonusSystemNode & parent)
 	CBonusSystemNode::treeHasChanged();
 }
 
+
+void CBonusSystemNode::detachFromSource(const CBonusSystemNode & parent)
+{
+	assert(vstd::contains(parentsToInherit, &parent));
+
+	if(!isHypothetic())
+	{
+		if(parent.actsAsBonusSourceOnly())
+			parent.removedRedDescendant(*this);
+	}
+
+	if (vstd::contains(parentsToPropagate, &parent))
+	{
+		parentsToInherit -= &parent;
+	}
+	else
+	{
+		logBonus->error("Error on Detach. Node %s (nodeType=%d) has not parent %s (nodeType=%d)"
+			, nodeShortInfo(), nodeType, parent.nodeShortInfo(), parent.nodeType);
+	}
+
+	CBonusSystemNode::treeHasChanged();
+}
+
 void CBonusSystemNode::removeBonusesRecursive(const CSelector & s)
 {
 	removeBonuses(s);
@@ -410,13 +443,16 @@ void CBonusSystemNode::childDetached(CBonusSystemNode & child)
 
 void CBonusSystemNode::detachFromAll()
 {
-	while(!parents.empty())
-		detachFrom(*parents.front());
+	while(!parentsToPropagate.empty())
+		detachFrom(*parentsToPropagate.front());
+
+	while(!parentsToInherit.empty())
+		detachFromSource(*parentsToInherit.front());
 }
 
 bool CBonusSystemNode::isIndependentNode() const
 {
-	return parents.empty() && children.empty();
+	return parentsToInherit.empty() && parentsToPropagate.empty() && children.empty();
 }
 
 std::string CBonusSystemNode::nodeName() const
@@ -434,7 +470,6 @@ std::string CBonusSystemNode::nodeShortInfo() const
 void CBonusSystemNode::deserializationFix()
 {
 	exportBonuses();
-
 }
 
 void CBonusSystemNode::getRedParents(TCNodes & out) const
@@ -460,9 +495,7 @@ void CBonusSystemNode::getRedParents(TCNodes & out) const
 
 void CBonusSystemNode::getRedChildren(TNodes &out)
 {
-	TNodes lparents;
-	getParents(lparents);
-	for(CBonusSystemNode *pname : lparents)
+	for(CBonusSystemNode *pname : parentsToPropagate)
 	{
 		if(!pname->actsAsBonusSourceOnly())
 		{
@@ -479,7 +512,7 @@ void CBonusSystemNode::getRedChildren(TNodes &out)
 	}
 }
 
-void CBonusSystemNode::newRedDescendant(CBonusSystemNode & descendant)
+void CBonusSystemNode::newRedDescendant(CBonusSystemNode & descendant) const
 {
 	for(const auto & b : exportedBonuses)
 	{
@@ -489,7 +522,7 @@ void CBonusSystemNode::newRedDescendant(CBonusSystemNode & descendant)
 	TCNodes redParents;
 	getRedAncestors(redParents); //get all red parents recursively
 
-	for(auto * parent : redParents)
+	for(const auto * parent : redParents)
 	{
 		for(const auto & b : parent->exportedBonuses)
 		{
@@ -499,7 +532,7 @@ void CBonusSystemNode::newRedDescendant(CBonusSystemNode & descendant)
 	}
 }
 
-void CBonusSystemNode::removedRedDescendant(CBonusSystemNode & descendant)
+void CBonusSystemNode::removedRedDescendant(CBonusSystemNode & descendant) const
 {
 	for(const auto & b : exportedBonuses)
 		if(b->propagator)
@@ -550,7 +583,7 @@ CBonusSystemNode::ENodeTypes CBonusSystemNode::getNodeType() const
 
 const TNodesVector& CBonusSystemNode::getParentNodes() const
 {
-	return parents;
+	return parentsToPropagate;
 }
 
 void CBonusSystemNode::setNodeType(CBonusSystemNode::ENodeTypes type)

lib/bonuses/CBonusSystemNode.h

@@ -19,6 +19,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 using TNodes = std::set<CBonusSystemNode *>;
 using TCNodes = std::set<const CBonusSystemNode *>;
 using TNodesVector = std::vector<CBonusSystemNode *>;
+using TCNodesVector = std::vector<const CBonusSystemNode *>;
 
 class DLL_LINKAGE CBonusSystemNode : public virtual IBonusBearer, public boost::noncopyable
 {
@@ -33,7 +34,8 @@ private:
 	BonusList bonuses; //wielded bonuses (local or up-propagated here)
 	BonusList exportedBonuses; //bonuses coming from this node (wielded or propagated away)
 
-	TNodesVector parents; // we inherit bonuses from them, we may attach our bonuses to them
+	TCNodesVector parentsToInherit; // we inherit bonuses from them
+	TNodesVector parentsToPropagate; // we may attach our bonuses to them
 	TNodesVector children;
 
 	ENodeTypes nodeType;
@@ -66,8 +68,8 @@ private:
 	void unpropagateBonus(const std::shared_ptr<Bonus> & b);
 	bool actsAsBonusSourceOnly() const;
 
-	void newRedDescendant(CBonusSystemNode & descendant); //propagation needed
-	void removedRedDescendant(CBonusSystemNode & descendant); //de-propagation needed
+	void newRedDescendant(CBonusSystemNode & descendant) const; //propagation needed
+	void removedRedDescendant(CBonusSystemNode & descendant) const; //de-propagation needed
 
 	std::string nodeShortInfo() const;
 
@@ -89,11 +91,12 @@ public:
 	std::shared_ptr<const Bonus> getBonusLocalFirst(const CSelector & selector) const;
 
 	//non-const interface
-	void getParents(TNodes &out);  //retrieves list of parent nodes (nodes to inherit bonuses from)
 	std::shared_ptr<Bonus> getBonusLocalFirst(const CSelector & selector);
 
 	void attachTo(CBonusSystemNode & parent);
+	void attachToSource(const CBonusSystemNode & parent);
 	void detachFrom(CBonusSystemNode & parent);
+	void detachFromSource(const 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