CCombinedArtifactInstance refactoring

client/widgets/CArtifactsOfHeroAltar.cpp

@@ -98,13 +98,10 @@ void CArtifactsOfHeroAltar::deleteFromVisible(const CArtifactInstance * artInst)
 	}
 	else
 	{
-		if(artInst->canBeDisassembled())
+		for(const auto & part : artInst->partsInfo)
 		{
-			for(const auto & part : dynamic_cast<const CCombinedArtifactInstance*>(artInst)->constituentsInfo)
+			if(part.slot != ArtifactPosition::PRE_FIRST)
-			{
+				getArtPlace(part.slot)->setArtifact(nullptr);
-				if(part.slot != ArtifactPosition::PRE_FIRST)
-					getArtPlace(part.slot)->setArtifact(nullptr);
-			}
 		}
 	}
 }

lib/ArtifactUtils.cpp

@@ -165,24 +165,23 @@ DLL_LINKAGE CArtifactInstance * ArtifactUtils::createScroll(const SpellID & sid)
 
 DLL_LINKAGE CArtifactInstance * ArtifactUtils::createNewArtifactInstance(CArtifact * art)
 {
+	assert(art);
+
+	auto * artInst = new CArtifactInstance(art);
 	if(art->canBeDisassembled())
 	{
-		auto * ret = new CCombinedArtifactInstance(art);
+		assert(art->constituents);
-		ret->createConstituents();
+		for(const auto & part : *art->constituents)
-		return ret;
+			artInst->addArtInstAsPart(ArtifactUtils::createNewArtifactInstance(part), ArtifactPosition::PRE_FIRST);
 	}
-	else
+	if(dynamic_cast<CGrowingArtifact*>(art))
 	{
-		auto * ret = new CArtifactInstance(art);
+		auto bonus = std::make_shared<Bonus>();
-		if(dynamic_cast<CGrowingArtifact*>(art))
+		bonus->type = BonusType::LEVEL_COUNTER;
-		{
+		bonus->val = 0;
-			auto bonus = std::make_shared<Bonus>();
+		artInst->addNewBonus(bonus);
-			bonus->type = BonusType::LEVEL_COUNTER;
-			bonus->val = 0;
-			ret->addNewBonus(bonus);
-		}
-		return ret;
 	}
+	return artInst;
 }
 
 DLL_LINKAGE CArtifactInstance * ArtifactUtils::createNewArtifactInstance(const ArtifactID & aid)
@@ -211,10 +210,9 @@ DLL_LINKAGE CArtifactInstance * ArtifactUtils::createArtifact(CMap * map, const
 	map->addNewArtifactInstance(art);
 	if(art->artType && art->canBeDisassembled())
 	{
-		auto * combined = dynamic_cast<CCombinedArtifactInstance*>(art);
+		for(auto & part : art->partsInfo)
-		for(CCombinedArtifactInstance::ConstituentInfo & ci : combined->constituentsInfo)
 		{
-			map->addNewArtifactInstance(ci.art);
+			map->addNewArtifactInstance(part.art);
 		}
 	}
 	return art;

lib/CArtHandler.cpp

@@ -854,6 +854,11 @@ void CArtifactInstance::putAt(const ArtifactLocation & al)
 void CArtifactInstance::removeFrom(const ArtifactLocation & al)
 {
 	al.getHolderArtSet()->removeArtifact(al.slot);
+	for(auto & part : partsInfo)
+	{
+		if(part.slot != ArtifactPosition::PRE_FIRST)
+			part.slot = ArtifactPosition::PRE_FIRST;
+	}
 }
 
 bool CArtifactInstance::canBeDisassembled() const
@@ -870,6 +875,8 @@ void CArtifactInstance::move(const ArtifactLocation & src, const ArtifactLocatio
 void CArtifactInstance::deserializationFix()
 {
 	setType(artType);
+	for(PartInfo & part : partsInfo)
+		attachTo(*part.art);
 }
 
 SpellID CArtifactInstance::getScrollSpellID() const
@@ -885,74 +892,28 @@ SpellID CArtifactInstance::getScrollSpellID() const
 
 bool CArtifactInstance::isPart(const CArtifactInstance *supposedPart) const
 {
-	return supposedPart == this;
+	if(supposedPart == this)
-}
-
-CCombinedArtifactInstance::CCombinedArtifactInstance(CArtifact *Art)
-	: CArtifactInstance(Art) //TODO: seems unused, but need to be written
-{
-}
-
-void CCombinedArtifactInstance::createConstituents()
-{
-	assert(artType);
-	assert(artType->constituents);
-
-	for(const CArtifact * art : *artType->constituents)
-	{
-		addAsConstituent(ArtifactUtils::createNewArtifactInstance(art->getId()), ArtifactPosition::PRE_FIRST);
-	}
-}
-
-void CCombinedArtifactInstance::addAsConstituent(CArtifactInstance * art, const ArtifactPosition & slot)
-{
-	assert(vstd::contains_if(*artType->constituents, [=](const CArtifact * constituent){
-		return constituent->getId() == art->artType->getId();
-	}));
-	assert(art->getParentNodes().size() == 1  &&  art->getParentNodes().front() == art->artType);
-	constituentsInfo.emplace_back(art, slot);
-	attachTo(*art);
-}
-
-void CCombinedArtifactInstance::removeFrom(const ArtifactLocation & al)
-{
-	CArtifactInstance::removeFrom(al);
-	for(auto & part : constituentsInfo)
-	{
-		if(part.slot != ArtifactPosition::PRE_FIRST)
-			part.slot = ArtifactPosition::PRE_FIRST;
-	}
-}
-
-void CCombinedArtifactInstance::deserializationFix()
-{
-	for(ConstituentInfo &ci : constituentsInfo)
-		attachTo(*ci.art);
-}
-
-bool CCombinedArtifactInstance::isPart(const CArtifactInstance *supposedPart) const
-{
-	bool me = CArtifactInstance::isPart(supposedPart);
-	if(me)
 		return true;
 
 	//check for constituents
-	for(const ConstituentInfo &constituent : constituentsInfo)
+	for(const PartInfo & constituent : partsInfo)
 		if(constituent.art == supposedPart)
 			return true;
 
 	return false;
 }
 
-CCombinedArtifactInstance::ConstituentInfo::ConstituentInfo(CArtifactInstance * Art, const ArtifactPosition & Slot):
+void CCombinedArtifactInstance::addArtInstAsPart(CArtifactInstance * art, const ArtifactPosition & slot)
-	art(Art),
-	slot(Slot)
 {
-}
+	auto artInst = static_cast<CArtifactInstance*>(this);
-
+	assert(vstd::contains_if(*artInst->artType->constituents,
-bool CCombinedArtifactInstance::ConstituentInfo::operator==(const ConstituentInfo &rhs) const
+		[=](const CArtifact * partType)
-{
+		{
-	return art == rhs.art && slot == rhs.slot;
+			return partType->getId() == art->getTypeId();
+		}));
+	assert(art->getParentNodes().size() == 1  &&  art->getParentNodes().front() == art->artType);
+	partsInfo.emplace_back(art, slot);
+	artInst->attachTo(*art);
 }
 
 CArtifactSet::~CArtifactSet() = default;
@@ -1090,8 +1051,7 @@ void CArtifactSet::putArtifact(ArtifactPosition slot, CArtifactInstance * art)
 	if(art->artType->canBeDisassembled() && ArtifactUtils::isSlotEquipment(slot))
 	{
 		const CArtifactInstance * mainPart = nullptr;
-		auto & parts = dynamic_cast<CCombinedArtifactInstance*>(art)->constituentsInfo;
+		for(const auto & part : art->partsInfo)
-		for(const auto & part : parts)
 			if(vstd::contains(part.art->artType->possibleSlots.at(bearerType()), slot)
 				&& (part.slot == ArtifactPosition::PRE_FIRST))
 			{
@@ -1099,7 +1059,7 @@ void CArtifactSet::putArtifact(ArtifactPosition slot, CArtifactInstance * art)
 				break;
 			}
 
-		for(auto & part : parts)
+		for(auto & part : art->partsInfo)
 		{
 			if(part.art != mainPart)
 			{
@@ -1120,8 +1080,7 @@ void CArtifactSet::removeArtifact(ArtifactPosition slot)
 	{
 		if(art->canBeDisassembled())
 		{
-			auto combinedArt = dynamic_cast<CCombinedArtifactInstance*>(art);
+			for(auto & part : art->partsInfo)
-			for(auto & part : combinedArt->constituentsInfo)
 			{
 				if(getArt(part.slot, false))
 					eraseArtSlot(part.slot);
@@ -1131,19 +1090,18 @@ void CArtifactSet::removeArtifact(ArtifactPosition slot)
 	}
 }
 
-std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *> CArtifactSet::searchForConstituent(const ArtifactID & aid) const
+std::pair<const CArtifactInstance *, const CArtifactInstance *> CArtifactSet::searchForConstituent(const ArtifactID & aid) const
 {
 	for(const auto & slot : artifactsInBackpack)
 	{
 		auto art = slot.artifact;
 		if(art->canBeDisassembled())
 		{
-			auto * ass = dynamic_cast<CCombinedArtifactInstance *>(art.get());
+			for(auto& ci : art->partsInfo)
-			for(auto& ci : ass->constituentsInfo)
 			{
 				if(ci.art->getTypeId() == aid)
 				{
-					return {ass, ci.art};
+					return {art, ci.art};
 				}
 			}
 		}
@@ -1156,7 +1114,7 @@ const CArtifactInstance * CArtifactSet::getHiddenArt(const ArtifactID & aid) con
 	return searchForConstituent(aid).second;
 }
 
-const CCombinedArtifactInstance * CArtifactSet::getAssemblyByConstituent(const ArtifactID & aid) const
+const CArtifactInstance * CArtifactSet::getAssemblyByConstituent(const ArtifactID & aid) const
 {
 	return searchForConstituent(aid).first;
 }

lib/CArtHandler.h

@@ -137,12 +137,33 @@ public:
 	}
 };
 
-class DLL_LINKAGE CArtifactInstance : public CBonusSystemNode
+class CCombinedArtifactInstance
+{
+protected:
+	CCombinedArtifactInstance() = default;
+public:
+	struct PartInfo
+	{
+		ConstTransitivePtr<CArtifactInstance> art;
+		ArtifactPosition slot;
+		template <typename Handler> void serialize(Handler & h, const int version)
+		{
+			h & art;
+			h & slot;
+		}
+		PartInfo(CArtifactInstance * art = nullptr, const ArtifactPosition & slot = ArtifactPosition::PRE_FIRST)
+			: art(art), slot(slot) {};
+	};
+	std::vector<PartInfo> partsInfo;
+	void addArtInstAsPart(CArtifactInstance * art, const ArtifactPosition & slot);
+};
+
+class DLL_LINKAGE CArtifactInstance : public CBonusSystemNode, public CCombinedArtifactInstance
 {
 protected:
 	void init();
 public:
-	CArtifactInstance(CArtifact * Art);
+	CArtifactInstance(CArtifact * art);
 	CArtifactInstance();
 
 	ConstTransitivePtr<CArtifact> artType;
@@ -150,66 +171,27 @@ public:
 
 	std::string nodeName() const override;
 	void deserializationFix();
-	void setType(CArtifact *Art);
+	void setType(CArtifact * art);
 
 	std::string getDescription() const;
 	SpellID getScrollSpellID() const; //to be used with scrolls (and similar arts), -1 if none
 
 	ArtifactID getTypeId() const;
-	bool canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved = false) const;  //forwards to the above one
+	bool canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved = false) const;
-	virtual bool canBeDisassembled() const;
+	bool canBeDisassembled() const;
 	/// Checks if this a part of this artifact: artifact instance is a part
 	/// of itself, additionally truth is returned for constituents of combined arts
-	virtual bool isPart(const CArtifactInstance *supposedPart) const;
+	bool isPart(const CArtifactInstance * supposedPart) const;
-
+	void putAt(const ArtifactLocation & al);
-	virtual void putAt(const ArtifactLocation & al);
+	void removeFrom(const ArtifactLocation & al);
-	virtual void removeFrom(const ArtifactLocation & al);
+	void move(const ArtifactLocation & src, const ArtifactLocation & dst);
-	virtual void move(const ArtifactLocation & src, const ArtifactLocation & dst);
 
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & static_cast<CBonusSystemNode&>(*this);
 		h & artType;
 		h & id;
-		BONUS_TREE_DESERIALIZATION_FIX
+		h & partsInfo;
-	}
-};
-
-class DLL_LINKAGE CCombinedArtifactInstance : public CArtifactInstance
-{
-public:
-	CCombinedArtifactInstance(CArtifact * Art);
-	struct ConstituentInfo
-	{
-		ConstTransitivePtr<CArtifactInstance> art;
-		ArtifactPosition slot;
-		template <typename Handler> void serialize(Handler &h, const int version)
-		{
-			h & art;
-			h & slot;
-		}
-
-		bool operator==(const ConstituentInfo &rhs) const;
-		ConstituentInfo(CArtifactInstance * art = nullptr, const ArtifactPosition & slot = ArtifactPosition::PRE_FIRST);
-	};
-
-	std::vector<ConstituentInfo> constituentsInfo;
-
-	bool isPart(const CArtifactInstance *supposedPart) const override;
-	void createConstituents();
-	void addAsConstituent(CArtifactInstance * art, const ArtifactPosition & slot);
-	void removeFrom(const ArtifactLocation & al) override;
-
-	CCombinedArtifactInstance() = default;
-
-	void deserializationFix();
-
-	friend class CArtifactInstance;
-	friend struct AssembledArtifact;
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & static_cast<CArtifactInstance&>(*this);
-		h & constituentsInfo;
 		BONUS_TREE_DESERIALIZATION_FIX
 	}
 };
@@ -313,7 +295,7 @@ public:
 	const ArtifactPosition getSlotByInstance(const CArtifactInstance * artInst) const;
 	/// Search for constituents of assemblies in backpack which do not have an ArtifactPosition
 	const CArtifactInstance * getHiddenArt(const ArtifactID & aid) const;
-	const CCombinedArtifactInstance * getAssemblyByConstituent(const ArtifactID & aid) const;
+	const CArtifactInstance * getAssemblyByConstituent(const ArtifactID & aid) const;
 	/// Checks if hero possess artifact of given id (either in backack or worn)
 	bool hasArt(const ArtifactID & aid, bool onlyWorn = false, bool searchBackpackAssemblies = false, bool allowLocked = true) const;
 	bool hasArtBackpack(const ArtifactID & aid) const;
@@ -335,7 +317,7 @@ public:
 
 	void serializeJsonArtifacts(JsonSerializeFormat & handler, const std::string & fieldName, CMap * map);
 protected:
-	std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *> searchForConstituent(const ArtifactID & aid) const;
+	std::pair<const CArtifactInstance *, const CArtifactInstance *> searchForConstituent(const ArtifactID & aid) const;
 
 private:
 	void serializeJsonHero(JsonSerializeFormat & handler, CMap * map);

lib/NetPacksLib.cpp

@@ -1524,12 +1524,17 @@ void NewObject::applyGs(CGameState *gs)
 void NewArtifact::applyGs(CGameState *gs)
 {
 	assert(!vstd::contains(gs->map->artInstances, art));
-	gs->map->addNewArtifactInstance(art);
-
 	assert(!art->getParentNodes().size());
+	assert(art->artType);
+
 	art->setType(art->artType);
-	if(auto * cart = dynamic_cast<CCombinedArtifactInstance *>(art.get()))
+	if(art->canBeDisassembled())
-		cart->createConstituents();
+	{
+		assert(art->artType->constituents);
+		for(const auto & part : *art->artType->constituents)
+			art->addArtInstAsPart(ArtifactUtils::createNewArtifactInstance(part), ArtifactPosition::PRE_FIRST);
+	}
+	gs->map->addNewArtifactInstance(art);
 }
 
 const CStackInstance * StackLocation::getStack()
@@ -1930,7 +1935,7 @@ void AssembledArtifact::applyGs(CGameState *gs)
 			return art->getId() == builtArt->getId();
 		}));
 
-	auto * combinedArt = new CCombinedArtifactInstance(builtArt);
+	auto * combinedArt = new CArtifactInstance(builtArt);
 	gs->map->addNewArtifactInstance(combinedArt);
 	// Retrieve all constituents
 	for(const CArtifact * constituent : *builtArt->constituents)
@@ -1955,7 +1960,7 @@ void AssembledArtifact::applyGs(CGameState *gs)
 			al.slot = std::min(al.slot, pos);
 			pos = ArtifactPosition::PRE_FIRST;
 		}
-		combinedArt->addAsConstituent(constituentInstance, pos);
+		combinedArt->addArtInstAsPart(constituentInstance, pos);
 	}
 
 	//put new combined artifacts
@@ -1964,19 +1969,19 @@ void AssembledArtifact::applyGs(CGameState *gs)
 
 void DisassembledArtifact::applyGs(CGameState *gs)
 {
-	auto * disassembled = dynamic_cast<CCombinedArtifactInstance *>(al.getArt());
+	auto * disassembled = al.getArt();
 	assert(disassembled);
 
-	std::vector<CCombinedArtifactInstance::ConstituentInfo> constituents = disassembled->constituentsInfo;
+	auto parts = disassembled->partsInfo;
 	disassembled->removeFrom(al);
-	for(CCombinedArtifactInstance::ConstituentInfo &ci : constituents)
+	for(auto & part : parts)
 	{
-		ArtifactLocation constituentLoc = al;
+		ArtifactLocation partLoc = al;
-		constituentLoc.slot = (ci.slot >= 0 ? ci.slot : al.slot); //-1 is slot of main constituent -> it'll replace combined artifact in its pos
+		// ArtifactPosition::PRE_FIRST is value of main part slot -> it'll replace combined artifact in its pos
-		disassembled->detachFrom(*ci.art);
+		partLoc.slot = (ArtifactUtils::isSlotEquipment(part.slot) ? part.slot : al.slot);
-		ci.art->putAt(constituentLoc);
+		disassembled->detachFrom(*part.art);
+		part.art->putAt(partLoc);
 	}
-
 	gs->map->eraseArtifactInstance(disassembled);
 }
 

lib/mapObjects/CQuest.cpp

@@ -153,7 +153,7 @@ bool CQuest::checkQuest(const CGHeroInstance * h) const
 				if(h->getArtPosCount(elem.first, false, true, true) < elem.second)
 					return false;
 				if(!h->hasArt(elem.first))
-					reqSlots += h->getAssemblyByConstituent(elem.first)->constituentsInfo.size() - 2;
+					reqSlots += h->getAssemblyByConstituent(elem.first)->partsInfo.size() - 2;
 			}
 			if(ArtifactUtils::isBackpackFreeSlots(h, reqSlots))
 				return true;
@@ -804,7 +804,7 @@ void CGSeerHut::finishQuest(const CGHeroInstance * h, ui32 accept) const
 					{
 						const auto * assembly = h->getAssemblyByConstituent(elem);
 						assert(assembly);
-						auto parts = assembly->constituentsInfo;
+						auto parts = assembly->partsInfo;
 
 						// Remove the assembly
 						cb->removeArtifact(ArtifactLocation(h, h->getArtPos(assembly)));

lib/registerTypes/RegisterTypes.h

@@ -218,7 +218,6 @@ void registerTypesMapObjects2(Serializer &s)
 	s.template registerType<CBonusSystemNode, BattleInfo>();
 	//s.template registerType<QuestInfo>();
 	s.template registerType<CBonusSystemNode, CArtifactInstance>();
-	s.template registerType<CArtifactInstance, CCombinedArtifactInstance>();
 
 	//s.template registerType<CObstacleInstance>();
 		s.template registerType<CObstacleInstance, SpellCreatedObstacle>();

server/CGameHandler.cpp

@@ -6806,17 +6806,12 @@ bool CGameHandler::giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact
 		COMPLAIN_RET_FALSE_IF(!artType->canBePutAt(h, pos, false), "Cannot put artifact in that slot!");
 	}
 
-	CArtifactInstance * newArtInst = nullptr;
+	auto * newArtInst = new CArtifactInstance();
-	if(artType->canBeDisassembled())
-		newArtInst = new CCombinedArtifactInstance();
-	else
-		newArtInst = new CArtifactInstance();
-
 	newArtInst->artType = artType; // *NOT* via settype -> all bonus-related stuff must be done by NewArtifact apply
 
 	NewArtifact na;
 	na.art = newArtInst;
-	sendAndApply(&na); // -> updates a!!!, will create a on other machines
+	sendAndApply(&na); // -> updates newArtInst!!!
 
 	if(giveHeroArtifact(h, newArtInst, pos))
 		return true;