Merge pull request #3574 from SoundSSGood/altar-fixes

Artifacts altar related fixes

client/widgets/CArtifactsOfHeroAltar.cpp

@@ -20,14 +20,12 @@
 #include "../../lib/networkPacks/ArtifactLocation.h"
 
 CArtifactsOfHeroAltar::CArtifactsOfHeroAltar(const Point & position)
-	: visibleArtSet(ArtBearer::ArtBearer::HERO)
 {
 	init(
 		std::bind(&CArtifactsOfHeroBase::clickPrassedArtPlace, this, _1, _2),
 		std::bind(&CArtifactsOfHeroBase::showPopupArtPlace, this, _1, _2),
 		position,
 		std::bind(&CArtifactsOfHeroAltar::scrollBackpack, this, _1));
-	pickedArtFromSlot = ArtifactPosition::PRE_FIRST;
 
 	// The backpack is in the altar window above and to the right
 	for(auto & slot : backpack)
@@ -40,81 +38,3 @@ CArtifactsOfHeroAltar::~CArtifactsOfHeroAltar()
 {
 	putBackPickedArtifact();
 }
-
-void CArtifactsOfHeroAltar::setHero(const CGHeroInstance * hero)
-{
-	if(hero)
-	{
-		visibleArtSet.artifactsWorn = hero->artifactsWorn;
-		visibleArtSet.artifactsInBackpack = hero->artifactsInBackpack;
-		CArtifactsOfHeroBase::setHero(hero);
-	}
-}
-
-void CArtifactsOfHeroAltar::updateWornSlots()
-{
-	for(auto place : artWorn)
-		setSlotData(getArtPlace(place.first), place.first, visibleArtSet);
-}
-
-void CArtifactsOfHeroAltar::updateBackpackSlots()
-{
-	for(auto artPlace : backpack)
-		setSlotData(getArtPlace(artPlace->slot), artPlace->slot, visibleArtSet);
-}
-
-void CArtifactsOfHeroAltar::scrollBackpack(int offset)
-{
-	CArtifactsOfHeroBase::scrollBackpackForArtSet(offset, visibleArtSet);
-	redraw();
-}
-
-void CArtifactsOfHeroAltar::pickUpArtifact(CArtPlace & artPlace)
-{
-	if(const auto art = artPlace.getArt())
-	{
-		pickedArtFromSlot = artPlace.slot;
-		artPlace.setArtifact(nullptr);
-		deleteFromVisible(art);
-		if(ArtifactUtils::isSlotBackpack(pickedArtFromSlot))
-			pickedArtFromSlot = curHero->getSlotByInstance(art);
-		assert(pickedArtFromSlot != ArtifactPosition::PRE_FIRST);
-		LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, pickedArtFromSlot), ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
-	}
-}
-
-void CArtifactsOfHeroAltar::swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc)
-{
-	LOCPLINT->cb->swapArtifacts(srcLoc, dstLoc);
-	const auto pickedArtInst = curHero->getArt(ArtifactPosition::TRANSITION_POS);
-	assert(pickedArtInst);
-	visibleArtSet.putArtifact(dstLoc.slot, const_cast<CArtifactInstance*>(pickedArtInst));
-}
-
-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->id, slot), ArtifactLocation(curHero->id, pickedArtFromSlot));
-		pickedArtFromSlot = ArtifactPosition::PRE_FIRST;
-	}
-}
-
-void CArtifactsOfHeroAltar::deleteFromVisible(const CArtifactInstance * artInst)
-{
-	const auto slot = visibleArtSet.getSlotByInstance(artInst);
-	visibleArtSet.removeArtifact(slot);
-	if(ArtifactUtils::isSlotBackpack(slot))
-	{
-		scrollBackpackForArtSet(0, visibleArtSet);
-	}
-	else
-	{
-		for(const auto & part : artInst->getPartsInfo())
-		{
-			if(part.slot != ArtifactPosition::PRE_FIRST)
-				getArtPlace(part.slot)->setArtifact(nullptr);
-		}
-	}
-}

client/widgets/CArtifactsOfHeroAltar.h

@@ -16,18 +16,6 @@
 class CArtifactsOfHeroAltar : public CArtifactsOfHeroBase
 {
 public:
-	std::set<const CArtifactInstance*> artifactsOnAltar;
-	ArtifactPosition pickedArtFromSlot;
-	CArtifactFittingSet visibleArtSet;
-
 	CArtifactsOfHeroAltar(const Point & position);
 	~CArtifactsOfHeroAltar();
-	void setHero(const CGHeroInstance * hero) override;
-	void updateWornSlots() override;
-	void updateBackpackSlots() override;
-	void scrollBackpack(int offset) override;
-	void pickUpArtifact(CArtPlace & artPlace);
-	void swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc);
-	void pickedArtMoveToAltar(const ArtifactPosition & slot);
-	void deleteFromVisible(const CArtifactInstance * artInst);
 };

client/widgets/CArtifactsOfHeroBackpack.cpp

@@ -41,17 +41,6 @@ CArtifactsOfHeroBackpack::CArtifactsOfHeroBackpack()
 	initAOHbackpack(visibleCapacityMax, backpackCap < 0 || visibleCapacityMax < backpackCap);
 }
 
-void CArtifactsOfHeroBackpack::swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc)
-{
-	LOCPLINT->cb->swapArtifacts(srcLoc, dstLoc);
-}
-
-void CArtifactsOfHeroBackpack::pickUpArtifact(CArtPlace & artPlace)
-{
-	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, artPlace.slot),
-		ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
-}
-
 void CArtifactsOfHeroBackpack::scrollBackpack(int offset)
 {
 	if(backpackListBox)
@@ -60,7 +49,7 @@ void CArtifactsOfHeroBackpack::scrollBackpack(int offset)
 	auto slot = ArtifactPosition::BACKPACK_START + backpackPos;
 	for(auto artPlace : backpack)
 	{
-		setSlotData(artPlace, slot, *curHero);
+		setSlotData(artPlace, slot);
 		slot = slot + 1;
 	}
 	redraw();
@@ -188,9 +177,9 @@ void CArtifactsOfHeroQuickBackpack::setHero(const CGHeroInstance * hero)
 		initAOHbackpack(requiredSlots, false);
 		auto artPlace = backpack.begin();
 		for(auto & art : filteredArts)
-			setSlotData(*artPlace++, curHero->getSlotByInstance(art.second), *curHero);
+			setSlotData(*artPlace++, curHero->getSlotByInstance(art.second));
 		for(auto & art : filteredScrolls)
-			setSlotData(*artPlace++, curHero->getSlotByInstance(art.second), *curHero);
+			setSlotData(*artPlace++, curHero->getSlotByInstance(art.second));
 	}
 }
 
@@ -215,5 +204,5 @@ void CArtifactsOfHeroQuickBackpack::swapSelected()
 			break;
 		}
 	if(backpackLoc.slot != ArtifactPosition::PRE_FIRST && filterBySlot != ArtifactPosition::PRE_FIRST && curHero)
-		swapArtifacts(backpackLoc, ArtifactLocation(curHero->id, filterBySlot));
+		LOCPLINT->cb->swapArtifacts(backpackLoc, ArtifactLocation(curHero->id, filterBySlot));
 }

client/widgets/CArtifactsOfHeroBackpack.h

@@ -24,8 +24,6 @@ class CArtifactsOfHeroBackpack : public CArtifactsOfHeroBase
 public:
 	CArtifactsOfHeroBackpack(size_t slotsColumnsMax, size_t slotsRowsMax);
 	CArtifactsOfHeroBackpack();
-	void swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc);
-	void pickUpArtifact(CArtPlace & artPlace);
 	void scrollBackpack(int offset) override;
 	void updateBackpackSlots() override;
 	size_t getActiveSlotRowsNum();

client/widgets/CArtifactsOfHeroBase.cpp

@@ -123,7 +123,7 @@ void CArtifactsOfHeroBase::setHero(const CGHeroInstance * hero)
 
 	for(auto slot : artWorn)
 	{
-		setSlotData(slot.second, slot.first, *curHero);
+		setSlotData(slot.second, slot.first);
 	}
 	scrollBackpack(0);
 }
@@ -134,16 +134,10 @@ const CGHeroInstance * CArtifactsOfHeroBase::getHero() const
 }
 
 void CArtifactsOfHeroBase::scrollBackpack(int offset)
-{
-	scrollBackpackForArtSet(offset, *curHero);
-	redraw();
-}
-
-void CArtifactsOfHeroBase::scrollBackpackForArtSet(int offset, const CArtifactSet & artSet)
 {
 	// offset==-1 => to left; offset==1 => to right
 	using slotInc = std::function<ArtifactPosition(ArtifactPosition&)>;
-	auto artsInBackpack = static_cast<int>(artSet.artifactsInBackpack.size());
+	auto artsInBackpack = static_cast<int>(curHero->artifactsInBackpack.size());
 	auto scrollingPossible = artsInBackpack > backpack.size();
 
 	slotInc inc_straight = [](ArtifactPosition & slot) -> ArtifactPosition
@@ -170,7 +164,7 @@ void CArtifactsOfHeroBase::scrollBackpackForArtSet(int offset, const CArtifactSe
 	auto slot = ArtifactPosition(ArtifactPosition::BACKPACK_START + backpackPos);
 	for(auto artPlace : backpack)
 	{
-		setSlotData(artPlace, slot, artSet);
+		setSlotData(artPlace, slot);
 		slot = inc(slot);
 	}
 
@@ -179,6 +173,8 @@ void CArtifactsOfHeroBase::scrollBackpackForArtSet(int offset, const CArtifactSe
 		leftBackpackRoll->block(!scrollingPossible);
 	if(rightBackpackRoll)
 		rightBackpackRoll->block(!scrollingPossible);
+
+	redraw();
 }
 
 void CArtifactsOfHeroBase::markPossibleSlots(const CArtifactInstance * art, bool assumeDestRemoved)
@@ -235,7 +231,7 @@ void CArtifactsOfHeroBase::updateBackpackSlots()
 
 void CArtifactsOfHeroBase::updateSlot(const ArtifactPosition & slot)
 {
-	setSlotData(getArtPlace(slot), slot, *curHero);
+	setSlotData(getArtPlace(slot), slot);
 }
 
 const CArtifactInstance * CArtifactsOfHeroBase::getPickedArtifact()
@@ -256,7 +252,7 @@ void CArtifactsOfHeroBase::addGestureCallback(CArtPlace::ClickFunctor callback)
 	}
 }
 
-void CArtifactsOfHeroBase::setSlotData(ArtPlacePtr artPlace, const ArtifactPosition & slot, const CArtifactSet & artSet)
+void CArtifactsOfHeroBase::setSlotData(ArtPlacePtr artPlace, const ArtifactPosition & slot)
 {
 	// Spurious call from artifactMoved in attempt to update hidden backpack slot
 	if(!artPlace && ArtifactUtils::isSlotBackpack(slot))
@@ -265,7 +261,7 @@ void CArtifactsOfHeroBase::setSlotData(ArtPlacePtr artPlace, const ArtifactPosit
 	}
 
 	artPlace->slot = slot;
-	if(auto slotInfo = artSet.getSlot(slot))
+	if(auto slotInfo = curHero->getSlot(slot))
 	{
 		artPlace->lockSlot(slotInfo->locked);
 		artPlace->setArtifact(slotInfo->artifact);
@@ -278,7 +274,7 @@ void CArtifactsOfHeroBase::setSlotData(ArtPlacePtr artPlace, const ArtifactPosit
 				arts.insert(std::pair(combinedArt, 0));
 				for(const auto part : combinedArt->getConstituents())
 				{
-					if(artSet.hasArt(part->getId(), false))
+					if(curHero->hasArt(part->getId(), false))
 						arts.at(combinedArt)++;
 				}
 			}

client/widgets/CArtifactsOfHeroBase.h

@@ -69,6 +69,5 @@ protected:
 	virtual void init(CHeroArtPlace::ClickFunctor lClickCallback, CHeroArtPlace::ClickFunctor showPopupCallback,
 		const Point & position, BpackScrollFunctor scrollCallback);
 	// Assigns an artifacts to an artifact place depending on it's new slot ID
-	virtual void setSlotData(ArtPlacePtr artPlace, const ArtifactPosition & slot, const CArtifactSet & artSet);
-	virtual void scrollBackpackForArtSet(int offset, const CArtifactSet & artSet);
+	virtual void setSlotData(ArtPlacePtr artPlace, const ArtifactPosition & slot);
 };

client/widgets/CArtifactsOfHeroKingdom.cpp

@@ -50,15 +50,3 @@ CArtifactsOfHeroKingdom::~CArtifactsOfHeroKingdom()
 {
 	putBackPickedArtifact();
 }
-
-void CArtifactsOfHeroKingdom::swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc)
-{
-	LOCPLINT->cb->swapArtifacts(srcLoc, dstLoc);
-}
-
-void CArtifactsOfHeroKingdom::pickUpArtifact(CArtPlace & artPlace)
-{
-	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, artPlace.slot),
-		ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
-}
-

client/widgets/CArtifactsOfHeroKingdom.h

@@ -20,9 +20,8 @@ VCMI_LIB_NAMESPACE_END
 class CArtifactsOfHeroKingdom : public CArtifactsOfHeroBase
 {
 public:
+	CArtifactsOfHeroKingdom() = delete;
 	CArtifactsOfHeroKingdom(ArtPlaceMap ArtWorn, std::vector<ArtPlacePtr> Backpack,
 		std::shared_ptr<CButton> leftScroll, std::shared_ptr<CButton> rightScroll);
 	~CArtifactsOfHeroKingdom();
-	void swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc);
-	void pickUpArtifact(CArtPlace & artPlace);
-};
+};

client/widgets/CArtifactsOfHeroMain.cpp

@@ -30,14 +30,3 @@ CArtifactsOfHeroMain::~CArtifactsOfHeroMain()
 {
 	putBackPickedArtifact();
 }
-
-void CArtifactsOfHeroMain::swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc)
-{
-	LOCPLINT->cb->swapArtifacts(srcLoc, dstLoc);
-}
-
-void CArtifactsOfHeroMain::pickUpArtifact(CArtPlace & artPlace)
-{
-	LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero->id, artPlace.slot),
-		ArtifactLocation(curHero->id, ArtifactPosition::TRANSITION_POS));
-}

client/widgets/CArtifactsOfHeroMain.h

@@ -22,6 +22,4 @@ class CArtifactsOfHeroMain : public CArtifactsOfHeroBase
 public:
 	CArtifactsOfHeroMain(const Point & position);
 	~CArtifactsOfHeroMain();
-	void swapArtifacts(const ArtifactLocation & srcLoc, const ArtifactLocation & dstLoc);
-	void pickUpArtifact(CArtPlace & artPlace);
 };

client/widgets/CArtifactsOfHeroMarket.cpp

@@ -28,12 +28,12 @@ CArtifactsOfHeroMarket::CArtifactsOfHeroMarket(const Point & position)
 
 void CArtifactsOfHeroMarket::scrollBackpack(int offset)
 {
-	CArtifactsOfHeroBase::scrollBackpackForArtSet(offset, *curHero);
+	CArtifactsOfHeroBase::scrollBackpack(offset);
 
 	// We may have highlight on one of backpack artifacts
 	if(selectArtCallback)
 	{
-		for(auto & artPlace : backpack)
+		for(const auto & artPlace : backpack)
 		{
 			if(artPlace->isSelected())
 			{
@@ -42,5 +42,4 @@ void CArtifactsOfHeroMarket::scrollBackpack(int offset)
 			}
 		}
 	}
-	redraw();
 }

client/widgets/CWindowWithArtifacts.cpp

@@ -33,6 +33,8 @@
 #include "../../lib/networkPacks/ArtifactLocation.h"
 #include "../../lib/CConfigHandler.h"
 
+#include "../../CCallback.h"
+
 void CWindowWithArtifacts::addSet(CArtifactsOfHeroPtr artSet)
 {
 	artSets.emplace_back(artSet);
@@ -81,31 +83,14 @@ const CArtifactInstance * CWindowWithArtifacts::getPickedArtifact()
 
 void CWindowWithArtifacts::clickPressedArtPlaceHero(CArtifactsOfHeroBase & artsInst, CArtPlace & artPlace, const Point & cursorPosition)
 {
-	const auto artSetWeak = findAOHbyRef(artsInst);
-	assert(artSetWeak.has_value());
+	const auto artSet = findAOHbyRef(artsInst);
+	assert(artSet.has_value());
 
 	if(artPlace.isLocked())
 		return;
 
-	const auto checkSpecialArts = [](const CGHeroInstance * hero, CArtPlace & artPlace) -> bool
-	{
-		if(artPlace.getArt()->getTypeId() == ArtifactID::SPELLBOOK)
-		{
-			GH.windows().createAndPushWindow<CSpellWindow>(hero, LOCPLINT, LOCPLINT->battleInt.get());
-			return false;
-		}
-		if(artPlace.getArt()->getTypeId() == ArtifactID::CATAPULT)
-		{
-			// The Catapult must be equipped
-			std::vector<std::shared_ptr<CComponent>> catapult(1, std::make_shared<CComponent>(ComponentType::ARTIFACT, ArtifactID(ArtifactID::CATAPULT)));
-			LOCPLINT->showInfoDialog(CGI->generaltexth->allTexts[312], catapult);
-			return false;
-		}
-		return true;
-	};
-
 	std::visit(
-		[checkSpecialArts, this, &artPlace](auto artSetWeak) -> void
+		[this, &artPlace](auto artSetWeak) -> void
 		{
 			const auto artSetPtr = artSetWeak.lock();
 
@@ -153,26 +138,23 @@ void CWindowWithArtifacts::clickPressedArtPlaceHero(CArtifactsOfHeroBase & artsI
 							isTransferAllowed = false;
 					}
 					if(isTransferAllowed)
-						artSetPtr->swapArtifacts(srcLoc, dstLoc);
+						LOCPLINT->cb->swapArtifacts(srcLoc, dstLoc);
 				}
-				else
+				else if(auto art = artPlace.getArt())
 				{
-					if(artPlace.getArt())
+					if(artSetPtr->getHero()->getOwner() == LOCPLINT->playerID)
 					{
-						if(artSetPtr->getHero()->tempOwner == LOCPLINT->playerID)
-						{
-							if(checkSpecialArts(hero, artPlace))
-								artSetPtr->pickUpArtifact(artPlace);
-						}
-						else
-						{
-							for(const auto artSlot : ArtifactUtils::unmovableSlots())
-								if(artPlace.slot == artSlot)
-								{
-									msg = CGI->generaltexth->allTexts[21];
-									break;
-								}
-						}
+						if(checkSpecialArts(*art, hero, std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroAltar>> ? true : false))
+							LOCPLINT->cb->swapArtifacts(ArtifactLocation(artSetPtr->getHero()->id, artPlace.slot), ArtifactLocation(artSetPtr->getHero()->id, ArtifactPosition::TRANSITION_POS));
+					}
+					else
+					{
+						for(const auto artSlot : ArtifactUtils::unmovableSlots())
+							if(artPlace.slot == artSlot)
+							{
+								msg = CGI->generaltexth->allTexts[21];
+								break;
+							}
 					}
 				}
 
@@ -211,12 +193,11 @@ void CWindowWithArtifacts::clickPressedArtPlaceHero(CArtifactsOfHeroBase & artsI
 			else if constexpr(std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroQuickBackpack>>)
 			{
 				const auto hero = artSetPtr->getHero();
-				artSetPtr->swapArtifacts(ArtifactLocation(hero->id, artPlace.slot),
-					ArtifactLocation(hero->id, artSetPtr->getFilterSlot()));
+				LOCPLINT->cb->swapArtifacts(ArtifactLocation(hero->id, artPlace.slot), ArtifactLocation(hero->id, artSetPtr->getFilterSlot()));
 				if(closeCallback)
 					closeCallback();
 			}
-		}, artSetWeak.value());
+		}, artSet.value());
 }
 
 void CWindowWithArtifacts::showPopupArtPlaceHero(CArtifactsOfHeroBase & artsInst, CArtPlace & artPlace, const Point & cursorPosition)
@@ -234,6 +215,7 @@ void CWindowWithArtifacts::showPopupArtPlaceHero(CArtifactsOfHeroBase & artsInst
 
 			// Hero (Main, Exchange) window, Kingdom window, Backpack window right click handler
 			if constexpr(
+				std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroAltar>> ||
 				std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroMain>> ||
 				std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroKingdom>> ||
 				std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroBackpack>>)
@@ -254,7 +236,6 @@ void CWindowWithArtifacts::showPopupArtPlaceHero(CArtifactsOfHeroBase & artsInst
 			}
 			// Altar window, Market window right click handler
 			else if constexpr(
-				std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroAltar>> ||
 				std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroMarket>> ||
 				std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroQuickBackpack>>)
 			{
@@ -469,3 +450,31 @@ void CWindowWithArtifacts::markPossibleSlots()
 			std::visit(artifactAssembledBody, artSetWeak);
 	}
 }
+
+bool CWindowWithArtifacts::checkSpecialArts(const CArtifactInstance & artInst, const CGHeroInstance * hero, bool isTrade)
+{
+	const auto artId = artInst.getTypeId();
+	
+	if(artId == ArtifactID::SPELLBOOK)
+	{
+		GH.windows().createAndPushWindow<CSpellWindow>(hero, LOCPLINT, LOCPLINT->battleInt.get());
+		return false;
+	}
+	if(artId == ArtifactID::CATAPULT)
+	{
+		// The Catapult must be equipped
+		LOCPLINT->showInfoDialog(CGI->generaltexth->allTexts[312],
+			std::vector<std::shared_ptr<CComponent>>(1, std::make_shared<CComponent>(ComponentType::ARTIFACT, ArtifactID(ArtifactID::CATAPULT))));
+		return false;
+	}
+	if(isTrade)
+	{
+		if(!artInst.artType->isTradable())
+		{
+			LOCPLINT->showInfoDialog(CGI->generaltexth->allTexts[21],
+				std::vector<std::shared_ptr<CComponent>>(1, std::make_shared<CComponent>(ComponentType::ARTIFACT, artId)));
+			return false;
+		}
+	}
+	return true;
+}

client/widgets/CWindowWithArtifacts.h

@@ -50,4 +50,5 @@ protected:
 	std::optional<std::tuple<const CGHeroInstance*, const CArtifactInstance*>> getState();
 	std::optional<CArtifactsOfHeroPtr> findAOHbyRef(CArtifactsOfHeroBase & artsInst);
 	void markPossibleSlots();
+	bool checkSpecialArts(const CArtifactInstance & artInst, const CGHeroInstance * hero, bool isTrade);
 };

client/widgets/markets/CAltarArtifacts.cpp

@@ -12,7 +12,6 @@
 #include "CAltarArtifacts.h"
 
 #include "../../gui/CGuiHandler.h"
-#include "../../gui/CursorHandler.h"
 #include "../../widgets/Buttons.h"
 #include "../../widgets/TextControls.h"
 
@@ -31,6 +30,11 @@ CAltarArtifacts::CAltarArtifacts(const IMarket * market, const CGHeroInstance *
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255 - DISPOSE);
 
+	assert(market);
+	auto altarObj = dynamic_cast<const CGArtifactsAltar*>(market);
+	altarId = altarObj->id;
+	altarArtifacts = altarObj;
+
 	deal = std::make_shared<CButton>(dealButtonPos, AnimationPath::builtin("ALTSACR.DEF"),
 		CGI->generaltexth->zelp[585], [this]() {CAltarArtifacts::makeDeal(); });
 	labels.emplace_back(std::make_shared<CLabel>(450, 34, FONT_SMALL, ETextAlignment::CENTER, Colors::YELLOW, CGI->generaltexth->allTexts[477]));
@@ -46,8 +50,8 @@ CAltarArtifacts::CAltarArtifacts(const IMarket * market, const CGHeroInstance *
 		CGI->generaltexth->zelp[570], std::bind(&CAltarArtifacts::sacrificeBackpack, this));
 	sacrificeBackpackButton->block(hero->artifactsInBackpack.empty());
 
-	arts = std::make_shared<CArtifactsOfHeroAltar>(Point(-365, -11));
-	arts->setHero(hero);
+	heroArts = std::make_shared<CArtifactsOfHeroAltar>(Point(-365, -11));
+	heroArts->setHero(hero);
 
 	int slotNum = 0;
 	for(auto & altarSlotPos : posSlotsAltar)
@@ -65,151 +69,149 @@ CAltarArtifacts::CAltarArtifacts(const IMarket * market, const CGHeroInstance *
 
 TExpType CAltarArtifacts::calcExpAltarForHero()
 {
-	auto artifactsOfHero = std::dynamic_pointer_cast<CArtifactsOfHeroAltar>(arts);
 	TExpType expOnAltar(0);
-	for(const auto art : artifactsOfHero->artifactsOnAltar)
-	{
-		int dmp = 0;
-		int expOfArt = 0;
-		market->getOffer(art->getTypeId(), 0, dmp, expOfArt, EMarketMode::ARTIFACT_EXP);
-		expOnAltar += expOfArt;
-	}
-	auto resultExp = hero->calculateXp(expOnAltar);
-	expForHero->setText(std::to_string(resultExp));
-	return resultExp;
+	for(const auto & tradeSlot : tradeSlotsMap)
+		expOnAltar += calcExpCost(tradeSlot.first);
+	expForHero->setText(std::to_string(expOnAltar));
+	return expOnAltar;
 }
 
 void CAltarArtifacts::makeDeal()
 {
 	std::vector<TradeItemSell> positions;
-	for(const auto art : arts->artifactsOnAltar)
+	for(const auto & [artInst, altarSlot] : tradeSlotsMap)
 	{
-		positions.push_back(hero->getSlotByInstance(art));
+		positions.push_back(artInst->getId());
 	}
-	std::sort(positions.begin(), positions.end());
-	std::reverse(positions.begin(), positions.end());
-
 	LOCPLINT->cb->trade(market, EMarketMode::ARTIFACT_EXP, positions, std::vector<TradeItemBuy>(), std::vector<ui32>(), hero);
-	arts->artifactsOnAltar.clear();
 
+	tradeSlotsMap.clear();
+	// The event for removing artifacts from the altar will not be triggered. Therefore, we clean the altar immediately.
 	for(auto item : items[0])
 	{
 		item->setID(-1);
 		item->subtitle.clear();
 	}
-	deal->block(true);
 	calcExpAltarForHero();
+	deal->block(tradeSlotsMap.empty());
 }
 
 void CAltarArtifacts::sacrificeAll()
 {
-	std::vector<ConstTransitivePtr<CArtifactInstance>> artsForMove;
-	for(const auto & [slot, slotInfo] : arts->getHero()->artifactsWorn)
-	{
-		if(!slotInfo.locked && slotInfo.artifact->artType->isTradable())
-			artsForMove.emplace_back(slotInfo.artifact);
-	}
-	for(auto artInst : artsForMove)
-		moveArtToAltar(nullptr, artInst);
-	arts->updateWornSlots();
-	sacrificeBackpack();
+	LOCPLINT->cb->bulkMoveArtifacts(heroArts->getHero()->id, altarId, false, true, true);
 }
 
 void CAltarArtifacts::sacrificeBackpack()
 {
-	while(!arts->visibleArtSet.artifactsInBackpack.empty())
-	{
-		if(!putArtOnAltar(nullptr, arts->visibleArtSet.artifactsInBackpack[0].artifact))
-			break;
-	};
-	calcExpAltarForHero();
+	LOCPLINT->cb->bulkMoveArtifacts(heroArts->getHero()->id, altarId, false, false, true);
 }
 
 void CAltarArtifacts::setSelectedArtifact(const CArtifactInstance * art)
 {
-	if(art)
-	{
-		selectedArt->setArtifact(art);
-		int dmp = 0;
-		int exp = 0;
-		market->getOffer(art->getTypeId(), 0, dmp, exp, EMarketMode::ARTIFACT_EXP);
-		selectedCost->setText(std::to_string(hero->calculateXp(exp)));
-	}
-	else
-	{
-		selectedArt->setArtifact(nullptr);
-		selectedCost->setText("");
-	}
+	selectedArt->setArtifact(art);
+	selectedCost->setText(art == nullptr ? "" : std::to_string(calcExpCost(art)));
 }
 
-void CAltarArtifacts::moveArtToAltar(std::shared_ptr<CTradeableItem> altarSlot, const CArtifactInstance * art)
+std::shared_ptr<CArtifactsOfHeroAltar> CAltarArtifacts::getAOHset() const
 {
-	if(putArtOnAltar(altarSlot, art))
-	{
-		CCS->curh->dragAndDropCursor(nullptr);
-		arts->unmarkSlots();
-	}
+	return heroArts;
 }
 
-std::shared_ptr<CArtifactsOfHeroAltar> CAltarArtifacts::getAOHset() const
+ObjectInstanceID CAltarArtifacts::getObjId() const
 {
-	return arts;
+	return altarId;
 }
 
-bool CAltarArtifacts::putArtOnAltar(std::shared_ptr<CTradeableItem> altarSlot, const CArtifactInstance * art)
+void CAltarArtifacts::updateSlots()
 {
-	if(!art->artType->isTradable())
+	assert(altarArtifacts->artifactsInBackpack.size() <= GameConstants::ALTAR_ARTIFACTS_SLOTS);
+	assert(tradeSlotsMap.size() <= GameConstants::ALTAR_ARTIFACTS_SLOTS);
+	
+	auto slotsToAdd = tradeSlotsMap;
+	for(auto & altarSlot : items[0])
+		if(altarSlot->id != -1)
+		{
+			if(tradeSlotsMap.find(altarSlot->getArtInstance()) == tradeSlotsMap.end())
+			{
+				altarSlot->setID(-1);
+				altarSlot->subtitle.clear();
+			}
+			else
+			{
+				slotsToAdd.erase(altarSlot->getArtInstance());
+			}
+		}
+
+	for(auto & tradeSlot : slotsToAdd)
 	{
-		logGlobal->warn("Cannot put special artifact on altar!");
-		return false;
+		assert(tradeSlot.second->id == -1);
+		assert(altarArtifacts->getSlotByInstance(tradeSlot.first) != ArtifactPosition::PRE_FIRST);
+		tradeSlot.second->setArtInstance(tradeSlot.first);
+		tradeSlot.second->subtitle = std::to_string(calcExpCost(tradeSlot.first));
 	}
-
-	if(!altarSlot || altarSlot->id != -1)
+	for(auto & slotInfo : altarArtifacts->artifactsInBackpack)
 	{
-		int slotIndex = -1;
-		while(items[0][++slotIndex]->id >= 0 && slotIndex + 1 < items[0].size());
-		slotIndex = items[0][slotIndex]->id == -1 ? slotIndex : -1;
-		if(slotIndex < 0)
+		if(tradeSlotsMap.find(slotInfo.artifact) == tradeSlotsMap.end())
 		{
-			logGlobal->warn("No free slots on altar!");
-			return false;
+			for(auto & altarSlot : items[0])
+				if(altarSlot->id == -1)
+				{
+					altarSlot->setArtInstance(slotInfo.artifact);
+					altarSlot->subtitle = std::to_string(calcExpCost(slotInfo.artifact));
+					tradeSlotsMap.try_emplace(slotInfo.artifact, altarSlot);
+					break;
+				}
 		}
-		altarSlot = items[0][slotIndex];
 	}
+	calcExpAltarForHero();
+	deal->block(tradeSlotsMap.empty());
+}
 
-	int dmp = 0;
-	int exp = 0;
-	market->getOffer(art->artType->getId(), 0, dmp, exp, EMarketMode::ARTIFACT_EXP);
-	exp = static_cast<int>(hero->calculateXp(exp));
-
-	arts->artifactsOnAltar.insert(art);
-	altarSlot->setArtInstance(art);
-	altarSlot->subtitle = std::to_string(exp);
-
-	deal->block(false);
-	return true;
-};
+void CAltarArtifacts::putBackArtifacts()
+{
+	// TODO: If the backpack capacity limit is enabled, artifacts may remain on the altar.
+	// Perhaps should be erased in CGameHandler::objectVisitEnded if id of visited object will be available
+	if(!altarArtifacts->artifactsInBackpack.empty())
+		LOCPLINT->cb->bulkMoveArtifacts(altarId, heroArts->getHero()->id, false, true, true);
+}
 
-void CAltarArtifacts::onSlotClickPressed(const std::shared_ptr<CTradeableItem> & newSlot, std::shared_ptr<CTradeableItem> & hCurSlot)
+void CAltarArtifacts::onSlotClickPressed(const std::shared_ptr<CTradeableItem> & altarSlot, std::shared_ptr<CTradeableItem> & hCurSlot)
 {
-	const auto pickedArtInst = arts->getPickedArtifact();
-	if(pickedArtInst)
+	assert(altarSlot);
+
+	if(const auto pickedArtInst = heroArts->getPickedArtifact())
 	{
-		arts->pickedArtMoveToAltar(ArtifactPosition::TRANSITION_POS);
-		moveArtToAltar(newSlot, pickedArtInst);
+		if(pickedArtInst->canBePutAt(altarArtifacts))
+		{
+			if(pickedArtInst->artType->isTradable())
+			{
+				if(altarSlot->id == -1)
+					tradeSlotsMap.try_emplace(pickedArtInst, altarSlot);
+				deal->block(false);
+
+				LOCPLINT->cb->swapArtifacts(ArtifactLocation(heroArts->getHero()->id, ArtifactPosition::TRANSITION_POS),
+					ArtifactLocation(altarId, ArtifactPosition::ALTAR));
+			}
+			else
+			{
+				logGlobal->warn("Cannot put special artifact on altar!");
+				return;
+			}
+		}
 	}
-	else if(const CArtifactInstance * art = newSlot->getArtInstance())
+	else if(const CArtifactInstance * art = altarSlot->getArtInstance())
 	{
-		const auto hero = arts->getHero();
-		const auto slot = hero->getSlotByInstance(art);
+		const auto slot = altarArtifacts->getSlotByInstance(art);
 		assert(slot != ArtifactPosition::PRE_FIRST);
-		LOCPLINT->cb->swapArtifacts(ArtifactLocation(hero->id, slot),
-			ArtifactLocation(hero->id, ArtifactPosition::TRANSITION_POS));
-		arts->pickedArtFromSlot = slot;
-		arts->artifactsOnAltar.erase(art);
-		newSlot->setID(-1);
-		newSlot->subtitle.clear();
-		deal->block(!arts->artifactsOnAltar.size());
+		LOCPLINT->cb->swapArtifacts(ArtifactLocation(altarId, slot), ArtifactLocation(hero->id, ArtifactPosition::TRANSITION_POS));
+		tradeSlotsMap.erase(art);
 	}
-	calcExpAltarForHero();
+}
+
+TExpType CAltarArtifacts::calcExpCost(const CArtifactInstance * art)
+{
+	int dmp = 0;
+	int expOfArt = 0;
+	market->getOffer(art->getTypeId(), 0, dmp, expOfArt, EMarketMode::ARTIFACT_EXP);
+	return hero->calculateXp(expOfArt);
 }

client/widgets/markets/CAltarArtifacts.h

@@ -21,14 +21,19 @@ public:
 	void sacrificeAll() override;
 	void sacrificeBackpack();
 	void setSelectedArtifact(const CArtifactInstance * art);
-	void moveArtToAltar(std::shared_ptr<CTradeableItem>, const CArtifactInstance * art);
 	std::shared_ptr<CArtifactsOfHeroAltar> getAOHset() const;
+	ObjectInstanceID getObjId() const;
+	void updateSlots();
+	void putBackArtifacts();
 
 private:
+	ObjectInstanceID altarId;
+	const CArtifactSet * altarArtifacts;
 	std::shared_ptr<CArtPlace> selectedArt;
 	std::shared_ptr<CLabel> selectedCost;
 	std::shared_ptr<CButton> sacrificeBackpackButton;
-	std::shared_ptr<CArtifactsOfHeroAltar> arts;
+	std::shared_ptr<CArtifactsOfHeroAltar> heroArts;
+	std::map<const CArtifactInstance*, std::shared_ptr<CTradeableItem>> tradeSlotsMap;
 
 	const std::vector<Point> posSlotsAltar =
 	{
@@ -42,6 +47,6 @@ private:
 		Point(452, 333)
 	};
 
-	bool putArtOnAltar(std::shared_ptr<CTradeableItem> altarSlot, const CArtifactInstance * art);
-	void onSlotClickPressed(const std::shared_ptr<CTradeableItem> & newSlot, std::shared_ptr<CTradeableItem> & hCurSlot) override;
+	void onSlotClickPressed(const std::shared_ptr<CTradeableItem> & altarSlot, std::shared_ptr<CTradeableItem> & hCurSlot) override;
+	TExpType calcExpCost(const CArtifactInstance * art);
 };

client/windows/CAltarWindow.cpp

@@ -19,6 +19,7 @@
 
 #include "../CGameInfo.h"
 
+#include "../lib/networkPacks/ArtifactLocation.h"
 #include "../../lib/CGeneralTextHandler.h"
 #include "../../lib/CHeroHandler.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
@@ -78,14 +79,18 @@ void CAltarWindow::createAltarArtifacts(const IMarket * market, const CGHeroInst
 	auto altarArtifacts = std::make_shared<CAltarArtifacts>(market, hero);
 	altar = altarArtifacts;
 	artSets.clear();
-	addSetAndCallbacks(altarArtifacts->getAOHset());
+	addSetAndCallbacks(altarArtifacts->getAOHset()); altarArtifacts->putBackArtifacts();
 
 	changeModeButton = std::make_shared<CButton>(Point(516, 421), AnimationPath::builtin("ALTSACC.DEF"),
 		CGI->generaltexth->zelp[572], std::bind(&CAltarWindow::createAltarCreatures, this, market, hero));
 	if(altar->hero->getAlignment() == EAlignment::GOOD)
 		changeModeButton->block(true);
 	quitButton = std::make_shared<CButton>(Point(516, 520), AnimationPath::builtin("IOK6432.DEF"),
-		CGI->generaltexth->zelp[568], std::bind(&CAltarWindow::close, this), EShortcut::GLOBAL_RETURN);
+		CGI->generaltexth->zelp[568], [this, altarArtifacts]()
+		{
+			altarArtifacts->putBackArtifacts();
+			CAltarWindow::close();
+		}, EShortcut::GLOBAL_RETURN);
 	altar->setRedrawParent(true);
 	redraw();
 }
@@ -115,6 +120,9 @@ void CAltarWindow::artifactMoved(const ArtifactLocation & srcLoc, const Artifact
 
 	if(auto altarArtifacts = std::static_pointer_cast<CAltarArtifacts>(altar))
 	{
+		if(srcLoc.artHolder == altarArtifacts->getObjId() || destLoc.artHolder == altarArtifacts->getObjId())
+			altarArtifacts->updateSlots();
+
 		if(const auto pickedArt = getPickedArtifact())
 			altarArtifacts->setSelectedArtifact(pickedArt);
 		else

lib/ArtifactUtils.cpp

@@ -151,11 +151,16 @@ DLL_LINKAGE bool ArtifactUtils::isSlotEquipment(const ArtifactPosition & slot)
 
 DLL_LINKAGE bool ArtifactUtils::isBackpackFreeSlots(const CArtifactSet * target, const size_t reqSlots)
 {
-	const auto backpackCap = VLC->settings()->getInteger(EGameSettings::HEROES_BACKPACK_CAP);
-	if(backpackCap < 0)
-		return true;
+	if(target->bearerType() == ArtBearer::HERO)
+	{
+		const auto backpackCap = VLC->settings()->getInteger(EGameSettings::HEROES_BACKPACK_CAP);
+		if(backpackCap < 0)
+			return true;
+		else
+			return target->artifactsInBackpack.size() + reqSlots <= backpackCap;
+	}
 	else
-		return target->artifactsInBackpack.size() + reqSlots <= backpackCap;
+		return false;
 }
 
 DLL_LINKAGE std::vector<const CArtifact*> ArtifactUtils::assemblyPossibilities(

lib/CArtHandler.cpp

@@ -181,7 +181,7 @@ bool CArtifact::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, b
 {
 	auto simpleArtCanBePutAt = [this](const CArtifactSet * artSet, ArtifactPosition slot, bool assumeDestRemoved) -> bool
 	{
-		if(ArtifactUtils::isSlotBackpack(slot))
+		if(artSet->bearerType() == ArtBearer::HERO && ArtifactUtils::isSlotBackpack(slot))
 		{
 			if(isBig() || (!assumeDestRemoved && !ArtifactUtils::isBackpackFreeSlots(artSet)))
 				return false;
@@ -258,6 +258,7 @@ CArtifact::CArtifact()
 	possibleSlots[ArtBearer::HERO]; //we want to generate map entry even if it will be empty
 	possibleSlots[ArtBearer::CREATURE]; //we want to generate map entry even if it will be empty
 	possibleSlots[ArtBearer::COMMANDER];
+	possibleSlots[ArtBearer::ALTAR];
 }
 
 //This destructor should be placed here to avoid side effects
@@ -476,6 +477,9 @@ CArtifact * CArtHandler::loadFromJson(const std::string & scope, const JsonNode
 		}
 	});
 
+	if(art->isTradable())
+		art->possibleSlots.at(ArtBearer::ALTAR).push_back(ArtifactPosition::ALTAR);
+
 	return art;
 }
 
@@ -906,6 +910,9 @@ const ArtSlotInfo * CArtifactSet::getSlot(const ArtifactPosition & pos) const
 
 bool CArtifactSet::isPositionFree(const ArtifactPosition & pos, bool onlyLockCheck) const
 {
+	if(bearerType() == ArtBearer::ALTAR)
+		return artifactsInBackpack.size() < GameConstants::ALTAR_ARTIFACTS_SLOTS;
+
 	if(const ArtSlotInfo *s = getSlot(pos))
 		return (onlyLockCheck || !s->artifact) && !s->locked;
 

lib/CArtHandler.h

@@ -30,7 +30,8 @@ class JsonSerializeFormat;
 #define ART_BEARER_LIST \
 	ART_BEARER(HERO)\
 	ART_BEARER(CREATURE)\
-	ART_BEARER(COMMANDER)
+	ART_BEARER(COMMANDER)\
+	ART_BEARER(ALTAR)
 
 namespace ArtBearer
 {

lib/CGameInfoCallback.cpp

@@ -954,20 +954,9 @@ const CGObjectInstance * CGameInfoCallback::getObjInstance( ObjectInstanceID oid
 	return gs->map->objects[oid.num];
 }
 
-CArtifactSet * CGameInfoCallback::getArtSet(const ArtifactLocation & loc) const
+const 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;
-	}
+	return gs->getArtSet(loc);
 }
 
 std::vector<ObjectInstanceID> CGameInfoCallback::getVisibleTeleportObjects(std::vector<ObjectInstanceID> ids, PlayerColor player) const

lib/CGameInfoCallback.h

@@ -179,7 +179,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 CArtifactSet * getArtSet(const ArtifactLocation & loc) const;
 	//virtual const CGObjectInstance * getArmyInstance(ObjectInstanceID oid) const;
 
 	//objects

lib/IGameCallback.cpp

@@ -20,11 +20,13 @@
 #include "bonuses/Propagators.h"
 #include "bonuses/Updaters.h"
 
+#include "networkPacks/ArtifactLocation.h"
 #include "serializer/CLoadFile.h"
 #include "serializer/CSaveFile.h"
 #include "rmg/CMapGenOptions.h"
 #include "mapObjectConstructors/AObjectTypeHandler.h"
 #include "mapObjectConstructors/CObjectClassesHandler.h"
+#include "mapObjects/CGMarket.h"
 #include "mapObjects/CGTownInstance.h"
 #include "mapObjects/CObjectHandler.h"
 #include "mapObjects/CQuest.h"
@@ -268,6 +270,32 @@ CArmedInstance * CNonConstInfoCallback::getArmyInstance(const ObjectInstanceID &
 	return dynamic_cast<CArmedInstance *>(getObjInstance(oid));
 }
 
+CArtifactSet * CNonConstInfoCallback::getArtSet(const ArtifactLocation & loc)
+{
+	if(auto hero = 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;
+		}
+	}
+	else if(auto market = dynamic_cast<CGArtifactsAltar*>(getObjInstance(loc.artHolder)))
+	{
+		return market;
+	}
+	else
+	{
+		return nullptr;
+	}
+}
+
 bool IGameCallback::isVisitCoveredByAnotherQuery(const CGObjectInstance *obj, const CGHeroInstance *hero)
 {
 	//only server knows

lib/IGameCallback.h

@@ -146,6 +146,7 @@ public:
 	using CGameInfoCallback::getTile;
 	using CGameInfoCallback::getArtInstance;
 	using CGameInfoCallback::getObjInstance;
+	using CGameInfoCallback::getArtSet;
 
 	PlayerState * getPlayerState(const PlayerColor & color, bool verbose = true);
 	TeamState * getTeam(const TeamID & teamID); //get team by team ID
@@ -156,6 +157,7 @@ public:
 	CArtifactInstance * getArtInstance(const ArtifactInstanceID & aid);
 	CGObjectInstance * getObjInstance(const ObjectInstanceID & oid);
 	CArmedInstance * getArmyInstance(const ObjectInstanceID & oid);
+	CArtifactSet * getArtSet(const ArtifactLocation & loc);
 
 	virtual void updateEntity(Metatype metatype, int32_t index, const JsonNode & data) = 0;
 };

lib/constants/EntityIdentifiers.h

@@ -612,7 +612,10 @@ public:
 		CREATURE_SLOT = 0,
 		
 		// Commander
-		COMMANDER1 = 0, COMMANDER2, COMMANDER3, COMMANDER4, COMMANDER5, COMMANDER6
+		COMMANDER1 = 0, COMMANDER2, COMMANDER3, COMMANDER4, COMMANDER5, COMMANDER6,
+
+		// Altar
+		ALTAR = BACKPACK_START
 	};
 
 	static_assert(MISC5 < BACKPACK_START, "incorrect number of artifact slots");

lib/constants/NumericConstants.h

@@ -51,6 +51,7 @@ namespace GameConstants
 
 	constexpr ui32 BASE_MOVEMENT_COST = 100; //default cost for non-diagonal movement
 	constexpr int64_t PLAYER_RESOURCES_CAP = 1000 * 1000 * 1000;
+	constexpr int ALTAR_ARTIFACTS_SLOTS = 22;
 }
 
 VCMI_LIB_NAMESPACE_END

lib/mapObjectConstructors/CommonConstructors.cpp

@@ -238,6 +238,11 @@ CGMarket * MarketInstanceConstructor::createObject(IGameCallback * cb) const
 				return new CGUniversity(cb);
 		}
 	}
+	else if(marketModes.size() == 2)
+	{
+		if(vstd::contains(marketModes, EMarketMode::ARTIFACT_EXP))
+			return new CGArtifactsAltar(cb);
+	}
 	return new CGMarket(cb);
 }
 

lib/mapObjects/CGMarket.cpp

@@ -113,4 +113,9 @@ void CGUniversity::onHeroVisit(const CGHeroInstance * h) const
 	cb->showObjectWindow(this, EOpenWindowMode::UNIVERSITY_WINDOW, h, true);
 }
 
+ArtBearer::ArtBearer CGArtifactsAltar::bearerType() const
+{
+	return ArtBearer::ALTAR;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/mapObjects/CGMarket.h

@@ -11,6 +11,7 @@
 
 #include "CGObjectInstance.h"
 #include "IMarket.h"
+#include "../CArtHandler.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
@@ -80,4 +81,18 @@ public:
 	}
 };
 
+class DLL_LINKAGE CGArtifactsAltar : public CGMarket, public CArtifactSet
+{
+public:
+	using CGMarket::CGMarket;
+
+	ArtBearer::ArtBearer bearerType() const override;
+
+	template <typename Handler> void serialize(Handler & h)
+	{
+		h & static_cast<CGMarket&>(*this);
+		h & static_cast<CArtifactSet&>(*this);
+	}
+};
+
 VCMI_LIB_NAMESPACE_END

lib/networkPacks/NetPacksLib.cpp

@@ -1758,35 +1758,35 @@ void PutArtifact::applyGs(CGameState *gs)
 
 void EraseArtifact::applyGs(CGameState *gs)
 {
-	const auto hero = gs->getHero(al.artHolder);
-	assert(hero);
-	const auto slot = hero->getSlot(al.slot);
+	const auto artSet = gs->getArtSet(al.artHolder);
+	assert(artSet);
+	const auto slot = artSet->getSlot(al.slot);
 	if(slot->locked)
 	{
 		logGlobal->debug("Erasing locked artifact: %s", slot->artifact->artType->getNameTranslated());
 		DisassembledArtifact dis;
 		dis.al.artHolder = al.artHolder;
 		
-		for(auto & slotInfo : hero->artifactsWorn)
+		for(auto & slotInfo : artSet->artifactsWorn)
 		{
 			auto art = slotInfo.second.artifact;
 			if(art->isCombined() && art->isPart(slot->artifact))
 			{
-				dis.al.slot = hero->getArtPos(art);
+				dis.al.slot = artSet->getArtPos(art);
 				break;
 			}
 		}
 		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());
+		logGlobal->debug("Found the corresponding assembly: %s", artSet->getArt(dis.al.slot)->artType->getNameTranslated());
 		dis.applyGs(gs);
 	}
 	else
 	{
 		logGlobal->debug("Erasing artifact %s", slot->artifact->artType->getNameTranslated());
 	}
-	auto art = hero->getArt(al.slot);
+	auto art = artSet->getArt(al.slot);
 	assert(art);
-	art->removeFrom(*hero, al.slot);
+	art->removeFrom(*artSet, al.slot);
 }
 
 void MoveArtifact::applyGs(CGameState * gs)

lib/networkPacks/TradeItem.h

@@ -14,7 +14,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-using TradeItemSell = VariantIdentifier<GameResID, SlotID, ArtifactPosition, ArtifactInstanceID>;
+using TradeItemSell = VariantIdentifier<GameResID, SlotID, ArtifactInstanceID>;
 using TradeItemBuy = VariantIdentifier<GameResID, PlayerColor, ArtifactID, SecondarySkill>;
 
 VCMI_LIB_NAMESPACE_END

lib/registerTypes/RegisterTypesMapObjects.h

@@ -54,6 +54,7 @@ void registerTypesMapObjects(Serializer &s)
 	s.template registerType<CGObjectInstance, CGMarket>();
 		s.template registerType<CGMarket, CGBlackMarket>();
 		s.template registerType<CGMarket, CGUniversity>();
+		s.template registerType<CGMarket, CGArtifactsAltar>();
 	s.template registerType<CGObjectInstance, CGHeroPlaceholder>();
 
 	s.template registerType<CGObjectInstance, CArmedInstance>(); s.template registerType<CBonusSystemNode, CArmedInstance>(); s.template registerType<CCreatureSet, CArmedInstance>();

server/CGameHandler.cpp

@@ -2713,17 +2713,17 @@ bool CGameHandler::moveArtifact(const ArtifactLocation & src, const ArtifactLoca
 		COMPLAIN_RET("That heroes cannot make any exchange!");
 
 	const auto srcArtifact = srcArtSet->getArt(src.slot);
-	const auto dstArtifact = dstArtSet->getArt(dst.slot);
+	const bool isDstSlotOccupied = dstArtSet->bearerType() == ArtBearer::ALTAR ? false : dstArtSet->getArt(dst.slot) != nullptr;
 	const bool isDstSlotBackpack = dstArtSet->bearerType() == ArtBearer::HERO ? ArtifactUtils::isSlotBackpack(dst.slot) : false;
 
 	if(srcArtifact == nullptr)
 		COMPLAIN_RET("No artifact to move!");
-	if(dstArtifact && getHero(src.artHolder)->getOwner() != getHero(dst.artHolder)->getOwner() && !isDstSlotBackpack)
+	if(isDstSlotOccupied && getOwner(src.artHolder) != getOwner(dst.artHolder) && !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(dstArtSet, dst.slot, true))
+	if((!srcArtifact || !isDstSlotBackpack) && !srcArtifact->canBePutAt(dstArtSet, dst.slot, true))
 		COMPLAIN_RET("Cannot move artifact!");
 
 	auto srcSlotInfo = srcArtSet->getSlot(src.slot);
@@ -2749,7 +2749,7 @@ bool CGameHandler::moveArtifact(const ArtifactLocation & src, const ArtifactLoca
 	ma.dstCreature = dst.creature;
 	
 	// Check if dst slot is occupied
-	if(!isDstSlotBackpack && dstArtifact)
+	if(!isDstSlotBackpack && isDstSlotOccupied)
 	{
 		// Previous artifact must be removed
 		ma.artsPack1.push_back(BulkMoveArtifacts::LinkedSlots(dstSlot, src.slot));
@@ -2767,27 +2767,26 @@ bool CGameHandler::moveArtifact(const ArtifactLocation & src, const ArtifactLoca
 	return true;
 }
 
-bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap, bool equipped, bool backpack)
+bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcId, ObjectInstanceID dstId, bool swap, bool equipped, bool backpack)
 {
 	// Make sure exchange is even possible between the two heroes.
-	if(!isAllowedExchange(srcHero, dstHero))
+	if(!isAllowedExchange(srcId, dstId))
 		COMPLAIN_RET("That heroes cannot make any exchange!");
 
-	auto psrcHero = getHero(srcHero);
-	auto pdstHero = getHero(dstHero);
-	if((!psrcHero) || (!pdstHero))
+	auto psrcSet = getArtSet(srcId);
+	auto pdstSet = getArtSet(dstId);
+	if((!psrcSet) || (!pdstSet))
 		COMPLAIN_RET("bulkMoveArtifacts: wrong hero's ID");
 
-	BulkMoveArtifacts ma(srcHero, dstHero, swap);
+	BulkMoveArtifacts ma(srcId, dstId, swap);
 	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());
+	CArtifactFittingSet artFittingSet(pdstSet->bearerType());
 
-	auto moveArtifact = [this, &artFittingSet](const CArtifactInstance * artifact,
-		ArtifactPosition srcSlot, const CGHeroInstance * dstHero,
-		std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
+	auto moveArtifact = [this, &artFittingSet, dstId](const CArtifactInstance * artifact,
+		ArtifactPosition srcSlot, std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
 	{
 		assert(artifact);
 		auto dstSlot = ArtifactUtils::getArtAnyPosition(&artFittingSet, artifact->getTypeId());
@@ -2796,20 +2795,23 @@ bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID
 			artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
 			slots.push_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
 
-			if(ArtifactUtils::checkSpellbookIsNeeded(dstHero, artifact->getTypeId(), dstSlot))
-				giveHeroNewArtifact(dstHero, ArtifactID(ArtifactID::SPELLBOOK).toArtifact(), ArtifactPosition::SPELLBOOK);
+			// TODO Shouldn't be here. Possibly in callback after equipping the artifact
+			if(auto dstHero = getHero(dstId))
+			{
+				if(ArtifactUtils::checkSpellbookIsNeeded(dstHero, artifact->getTypeId(), dstSlot))
+					giveHeroNewArtifact(dstHero, ArtifactID(ArtifactID::SPELLBOOK).toArtifact(), ArtifactPosition::SPELLBOOK);
+			}
 		}
 	};
 
 	if(swap)
 	{
-		auto moveArtsWorn = [moveArtifact](const CGHeroInstance * srcHero, const CGHeroInstance * dstHero,
-			std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
+		auto moveArtsWorn = [moveArtifact](const CArtifactSet * srcArtSet, std::vector<BulkMoveArtifacts::LinkedSlots> & slots)
 		{
-			for(auto & artifact : srcHero->artifactsWorn)
+			for(auto & artifact : srcArtSet->artifactsWorn)
 			{
 				if(ArtifactUtils::isArtRemovable(artifact))
-					moveArtifact(artifact.second.getArt(), artifact.first, dstHero, slots);
+					moveArtifact(artifact.second.getArt(), artifact.first, slots);
 			}
 		};
 		auto moveArtsInBackpack = [](const CArtifactSet * artSet,
@@ -2824,41 +2826,41 @@ bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID
 		if(equipped)
 		{
 			// Move over artifacts that are worn srcHero -> dstHero
-			moveArtsWorn(psrcHero, pdstHero, slotsSrcDst);
+			moveArtsWorn(psrcSet, slotsSrcDst);
 			artFittingSet.artifactsWorn.clear();
 			// Move over artifacts that are worn dstHero -> srcHero
-			moveArtsWorn(pdstHero, psrcHero, slotsDstSrc);
+			moveArtsWorn(pdstSet, slotsDstSrc);
 		}
 		if(backpack)
 		{
 			// Move over artifacts that are in backpack srcHero -> dstHero
-			moveArtsInBackpack(psrcHero, slotsSrcDst);
+			moveArtsInBackpack(psrcSet, slotsSrcDst);
 			// Move over artifacts that are in backpack dstHero -> srcHero
-			moveArtsInBackpack(pdstHero, slotsDstSrc);
+			moveArtsInBackpack(pdstSet, slotsDstSrc);
 		}
 	}
 	else
 	{
-		artFittingSet.artifactsInBackpack = pdstHero->artifactsInBackpack;
-		artFittingSet.artifactsWorn = pdstHero->artifactsWorn;
+		artFittingSet.artifactsInBackpack = pdstSet->artifactsInBackpack;
+		artFittingSet.artifactsWorn = pdstSet->artifactsWorn;
 		if(equipped)
 		{
 			// Move over artifacts that are worn
-			for(auto & artInfo : psrcHero->artifactsWorn)
+			for(auto & artInfo : psrcSet->artifactsWorn)
 			{
 				if(ArtifactUtils::isArtRemovable(artInfo))
 				{
-					moveArtifact(psrcHero->getArt(artInfo.first), artInfo.first, pdstHero, slotsSrcDst);
+					moveArtifact(psrcSet->getArt(artInfo.first), artInfo.first, slotsSrcDst);
 				}
 			}
 		}
 		if(backpack)
 		{
 			// Move over artifacts that are in backpack
-			for(auto & slotInfo : psrcHero->artifactsInBackpack)
+			for(auto & slotInfo : psrcSet->artifactsInBackpack)
 			{
-				moveArtifact(psrcHero->getArt(psrcHero->getArtPos(slotInfo.artifact)),
-					psrcHero->getArtPos(slotInfo.artifact), pdstHero, slotsSrcDst);
+				moveArtifact(psrcSet->getArt(psrcSet->getArtPos(slotInfo.artifact)),
+					psrcSet->getArtPos(slotInfo.artifact), slotsSrcDst);
 			}
 		}
 	}
@@ -3432,6 +3434,12 @@ bool CGameHandler::isAllowedExchange(ObjectInstanceID id1, ObjectInstanceID id2)
 				return true;
 		}
 
+		auto market = dynamic_cast<const IMarket*>(o1);
+		if(market == nullptr)
+			market = dynamic_cast<const IMarket*>(o2);
+		if(market)
+			return market->allowsTrade(EMarketMode::ARTIFACT_EXP);
+
 		if (o1->ID == Obj::HERO && o2->ID == Obj::HERO)
 		{
 			const CGHeroInstance *h1 = static_cast<const CGHeroInstance*>(o1);
@@ -3775,10 +3783,15 @@ bool CGameHandler::sacrificeCreatures(const IMarket * market, const CGHeroInstan
 	return true;
 }
 
-bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, const std::vector<ArtifactPosition> & slot)
+bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, const std::vector<ArtifactInstanceID> & arts)
 {
 	if (!hero)
 		COMPLAIN_RET("You need hero to sacrifice artifact!");
+	if(hero->getAlignment() == EAlignment::EVIL)
+		COMPLAIN_RET("Evil hero can't sacrifice artifact!");
+
+	assert(m);
+	auto altarObj = dynamic_cast<const CGArtifactsAltar*>(m);
 
 	int expSum = 0;
 	auto finish = [this, &hero, &expSum]()
@@ -3786,34 +3799,28 @@ bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * h
 		giveExperience(hero, hero->calculateXp(expSum));
 	};
 
-	for(int i = 0; i < slot.size(); ++i)
+	for(const auto & artInstId : arts)
 	{
-		ArtifactLocation al(hero->id, slot[i]);
-		const CArtifactInstance * a = hero->getArt(al.slot);
-
-		if(!a)
+		if(auto art = altarObj->getArtByInstanceId(artInstId))
 		{
-			finish();
-			COMPLAIN_RET("Cannot find artifact to sacrifice!");
+			if(art->artType->isTradable())
+			{
+				int dmp;
+				int expToGive;
+				m->getOffer(art->getTypeId(), 0, dmp, expToGive, EMarketMode::ARTIFACT_EXP);
+				expSum += expToGive;
+				removeArtifact(ArtifactLocation(altarObj->id, altarObj->getSlotByInstance(art)));
+			}
+			else
+			{
+				COMPLAIN_RET("Cannot sacrifice not tradable artifact!");
+			}
 		}
-
-		const CArtifactInstance * art = hero->getArt(slot[i]);
-
-		if(!art)
+		else
 		{
 			finish();
-			COMPLAIN_RET("No artifact at position to sacrifice!");
+			COMPLAIN_RET("Cannot find artifact to sacrifice!");
 		}
-
-		si32 typId = art->artType->getId();
-		int dmp;
-		int expToGive;
-
-		m->getOffer(typId, 0, dmp, expToGive, EMarketMode::ARTIFACT_EXP);
-
-		expSum += expToGive;
-
-		removeArtifact(al);
 	}
 
 	finish();

server/CGameHandler.h

@@ -130,7 +130,7 @@ public:
 	bool putArtifact(const ArtifactLocation & al, const CArtifactInstance * art, std::optional<bool> askAssemble) override;
 	void removeArtifact(const ArtifactLocation &al) override;
 	bool moveArtifact(const ArtifactLocation & src, const ArtifactLocation & dst) override;
-	bool bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap, bool equipped, bool backpack);
+	bool bulkMoveArtifacts(ObjectInstanceID srcId, ObjectInstanceID dstId, bool swap, bool equipped, bool backpack);
 	bool eraseArtifactByClient(const ArtifactLocation & al);
 	void synchronizeArtifactHandlerLists();
 
@@ -262,7 +262,7 @@ public:
 	bool isPlayerOwns(CPackForServer * pack, ObjectInstanceID id);
 
 	void run(bool resume);
-	bool sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, const std::vector<ArtifactPosition> & slot);
+	bool sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, const std::vector<ArtifactInstanceID> & arts);
 	void spawnWanderingMonsters(CreatureID creatureID);
 
 	// Check for victory and loss conditions

server/NetPacksServer.cpp

@@ -134,13 +134,15 @@ void ApplyGhNetPackVisitor::visitGarrisonHeroSwap(GarrisonHeroSwap & pack)
 
 void ApplyGhNetPackVisitor::visitExchangeArtifacts(ExchangeArtifacts & pack)
 {
-	gh.throwIfWrongPlayer(&pack, gh.getOwner(pack.src.artHolder)); //second hero can be ally
+	if(gh.getHero(pack.src.artHolder))
+		gh.throwIfWrongPlayer(&pack, gh.getOwner(pack.src.artHolder)); //second hero can be ally
 	result = gh.moveArtifact(pack.src, pack.dst);
 }
 
 void ApplyGhNetPackVisitor::visitBulkExchangeArtifacts(BulkExchangeArtifacts & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.srcHero);
+	if(dynamic_cast<const IMarket*>(gh.getObj(pack.srcHero)) == nullptr)
+		gh.throwIfWrongOwner(&pack, pack.srcHero);
 	if(pack.swap)
 		gh.throwIfWrongOwner(&pack, pack.dstHero);
 	result = gh.bulkMoveArtifacts(pack.srcHero, pack.dstHero, pack.swap, pack.equipped, pack.backpack);
@@ -260,9 +262,9 @@ void ApplyGhNetPackVisitor::visitTradeOnMarketplace(TradeOnMarketplace & pack)
 	}
 	case EMarketMode::ARTIFACT_EXP:
 	{
-		std::vector<ArtifactPosition> positions;
-		for(auto const & slot : pack.r1)
-			positions.push_back(slot.as<ArtifactPosition>());
+		std::vector<ArtifactInstanceID> positions;
+		for(auto const & artInstId : pack.r1)
+			positions.push_back(artInstId.as<ArtifactInstanceID>());
 
 		result = gh.sacrificeArtifact(market, hero, positions);
 		return;

server/queries/MapQueries.cpp

@@ -200,6 +200,9 @@ bool OpenWindowQuery::blocksPack(const CPack *pack) const
 		if(dynamic_ptr_cast<ExchangeArtifacts>(pack) != nullptr)
 			return false;
 
+		if(dynamic_ptr_cast<BulkExchangeArtifacts>(pack) != nullptr)
+			return false;
+
 		if(dynamic_ptr_cast<AssembleArtifacts>(pack))
 			return false;