Merge pull request #3069 from SoundSSGood/artifact-location-id

ArtifactLocation now use ID for artHolder identification

AI/Nullkiller/AIGateway.cpp

@@ -9,6 +9,7 @@
  */
 #include "StdInc.h"
 
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/UnlockGuard.h"
 #include "../../lib/mapObjects/MapObjects.h"
 #include "../../lib/mapObjects/ObjectTemplate.h"
@@ -995,21 +996,21 @@ void AIGateway::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance
 				for(auto p : h->artifactsWorn)
 				{
 					if(p.second.artifact)
-						allArtifacts.push_back(ArtifactLocation(h, p.first));
+						allArtifacts.push_back(ArtifactLocation(h->id, p.first));
 				}
 			}
 			for(auto slot : h->artifactsInBackpack)
-				allArtifacts.push_back(ArtifactLocation(h, h->getArtPos(slot.artifact)));
+				allArtifacts.push_back(ArtifactLocation(h->id, h->getArtPos(slot.artifact)));
 
 			if(otherh)
 			{
 				for(auto p : otherh->artifactsWorn)
 				{
 					if(p.second.artifact)
-						allArtifacts.push_back(ArtifactLocation(otherh, p.first));
+						allArtifacts.push_back(ArtifactLocation(otherh->id, p.first));
 				}
 				for(auto slot : otherh->artifactsInBackpack)
-					allArtifacts.push_back(ArtifactLocation(otherh, otherh->getArtPos(slot.artifact)));
+					allArtifacts.push_back(ArtifactLocation(otherh->id, otherh->getArtPos(slot.artifact)));
 			}
 			//we give stuff to one hero or another, depending on giveStuffToFirstHero
 
@@ -1021,13 +1022,13 @@ void AIGateway::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance
 
 			for(auto location : allArtifacts)
 			{
-				if(location.relatedObj() == target && location.slot < ArtifactPosition::AFTER_LAST)
+				if(location.artHolder == target->id && ArtifactUtils::isSlotEquipment(location.slot))
 					continue; //don't reequip artifact we already wear
 
 				if(location.slot == ArtifactPosition::MACH4) // don't attempt to move catapult
 					continue;
 
-				auto s = location.getSlot();
+				auto s = cb->getHero(location.artHolder)->getSlot(location.slot);
 				if(!s || s->locked) //we can't move locks
 					continue;
 				auto artifact = s->artifact;
@@ -1038,9 +1039,9 @@ void AIGateway::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance
 				bool emptySlotFound = false;
 				for(auto slot : artifact->artType->getPossibleSlots().at(target->bearerType()))
 				{
-					ArtifactLocation destLocation(target, slot);
-					if(target->isPositionFree(slot) && artifact->canBePutAt(destLocation, true)) //combined artifacts are not always allowed to move
+					if(target->isPositionFree(slot) && artifact->canBePutAt(target, slot, true)) //combined artifacts are not always allowed to move
 					{
+						ArtifactLocation destLocation(target->id, slot);
 						cb->swapArtifacts(location, destLocation); //just put into empty slot
 						emptySlotFound = true;
 						changeMade = true;
@@ -1054,11 +1055,11 @@ void AIGateway::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance
 						auto otherSlot = target->getSlot(slot);
 						if(otherSlot && otherSlot->artifact) //we need to exchange artifact for better one
 						{
-							ArtifactLocation destLocation(target, slot);
 							//if that artifact is better than what we have, pick it
-							if(compareArtifacts(artifact, otherSlot->artifact) && artifact->canBePutAt(destLocation, true)) //combined artifacts are not always allowed to move
+							if(compareArtifacts(artifact, otherSlot->artifact) && artifact->canBePutAt(target, slot, true)) //combined artifacts are not always allowed to move
 							{
-								cb->swapArtifacts(location, ArtifactLocation(target, target->getArtPos(otherSlot->artifact)));
+								ArtifactLocation destLocation(target->id, slot);
+								cb->swapArtifacts(location, ArtifactLocation(target->id, target->getArtPos(otherSlot->artifact)));
 								changeMade = true;
 								break;
 							}

AI/VCAI/VCAI.cpp

@@ -14,6 +14,7 @@
 #include "BuildingManager.h"
 #include "Goals/Goals.h"
 
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/UnlockGuard.h"
 #include "../../lib/mapObjects/MapObjects.h"
 #include "../../lib/mapObjects/ObjectTemplate.h"
@@ -1166,21 +1167,21 @@ void VCAI::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance * ot
 				for(auto p : h->artifactsWorn)
 				{
 					if(p.second.artifact)
-						allArtifacts.push_back(ArtifactLocation(h, p.first));
+						allArtifacts.push_back(ArtifactLocation(h->id, p.first));
 				}
 			}
 			for(auto slot : h->artifactsInBackpack)
-				allArtifacts.push_back(ArtifactLocation(h, h->getArtPos(slot.artifact)));
+				allArtifacts.push_back(ArtifactLocation(h->id, h->getArtPos(slot.artifact)));
 
 			if(otherh)
 			{
 				for(auto p : otherh->artifactsWorn)
 				{
 					if(p.second.artifact)
-						allArtifacts.push_back(ArtifactLocation(otherh, p.first));
+						allArtifacts.push_back(ArtifactLocation(otherh->id, p.first));
 				}
 				for(auto slot : otherh->artifactsInBackpack)
-					allArtifacts.push_back(ArtifactLocation(otherh, otherh->getArtPos(slot.artifact)));
+					allArtifacts.push_back(ArtifactLocation(otherh->id, otherh->getArtPos(slot.artifact)));
 			}
 			//we give stuff to one hero or another, depending on giveStuffToFirstHero
 
@@ -1195,10 +1196,10 @@ void VCAI::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance * ot
 				if(location.slot == ArtifactPosition::MACH4 || location.slot == ArtifactPosition::SPELLBOOK)
 					continue; // don't attempt to move catapult and spellbook
 
-				if(location.relatedObj() == target && location.slot < ArtifactPosition::AFTER_LAST)
+				if(location.artHolder == target->id && ArtifactUtils::isSlotEquipment(location.slot))
 					continue; //don't reequip artifact we already wear
 
-				auto s = location.getSlot();
+				auto s = cb->getHero(location.artHolder)->getSlot(location.slot);
 				if(!s || s->locked) //we can't move locks
 					continue;
 				auto artifact = s->artifact;
@@ -1209,9 +1210,9 @@ void VCAI::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance * ot
 				bool emptySlotFound = false;
 				for(auto slot : artifact->artType->getPossibleSlots().at(target->bearerType()))
 				{
-					ArtifactLocation destLocation(target, slot);
-					if(target->isPositionFree(slot) && artifact->canBePutAt(destLocation, true)) //combined artifacts are not always allowed to move
+					if(target->isPositionFree(slot) && artifact->canBePutAt(target, slot, true)) //combined artifacts are not always allowed to move
 					{
+						ArtifactLocation destLocation(target->id, slot);
 						cb->swapArtifacts(location, destLocation); //just put into empty slot
 						emptySlotFound = true;
 						changeMade = true;
@@ -1225,11 +1226,11 @@ void VCAI::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance * ot
 						auto otherSlot = target->getSlot(slot);
 						if(otherSlot && otherSlot->artifact) //we need to exchange artifact for better one
 						{
-							ArtifactLocation destLocation(target, slot);
 							//if that artifact is better than what we have, pick it
-							if(compareArtifacts(artifact, otherSlot->artifact) && artifact->canBePutAt(destLocation, true)) //combined artifacts are not always allowed to move
+							if(compareArtifacts(artifact, otherSlot->artifact) && artifact->canBePutAt(target, slot, true)) //combined artifacts are not always allowed to move
 							{
-								cb->swapArtifacts(location, ArtifactLocation(target, target->getArtPos(otherSlot->artifact)));
+								ArtifactLocation destLocation(target->id, slot);
+								cb->swapArtifacts(location, ArtifactLocation(target->id, target->getArtPos(otherSlot->artifact)));
 								changeMade = true;
 								break;
 							}

client/CPlayerInterface.cpp

@@ -1753,8 +1753,7 @@ void CPlayerInterface::requestReturningToMainMenu(bool won)
 
 void CPlayerInterface::askToAssembleArtifact(const ArtifactLocation &al)
 {
-	auto hero = std::visit(HeroObjectRetriever(), al.artHolder);
-	if(hero)
+	if(auto hero = cb->getHero(al.artHolder))
 	{
 		auto art = hero->getArt(al.slot);
 		if(art == nullptr)
@@ -1770,15 +1769,13 @@ void CPlayerInterface::askToAssembleArtifact(const ArtifactLocation &al)
 void CPlayerInterface::artifactPut(const ArtifactLocation &al)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
-	auto hero = std::visit(HeroObjectRetriever(), al.artHolder);
-	adventureInt->onHeroChanged(hero);
+	adventureInt->onHeroChanged(cb->getHero(al.artHolder));
 }
 
 void CPlayerInterface::artifactRemoved(const ArtifactLocation &al)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
-	auto hero = std::visit(HeroObjectRetriever(), al.artHolder);
-	adventureInt->onHeroChanged(hero);
+	adventureInt->onHeroChanged(cb->getHero(al.artHolder));
 
 	for(auto artWin : GH.windows().findWindows<CArtifactHolder>())
 		artWin->artifactRemoved(al);
@@ -1789,8 +1786,7 @@ void CPlayerInterface::artifactRemoved(const ArtifactLocation &al)
 void CPlayerInterface::artifactMoved(const ArtifactLocation &src, const ArtifactLocation &dst)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
-	auto hero = std::visit(HeroObjectRetriever(), dst.artHolder);
-	adventureInt->onHeroChanged(hero);
+	adventureInt->onHeroChanged(cb->getHero(dst.artHolder));
 
 	bool redraw = true;
 	// If a bulk transfer has arrived, then redrawing only the last art movement.
@@ -1815,8 +1811,7 @@ void CPlayerInterface::bulkArtMovementStart(size_t numOfArts)
 void CPlayerInterface::artifactAssembled(const ArtifactLocation &al)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
-	auto hero = std::visit(HeroObjectRetriever(), al.artHolder);
-	adventureInt->onHeroChanged(hero);
+	adventureInt->onHeroChanged(cb->getHero(al.artHolder));
 
 	for(auto artWin : GH.windows().findWindows<CArtifactHolder>())
 		artWin->artifactAssembled(al);
@@ -1825,8 +1820,7 @@ void CPlayerInterface::artifactAssembled(const ArtifactLocation &al)
 void CPlayerInterface::artifactDisassembled(const ArtifactLocation &al)
 {
 	EVENT_HANDLER_CALLED_BY_CLIENT;
-	auto hero = std::visit(HeroObjectRetriever(), al.artHolder);
-	adventureInt->onHeroChanged(hero);
+	adventureInt->onHeroChanged(cb->getHero(al.artHolder));
 
 	for(auto artWin : GH.windows().findWindows<CArtifactHolder>())
 		artWin->artifactDisassembled(al);

client/NetPacksClient.cpp

@@ -267,14 +267,14 @@ void ApplyClientNetPackVisitor::visitBulkSmartRebalanceStacks(BulkSmartRebalance
 
 void ApplyClientNetPackVisitor::visitPutArtifact(PutArtifact & pack)
 {
-	callInterfaceIfPresent(cl, pack.al.owningPlayer(), &IGameEventsReceiver::artifactPut, pack.al);
+	callInterfaceIfPresent(cl, cl.getOwner(pack.al.artHolder), &IGameEventsReceiver::artifactPut, pack.al);
 	if(pack.askAssemble)
-		callInterfaceIfPresent(cl, pack.al.owningPlayer(), &IGameEventsReceiver::askToAssembleArtifact, pack.al);
+		callInterfaceIfPresent(cl, cl.getOwner(pack.al.artHolder), &IGameEventsReceiver::askToAssembleArtifact, pack.al);
 }
 
 void ApplyClientNetPackVisitor::visitEraseArtifact(EraseArtifact & pack)
 {
-	callInterfaceIfPresent(cl, pack.al.owningPlayer(), &IGameEventsReceiver::artifactRemoved, pack.al);
+	callInterfaceIfPresent(cl, cl.getOwner(pack.al.artHolder), &IGameEventsReceiver::artifactRemoved, pack.al);
 }
 
 void ApplyClientNetPackVisitor::visitMoveArtifact(MoveArtifact & pack)
@@ -286,9 +286,9 @@ void ApplyClientNetPackVisitor::visitMoveArtifact(MoveArtifact & pack)
 			callInterfaceIfPresent(cl, player, &IGameEventsReceiver::askToAssembleArtifact, pack.dst);
 	};
 
-	moveArtifact(pack.src.owningPlayer());
-	if(pack.src.owningPlayer() != pack.dst.owningPlayer())
-		moveArtifact(pack.dst.owningPlayer());
+	moveArtifact(cl.getOwner(pack.src.artHolder));
+	if(cl.getOwner(pack.src.artHolder) != cl.getOwner(pack.dst.artHolder))
+		moveArtifact(cl.getOwner(pack.dst.artHolder));
 
 	cl.invalidatePaths(); // hero might have equipped/unequipped Angel Wings
 }
@@ -306,8 +306,8 @@ void ApplyClientNetPackVisitor::visitBulkMoveArtifacts(BulkMoveArtifacts & pack)
 		}
 	};
 
-	auto srcOwner = std::get<ConstTransitivePtr<CGHeroInstance>>(pack.srcArtHolder)->tempOwner;
-	auto dstOwner = std::get<ConstTransitivePtr<CGHeroInstance>>(pack.dstArtHolder)->tempOwner;
+	auto srcOwner = cl.getOwner(pack.srcArtHolder);
+	auto dstOwner = cl.getOwner(pack.dstArtHolder);
 
 	// Begin a session of bulk movement of arts. It is not necessary but useful for the client optimization.
 	callInterfaceIfPresent(cl, srcOwner, &IGameEventsReceiver::bulkArtMovementStart, pack.artsPack0.size() + pack.artsPack1.size());
@@ -321,14 +321,14 @@ void ApplyClientNetPackVisitor::visitBulkMoveArtifacts(BulkMoveArtifacts & pack)
 
 void ApplyClientNetPackVisitor::visitAssembledArtifact(AssembledArtifact & pack)
 {
-	callInterfaceIfPresent(cl, pack.al.owningPlayer(), &IGameEventsReceiver::artifactAssembled, pack.al);
+	callInterfaceIfPresent(cl, cl.getOwner(pack.al.artHolder), &IGameEventsReceiver::artifactAssembled, pack.al);
 
 	cl.invalidatePaths(); // hero might have equipped/unequipped Angel Wings
 }
 
 void ApplyClientNetPackVisitor::visitDisassembledArtifact(DisassembledArtifact & pack)
 {
-	callInterfaceIfPresent(cl, pack.al.owningPlayer(), &IGameEventsReceiver::artifactDisassembled, pack.al);
+	callInterfaceIfPresent(cl, cl.getOwner(pack.al.artHolder), &IGameEventsReceiver::artifactDisassembled, pack.al);
 
 	cl.invalidatePaths(); // hero might have equipped/unequipped Angel Wings
 }

client/widgets/CArtifactHolder.cpp

@@ -121,10 +121,11 @@ void CCommanderArtPlace::returnArtToHeroCallback()
 	}
 	else
 	{
-		ArtifactLocation src(commanderOwner->commander.get(), artifactPos);
-		ArtifactLocation dst(commanderOwner, freeSlot);
+		ArtifactLocation src(commanderOwner->id, artifactPos);
+		src.creature = SlotID::COMMANDER_SLOT_PLACEHOLDER;
+		ArtifactLocation dst(commanderOwner->id, freeSlot);
 
-		if(ourArt->canBePutAt(dst, true))
+		if(ourArt->canBePutAt(commanderOwner, freeSlot, true))
 		{
 			LOCPLINT->cb->swapArtifacts(src, dst);
 			setArtifact(nullptr);

client/widgets/CArtifactsOfHeroAltar.cpp

@@ -72,7 +72,7 @@ void CArtifactsOfHeroAltar::pickUpArtifact(CHeroArtPlace & artPlace)
 		if(ArtifactUtils::isSlotBackpack(pickedArtFromSlot))
 			pickedArtFromSlot = curHero->getSlotByInstance(art);
 		assert(pickedArtFromSlot != ArtifactPosition::PRE_FIRST);
-		LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero, pickedArtFromSlot), ArtifactLocation(curHero, ArtifactPosition::TRANSITION_POS));
+		LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, pickedArtFromSlot), ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
 	}
 }
 
@@ -89,7 +89,7 @@ void CArtifactsOfHeroAltar::pickedArtMoveToAltar(const ArtifactPosition & slot)
 	if(ArtifactUtils::isSlotBackpack(slot) || ArtifactUtils::isSlotEquipment(slot) || slot == ArtifactPosition::TRANSITION_POS)
 	{
 		assert(curHero->getSlot(slot)->getArt());
-		LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero, slot), ArtifactLocation(curHero, pickedArtFromSlot));
+		LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, slot), ArtifactLocation(curHero->id, pickedArtFromSlot));
 		pickedArtFromSlot = ArtifactPosition::PRE_FIRST;
 	}
 }

client/widgets/CArtifactsOfHeroBackpack.cpp

@@ -80,8 +80,8 @@ void CArtifactsOfHeroBackpack::swapArtifacts(const ArtifactLocation & srcLoc, co
 
 void CArtifactsOfHeroBackpack::pickUpArtifact(CHeroArtPlace & artPlace)
 {
-	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero, artPlace.slot),
-		ArtifactLocation(curHero, ArtifactPosition::TRANSITION_POS));
+	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, artPlace.slot),
+		ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
 }
 
 void CArtifactsOfHeroBackpack::scrollBackpack(int offset)

client/widgets/CArtifactsOfHeroBase.cpp

@@ -39,11 +39,11 @@ void CArtifactsOfHeroBase::putBackPickedArtifact()
 		auto slot = ArtifactUtils::getArtAnyPosition(curHero, curHero->artifactsTransitionPos.begin()->artifact->getTypeId());
 		if(slot == ArtifactPosition::PRE_FIRST)
 		{
-			LOCPLINT->cb->eraseArtifactByClient(ArtifactLocation(curHero, ArtifactPosition::TRANSITION_POS));
+			LOCPLINT->cb->eraseArtifactByClient(ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
 		}
 		else
 		{
-			LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero, ArtifactPosition::TRANSITION_POS), ArtifactLocation(curHero, slot));
+			LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS), ArtifactLocation(curHero->id, slot));
 		}
 	}
 	if(putBackPickedArtCallback)
@@ -178,7 +178,7 @@ void CArtifactsOfHeroBase::scrollBackpackForArtSet(int offset, const CArtifactSe
 void CArtifactsOfHeroBase::markPossibleSlots(const CArtifactInstance * art, bool assumeDestRemoved)
 {
 	for(auto artPlace : artWorn)
-		artPlace.second->selectSlot(art->artType->canBePutAt(curHero, artPlace.second->slot, assumeDestRemoved));
+		artPlace.second->selectSlot(art->canBePutAt(curHero, artPlace.second->slot, assumeDestRemoved));
 }
 
 void CArtifactsOfHeroBase::unmarkSlots()

client/widgets/CArtifactsOfHeroKingdom.cpp

@@ -13,6 +13,7 @@
 #include "Buttons.h"
 
 #include "../CPlayerInterface.h"
+#include "../../lib/mapObjects/CGHeroInstance.h"
 
 #include "../../CCallback.h"
 #include "../../lib/networkPacks/ArtifactLocation.h"
@@ -56,7 +57,7 @@ void CArtifactsOfHeroKingdom::swapArtifacts(const ArtifactLocation & srcLoc, con
 
 void CArtifactsOfHeroKingdom::pickUpArtifact(CHeroArtPlace & artPlace)
 {
-	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero, artPlace.slot),
-		ArtifactLocation(curHero, ArtifactPosition::TRANSITION_POS));
+	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, artPlace.slot),
+		ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
 }
 

client/widgets/CArtifactsOfHeroMain.cpp

@@ -11,6 +11,7 @@
 #include "CArtifactsOfHeroMain.h"
 
 #include "../CPlayerInterface.h"
+#include "../../lib/mapObjects/CGHeroInstance.h"
 
 #include "../../CCallback.h"
 #include "../../lib/networkPacks/ArtifactLocation.h"
@@ -36,6 +37,6 @@ void CArtifactsOfHeroMain::swapArtifacts(const ArtifactLocation & srcLoc, const
 
 void CArtifactsOfHeroMain::pickUpArtifact(CHeroArtPlace & artPlace)
 {
-	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero, artPlace.slot),
-		ArtifactLocation(curHero, ArtifactPosition::TRANSITION_POS));
+	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, artPlace.slot),
+		ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
 }

client/widgets/CGarrisonInt.cpp

@@ -196,16 +196,18 @@ bool CGarrisonSlot::highlightOrDropArtifact()
 			artSelected = true;
 			if (myStack) // try dropping the artifact only if the slot isn't empty
 			{
-				ArtifactLocation src(srcHero, ArtifactPosition::TRANSITION_POS);
-				ArtifactLocation dst(myStack, ArtifactPosition::CREATURE_SLOT);
-				if(pickedArtInst->canBePutAt(dst, true))
+				ArtifactLocation src(srcHero->id, ArtifactPosition::TRANSITION_POS);
+				ArtifactLocation dst(getObj()->id, ArtifactPosition::CREATURE_SLOT);
+				dst.creature = getSlot();
+
+				if(pickedArtInst->canBePutAt(myStack, ArtifactPosition::CREATURE_SLOT, true))
 				{	//equip clicked stack
-					if(dst.getArt())
+					if(auto dstArt = myStack->getArt(ArtifactPosition::CREATURE_SLOT))
 					{
 						//creature can wear only one active artifact
 						//if we are placing a new one, the old one will be returned to the hero's backpack
-						LOCPLINT->cb->swapArtifacts(dst, ArtifactLocation(srcHero,
-							ArtifactUtils::getArtBackpackPosition(srcHero, dst.getArt()->getTypeId())));
+						LOCPLINT->cb->swapArtifacts(dst, ArtifactLocation(srcHero->id,
+							ArtifactUtils::getArtBackpackPosition(srcHero, dstArt->getTypeId())));
 					}
 					LOCPLINT->cb->swapArtifacts(src, dst);
 				}

client/widgets/CWindowWithArtifacts.cpp

@@ -122,8 +122,8 @@ void CWindowWithArtifacts::leftClickArtPlaceHero(CArtifactsOfHeroBase & artsInst
 
 				if(pickedArtInst)
 				{
-					auto srcLoc = ArtifactLocation(heroPickedArt, ArtifactPosition::TRANSITION_POS);
-					auto dstLoc = ArtifactLocation(hero, artPlace.slot);
+					auto srcLoc = ArtifactLocation(heroPickedArt->id, ArtifactPosition::TRANSITION_POS);
+					auto dstLoc = ArtifactLocation(hero->id, artPlace.slot);
 
 					if(ArtifactUtils::isSlotBackpack(artPlace.slot))
 					{
@@ -141,7 +141,7 @@ void CWindowWithArtifacts::leftClickArtPlaceHero(CArtifactsOfHeroBase & artsInst
 						}
 					}
 					// Check if artifact transfer is possible
-					else if(pickedArtInst->canBePutAt(dstLoc, true) && (!artPlace.getArt() || hero->tempOwner == LOCPLINT->playerID))
+					else if(pickedArtInst->canBePutAt(hero, artPlace.slot, true) && (!artPlace.getArt() || hero->tempOwner == LOCPLINT->playerID))
 					{
 						isTransferAllowed = true;
 					}
@@ -270,7 +270,7 @@ void CWindowWithArtifacts::artifactMoved(const ArtifactLocation & srcLoc, const
 	// we have a different artifact may look surprising... but it's valid.
 
 	auto pickedArtInst = std::get<const CArtifactInstance*>(curState.value());
-	assert(!pickedArtInst || destLoc.isHolder(std::get<const CGHeroInstance*>(curState.value())));
+	assert(!pickedArtInst || destLoc.artHolder == std::get<const CGHeroInstance*>(curState.value())->id);
 
 	auto artifactMovedBody = [this, withRedraw, &destLoc, &pickedArtInst](auto artSetWeak) -> void
 	{
@@ -316,7 +316,7 @@ void CWindowWithArtifacts::artifactMoved(const ArtifactLocation & srcLoc, const
 			}
 
 			// Make sure the status bar is updated so it does not display old text
-			if(destLoc.getHolderArtSet() == hero)
+			if(destLoc.artHolder == hero->id)
 			{
 				if(auto artPlace = artSetPtr->getArtPlace(destLoc.slot))
 					artPlace->hover(true);

client/windows/CCreatureWindow.cpp

@@ -974,11 +974,12 @@ void CStackWindow::removeStackArtifact(ArtifactPosition pos)
 	const auto slot = ArtifactUtils::getArtBackpackPosition(info->owner, art->getTypeId());
 	if(slot != ArtifactPosition::PRE_FIRST)
 	{
-		LOCPLINT->cb->swapArtifacts(ArtifactLocation(info->stackNode, pos), ArtifactLocation(info->owner, slot));
+		auto artLoc = ArtifactLocation(info->owner->id, pos);
+		artLoc.creature = info->stackNode->armyObj->findStack(info->stackNode);
+		LOCPLINT->cb->swapArtifacts(artLoc, ArtifactLocation(info->owner->id, slot));
 		stackArtifactButton.reset();
 		stackArtifactHelp.reset();
 		stackArtifactIcon.reset();
 		redraw();
 	}
 }
-

client/windows/CHeroWindow.cpp

@@ -334,20 +334,11 @@ void CHeroWindow::commanderWindow()
 	if(pickedArtInst)
 	{
 		const auto freeSlot = ArtifactUtils::getArtAnyPosition(curHero->commander, pickedArtInst->getTypeId());
-		if(freeSlot < ArtifactPosition::COMMANDER_AFTER_LAST) //we don't want to put it in commander's backpack!
+		if(vstd::contains(ArtifactUtils::commanderSlots(), freeSlot)) // We don't want to put it in commander's backpack!
 		{
-			ArtifactLocation src(hero, ArtifactPosition::TRANSITION_POS);
-			ArtifactLocation dst(curHero->commander.get(), freeSlot);
-
-			if(pickedArtInst->canBePutAt(dst, true))
-			{	//equip clicked stack
-				if(dst.getArt())
-				{
-					LOCPLINT->cb->swapArtifacts(dst, ArtifactLocation(hero,
-						ArtifactUtils::getArtBackpackPosition(hero, pickedArtInst->getTypeId())));
-				}
-				LOCPLINT->cb->swapArtifacts(src, dst);
-			}
+			ArtifactLocation dst(curHero->id, freeSlot);
+			dst.creature = SlotID::COMMANDER_SLOT_PLACEHOLDER;
+			LOCPLINT->cb->swapArtifacts(ArtifactLocation(hero->id, ArtifactPosition::TRANSITION_POS), dst);
 		}
 	}
 	else

client/windows/CTradeWindow.cpp

@@ -191,8 +191,8 @@ void CTradeWindow::CTradeableItem::clickPressed(const Point & cursorPosition)
 				const auto hero = aw->arts->getHero();
 				const auto slot = hero->getSlotByInstance(art);
 				assert(slot != ArtifactPosition::PRE_FIRST);
-				LOCPLINT->cb->swapArtifacts(ArtifactLocation(hero, slot),
-					ArtifactLocation(hero, ArtifactPosition::TRANSITION_POS));
+				LOCPLINT->cb->swapArtifacts(ArtifactLocation(hero->id, slot),
+					ArtifactLocation(hero->id, ArtifactPosition::TRANSITION_POS));
 				aw->arts->pickedArtFromSlot = slot;
 				aw->arts->artifactsOnAltar.erase(art);
 				setID(-1);

lib/ArtifactUtils.cpp

@@ -74,6 +74,49 @@ DLL_LINKAGE const std::vector<ArtifactPosition> & ArtifactUtils::constituentWorn
 	return positions;
 }
 
+DLL_LINKAGE const std::vector<ArtifactPosition> & ArtifactUtils::allWornSlots()
+{
+	static const std::vector<ArtifactPosition> positions =
+	{
+		ArtifactPosition::HEAD,
+		ArtifactPosition::SHOULDERS,
+		ArtifactPosition::NECK,
+		ArtifactPosition::RIGHT_HAND,
+		ArtifactPosition::LEFT_HAND,
+		ArtifactPosition::TORSO,
+		ArtifactPosition::RIGHT_RING,
+		ArtifactPosition::LEFT_RING,
+		ArtifactPosition::FEET,
+		ArtifactPosition::MISC1,
+		ArtifactPosition::MISC2,
+		ArtifactPosition::MISC3,
+		ArtifactPosition::MISC4,
+		ArtifactPosition::MISC5,
+		ArtifactPosition::MACH1,
+		ArtifactPosition::MACH2,
+		ArtifactPosition::MACH3,
+		ArtifactPosition::MACH4,
+		ArtifactPosition::SPELLBOOK
+	};
+
+	return positions;
+}
+
+DLL_LINKAGE const std::vector<ArtifactPosition> & ArtifactUtils::commanderSlots()
+{
+	static const std::vector<ArtifactPosition> positions =
+	{
+		ArtifactPosition::COMMANDER1,
+		ArtifactPosition::COMMANDER2,
+		ArtifactPosition::COMMANDER3,
+		ArtifactPosition::COMMANDER4,
+		ArtifactPosition::COMMANDER5,
+		ArtifactPosition::COMMANDER6
+	};
+
+	return positions;
+}
+
 DLL_LINKAGE bool ArtifactUtils::isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot)
 {
 	return slot.second.artifact

lib/ArtifactUtils.h

@@ -31,6 +31,8 @@ namespace ArtifactUtils
 	// TODO: Make this constexpr when the toolset is upgraded
 	DLL_LINKAGE const std::vector<ArtifactPosition> & unmovableSlots();
 	DLL_LINKAGE const std::vector<ArtifactPosition> & constituentWornSlots();
+	DLL_LINKAGE const std::vector<ArtifactPosition> & allWornSlots();
+	DLL_LINKAGE const std::vector<ArtifactPosition> & commanderSlots();
 	DLL_LINKAGE bool isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot);
 	DLL_LINKAGE bool checkSpellbookIsNeeded(const CGHeroInstance * heroPtr, const ArtifactID & artID, const ArtifactPosition & slot);
 	DLL_LINKAGE bool isSlotBackpack(const ArtifactPosition & slot);

lib/CArtHandler.cpp

@@ -693,8 +693,8 @@ void CArtHandler::makeItCommanderArt(CArtifact * a, bool onlyCommander)
 		a->possibleSlots[ArtBearer::HERO].clear();
 		a->possibleSlots[ArtBearer::CREATURE].clear();
 	}
-	for (int i = ArtifactPosition::COMMANDER1; i <= ArtifactPosition::COMMANDER6; ++i)
-		a->possibleSlots[ArtBearer::COMMANDER].push_back(ArtifactPosition(i));
+	for(const auto & slot : ArtifactUtils::commanderSlots())
+		a->possibleSlots[ArtBearer::COMMANDER].push_back(ArtifactPosition(slot));
 }
 
 bool CArtHandler::legalArtifact(const ArtifactID & id)
@@ -975,9 +975,9 @@ const ArtSlotInfo * CArtifactSet::getSlot(const ArtifactPosition & pos) const
 	}
 	if(vstd::contains(artifactsWorn, pos))
 		return &artifactsWorn.at(pos);
-	if(pos >= ArtifactPosition::AFTER_LAST )
+	if(ArtifactUtils::isSlotBackpack(pos))
 	{
-		int backpackPos = (int)pos - ArtifactPosition::BACKPACK_START;
+		auto backpackPos = pos - ArtifactPosition::BACKPACK_START;
 		if(backpackPos < 0 || backpackPos >= artifactsInBackpack.size())
 			return nullptr;
 		else
@@ -1080,9 +1080,9 @@ void CArtifactSet::serializeJsonArtifacts(JsonSerializeFormat & handler, const s
 
 void CArtifactSet::serializeJsonHero(JsonSerializeFormat & handler, CMap * map)
 {
-	for(ArtifactPosition ap = ArtifactPosition::HEAD; ap < ArtifactPosition::AFTER_LAST; ap.advance(1))
+	for(const auto & slot : ArtifactUtils::allWornSlots())
 	{
-		serializeJsonSlot(handler, ap, map);
+		serializeJsonSlot(handler, slot, map);
 	}
 
 	std::vector<ArtifactID> backpackTemp;

lib/CArtifactInstance.cpp

@@ -155,9 +155,9 @@ void CArtifactInstance::setId(ArtifactInstanceID id)
 	this->id = id;
 }
 
-bool CArtifactInstance::canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved) const
+bool CArtifactInstance::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, bool assumeDestRemoved) const
 {
-	return artType->canBePutAt(al.getHolderArtSet(), al.slot, assumeDestRemoved);
+	return artType->canBePutAt(artSet, slot, assumeDestRemoved);
 }
 
 bool CArtifactInstance::isCombined() const
@@ -165,15 +165,15 @@ bool CArtifactInstance::isCombined() const
 	return artType->isCombined();
 }
 
-void CArtifactInstance::putAt(const ArtifactLocation & al)
+void CArtifactInstance::putAt(CArtifactSet & set, const ArtifactPosition slot)
 {
-	auto placementMap = al.getHolderArtSet()->putArtifact(al.slot, this);
+	auto placementMap = set.putArtifact(slot, this);
 	addPlacementMap(placementMap);
 }
 
-void CArtifactInstance::removeFrom(const ArtifactLocation & al)
+void CArtifactInstance::removeFrom(CArtifactSet & set, const ArtifactPosition slot)
 {
-	al.getHolderArtSet()->removeArtifact(al.slot);
+	set.removeArtifact(slot);
 	for(auto & part : partsInfo)
 	{
 		if(part.slot != ArtifactPosition::PRE_FIRST)
@@ -181,10 +181,10 @@ void CArtifactInstance::removeFrom(const ArtifactLocation & al)
 	}
 }
 
-void CArtifactInstance::move(const ArtifactLocation & src, const ArtifactLocation & dst)
+void CArtifactInstance::move(CArtifactSet & srcSet, const ArtifactPosition srcSlot, CArtifactSet & dstSet, const ArtifactPosition dstSlot)
 {
-	removeFrom(src);
-	putAt(dst);
+	removeFrom(srcSet, srcSlot);
+	putAt(dstSet, dstSlot);
 }
 
 void CArtifactInstance::deserializationFix()

lib/CArtifactInstance.h

@@ -84,11 +84,12 @@ public:
 	ArtifactInstanceID getId() const;
 	void setId(ArtifactInstanceID id);
 
-	bool canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved = false) const;
+	bool canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot = ArtifactPosition::FIRST_AVAILABLE,
+		bool assumeDestRemoved = false) const;
 	bool isCombined() const;
-	void putAt(const ArtifactLocation & al);
-	void removeFrom(const ArtifactLocation & al);
-	void move(const ArtifactLocation & src, const ArtifactLocation & dst);
+	void putAt(CArtifactSet & set, const ArtifactPosition slot);
+	void removeFrom(CArtifactSet & set, const ArtifactPosition slot);
+	void move(CArtifactSet & srcSet, const ArtifactPosition srcSlot, CArtifactSet & dstSet, const ArtifactPosition dstSlot);
 	
 	void deserializationFix();
 	template <typename Handler> void serialize(Handler & h, const int version)

lib/CCreatureSet.cpp

@@ -458,7 +458,7 @@ const CStackInstance & CCreatureSet::getStack(const SlotID & slot) const
 	return *getStackPtr(slot);
 }
 
-const CStackInstance * CCreatureSet::getStackPtr(const SlotID & slot) const
+CStackInstance * CCreatureSet::getStackPtr(const SlotID & slot) const
 {
 	if(hasStackAtSlot(slot))
 		return stacks.find(slot)->second;
@@ -870,7 +870,7 @@ ArtBearer::ArtBearer CStackInstance::bearerType() const
 CStackInstance::ArtPlacementMap CStackInstance::putArtifact(ArtifactPosition pos, CArtifactInstance * art)
 {
 	assert(!getArt(pos));
-	assert(art->artType->canBePutAt(this, pos));
+	assert(art->canBePutAt(this, pos));
 
 	attachTo(*art);
 	return CArtifactSet::putArtifact(pos, art);

lib/CCreatureSet.h

@@ -253,7 +253,7 @@ public:
 	void setToArmy(CSimpleArmy &src); //erases all our army and moves stacks from src to us; src MUST NOT be an armed object! WARNING: use it wisely. Or better do not use at all.
 
 	const CStackInstance & getStack(const SlotID & slot) const; //stack must exist
-	const CStackInstance * getStackPtr(const SlotID & slot) const; //if stack doesn't exist, returns nullptr
+	CStackInstance * getStackPtr(const SlotID & slot) const; //if stack doesn't exist, returns nullptr
 	const CCreature * getCreature(const SlotID & slot) const; //workaround of map issue;
 	int getStackCount(const SlotID & slot) const;
 	TExpType getStackExperience(const SlotID & slot) const;

lib/CGameInfoCallback.cpp

@@ -16,6 +16,7 @@
 #include "gameState/TavernHeroesPool.h"
 #include "gameState/QuestInfo.h"
 #include "mapObjects/CGHeroInstance.h"
+#include "networkPacks/ArtifactLocation.h"
 #include "CGeneralTextHandler.h"
 #include "StartInfo.h" // for StartInfo
 #include "battle/BattleInfo.h" // for BattleInfo
@@ -966,6 +967,22 @@ const CGObjectInstance * CGameInfoCallback::getObjInstance( ObjectInstanceID oid
 	return gs->map->objects[oid.num];
 }
 
+CArtifactSet * CGameInfoCallback::getArtSet(const ArtifactLocation & loc) const
+{
+	auto hero = const_cast<CGHeroInstance*>(getHero(loc.artHolder));
+	if(loc.creature.has_value())
+	{
+		if(loc.creature.value() == SlotID::COMMANDER_SLOT_PLACEHOLDER)
+			return hero->commander;
+		else
+			return hero->getStackPtr(loc.creature.value());
+	}
+	else
+	{
+		return hero;
+	}
+}
+
 std::vector<ObjectInstanceID> CGameInfoCallback::getVisibleTeleportObjects(std::vector<ObjectInstanceID> ids, PlayerColor player) const
 {
 	vstd::erase_if(ids, [&](const ObjectInstanceID & id) -> bool

lib/CGameInfoCallback.h

@@ -40,6 +40,8 @@ class CGameState;
 class PathfinderConfig;
 struct TurnTimerInfo;
 
+struct ArtifactLocation;
+class CArtifactSet;
 class CArmedInstance;
 class CGObjectInstance;
 class CGHeroInstance;
@@ -174,6 +176,7 @@ public:
 	virtual int64_t estimateSpellDamage(const CSpell * sp, const CGHeroInstance * hero) const; //estimates damage of given spell; returns 0 if spell causes no dmg
 	virtual const CArtifactInstance * getArtInstance(ArtifactInstanceID aid) const;
 	virtual const CGObjectInstance * getObjInstance(ObjectInstanceID oid) const;
+	virtual CArtifactSet * getArtSet(const ArtifactLocation & loc) const;
 	//virtual const CGObjectInstance * getArmyInstance(ObjectInstanceID oid) const;
 
 	//objects

lib/constants/EntityIdentifiers.h

@@ -607,20 +607,23 @@ public:
 		TRANSITION_POS = -3,
 		FIRST_AVAILABLE = -2,
 		PRE_FIRST = -1, //sometimes used as error, sometimes as first free in backpack
+		
+		// Hero
 		HEAD, SHOULDERS, NECK, RIGHT_HAND, LEFT_HAND, TORSO, //5
 		RIGHT_RING, LEFT_RING, FEET, //8
 		MISC1, MISC2, MISC3, MISC4, //12
 		MACH1, MACH2, MACH3, MACH4, //16
 		SPELLBOOK, MISC5, //18
-		AFTER_LAST,
-		//cres
+		BACKPACK_START = 19,
+		
+		// Creatures
 		CREATURE_SLOT = 0,
-		COMMANDER1 = 0, COMMANDER2, COMMANDER3, COMMANDER4, COMMANDER5, COMMANDER6, COMMANDER_AFTER_LAST,
-
-		BACKPACK_START = 19
+		
+		// Commander
+		COMMANDER1 = 0, COMMANDER2, COMMANDER3, COMMANDER4, COMMANDER5, COMMANDER6
 	};
 
-	static_assert (AFTER_LAST == BACKPACK_START, "incorrect number of artifact slots");
+	static_assert(MISC5 < BACKPACK_START, "incorrect number of artifact slots");
 
 	DLL_LINKAGE static si32 decode(const std::string & identifier);
 	DLL_LINKAGE static std::string encode(const si32 index);

lib/gameState/CGameState.cpp

@@ -2105,7 +2105,7 @@ bool CGameState::giveHeroArtifact(CGHeroInstance * h, const ArtifactID & aid)
 	 auto slot = ArtifactUtils::getArtAnyPosition(h, aid);
 	 if(ArtifactUtils::isSlotEquipment(slot) || ArtifactUtils::isSlotBackpack(slot))
 	 {
-		 ai->putAt(ArtifactLocation(h, slot));
+		 ai->putAt(*h, slot);
 		 return true;
 	 }
 	 else

lib/gameState/CGameStateCampaign.cpp

@@ -134,9 +134,9 @@ void CGameStateCampaign::trimCrossoverHeroesParameters(std::vector<CampaignHeroR
 
 				bool takeable = travelOptions.artifactsKeptByHero.count(art->artType->getId());
 
-				ArtifactLocation al(hero, artifactPosition);
-				if(!takeable  &&  !al.getSlot()->locked)  //don't try removing locked artifacts -> it crashes #1719
-					al.removeArtifact();
+				ArtifactLocation al(hero->id, artifactPosition);
+				if(!takeable && !hero->getSlot(al.slot)->locked)  //don't try removing locked artifacts -> it crashes #1719
+					hero->getArt(al.slot)->removeFrom(*hero, al.slot);
 			};
 
 			// process on copy - removal of artifact will invalidate container
@@ -300,7 +300,7 @@ void CGameStateCampaign::giveCampaignBonusToHero(CGHeroInstance * hero)
 			CArtifactInstance * scroll = ArtifactUtils::createScroll(SpellID(curBonus->info2));
 			const auto slot = ArtifactUtils::getArtAnyPosition(hero, scroll->getTypeId());
 			if(ArtifactUtils::isSlotEquipment(slot) || ArtifactUtils::isSlotBackpack(slot))
-				scroll->putAt(ArtifactLocation(hero, slot));
+				scroll->putAt(*hero, slot);
 			else
 				logGlobal->error("Cannot give starting scroll - no free slots!");
 			break;

lib/mapObjects/CGHeroInstance.cpp

@@ -1092,7 +1092,7 @@ std::string CGHeroInstance::getBiographyTextID() const
 
 CGHeroInstance::ArtPlacementMap CGHeroInstance::putArtifact(ArtifactPosition pos, CArtifactInstance * art)
 {
-	assert(art->artType->canBePutAt(this, pos));
+	assert(art->canBePutAt(this, pos));
 
 	if(ArtifactUtils::isSlotEquipment(pos))
 		attachTo(*art);
@@ -1135,7 +1135,7 @@ void CGHeroInstance::removeSpellbook()
 
 	if(hasSpellbook())
 	{
-		ArtifactLocation(this, ArtifactPosition(ArtifactPosition::SPELLBOOK)).removeArtifact();
+		getArt(ArtifactPosition::SPELLBOOK)->removeFrom(*this, ArtifactPosition::SPELLBOOK);
 	}
 }
 

lib/mapObjects/CQuest.cpp

@@ -144,7 +144,7 @@ void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h) const
 	{
 		if(h->hasArt(elem))
 		{
-			cb->removeArtifact(ArtifactLocation(h, h->getArtPos(elem, false)));
+			cb->removeArtifact(ArtifactLocation(h->id, h->getArtPos(elem, false)));
 		}
 		else
 		{
@@ -153,7 +153,7 @@ void CQuest::completeQuest(IGameCallback * cb, const CGHeroInstance *h) const
 			auto parts = assembly->getPartsInfo();
 
 			// Remove the assembly
-			cb->removeArtifact(ArtifactLocation(h, h->getArtPos(assembly)));
+			cb->removeArtifact(ArtifactLocation(h->id, h->getArtPos(assembly)));
 
 			// Disassemble this backpack artifact
 			for(const auto & ci : parts)

lib/mapping/MapFormatH3M.cpp

@@ -944,10 +944,9 @@ bool CMapLoaderH3M::loadArtifactToSlot(CGHeroInstance * hero, int slot)
 	// He has Shackles of War (normally - MISC slot artifact) in LEFT_HAND slot set in editor
 	// Artifact seems to be missing in game, so skip artifacts that don't fit target slot
 	auto * artifact = ArtifactUtils::createArtifact(map, artifactID);
-	auto dstLoc = ArtifactLocation(hero, ArtifactPosition(slot));
-	if(artifact->canBePutAt(dstLoc))
+	if(artifact->canBePutAt(hero, ArtifactPosition(slot)))
 	{
-		artifact->putAt(dstLoc);
+		artifact->putAt(*hero, ArtifactPosition(slot));
 	}
 	else
 	{

lib/networkPacks/ArtifactLocation.h

@@ -9,67 +9,34 @@
  */
 #pragma once
 
-#include "../ConstTransitivePtr.h"
 #include "../constants/EntityIdentifiers.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-class CGHeroInstance;
-class CStackInstance;
-class CArmedInstance;
-class CArtifactSet;
-class CBonusSystemNode;
-struct ArtSlotInfo;
-
-using TArtHolder = std::variant<ConstTransitivePtr<CGHeroInstance>, ConstTransitivePtr<CStackInstance>>;
-
 struct ArtifactLocation
 {
-	TArtHolder artHolder;//TODO: identify holder by id
-	ArtifactPosition slot = ArtifactPosition::PRE_FIRST;
+	ObjectInstanceID artHolder;
+	ArtifactPosition slot;
+	std::optional<SlotID> creature;
 
 	ArtifactLocation()
-		: artHolder(ConstTransitivePtr<CGHeroInstance>())
+		: artHolder(ObjectInstanceID::NONE)
+		, slot(ArtifactPosition::PRE_FIRST)
+		, creature(std::nullopt)
 	{
 	}
-	template<typename T>
-	ArtifactLocation(const T * ArtHolder, ArtifactPosition Slot)
-		: artHolder(const_cast<T *>(ArtHolder)) //we are allowed here to const cast -> change will go through one of our packages... do not abuse!
-		, slot(Slot)
+	ArtifactLocation(const ObjectInstanceID id, const ArtifactPosition & slot = ArtifactPosition::PRE_FIRST)
+		: artHolder(id)
+		, slot(slot)
+		, creature(std::nullopt)
 	{
 	}
-	ArtifactLocation(TArtHolder ArtHolder, const ArtifactPosition & Slot)
-		: artHolder(std::move(std::move(ArtHolder)))
-		, slot(Slot)
-	{
-	}
-
-	template <typename T>
-	bool isHolder(const T *t) const
-	{
-		if(auto ptrToT = std::get<ConstTransitivePtr<T>>(artHolder))
-		{
-			return ptrToT == t;
-		}
-		return false;
-	}
-
-	DLL_LINKAGE void removeArtifact(); // BE CAREFUL, this operation modifies holder (gs)
-
-	DLL_LINKAGE const CArmedInstance *relatedObj() const; //hero or the stack owner
-	DLL_LINKAGE PlayerColor owningPlayer() const;
-	DLL_LINKAGE CArtifactSet *getHolderArtSet();
-	DLL_LINKAGE CBonusSystemNode *getHolderNode();
-	DLL_LINKAGE CArtifactSet *getHolderArtSet() const;
-	DLL_LINKAGE const CBonusSystemNode *getHolderNode() const;
 
-	DLL_LINKAGE const CArtifactInstance *getArt() const;
-	DLL_LINKAGE CArtifactInstance *getArt();
-	DLL_LINKAGE const ArtSlotInfo *getSlot() const;
-	template <typename Handler> void serialize(Handler &h, const int version)
+	template <typename Handler> void serialize(Handler & h, const int version)
 	{
 		h & artHolder;
 		h & slot;
+		h & creature;
 	}
 };
 

lib/networkPacks/NetPacksLib.cpp

@@ -1574,67 +1574,6 @@ struct GetBase
 	}
 };
 
-
-void ArtifactLocation::removeArtifact()
-{
-	CArtifactInstance *a = getArt();
-	assert(a);
-	a->removeFrom(*this);
-}
-
-const CArmedInstance * ArtifactLocation::relatedObj() const
-{
-	return std::visit(ObjectRetriever(), artHolder);
-}
-
-PlayerColor ArtifactLocation::owningPlayer() const
-{
-	const auto * obj = relatedObj();
-	return obj ? obj->tempOwner : PlayerColor::NEUTRAL;
-}
-
-CArtifactSet *ArtifactLocation::getHolderArtSet()
-{
-	return std::visit(GetBase<CArtifactSet>(), artHolder);
-}
-
-CBonusSystemNode *ArtifactLocation::getHolderNode()
-{
-	return std::visit(GetBase<CBonusSystemNode>(), artHolder);
-}
-
-const CArtifactInstance *ArtifactLocation::getArt() const
-{
-	const auto * s = getSlot();
-	if(s)
-		return s->getArt();
-	else
-		return nullptr;
-}
-
-CArtifactSet * ArtifactLocation::getHolderArtSet() const
-{
-	auto * t = const_cast<ArtifactLocation *>(this);
-	return t->getHolderArtSet();
-}
-
-const CBonusSystemNode * ArtifactLocation::getHolderNode() const
-{
-	auto * t = const_cast<ArtifactLocation *>(this);
-	return t->getHolderNode();
-}
-
-CArtifactInstance *ArtifactLocation::getArt()
-{
-	const ArtifactLocation *t = this;
-	return const_cast<CArtifactInstance*>(t->getArt());
-}
-
-const ArtSlotInfo *ArtifactLocation::getSlot() const
-{
-	return getHolderArtSet()->getSlot(slot);
-}
-
 void ChangeStackCount::applyGs(CGameState * gs)
 {
 	auto * srcObj = gs->getArmyInstance(army);
@@ -1709,39 +1648,40 @@ void RebalanceStacks::applyGs(CGameState * gs)
 
 	if(srcCount == count) //moving whole stack
 	{
-		[[maybe_unused]] const CCreature *c = dst.army->getCreature(dst.slot);
+		const auto c = dst.army->getCreature(dst.slot);
 
 		if(c) //stack at dest -> merge
 		{
 			assert(c == srcType);
-			auto alHere = ArtifactLocation (src.getStack(), ArtifactPosition::CREATURE_SLOT);
-			auto alDest = ArtifactLocation (dst.getStack(), ArtifactPosition::CREATURE_SLOT);
-			auto * artHere = alHere.getArt();
-			auto * artDest = alDest.getArt();
-			if (artHere)
+			
+			const auto srcHero = dynamic_cast<CGHeroInstance*>(src.army.get());
+			const auto dstHero = dynamic_cast<CGHeroInstance*>(dst.army.get());
+			auto srcStack = const_cast<CStackInstance*>(src.getStack());
+			auto dstStack = const_cast<CStackInstance*>(dst.getStack());
+			if(auto srcArt = srcStack->getArt(ArtifactPosition::CREATURE_SLOT))
 			{
-				if (alDest.getArt())
+				if(auto dstArt = dstStack->getArt(ArtifactPosition::CREATURE_SLOT))
 				{
-					auto * hero = dynamic_cast<CGHeroInstance *>(src.army.get());
-					auto dstSlot = ArtifactUtils::getArtBackpackPosition(hero, alDest.getArt()->getTypeId());
-					if(hero && dstSlot != ArtifactPosition::PRE_FIRST)
+					auto dstSlot = ArtifactUtils::getArtBackpackPosition(srcHero, dstArt->getTypeId());
+					if(srcHero && dstSlot != ArtifactPosition::PRE_FIRST)
 					{
-						artDest->move (alDest, ArtifactLocation (hero, dstSlot));
+						dstArt->move(*dstStack, ArtifactPosition::CREATURE_SLOT, *srcHero, dstSlot);
 					}
 					//else - artifact cna be lost :/
 					else
 					{
 						EraseArtifact ea;
-						ea.al = alDest;
+						ea.al = ArtifactLocation(dstHero->id, ArtifactPosition::CREATURE_SLOT);
+						ea.al.creature = dst.slot;
 						ea.applyGs(gs);
 						logNetwork->warn("Cannot move artifact! No free slots");
 					}
-					artHere->move (alHere, alDest);
+					srcArt->move(*srcStack, ArtifactPosition::CREATURE_SLOT, *dstStack, ArtifactPosition::CREATURE_SLOT);
 					//TODO: choose from dialog
 				}
 				else //just move to the other slot before stack gets erased
 				{
-					artHere->move (alHere, alDest);
+					srcArt->move(*srcStack, ArtifactPosition::CREATURE_SLOT, *dstStack, ArtifactPosition::CREATURE_SLOT);
 				}
 			}
 			if (stackExp)
@@ -1811,53 +1751,57 @@ void BulkSmartRebalanceStacks::applyGs(CGameState * gs)
 
 void PutArtifact::applyGs(CGameState *gs)
 {
-	assert(art->canBePutAt(al));
 	// Ensure that artifact has been correctly added via NewArtifact pack
 	assert(vstd::contains(gs->map->artInstances, art));
 	assert(!art->getParentNodes().empty());
-	art->putAt(al);
+	auto hero = gs->getHero(al.artHolder);
+	assert(hero);
+	assert(art && art->canBePutAt(hero, al.slot));
+	art->putAt(*hero, al.slot);
 }
 
 void EraseArtifact::applyGs(CGameState *gs)
 {
-	const auto * slot = al.getSlot();
+	const auto hero = gs->getHero(al.artHolder);
+	assert(hero);
+	const auto slot = hero->getSlot(al.slot);
 	if(slot->locked)
 	{
 		logGlobal->debug("Erasing locked artifact: %s", slot->artifact->artType->getNameTranslated());
 		DisassembledArtifact dis;
 		dis.al.artHolder = al.artHolder;
-		auto * aset = al.getHolderArtSet();
-		#ifndef NDEBUG
-		bool found = false;
-		#endif
-		for(auto& p : aset->artifactsWorn)
+		
+		for(auto & slotInfo : hero->artifactsWorn)
 		{
-			auto art = p.second.artifact;
+			auto art = slotInfo.second.artifact;
 			if(art->isCombined() && art->isPart(slot->artifact))
 			{
-				dis.al.slot = aset->getArtPos(art);
-				#ifndef NDEBUG
-				found = true;
-				#endif
+				dis.al.slot = hero->getArtPos(art);
 				break;
 			}
 		}
-		assert(found && "Failed to determine the assembly this locked artifact belongs to");
-		logGlobal->debug("Found the corresponding assembly: %s", dis.al.getSlot()->artifact->artType->getNameTranslated());
+		assert((dis.al.slot != ArtifactPosition::PRE_FIRST) && "Failed to determine the assembly this locked artifact belongs to");
+		logGlobal->debug("Found the corresponding assembly: %s", hero->getArt(dis.al.slot)->artType->getNameTranslated());
 		dis.applyGs(gs);
 	}
 	else
 	{
 		logGlobal->debug("Erasing artifact %s", slot->artifact->artType->getNameTranslated());
 	}
-	al.removeArtifact();
+	auto art = hero->getArt(al.slot);
+	assert(art);
+	art->removeFrom(*hero, al.slot);
 }
 
 void MoveArtifact::applyGs(CGameState * gs)
 {
-	CArtifactInstance * art = src.getArt();
-	assert(!ArtifactUtils::isSlotEquipment(dst.slot) || !dst.getArt());
-	art->move(src, dst);
+	auto srcHero = gs->getArtSet(src);
+	auto dstHero = gs->getArtSet(dst);
+	assert(srcHero);
+	assert(dstHero);
+	auto art = srcHero->getArt(src.slot);
+	assert(art && art->canBePutAt(dstHero, dst.slot));
+	art->move(*srcHero, src.slot, *dstHero, dst.slot);
 }
 
 void BulkMoveArtifacts::applyGs(CGameState * gs)
@@ -1869,8 +1813,8 @@ void BulkMoveArtifacts::applyGs(CGameState * gs)
 		BULK_PUT
 	};
 
-	auto bulkArtsOperation = [this](std::vector<LinkedSlots> & artsPack, 
-		CArtifactSet * artSet, EBulkArtsOp operation) -> void
+	auto bulkArtsOperation = [this, gs](std::vector<LinkedSlots> & artsPack, 
+		CArtifactSet & artSet, EBulkArtsOp operation) -> void
 	{
 		int numBackpackArtifactsMoved = 0;
 		for(auto & slot : artsPack)
@@ -1883,21 +1827,18 @@ void BulkMoveArtifacts::applyGs(CGameState * gs)
 			{
 				srcPos = ArtifactPosition(srcPos.num - numBackpackArtifactsMoved);
 			}
-			const auto * slotInfo = artSet->getSlot(srcPos);
-			assert(slotInfo);
-			auto * art = const_cast<CArtifactInstance *>(slotInfo->getArt());
+			auto * art = artSet.getArt(srcPos);
 			assert(art);
 			switch(operation)
 			{
 			case EBulkArtsOp::BULK_MOVE:
-				const_cast<CArtifactInstance*>(art)->move(
-					ArtifactLocation(srcArtHolder, srcPos), ArtifactLocation(dstArtHolder, slot.dstPos));
+				art->move(artSet, srcPos, *gs->getHero(dstArtHolder), slot.dstPos);
 				break;
 			case EBulkArtsOp::BULK_REMOVE:
-				art->removeFrom(ArtifactLocation(dstArtHolder, srcPos));
+				art->removeFrom(artSet, srcPos);
 				break;
 			case EBulkArtsOp::BULK_PUT:
-				art->putAt(ArtifactLocation(srcArtHolder, slot.dstPos));
+				art->putAt(*gs->getHero(srcArtHolder), slot.dstPos);
 				break;
 			default:
 				break;
@@ -1910,37 +1851,38 @@ void BulkMoveArtifacts::applyGs(CGameState * gs)
 		}
 	};
 	
+	auto * leftSet = gs->getArtSet(ArtifactLocation(srcArtHolder));
 	if(swap)
 	{
 		// Swap
-		auto * leftSet = getSrcHolderArtSet();
-		auto * rightSet = getDstHolderArtSet();
+		auto * rightSet = gs->getArtSet(ArtifactLocation(dstArtHolder));
 		CArtifactFittingSet artFittingSet(leftSet->bearerType());
 
 		artFittingSet.artifactsWorn = rightSet->artifactsWorn;
 		artFittingSet.artifactsInBackpack = rightSet->artifactsInBackpack;
 
-		bulkArtsOperation(artsPack1, rightSet, EBulkArtsOp::BULK_REMOVE);
-		bulkArtsOperation(artsPack0, leftSet, EBulkArtsOp::BULK_MOVE);
-		bulkArtsOperation(artsPack1, &artFittingSet, EBulkArtsOp::BULK_PUT);
+		bulkArtsOperation(artsPack1, *rightSet, EBulkArtsOp::BULK_REMOVE);
+		bulkArtsOperation(artsPack0, *leftSet, EBulkArtsOp::BULK_MOVE);
+		bulkArtsOperation(artsPack1, artFittingSet, EBulkArtsOp::BULK_PUT);
 	}
 	else
 	{
-		bulkArtsOperation(artsPack0, getSrcHolderArtSet(), EBulkArtsOp::BULK_MOVE);
+		bulkArtsOperation(artsPack0, *leftSet, EBulkArtsOp::BULK_MOVE);
 	}
 }
 
 void AssembledArtifact::applyGs(CGameState *gs)
 {
-	CArtifactSet * artSet = al.getHolderArtSet();
-	const CArtifactInstance * transformedArt = al.getArt();
+	auto hero = gs->getHero(al.artHolder);
+	assert(hero);
+	const auto transformedArt = hero->getArt(al.slot);
 	assert(transformedArt);
-	assert(vstd::contains_if(ArtifactUtils::assemblyPossibilities(artSet, transformedArt->getTypeId()), [=](const CArtifact * art)->bool
+	assert(vstd::contains_if(ArtifactUtils::assemblyPossibilities(hero, transformedArt->getTypeId()), [=](const CArtifact * art)->bool
 		{
 			return art->getId() == builtArt->getId();
 		}));
 
-	const auto transformedArtSlot = artSet->getSlotByInstance(transformedArt);
+	const auto transformedArtSlot = hero->getSlotByInstance(transformedArt);
 	auto * combinedArt = new CArtifactInstance(builtArt);
 	gs->map->addNewArtifactInstance(combinedArt);
 
@@ -1952,7 +1894,7 @@ void AssembledArtifact::applyGs(CGameState *gs)
 		if(transformedArt->getTypeId() == constituent->getId())
 			slot = transformedArtSlot;
 		else
-			slot = artSet->getArtPos(constituent->getId(), false, false);
+			slot = hero->getArtPos(constituent->getId(), false, false);
 
 		assert(slot != ArtifactPosition::PRE_FIRST);
 		slotsInvolved.emplace_back(slot);
@@ -1972,8 +1914,8 @@ void AssembledArtifact::applyGs(CGameState *gs)
 				break;
 			}
 
-			if(!vstd::contains(combinedArt->artType->getPossibleSlots().at(artSet->bearerType()), al.slot)
-				&& vstd::contains(combinedArt->artType->getPossibleSlots().at(artSet->bearerType()), slot))
+			if(!vstd::contains(combinedArt->artType->getPossibleSlots().at(hero->bearerType()), al.slot)
+				&& vstd::contains(combinedArt->artType->getPossibleSlots().at(hero->bearerType()), slot))
 				al.slot = slot;
 		}
 		else
@@ -1986,8 +1928,8 @@ void AssembledArtifact::applyGs(CGameState *gs)
 	// Delete parts from hero
 	for(const auto slot : slotsInvolved)
 	{
-		const auto constituentInstance = artSet->getArt(slot);
-		constituentInstance->removeFrom(ArtifactLocation(al.artHolder, slot));
+		const auto constituentInstance = hero->getArt(slot);
+		constituentInstance->removeFrom(*hero, slot);
 
 		if(ArtifactUtils::isSlotEquipment(al.slot) && slot != al.slot)
 			combinedArt->addPart(constituentInstance, slot);
@@ -1996,25 +1938,26 @@ void AssembledArtifact::applyGs(CGameState *gs)
 	}
 
 	// Put new combined artifacts
-	combinedArt->putAt(al);
+	combinedArt->putAt(*hero, al.slot);
 }
 
 void DisassembledArtifact::applyGs(CGameState *gs)
 {
-	auto * disassembled = al.getArt();
-	assert(disassembled);
+	auto hero = gs->getHero(al.artHolder);
+	assert(hero);
+	auto disassembledArt = hero->getArt(al.slot);
+	assert(disassembledArt);
 
-	auto parts = disassembled->getPartsInfo();
-	disassembled->removeFrom(al);
+	auto parts = disassembledArt->getPartsInfo();
+	disassembledArt->removeFrom(*hero, al.slot);
 	for(auto & part : parts)
 	{
-		ArtifactLocation partLoc = al;
 		// ArtifactPosition::PRE_FIRST is value of main part slot -> it'll replace combined artifact in its pos
-		partLoc.slot = (ArtifactUtils::isSlotEquipment(part.slot) ? part.slot : al.slot);
-		disassembled->detachFrom(*part.art);
-		part.art->putAt(partLoc);
+		auto slot = (ArtifactUtils::isSlotEquipment(part.slot) ? part.slot : al.slot);
+		disassembledArt->detachFrom(*part.art);
+		part.art->putAt(*hero, slot);
 	}
-	gs->map->eraseArtifactInstance(disassembled);
+	gs->map->eraseArtifactInstance(disassembledArt);
 }
 
 void HeroVisit::applyGs(CGameState *gs)
@@ -2603,14 +2546,4 @@ const CArtifactInstance * ArtSlotInfo::getArt() const
 	return artifact;
 }
 
-CArtifactSet * BulkMoveArtifacts::getSrcHolderArtSet()
-{
-	return std::visit(GetBase<CArtifactSet>(), srcArtHolder);
-}
-
-CArtifactSet * BulkMoveArtifacts::getDstHolderArtSet()
-{
-	return std::visit(GetBase<CArtifactSet>(), dstArtHolder);
-}
-
 VCMI_LIB_NAMESPACE_END

lib/networkPacks/PacksForClient.h

@@ -1060,14 +1060,16 @@ struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 		}
 	};
 
-	TArtHolder srcArtHolder;
-	TArtHolder dstArtHolder;
+	ObjectInstanceID srcArtHolder;
+	ObjectInstanceID dstArtHolder;
 
 	BulkMoveArtifacts()
-		: swap(false)
+		: srcArtHolder(ObjectInstanceID::NONE)
+		, dstArtHolder(ObjectInstanceID::NONE)
+		, swap(false)
 	{
 	}
-	BulkMoveArtifacts(TArtHolder srcArtHolder, TArtHolder dstArtHolder, bool swap)
+	BulkMoveArtifacts(const ObjectInstanceID srcArtHolder, const ObjectInstanceID dstArtHolder, bool swap)
 		: srcArtHolder(std::move(srcArtHolder))
 		, dstArtHolder(std::move(dstArtHolder))
 		, swap(swap)
@@ -1079,8 +1081,6 @@ struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 	std::vector<LinkedSlots> artsPack0;
 	std::vector<LinkedSlots> artsPack1;
 	bool swap;
-	CArtifactSet * getSrcHolderArtSet();
-	CArtifactSet * getDstHolderArtSet();
 
 	void visitTyped(ICPackVisitor & visitor) override;
 

server/CGameHandler.cpp

@@ -2270,7 +2270,7 @@ bool CGameHandler::buildStructure(ObjectInstanceID tid, BuildingID requestedID,
 				if(!t->visitingHero || !t->visitingHero->hasArt(ArtifactID::GRAIL))
 					COMPLAIN_RET("Cannot build this without grail!")
 				else
-					removeArtifact(ArtifactLocation(t->visitingHero, t->visitingHero->getArtPos(ArtifactID::GRAIL, false)));
+					removeArtifact(ArtifactLocation(t->visitingHero->id, t->visitingHero->getArtPos(ArtifactID::GRAIL, false)));
 			}
 			break;
 		}
@@ -2664,65 +2664,66 @@ bool CGameHandler::garrisonSwap(ObjectInstanceID tid)
 
 // With the amount of changes done to the function, it's more like transferArtifacts.
 // Function moves artifact from src to dst. If dst is not a backpack and is already occupied, old dst art goes to backpack and is replaced.
-bool CGameHandler::moveArtifact(const ArtifactLocation &al1, const ArtifactLocation &al2)
+bool CGameHandler::moveArtifact(const ArtifactLocation & src, const ArtifactLocation & dst)
 {
-	ArtifactLocation src = al1, dst = al2;
-	const PlayerColor srcPlayer = src.owningPlayer(), dstPlayer = dst.owningPlayer();
-	const CArmedInstance *srcObj = src.relatedObj(), *dstObj = dst.relatedObj();
+	ArtifactLocation srcLoc = src, dstLoc = dst;
+	const auto srcArtSet = getArtSet(srcLoc);
+	const auto dstArtSet = getArtSet(dstLoc);
+	assert(srcArtSet);
+	assert(dstArtSet);
 
 	// Make sure exchange is even possible between the two heroes.
-	if(!isAllowedExchange(srcObj->id, dstObj->id))
+	if(!isAllowedExchange(srcLoc.artHolder, dstLoc.artHolder))
 		COMPLAIN_RET("That heroes cannot make any exchange!");
 
-	const CArtifactInstance *srcArtifact = src.getArt();
-	const CArtifactInstance *destArtifact = dst.getArt();
-	const bool isDstSlotBackpack = ArtifactUtils::isSlotBackpack(dst.slot);
+	const auto srcArtifact = srcArtSet->getArt(srcLoc.slot);
+	const auto dstArtifact = dstArtSet->getArt(dstLoc.slot);
+	const bool isDstSlotBackpack = dstArtSet->bearerType() == ArtBearer::HERO ? ArtifactUtils::isSlotBackpack(dstLoc.slot) : false;
 
 	if(srcArtifact == nullptr)
 		COMPLAIN_RET("No artifact to move!");
-	if(destArtifact && srcPlayer != dstPlayer && !isDstSlotBackpack)
+	if(dstArtifact && getHero(src.artHolder)->getOwner() != getHero(dst.artHolder)->getOwner() && !isDstSlotBackpack)
 		COMPLAIN_RET("Can't touch artifact on hero of another player!");
 
 	// Check if src/dest slots are appropriate for the artifacts exchanged.
 	// Moving to the backpack is always allowed.
-	if((!srcArtifact || !isDstSlotBackpack)
-		&& srcArtifact && !srcArtifact->canBePutAt(dst, true))
+	if((!srcArtifact || !isDstSlotBackpack) && srcArtifact && !srcArtifact->canBePutAt(dstArtSet, dstLoc.slot, true))
 		COMPLAIN_RET("Cannot move artifact!");
 
-	auto srcSlot = src.getSlot();
-	auto dstSlot = dst.getSlot();
+	auto srcSlotInfo = srcArtSet->getSlot(srcLoc.slot);
+	auto dstSlotInfo = dstArtSet->getSlot(dstLoc.slot);
 
-	if((srcSlot && srcSlot->locked) || (dstSlot && dstSlot->locked))
+	if((srcSlotInfo && srcSlotInfo->locked) || (dstSlotInfo && dstSlotInfo->locked))
 		COMPLAIN_RET("Cannot move artifact locks.");
 
 	if(isDstSlotBackpack && srcArtifact->artType->isBig())
 		COMPLAIN_RET("Cannot put big artifacts in backpack!");
-	if(src.slot == ArtifactPosition::MACH4 || dst.slot == ArtifactPosition::MACH4)
+	if(srcLoc.slot == ArtifactPosition::MACH4 || dstLoc.slot == ArtifactPosition::MACH4)
 		COMPLAIN_RET("Cannot move catapult!");
 
 	if(isDstSlotBackpack)
 	{
-		if(!ArtifactUtils::isBackpackFreeSlots(dst.getHolderArtSet()))
+		if(!ArtifactUtils::isBackpackFreeSlots(dstArtSet))
 			COMPLAIN_RET("Backpack is full!");
-		vstd::amin(dst.slot, ArtifactPosition::BACKPACK_START + dst.getHolderArtSet()->artifactsInBackpack.size());
+		vstd::amin(dstLoc.slot, ArtifactPosition::BACKPACK_START + dstArtSet->artifactsInBackpack.size());
 	}
 
-	if(!(src.slot == ArtifactPosition::TRANSITION_POS && dst.slot == ArtifactPosition::TRANSITION_POS))
+	if(!(srcLoc.slot == ArtifactPosition::TRANSITION_POS && dstLoc.slot == ArtifactPosition::TRANSITION_POS))
 	{
-		if(src.slot == dst.slot && src.artHolder == dst.artHolder)
+		if(srcLoc.slot == dstLoc.slot && srcLoc.artHolder == dstLoc.artHolder)
 			COMPLAIN_RET("Won't move artifact: Dest same as source!");
 
 		// Check if dst slot is occupied
-		if(!isDstSlotBackpack && destArtifact)
+		if(!isDstSlotBackpack && dstArtifact)
 		{
 			// Previous artifact must be removed first
-			moveArtifact(dst, ArtifactLocation(dst.artHolder, ArtifactPosition::TRANSITION_POS));
+			moveArtifact(dstLoc, ArtifactLocation(dstLoc.artHolder, ArtifactPosition::TRANSITION_POS));
 		}
 
 		try
 		{
-			auto hero = std::get<ConstTransitivePtr<CGHeroInstance>>(dst.artHolder);
-			if(ArtifactUtils::checkSpellbookIsNeeded(hero, srcArtifact->artType->getId(), dst.slot))
+			auto hero = getHero(dst.artHolder);
+			if(ArtifactUtils::checkSpellbookIsNeeded(hero, srcArtifact->artType->getId(), dstLoc.slot))
 				giveHeroNewArtifact(hero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
 		}
 		catch(const std::bad_variant_access &)
@@ -2730,8 +2731,8 @@ bool CGameHandler::moveArtifact(const ArtifactLocation &al1, const ArtifactLocat
 			// object other than hero received an art - ignore
 		}
 
-		MoveArtifact ma(&src, &dst);
-		if(src.artHolder == dst.artHolder)
+		MoveArtifact ma(&srcLoc, &dstLoc);
+		if(srcLoc.artHolder == dstLoc.artHolder)
 			ma.askAssemble = false;
 		sendAndApply(&ma);
 	}
@@ -2749,8 +2750,7 @@ bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID
 	if((!psrcHero) || (!pdstHero))
 		COMPLAIN_RET("bulkMoveArtifacts: wrong hero's ID");
 
-	BulkMoveArtifacts ma(static_cast<ConstTransitivePtr<CGHeroInstance>>(psrcHero),
-		static_cast<ConstTransitivePtr<CGHeroInstance>>(pdstHero), swap);
+	BulkMoveArtifacts ma(srcHero, dstHero, swap);
 	auto & slotsSrcDst = ma.artsPack0;
 	auto & slotsDstSrc = ma.artsPack1;
 
@@ -2854,7 +2854,7 @@ bool CGameHandler::assembleArtifacts(ObjectInstanceID heroID, ArtifactPosition a
 	if(!destArtifact)
 		COMPLAIN_RET("assembleArtifacts: there is no such artifact instance!");
 
-	const auto dstLoc = ArtifactLocation(hero, artifactSlot);
+	const auto dstLoc = ArtifactLocation(hero->id, artifactSlot);
 	if(assemble)
 	{
 		CArtifact * combinedArt = VLC->arth->objects[assembleTo];
@@ -2864,8 +2864,8 @@ bool CGameHandler::assembleArtifacts(ObjectInstanceID heroID, ArtifactPosition a
 		{
 			COMPLAIN_RET("assembleArtifacts: It's impossible to assemble requested artifact!");
 		}
-		if(!destArtifact->canBePutAt(dstLoc)
-			&& !destArtifact->canBePutAt(ArtifactLocation(hero, ArtifactPosition::BACKPACK_START)))
+		if(!destArtifact->canBePutAt(hero, artifactSlot)
+			&& !destArtifact->canBePutAt(hero, ArtifactPosition::BACKPACK_START))
 		{
 			COMPLAIN_RET("assembleArtifacts: It's impossible to give the artholder requested artifact!");
 		}
@@ -2897,15 +2897,15 @@ bool CGameHandler::assembleArtifacts(ObjectInstanceID heroID, ArtifactPosition a
 
 bool CGameHandler::eraseArtifactByClient(const ArtifactLocation & al)
 {
-	const auto * hero = getHero(al.relatedObj()->id);
+	const auto * hero = getHero(al.artHolder);
 	if(hero == nullptr)
 		COMPLAIN_RET("eraseArtifactByClient: wrong hero's ID");
 
-	const auto * art = al.getArt();
+	const auto * art = hero->getArt(al.slot);
 	if(art == nullptr)
 		COMPLAIN_RET("Cannot remove artifact!");
 
-	if(al.getArt()->artType->canBePutAt(hero) || al.slot != ArtifactPosition::TRANSITION_POS)
+	if(art->canBePutAt(hero) || al.slot != ArtifactPosition::TRANSITION_POS)
 		COMPLAIN_RET("Illegal artifact removal request");
 
 	removeArtifact(al);
@@ -3012,7 +3012,7 @@ bool CGameHandler::sellArtifact(const IMarket *m, const CGHeroInstance *h, Artif
 	int resVal = 0, dump = 1;
 	m->getOffer(art->artType->getId(), rid, dump, resVal, EMarketMode::ARTIFACT_RESOURCE);
 
-	removeArtifact(ArtifactLocation(h, h->getArtPos(art)));
+	removeArtifact(ArtifactLocation(h->id, h->getArtPos(art)));
 	giveResource(h->tempOwner, rid, resVal);
 	return true;
 }
@@ -3750,8 +3750,8 @@ bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * h
 
 	for(int i = 0; i < slot.size(); ++i)
 	{
-		ArtifactLocation al(hero, slot[i]);
-		const CArtifactInstance * a = al.getArt();
+		ArtifactLocation al(hero->id, slot[i]);
+		const CArtifactInstance * a = hero->getArt(al.slot);
 
 		if(!a)
 		{
@@ -3963,7 +3963,7 @@ bool CGameHandler::swapStacks(const StackLocation & sl1, const StackLocation & s
 bool CGameHandler::giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos)
 {
 	assert(a->artType);
-	ArtifactLocation al(h, ArtifactPosition::PRE_FIRST);
+	ArtifactLocation al(h->id, ArtifactPosition::PRE_FIRST);
 
 	if(pos == ArtifactPosition::FIRST_AVAILABLE)
 	{
@@ -3978,7 +3978,7 @@ bool CGameHandler::giveHeroArtifact(const CGHeroInstance * h, const CArtifactIns
 		al.slot = pos;
 	}
 
-	if(a->canBePutAt(al))
+	if(a->canBePutAt(h, al.slot))
 		putArtifact(al, a);
 	else
 		return false;

server/CGameHandler.h

@@ -130,7 +130,7 @@ public:
 	bool giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) override;
 	void putArtifact(const ArtifactLocation &al, const CArtifactInstance *a) override;
 	void removeArtifact(const ArtifactLocation &al) override;
-	bool moveArtifact(const ArtifactLocation & al1, const ArtifactLocation & al2) override;
+	bool moveArtifact(const ArtifactLocation & src, const ArtifactLocation & dst) override;
 	bool bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap, bool equipped, bool backpack);
 	bool eraseArtifactByClient(const ArtifactLocation & al);
 	void synchronizeArtifactHandlerLists();

server/NetPacksServer.cpp

@@ -134,7 +134,7 @@ void ApplyGhNetPackVisitor::visitGarrisonHeroSwap(GarrisonHeroSwap & pack)
 
 void ApplyGhNetPackVisitor::visitExchangeArtifacts(ExchangeArtifacts & pack)
 {
-	gh.throwIfWrongPlayer(&pack, pack.src.owningPlayer()); //second hero can be ally
+	gh.throwIfWrongPlayer(&pack, gh.getOwner(pack.src.artHolder)); //second hero can be ally
 	result = gh.moveArtifact(pack.src, pack.dst);
 }
 
@@ -154,7 +154,7 @@ void ApplyGhNetPackVisitor::visitAssembleArtifacts(AssembleArtifacts & pack)
 
 void ApplyGhNetPackVisitor::visitEraseArtifactByClient(EraseArtifactByClient & pack)
 {
-	gh.throwIfWrongPlayer(&pack, pack.al.owningPlayer());
+	gh.throwIfWrongPlayer(&pack, gh.getOwner(pack.al.artHolder));
 	result = gh.eraseArtifactByClient(pack.al);
 }
 

server/battles/BattleResultProcessor.cpp

@@ -79,7 +79,7 @@ CasualtiesAfterBattle::CasualtiesAfterBattle(const CBattleInfoCallback & battle,
 				logGlobal->debug("War machine has been destroyed");
 				auto hero = dynamic_ptr_cast<CGHeroInstance> (army);
 				if (hero)
-					removedWarMachines.push_back (ArtifactLocation(hero, hero->getArtPos(warMachine, true)));
+					removedWarMachines.push_back (ArtifactLocation(hero->id, hero->getArtPos(warMachine, true)));
 				else
 					logGlobal->error("War machine in army without hero");
 			}
@@ -339,7 +339,7 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 			if(slot != ArtifactPosition::PRE_FIRST)
 			{
 				arts.push_back(art);
-				ma->dst = ArtifactLocation(finishingBattle->winnerHero, slot);
+				ma->dst = ArtifactLocation(finishingBattle->winnerHero->id, slot);
 				if(ArtifactUtils::isSlotBackpack(slot))
 					ma->askAssemble = false;
 				gameHandler->sendAndApply(ma);
@@ -353,8 +353,8 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 			for (auto artSlot : artifactsWorn)
 			{
 				MoveArtifact ma;
-				ma.src = ArtifactLocation(finishingBattle->loserHero, artSlot.first);
-				const CArtifactInstance * art =  ma.src.getArt();
+				ma.src = ArtifactLocation(finishingBattle->loserHero->id, artSlot.first);
+				const CArtifactInstance * art = finishingBattle->loserHero->getArt(artSlot.first);
 				if (art && !art->artType->isBig() &&
 					art->artType->getId() != ArtifactID::SPELLBOOK)
 						// don't move war machines or locked arts (spellbook)
@@ -366,9 +366,9 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 			{
 				//we assume that no big artifacts can be found
 				MoveArtifact ma;
-				ma.src = ArtifactLocation(finishingBattle->loserHero,
+				ma.src = ArtifactLocation(finishingBattle->loserHero->id,
 					ArtifactPosition(ArtifactPosition::BACKPACK_START + slotNumber)); //backpack automatically shifts arts to beginning
-				const CArtifactInstance * art =  ma.src.getArt();
+				const CArtifactInstance * art = finishingBattle->loserHero->getArt(ArtifactPosition::BACKPACK_START + slotNumber);
 				if (art->artType->getId() != ArtifactID::GRAIL) //grail may not be won
 				{
 					sendMoveArtifact(art, &ma);
@@ -380,8 +380,9 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 				for (auto artSlot : artifactsWorn)
 				{
 					MoveArtifact ma;
-					ma.src = ArtifactLocation(finishingBattle->loserHero->commander.get(), artSlot.first);
-					const CArtifactInstance * art =  ma.src.getArt();
+					ma.src = ArtifactLocation(finishingBattle->loserHero->id, artSlot.first);
+					ma.src.creature = finishingBattle->loserHero->findStack(finishingBattle->loserHero->commander);
+					const auto art = finishingBattle->loserHero->commander->getArt(artSlot.first);
 					if (art && !art->artType->isBig())
 					{
 						sendMoveArtifact(art, &ma);
@@ -395,11 +396,12 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 		for (auto armySlot : battle.battleGetArmyObject(loser)->stacks)
 		{
 			auto artifactsWorn = armySlot.second->artifactsWorn;
-			for (auto artSlot : artifactsWorn)
+			for(const auto & artSlot : artifactsWorn)
 			{
 				MoveArtifact ma;
-				ma.src = ArtifactLocation(armySlot.second, artSlot.first);
-				const CArtifactInstance * art =  ma.src.getArt();
+				ma.src = ArtifactLocation(finishingBattle->loserHero->id, artSlot.first);
+				ma.src.creature = finishingBattle->loserHero->findStack(finishingBattle->loserHero->commander);
+				const auto art = finishingBattle->loserHero->commander->getArt(artSlot.first);
 				if (art && !art->artType->isBig())
 				{
 					sendMoveArtifact(art, &ma);

server/queries/MapQueries.cpp

@@ -17,20 +17,6 @@
 #include "../../lib/networkPacks/PacksForServer.h"
 #include "../../lib/serializer/Cast.h"
 
-struct GetEngagedHeroIds
-{
-	std::optional<ObjectInstanceID> operator()(const ConstTransitivePtr<CGHeroInstance> & h) const
-	{
-		return h->id;
-	}
-	std::optional<ObjectInstanceID> operator()(const ConstTransitivePtr<CStackInstance> & s) const
-	{
-		if(s->armyObj && s->armyObj->ID == Obj::HERO)
-			return s->armyObj->id;
-		return std::optional<ObjectInstanceID>();
-	}
-};
-
 TimerPauseQuery::TimerPauseQuery(CGameHandler * owner, PlayerColor player):
 	CQuery(owner)
 {
@@ -127,12 +113,12 @@ bool CGarrisonDialogQuery::blocksPack(const CPack * pack) const
 
 	if(auto arts = dynamic_ptr_cast<ExchangeArtifacts>(pack))
 	{
-		if(auto id1 = std::visit(GetEngagedHeroIds(), arts->src.artHolder))
-			if(!vstd::contains(ourIds, *id1))
+		if(auto id1 = arts->src.artHolder)
+			if(!vstd::contains(ourIds, id1))
 				return true;
 
-		if(auto id2 = std::visit(GetEngagedHeroIds(), arts->dst.artHolder))
-			if(!vstd::contains(ourIds, *id2))
+		if(auto id2 = arts->dst.artHolder)
+			if(!vstd::contains(ourIds, id2))
 				return true;
 		return false;
 	}
@@ -144,8 +130,8 @@ bool CGarrisonDialogQuery::blocksPack(const CPack * pack) const
 
 	if(auto art = dynamic_ptr_cast<EraseArtifactByClient>(pack))
 	{
-		if (auto id = std::visit(GetEngagedHeroIds(), art->al.artHolder))
-			return !vstd::contains(ourIds, *id);
+		if(auto id = art->al.artHolder)
+			return !vstd::contains(ourIds, id);
 	}
 
 	if(auto dismiss = dynamic_ptr_cast<AssembleArtifacts>(pack))

test/game/CGameStateTest.cpp

@@ -240,7 +240,7 @@ TEST_F(CGameStateTest, issue2765)
 		gameCallback->sendAndApply(&na);
 
 		PutArtifact pack;
-		pack.al = ArtifactLocation(defender, ArtifactPosition::MACH1);
+		pack.al = ArtifactLocation(defender->id, ArtifactPosition::MACH1);
 		pack.art = a;
 		gameCallback->sendAndApply(&pack);
 	}
@@ -334,7 +334,7 @@ TEST_F(CGameStateTest, battleResurrection)
 		gameCallback->sendAndApply(&na);
 
 		PutArtifact pack;
-		pack.al = ArtifactLocation(attacker, ArtifactPosition::SPELLBOOK);
+		pack.al = ArtifactLocation(attacker->id, ArtifactPosition::SPELLBOOK);
 		pack.art = a;
 		gameCallback->sendAndApply(&pack);
 	}