Fix 1723 quest crash on combined arts

lib/CArtHandler.cpp

@@ -280,16 +280,16 @@ ArtifactPosition CArtHandler::stringToSlot(std::string slotName)
 
 void CArtHandler::addSlot(CArtifact * art, const std::string & slotID)
 {
-	static const std::vector<ArtifactPosition> miscSlots = 
+	static const std::vector<ArtifactPosition> miscSlots =
 	{
 		ArtifactPosition::MISC1, ArtifactPosition::MISC2, ArtifactPosition::MISC3, ArtifactPosition::MISC4, ArtifactPosition::MISC5
 	};
-	
+
 	static const std::vector<ArtifactPosition> ringSlots =
 	{
 		ArtifactPosition::LEFT_RING, ArtifactPosition::RIGHT_RING
 	};
-	
+
 	if (slotID == "MISC")
 	{
 		vstd::concatenate(art->possibleSlots[ArtBearer::HERO], miscSlots);
@@ -323,7 +323,7 @@ void CArtHandler::loadSlots(CArtifact * art, const JsonNode & node)
 CArtifact::EartClass CArtHandler::stringToClass(std::string className)
 {
 	static const std::map<std::string, CArtifact::EartClass> artifactClassMap =
-	{	
+	{
 		{"TREASURE", CArtifact::ART_TREASURE},
 		{"MINOR", CArtifact::ART_MINOR},
 		{"MAJOR", CArtifact::ART_MAJOR},
@@ -1152,9 +1152,42 @@ const CArtifactInstance * CArtifactSet::getArtByInstanceId( ArtifactInstanceID a
 	return nullptr;
 }
 
-bool CArtifactSet::hasArt(ui32 aid, bool onlyWorn /*= false*/) const
+bool CArtifactSet::hasArt(ui32 aid, bool onlyWorn /*= false*/,
+                          bool searchBackpackAssemblies /*= false*/) const
+{
+	return getArtPos(aid, onlyWorn) != ArtifactPosition::PRE_FIRST ||
+	       (searchBackpackAssemblies && getHiddenArt(aid));
+}
+
+std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *>
+CArtifactSet::searchForConstituent(int aid) const
+{
+	for(auto & slot : artifactsInBackpack)
+	{
+		auto art = slot.artifact;
+		if(art->canBeDisassembled())
+		{
+			auto ass = static_cast<CCombinedArtifactInstance *>(art.get());
+			for(auto& ci : ass->constituentsInfo)
+			{
+				if(ci.art->artType->id == aid)
+				{
+					return {ass, ci.art};
+				}
+			}
+		}
+	}
+	return {nullptr, nullptr};
+}
+
+const CArtifactInstance *CArtifactSet::getHiddenArt(int aid) const
+{
+	return searchForConstituent(aid).second;
+}
+
+const CCombinedArtifactInstance *CArtifactSet::getAssemblyByConstituent(int aid) const
 {
-	return getArtPos(aid, onlyWorn) != ArtifactPosition::PRE_FIRST;
+	return searchForConstituent(aid).first;
 }
 
 const ArtSlotInfo * CArtifactSet::getSlot(ArtifactPosition pos) const

lib/CArtHandler.h

@@ -128,7 +128,9 @@ public:
 	virtual bool canBeDisassembled() const;
 	virtual void putAt(ArtifactLocation al);
 	virtual void removeFrom(ArtifactLocation al);
-	virtual bool isPart(const CArtifactInstance *supposedPart) const; //checks if this a part of this artifact: artifact instance is a part of itself, additionally truth is returned for consituents of combined arts
+	/// 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;
 
 	std::vector<const CArtifact *> assemblyPossibilities(const CArtifactSet *h) const;
 	void move(ArtifactLocation src, ArtifactLocation dst);
@@ -172,7 +174,7 @@ public:
 
 	void createConstituents();
 	void addAsConstituent(CArtifactInstance *art, ArtifactPosition slot);
-	CArtifactInstance *figureMainConstituent(const ArtifactLocation al); //main constituent is replcaed with us (combined art), not lock
+	CArtifactInstance *figureMainConstituent(const ArtifactLocation al); //main constituent is replaced with us (combined art), not lock
 
 	CCombinedArtifactInstance();
 
@@ -265,10 +267,8 @@ struct DLL_LINKAGE ArtSlotInfo
 	ConstTransitivePtr<CArtifactInstance> artifact;
 	ui8 locked; //if locked, then artifact points to the combined artifact
 
-	ArtSlotInfo()
-	{
-		locked = false;
-	}
+	ArtSlotInfo() : locked(false) {}
+
 	template <typename Handler> void serialize(Handler &h, const int version)
 	{
 		h & artifact & locked;
@@ -288,10 +288,16 @@ public:
 	const ArtSlotInfo *getSlot(ArtifactPosition pos) const;
 	const CArtifactInstance* getArt(ArtifactPosition pos, bool excludeLocked = true) const; //nullptr - no artifact
 	CArtifactInstance* getArt(ArtifactPosition pos, bool excludeLocked = true); //nullptr - no artifact
-	ArtifactPosition getArtPos(int aid, bool onlyWorn = true) const; //looks for equipped artifact with given ID and returns its slot ID or -1 if none(if more than one such artifact lower ID is returned)
+	/// Looks for equipped artifact with given ID and returns its slot ID or -1 if none
+	/// (if more than one such artifact lower ID is returned)
+	ArtifactPosition getArtPos(int aid, bool onlyWorn = true) const;
 	ArtifactPosition getArtPos(const CArtifactInstance *art) const;
 	const CArtifactInstance *getArtByInstanceId(ArtifactInstanceID artInstId) const;
-	bool hasArt(ui32 aid, bool onlyWorn = false) const; //checks if hero possess artifact of given id (either in backack or worn)
+	/// Search for constituents of assemblies in backpack which do not have an ArtifactPosition
+	const CArtifactInstance *getHiddenArt(int aid) const;
+	const CCombinedArtifactInstance *getAssemblyByConstituent(int aid) const;
+	/// Checks if hero possess artifact of given id (either in backack or worn)
+	bool hasArt(ui32 aid, bool onlyWorn = false, bool searchBackpackAssemblies = false) const;
 	bool isPositionFree(ArtifactPosition pos, bool onlyLockCheck = false) const;
 	si32 getArtTypeId(ArtifactPosition pos) const;
 
@@ -304,4 +310,7 @@ public:
 	}
 
 	void artDeserializationFix(CBonusSystemNode *node);
+
+protected:
+	std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *> searchForConstituent(int aid) const;
 };

lib/NetPacksLib.cpp

@@ -340,7 +340,7 @@ DLL_LINKAGE void RemoveBonus::applyGs( CGameState *gs )
 		if(b->source == source && b->sid == id)
 		{
 			bonus = *b; //backup bonus (to show to interfaces later)
-			node->removeBonus(b);			
+			node->removeBonus(b);
 			break;
 		}
 	}
@@ -754,7 +754,7 @@ DLL_LINKAGE const CArtifactInstance *ArtifactLocation::getArt() const
 			return s->artifact;
 		else
 		{
-            logNetwork->warnStream() << "ArtifactLocation::getArt: That location is locked!";
+			logNetwork->warnStream() << "ArtifactLocation::getArt: This location is locked!";
 			return nullptr;
 		}
 	}
@@ -914,6 +914,32 @@ DLL_LINKAGE void PutArtifact::applyGs( CGameState *gs )
 
 DLL_LINKAGE void EraseArtifact::applyGs( CGameState *gs )
 {
+	auto slot = al.getSlot();
+	if(slot->locked)
+	{
+		logGlobal->debugStream() << "Erasing locked artifact: " << slot->artifact->artType->Name();
+		DisassembledArtifact dis;
+		dis.al.artHolder = al.artHolder;
+		auto aset = al.getHolderArtSet();
+		bool found = false;
+		for(auto& p : aset->artifactsWorn)
+		{
+			auto art = p.second.artifact;
+			if(art->canBeDisassembled() && art->isPart(slot->artifact))
+			{
+				dis.al.slot = aset->getArtPos(art);
+				found = true;
+				break;
+			}
+		}
+		assert(found && "Failed to determine the assembly this locked artifact belongs to");
+		logGlobal->debugStream() << "Found the corresponding assembly: " << dis.al.getSlot()->artifact->artType->Name();
+		dis.applyGs(gs);
+	}
+	else
+	{
+		logGlobal->debugStream() << "Erasing artifact " << slot->artifact->artType->Name();
+	}
 	al.removeArtifact();
 }
 
@@ -1262,7 +1288,7 @@ DLL_LINKAGE void BattleStackAttacked::applyGs( CGameState *gs )
 	{
 		//"hide" killed creatures instead so we keep info about it
 		at->state.insert(EBattleStackState::DEAD_CLONE);
-		
+
 		for(CStack * s : gs->curB->stacks)
 		{
 			if(s->cloneID == at->ID)
@@ -1375,7 +1401,7 @@ void actualizeEffect(CStack * s, const Bonus & ef)
 			stackBonus->turnsRemain = std::max(stackBonus->turnsRemain, ef.turnsRemain);
 		}
 	}
-	CBonusSystemNode::treeHasChanged();	
+	CBonusSystemNode::treeHasChanged();
 }
 
 void actualizeEffect(CStack * s, const std::vector<Bonus> & ef)

lib/mapObjects/CQuest.cpp

@@ -70,7 +70,7 @@ bool CQuest::checkQuest (const CGHeroInstance * h) const
 		case MISSION_ART:
 			for (auto & elem : m5arts)
 			{
-				if (h->hasArt(elem))
+				if (h->hasArt(elem, false, true))
 					continue;
 				return false; //if the artifact was not found
 			}
@@ -630,6 +630,18 @@ void CGSeerHut::finishQuest(const CGHeroInstance * h, ui32 accept) const
 			case CQuest::MISSION_ART:
 				for (auto & elem : quest->m5arts)
 				{
+					if(!h->hasArt(elem))
+					{
+						// first we need to disassemble this backpack artifact
+						auto assembly = h->getAssemblyByConstituent(elem);
+						assert(assembly);
+						for(auto & ci : assembly->constituentsInfo)
+						{
+							cb->giveHeroNewArtifact(h, ci.art->artType, ArtifactPosition::PRE_FIRST);
+						}
+						// remove the assembly
+						cb->removeArtifact(ArtifactLocation(h, h->getArtPos(assembly)));
+					}
 					cb->removeArtifact(ArtifactLocation(h, h->getArtPos(elem, false)));
 				}
 				break;

server/CGameHandler.cpp

@@ -2113,7 +2113,6 @@ void CGameHandler::stopHeroVisitCastle(const CGTownInstance * obj, const CGHeroI
 
 void CGameHandler::removeArtifact(const ArtifactLocation &al)
 {
-	assert(al.getArt());
 	EraseArtifact ea;
 	ea.al = al;
 	sendAndApply(&ea);