sacrifice routine

client/widgets/CWindowWithArtifacts.cpp

@@ -87,7 +87,7 @@ void CWindowWithArtifacts::clickPressedArtPlaceHero(CArtifactsOfHeroBase & artsI
 	if(artPlace.isLocked())
 		return;
 
-	const auto checkSpecialArts = [](const CGHeroInstance * hero, CArtPlace & artPlace) -> bool
+	const auto checkSpecialArts = [](const CGHeroInstance * hero, CArtPlace & artPlace, bool isTrade) -> bool
 	{
 		if(artPlace.getArt()->getTypeId() == ArtifactID::SPELLBOOK)
 		{
@@ -97,10 +97,19 @@ void CWindowWithArtifacts::clickPressedArtPlaceHero(CArtifactsOfHeroBase & artsI
 		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);
+			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(!artPlace.getArt()->artType->isTradable())
+			{
+				LOCPLINT->showInfoDialog(CGI->generaltexth->allTexts[21],
+					std::vector<std::shared_ptr<CComponent>>(1, std::make_shared<CComponent>(ComponentType::ARTIFACT, artPlace.getArt()->getTypeId())));
+				return false;
+			}
+		}
 		return true;
 	};
 
@@ -155,24 +164,21 @@ void CWindowWithArtifacts::clickPressedArtPlaceHero(CArtifactsOfHeroBase & artsI
 					if(isTransferAllowed)
 						artSetPtr->swapArtifacts(srcLoc, dstLoc);
 				}
-				else
+				else if(auto art = artPlace.getArt())
 				{
-					if(artPlace.getArt())
+					if(artSetPtr->getHero()->tempOwner == 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(hero, artPlace, std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroAltar>> ? true : false))
+							artSetPtr->pickUpArtifact(artPlace);
+					}
+					else
+					{
+						for(const auto artSlot : ArtifactUtils::unmovableSlots())
+							if(artPlace.slot == artSlot)
+							{
+								msg = CGI->generaltexth->allTexts[21];
+								break;
+							}
 					}
 				}
 
@@ -234,6 +240,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 +261,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>>)
 			{

client/widgets/markets/CAltarArtifacts.cpp

@@ -67,6 +67,13 @@ CAltarArtifacts::CAltarArtifacts(const IMarket * market, const CGHeroInstance *
 	CTradeBase::deselect();
 };
 
+CAltarArtifacts::~CAltarArtifacts()
+{
+	// 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
+	LOCPLINT->cb->bulkMoveArtifacts(altarId, heroArts->getHero()->id, false, true, true);
+}
+
 TExpType CAltarArtifacts::calcExpAltarForHero()
 {
 	TExpType expOnAltar(0);
@@ -79,23 +86,21 @@ TExpType CAltarArtifacts::calcExpAltarForHero()
 void CAltarArtifacts::makeDeal()
 {
 	std::vector<TradeItemSell> positions;
-	for(const auto art : tradeSlotsMap)
+	for(const auto & [artInst, altarSlot] : tradeSlotsMap)
 	{
-		positions.push_back(hero->getSlotByInstance(art.first));
+		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);
-	heroArts->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()
@@ -110,15 +115,8 @@ void CAltarArtifacts::sacrificeBackpack()
 
 void CAltarArtifacts::setSelectedArtifact(const CArtifactInstance * art)
 {
-	if(art)
-	{
-		selectedCost->setText(std::to_string(calcExpCost(art)));
-	}
-	else
-	{
-		selectedArt->setArtifact(nullptr);
-		selectedCost->setText("");
-	}
+	selectedArt->setArtifact(art);
+	selectedCost->setText(art == nullptr ? "" : std::to_string(calcExpCost(art)));
 }
 
 std::shared_ptr<CArtifactsOfHeroAltar> CAltarArtifacts::getAOHset() const
@@ -133,8 +131,8 @@ ObjectInstanceID CAltarArtifacts::getObjId() const
 
 void CAltarArtifacts::updateSlots()
 {
-	assert(altarArtifacts->artifactsInBackpack.size() <= 22);
-	assert(tradeSlotsMap.size() <= 22);
+	assert(altarArtifacts->artifactsInBackpack.size() <= GameConstants::ALTAR_ARTIFACTS_SLOTS);
+	assert(tradeSlotsMap.size() <= GameConstants::ALTAR_ARTIFACTS_SLOTS);
 	
 	auto slotsToAdd = tradeSlotsMap;
 	for(auto & altarSlot : items[0])
@@ -209,7 +207,7 @@ void CAltarArtifacts::onSlotClickPressed(const std::shared_ptr<CTradeableItem> &
 	}
 }
 
-TExpType CAltarArtifacts::calcExpCost(const CArtifactInstance* art)
+TExpType CAltarArtifacts::calcExpCost(const CArtifactInstance * art)
 {
 	int dmp = 0;
 	int expOfArt = 0;

client/widgets/markets/CAltarArtifacts.h

@@ -16,6 +16,7 @@ class CAltarArtifacts : public CExperienceAltar
 {
 public:
 	CAltarArtifacts(const IMarket * market, const CGHeroInstance * hero);
+	~CAltarArtifacts();
 	TExpType calcExpAltarForHero() override;
 	void makeDeal() override;
 	void sacrificeAll() override;

lib/CArtHandler.cpp

@@ -258,7 +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].push_back(ArtifactPosition::ALTAR);
+	possibleSlots[ArtBearer::ALTAR];
 }
 
 //This destructor should be placed here to avoid side effects
@@ -477,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;
 }
 
@@ -908,7 +911,7 @@ const ArtSlotInfo * CArtifactSet::getSlot(const ArtifactPosition & pos) const
 bool CArtifactSet::isPositionFree(const ArtifactPosition & pos, bool onlyLockCheck) const
 {
 	if(bearerType() == ArtBearer::ALTAR)
-		return artifactsInBackpack.size() < 22;
+		return artifactsInBackpack.size() < GameConstants::ALTAR_ARTIFACTS_SLOTS;
 
 	if(const ArtSlotInfo *s = getSlot(pos))
 		return (onlyLockCheck || !s->artifact) && !s->locked;

lib/constants/EntityIdentifiers.h

@@ -341,7 +341,7 @@ public:
 	enum Type
 	{
 		NO_OBJ = -1,
-		ALTAR_OF_SACRIFICE [[deprecated]] = 2,
+		ALTAR_OF_SACRIFICE = 2,
 		ANCHOR_POINT = 3,
 		ARENA = 4,
 		ARTIFACT = 5,

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/networkPacks/NetPacksLib.cpp

@@ -1776,35 +1776,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

server/CGameHandler.cpp

@@ -3436,10 +3436,7 @@ bool CGameHandler::isAllowedExchange(ObjectInstanceID id1, ObjectInstanceID id2)
 
 		if(o1->ID == Obj::ALTAR_OF_SACRIFICE)
 		{
-			const auto visitingHero = getVisitingHero(o1);
-			const auto thisHero = static_cast<const CGHeroInstance*>(o2);
-			if(visitingHero == thisHero)
-				return true;
+			return true;
 		}
 		if(o2->ID == Obj::ALTAR_OF_SACRIFICE)
 		{
@@ -3791,10 +3788,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]()
@@ -3802,34 +3804,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

@@ -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

@@ -141,7 +141,8 @@ void ApplyGhNetPackVisitor::visitExchangeArtifacts(ExchangeArtifacts & pack)
 
 void ApplyGhNetPackVisitor::visitBulkExchangeArtifacts(BulkExchangeArtifacts & pack)
 {
-	gh.throwIfWrongOwner(&pack, pack.srcHero);
+	if(gh.getObj(pack.srcHero)->ID != MapObjectID::ALTAR_OF_SACRIFICE)
+		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);
@@ -261,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;