Merge pull request #1650 from SoundSSGood/arts-swap-regression-fix

Arts swap regression fixed

client/widgets/CArtifactHolder.cpp

@@ -633,7 +633,7 @@ CArtifactsOfHero::~CArtifactsOfHero()
 	if(!curHero->artifactsTransitionPos.empty())
 	{
 		auto artPlace = getArtPlace(
-			ArtifactUtils::getArtifactDstPosition(curHero->artifactsTransitionPos.begin()->artifact, curHero, curHero->bearerType()));
+			ArtifactUtils::getArtifactDstPosition(curHero->artifactsTransitionPos.begin()->artifact, curHero));
 		assert(artPlace);
 		assert(artPlace->ourOwner);
 		artPlace->setMeAsDest();
@@ -748,10 +748,7 @@ void CArtifactsOfHero::artifactMoved(const ArtifactLocation & src, const Artifac
 	if(withUIUpdate)
 	{
 		updateParentWindow();
-		// If backpack is changed, update it
-		if((isCurHeroSrc && ArtifactUtils::isSlotBackpack(src.slot))
-			|| (isCurHeroDst && ArtifactUtils::isSlotBackpack(dst.slot)))
-			scrollBackpack(0);
+		scrollBackpack(0);
 	}
 }
 

client/windows/GUIClasses.cpp

@@ -841,7 +841,7 @@ void CExchangeController::moveArtifact(
 {
 	auto srcLocation = ArtifactLocation(source, srcPosition);
 	auto dstLocation = ArtifactLocation(target,
-		ArtifactUtils::getArtifactDstPosition(source->getArt(srcPosition), target, target->bearerType()));
+		ArtifactUtils::getArtifactDstPosition(source->getArt(srcPosition), target));
 
 	cb->swapArtifacts(srcLocation, dstLocation);
 }

lib/CArtHandler.cpp

@@ -410,7 +410,7 @@ void CArtHandler::addSlot(CArtifact * art, const std::string & slotID)
 
 	static const std::vector<ArtifactPosition> ringSlots =
 	{
-		ArtifactPosition::LEFT_RING, ArtifactPosition::RIGHT_RING
+		ArtifactPosition::RIGHT_RING, ArtifactPosition::LEFT_RING
 	};
 
 	if (slotID == "MISC")
@@ -425,7 +425,7 @@ void CArtHandler::addSlot(CArtifact * art, const std::string & slotID)
 	{
 		auto slot = ArtifactPosition(slotID);
 		if (slot != ArtifactPosition::PRE_FIRST)
-			art->possibleSlots[ArtBearer::HERO].push_back (slot);
+			art->possibleSlots[ArtBearer::HERO].push_back(slot);
 	}
 }
 
@@ -440,6 +440,7 @@ void CArtHandler::loadSlots(CArtifact * art, const JsonNode & node)
 			for (const JsonNode & slot : node["slot"].Vector())
 				addSlot(art, slot.String());
 		}
+		std::sort(art->possibleSlots.at(ArtBearer::HERO).begin(), art->possibleSlots.at(ArtBearer::HERO).end());
 	}
 }
 
@@ -1565,17 +1566,14 @@ ArtBearer::ArtBearer CArtifactFittingSet::bearerType() const
 	return this->Bearer;
 }
 
-DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtifactDstPosition(	const CArtifactInstance * artifact,
-									const CArtifactSet * target, 
-									ArtBearer::ArtBearer bearer)
+DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtifactDstPosition(const CArtifactInstance * artifact,
+	const CArtifactSet * target)
 {
-	for(auto slot : artifact->artType->possibleSlots.at(bearer))
+	for(auto slot : artifact->artType->possibleSlots.at(target->bearerType()))
 	{
-		auto existingArtifact = target->getArt(slot);
 		auto existingArtInfo = target->getSlot(slot);
 
-		if(!existingArtifact
-			&& (!existingArtInfo || !existingArtInfo->locked)
+		if((!existingArtInfo || !existingArtInfo->locked)
 			&& artifact->canBePutAt(target, slot))
 		{
 			return slot;

lib/CArtHandler.h

@@ -384,9 +384,8 @@ protected:
 namespace ArtifactUtils
 {
 	// Calculates where an artifact gets placed when it gets transferred from one hero to another.
-	DLL_LINKAGE ArtifactPosition getArtifactDstPosition(	const CArtifactInstance * artifact, 
-								const CArtifactSet * target,
-								ArtBearer::ArtBearer bearer);
+	DLL_LINKAGE ArtifactPosition getArtifactDstPosition(const CArtifactInstance * artifact, 
+		const CArtifactSet * target);
 	// TODO: Make this constexpr when the toolset is upgraded
 	DLL_LINKAGE const std::vector<ArtifactPosition::EArtifactPosition> & unmovableSlots();
 	DLL_LINKAGE const std::vector<ArtifactPosition::EArtifactPosition> & constituentWornSlots();

server/CGameHandler.cpp

@@ -4001,36 +4001,45 @@ bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID
 	auto & slotsSrcDst = ma.artsPack0;
 	auto & slotsDstSrc = ma.artsPack1;
 
+	// Temporary fitting set for artifacts. Used to select available slots before sending data.
+	CArtifactFittingSet artFittingSet(pdstHero->bearerType());
+
+	auto moveArtifact = [this, &artFittingSet](const CArtifactInstance * artifact,
+		ArtifactPosition srcSlot, const CGHeroInstance * dstHero,
+		std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
+	{
+		assert(artifact);
+		auto dstSlot = ArtifactUtils::getArtifactDstPosition(artifact, &artFittingSet);
+		artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
+		slots.push_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
+
+		if(ArtifactUtils::checkSpellbookIsNeeded(dstHero, artifact->artType->getId(), dstSlot))
+			giveHeroNewArtifact(dstHero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
+	};
+
 	if(swap)
 	{
-		auto moveArtsWorn = [this](const CGHeroInstance * srcHero, const CGHeroInstance * dstHero,
+		auto moveArtsWorn = [moveArtifact](const CGHeroInstance * srcHero, const CGHeroInstance * dstHero,
 			std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
 		{
 			for(auto & artifact : srcHero->artifactsWorn)
 			{
-				if(artifact.second.locked)
-					continue;
-				if(!ArtifactUtils::isArtRemovable(artifact))
-					continue;
-				slots.push_back(BulkMoveArtifacts::LinkedSlots(artifact.first, artifact.first));
-
-				auto art = artifact.second.getArt();
-				assert(art);
-				if(ArtifactUtils::checkSpellbookIsNeeded(dstHero, art->artType->getId(), artifact.first))
-					giveHeroNewArtifact(dstHero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
+				if(ArtifactUtils::isArtRemovable(artifact))
+					moveArtifact(artifact.second.getArt(), artifact.first, dstHero, slots);
 			}
 		};
-		auto moveArtsInBackpack = [](const CGHeroInstance * pHero,
+		auto moveArtsInBackpack = [](const CArtifactSet * artSet,
 			std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
 		{
-			for(auto & slotInfo : pHero->artifactsInBackpack)
+			for(auto & slotInfo : artSet->artifactsInBackpack)
 			{
-				auto slot = pHero->getArtPos(slotInfo.artifact);
+				auto slot = artSet->getArtPos(slotInfo.artifact);
 				slots.push_back(BulkMoveArtifacts::LinkedSlots(slot, slot));
 			}
 		};
 		// Move over artifacts that are worn srcHero -> dstHero
 		moveArtsWorn(psrcHero, pdstHero, slotsSrcDst);
+		artFittingSet.artifactsWorn.clear();
 		// Move over artifacts that are worn dstHero -> srcHero
 		moveArtsWorn(pdstHero, psrcHero, slotsDstSrc);
 		// Move over artifacts that are in backpack srcHero -> dstHero
@@ -4040,35 +4049,22 @@ bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID
 	}
 	else
 	{
-		// Temporary fitting set for artifacts. Used to select available slots before sending data.
-		CArtifactFittingSet artFittingSet(pdstHero->bearerType());
 		artFittingSet.artifactsInBackpack = pdstHero->artifactsInBackpack;
 		artFittingSet.artifactsWorn = pdstHero->artifactsWorn;
 
-		auto moveArtifact = [this, &artFittingSet, &slotsSrcDst](const CArtifactInstance * artifact,
-			ArtifactPosition srcSlot, const CGHeroInstance * pdstHero) -> void
-		{
-			assert(artifact);
-			auto dstSlot = ArtifactUtils::getArtifactDstPosition(artifact, &artFittingSet, pdstHero->bearerType());
-			artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
-			slotsSrcDst.push_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
-
-			if(ArtifactUtils::checkSpellbookIsNeeded(pdstHero, artifact->artType->getId(), dstSlot))
-				giveHeroNewArtifact(pdstHero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
-		};
-
 		// Move over artifacts that are worn
 		for(auto & artInfo : psrcHero->artifactsWorn)
 		{
 			if(ArtifactUtils::isArtRemovable(artInfo))
 			{
-				moveArtifact(psrcHero->getArt(artInfo.first), artInfo.first, pdstHero);
+				moveArtifact(psrcHero->getArt(artInfo.first), artInfo.first, pdstHero, slotsSrcDst);
 			}
 		}
 		// Move over artifacts that are in backpack
 		for(auto & slotInfo : psrcHero->artifactsInBackpack)
 		{
-			moveArtifact(psrcHero->getArt(psrcHero->getArtPos(slotInfo.artifact)), psrcHero->getArtPos(slotInfo.artifact), pdstHero);
+			moveArtifact(psrcHero->getArt(psrcHero->getArtPos(slotInfo.artifact)),
+				psrcHero->getArtPos(slotInfo.artifact), pdstHero, slotsSrcDst);
 		}
 	}
 	sendAndApply(&ma);