fixed regressions

CCallback.cpp

@@ -173,9 +173,9 @@ bool CCallback::swapArtifacts(const ArtifactLocation &l1, const ArtifactLocation
  * @param assembleTo If assemble is true, this represents the artifact ID of the combination
  * artifact to assemble to. Otherwise it's not used.
  */
-void CCallback::assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)
+void CCallback::assembleArtifacts(const ObjectInstanceID & heroID, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)
 {
-	AssembleArtifacts aa(hero->id, artifactSlot, assemble, assembleTo);
+	AssembleArtifacts aa(heroID, artifactSlot, assemble, assembleTo);
 	sendRequest(&aa);
 }
 

CCallback.h

@@ -93,7 +93,7 @@ public:
 	virtual bool swapArtifacts(const ArtifactLocation &l1, const ArtifactLocation &l2)=0;
 	virtual void scrollBackpackArtifacts(ObjectInstanceID hero, bool left) = 0;
 	virtual void manageHeroCostume(ObjectInstanceID hero, size_t costumeIndex, bool saveCostume) = 0;
-	virtual void assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)=0;
+	virtual void assembleArtifacts(const ObjectInstanceID & heroID, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)=0;
 	virtual void eraseArtifactByClient(const ArtifactLocation & al)=0;
 	virtual bool dismissCreature(const CArmedInstance *obj, SlotID stackPos)=0;
 	virtual void endTurn()=0;
@@ -176,7 +176,7 @@ public:
 	int bulkMergeStacks(ObjectInstanceID armyId, SlotID srcSlot) override;
 	bool dismissHero(const CGHeroInstance * hero) override;
 	bool swapArtifacts(const ArtifactLocation &l1, const ArtifactLocation &l2) override;
-	void assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo) override;
+	void assembleArtifacts(const ObjectInstanceID & heroID, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo) override;
 	void bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap, bool equipped = true, bool backpack = true) override;
 	void scrollBackpackArtifacts(ObjectInstanceID hero, bool left) override;
 	void manageHeroCostume(ObjectInstanceID hero, size_t costumeIdx, bool saveCostume) override;

client/ArtifactsUIController.cpp

@@ -23,7 +23,22 @@
 #include "widgets/CComponent.h"
 #include "windows/CWindowWithArtifacts.h"
 
-bool ArtifactsUIController::askToAssemble(const CGHeroInstance * hero, const ArtifactPosition & slot, std::set<ArtifactID> * ignoredArtifacts)
+bool ArtifactsUIController::askToAssemble(const ArtifactLocation & al, const bool onlyEquipped, std::set<ArtifactID> * ignoredArtifacts)
+{
+	if(auto hero = LOCPLINT->cb->getHero(al.artHolder))
+	{
+		if(hero->getArt(al.slot) == nullptr)
+		{
+			logGlobal->error("artifact location %d points to nothing", al.slot.num);
+			return false;
+		}
+		return askToAssemble(hero, al.slot, onlyEquipped, ignoredArtifacts);
+	}
+	return false;
+}
+
+bool ArtifactsUIController::askToAssemble(const CGHeroInstance * hero, const ArtifactPosition & slot,
+	const bool onlyEquipped, std::set<ArtifactID> * ignoredArtifacts)
 {
 	assert(hero);
 	const auto art = hero->getArt(slot);
@@ -32,17 +47,23 @@ bool ArtifactsUIController::askToAssemble(const CGHeroInstance * hero, const Art
 	if(hero->tempOwner != LOCPLINT->playerID)
 		return false;
 
-	auto assemblyPossibilities = ArtifactUtils::assemblyPossibilities(hero, art->getTypeId(), true);
+	if(numOfArtsAskAssembleSession != 0)
+		numOfArtsAskAssembleSession--;
+	auto assemblyPossibilities = ArtifactUtils::assemblyPossibilities(hero, art->getTypeId(), onlyEquipped);
 	if(!assemblyPossibilities.empty())
 	{
 		auto askThread = new boost::thread([this, hero, art, slot, assemblyPossibilities, ignoredArtifacts]() -> void
 			{
-				boost::mutex::scoped_lock askLock(askAssembleArtifactsMutex);
+				boost::mutex::scoped_lock askLock(askAssembleArtifactMutex);
 				for(const auto combinedArt : assemblyPossibilities)
 				{
 					boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
-					if(ignoredArtifacts && vstd::contains(*ignoredArtifacts, combinedArt->getId()))
-						continue;
+					if(ignoredArtifacts)
+					{
+						if(vstd::contains(*ignoredArtifacts, combinedArt->getId()))
+							continue;
+						ignoredArtifacts->emplace(combinedArt->getId());
+					}
 
 					bool assembleConfirmed = false;
 					MetaString message = MetaString::createFromTextID(art->artType->getDescriptionTextID());
@@ -53,12 +74,10 @@ bool ArtifactsUIController::askToAssemble(const CGHeroInstance * hero, const Art
 					LOCPLINT->showYesNoDialog(message.toString(), [&assembleConfirmed, hero, slot, combinedArt]()
 						{
 							assembleConfirmed = true;
-							LOCPLINT->cb.get()->assembleArtifacts(hero, slot, true, combinedArt->getId());
+							LOCPLINT->cb.get()->assembleArtifacts(hero->id, slot, true, combinedArt->getId());
 						}, nullptr, {std::make_shared<CComponent>(ComponentType::ARTIFACT, combinedArt->getId())});
 
 					LOCPLINT->waitWhileDialog();
-					if(ignoredArtifacts)
-						ignoredArtifacts->emplace(combinedArt->getId());
 					if(assembleConfirmed)
 						break;
 				}
@@ -89,7 +108,7 @@ bool ArtifactsUIController::askToDisassemble(const CGHeroInstance * hero, const
 		message.appendRawString(CGI->generaltexth->allTexts[733]); // Do you wish to disassemble this artifact?
 		LOCPLINT->showYesNoDialog(message.toString(), [hero, slot]()
 			{
-				LOCPLINT->cb->assembleArtifacts(hero, slot, false, ArtifactID());
+				LOCPLINT->cb->assembleArtifacts(hero->id, slot, false, ArtifactID());
 			}, nullptr);
 		return true;
 	}
@@ -98,7 +117,7 @@ bool ArtifactsUIController::askToDisassemble(const CGHeroInstance * hero, const
 
 void ArtifactsUIController::artifactRemoved()
 {
-	for(auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
+	for(const auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
 		artWin->update();
 	LOCPLINT->waitWhileDialog();
 }
@@ -109,34 +128,34 @@ void ArtifactsUIController::artifactMoved()
 	if(numOfMovedArts != 0)
 		numOfMovedArts--;
 
-	for(auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
-		if(numOfMovedArts == 0)
+	if(numOfMovedArts == 0)
+		for(const auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
 		{
 			artWin->update();
-			artWin->redraw();
 		}
 	LOCPLINT->waitWhileDialog();
 }
 
-void ArtifactsUIController::bulkArtMovementStart(size_t numOfArts)
+void ArtifactsUIController::bulkArtMovementStart(size_t totalNumOfArts, size_t possibleAssemblyNumOfArts)
 {
-	numOfMovedArts = numOfArts;
+	assert(totalNumOfArts >= possibleAssemblyNumOfArts);
+	numOfMovedArts = totalNumOfArts;
 	if(numOfArtsAskAssembleSession == 0)
 	{
 		// Do not start the next session until the previous one is finished
-		numOfArtsAskAssembleSession = numOfArts; // TODO this is wrong
+		numOfArtsAskAssembleSession = possibleAssemblyNumOfArts;
 		ignoredArtifacts.clear();
 	}
 }
 
 void ArtifactsUIController::artifactAssembled()
 {
-	for(auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
+	for(const auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
 		artWin->update();
 }
 
 void ArtifactsUIController::artifactDisassembled()
 {
-	for(auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
+	for(const auto & artWin : GH.windows().findWindows<CWindowWithArtifacts>())
 		artWin->update();
 }

client/ArtifactsUIController.h

@@ -10,7 +10,8 @@
  #pragma once
 
 #include "../lib/constants/EntityIdentifiers.h"
- 
+#include "../lib/networkPacks/ArtifactLocation.h"
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 class CGHeroInstance;
@@ -24,14 +25,16 @@ public:
 	size_t numOfArtsAskAssembleSession;
 	std::set<ArtifactID> ignoredArtifacts;
 
-	boost::mutex askAssembleArtifactsMutex;
+	boost::mutex askAssembleArtifactMutex;
 
-	bool askToAssemble(const CGHeroInstance * hero, const ArtifactPosition & slot, std::set<ArtifactID> * ignoredArtifacts = nullptr);
+	bool askToAssemble(const ArtifactLocation & al, const bool onlyEquipped = false, std::set<ArtifactID> * ignoredArtifacts = nullptr);
+	bool askToAssemble(const CGHeroInstance * hero, const ArtifactPosition & slot, const bool onlyEquipped = false,
+		std::set<ArtifactID> * ignoredArtifacts = nullptr);
 	bool askToDisassemble(const CGHeroInstance * hero, const ArtifactPosition & slot);
 
 	void artifactRemoved();
 	void artifactMoved();
-	void bulkArtMovementStart(size_t numOfArts);
+	void bulkArtMovementStart(size_t totalNumOfArts, size_t possibleAssemblyNumOfArts);
 	void artifactAssembled();
 	void artifactDisassembled();
 };

client/CPlayerInterface.cpp

@@ -66,7 +66,6 @@
 
 #include "../CCallback.h"
 
-#include "../lib/CArtHandler.h"
 #include "../lib/CConfigHandler.h"
 #include "../lib/CGeneralTextHandler.h"
 #include "../lib/CHeroHandler.h"
@@ -1708,17 +1707,7 @@ void CPlayerInterface::showShipyardDialogOrProblemPopup(const IShipyard *obj)
 
 void CPlayerInterface::askToAssembleArtifact(const ArtifactLocation &al)
 {
-	if(auto hero = cb->getHero(al.artHolder))
-	{
-		if(hero->getArt(al.slot) == nullptr)
-		{
-			logGlobal->error("artifact location %d points to nothing", al.slot.num);
-			return;
-		}
-		askToAssemble(hero, al.slot, &ignoredArtifacts);
-		if(numOfArtsAskAssembleSession != 0)
-			numOfArtsAskAssembleSession--;
-	}
+	ArtifactsUIController::askToAssemble(al, true, &ignoredArtifacts);
 }
 
 void CPlayerInterface::artifactPut(const ArtifactLocation &al)
@@ -1741,9 +1730,9 @@ void CPlayerInterface::artifactMoved(const ArtifactLocation &src, const Artifact
 	ArtifactsUIController::artifactMoved();
 }
 
-void CPlayerInterface::bulkArtMovementStart(size_t numOfArts)
+void CPlayerInterface::bulkArtMovementStart(size_t totalNumOfArts, size_t possibleAssemblyNumOfArts)
 {
-	ArtifactsUIController::bulkArtMovementStart(numOfArts);
+	ArtifactsUIController::bulkArtMovementStart(totalNumOfArts, possibleAssemblyNumOfArts);
 }
 
 void CPlayerInterface::artifactAssembled(const ArtifactLocation &al)

client/CPlayerInterface.h

@@ -9,17 +9,16 @@
  */
 #pragma once
 
+#include "ArtifactsUIController.h"
+
 #include "../lib/FunctionList.h"
 #include "../lib/CGameInterface.h"
 #include "gui/CIntObject.h"
-#include "ArtifactsUIController.h"
 
 VCMI_LIB_NAMESPACE_BEGIN
 
 class Artifact;
-
 struct TryMoveHero;
-class CGHeroInstance;
 class CStack;
 class CCreature;
 struct CGPath;
@@ -97,7 +96,7 @@ protected: // Call-ins from server, should not be called directly, but only via
 	void artifactPut(const ArtifactLocation &al) override;
 	void artifactRemoved(const ArtifactLocation &al) override;
 	void artifactMoved(const ArtifactLocation &src, const ArtifactLocation &dst) override;
-	void bulkArtMovementStart(size_t numOfArts) override;
+	void bulkArtMovementStart(size_t totalNumOfArts, size_t possibleAssemblyNumOfArts) override;
 	void artifactAssembled(const ArtifactLocation &al) override;
 	void askToAssembleArtifact(const ArtifactLocation & dst) override;
 	void artifactDisassembled(const ArtifactLocation &al) override;

client/NetPacksClient.cpp

@@ -316,13 +316,25 @@ void ApplyClientNetPackVisitor::visitBulkMoveArtifacts(BulkMoveArtifacts & pack)
 		}
 	};
 
+	size_t possibleAssemblyNumOfArts = 0;
+	const auto calcPossibleAssemblyNumOfArts = [&possibleAssemblyNumOfArts](const auto & slotToMove)
+	{
+		if(slotToMove.askAssemble)
+			possibleAssemblyNumOfArts++;
+	};
+	std::for_each(pack.artsPack0.cbegin(), pack.artsPack0.cend(), calcPossibleAssemblyNumOfArts);
+	std::for_each(pack.artsPack1.cbegin(), pack.artsPack1.cend(), calcPossibleAssemblyNumOfArts);
+
+
 	// Begin a session of bulk movement of arts. It is not necessary but useful for the client optimization.
-	callInterfaceIfPresent(cl, pack.interfaceOwner, &IGameEventsReceiver::bulkArtMovementStart, pack.artsPack0.size() + pack.artsPack1.size());
+	callInterfaceIfPresent(cl, pack.interfaceOwner, &IGameEventsReceiver::bulkArtMovementStart,
+		pack.artsPack0.size() + pack.artsPack1.size(), possibleAssemblyNumOfArts);
 	if(pack.interfaceOwner != dstOwner)
-		callInterfaceIfPresent(cl, dstOwner, &IGameEventsReceiver::bulkArtMovementStart, pack.artsPack0.size() + pack.artsPack1.size());
+		callInterfaceIfPresent(cl, dstOwner, &IGameEventsReceiver::bulkArtMovementStart,
+			pack.artsPack0.size() + pack.artsPack1.size(), possibleAssemblyNumOfArts);
 
 	applyMove(pack.artsPack0);
-	if(pack.swap)
+	if(!pack.artsPack1.empty())
 		applyMove(pack.artsPack1);
 }
 

client/widgets/markets/CAltarArtifacts.cpp

@@ -146,7 +146,7 @@ void CAltarArtifacts::updateAltarSlots()
 	for(auto & tradeSlot : tradeSlotsMapNewArts)
 	{
 		assert(tradeSlot.first->id == -1);
-		assert(altarArtifacts->getSlotByInstance(tradeSlot.second) != ArtifactPosition::PRE_FIRST);
+		assert(altarArtifacts->getArtPos(tradeSlot.second) != ArtifactPosition::PRE_FIRST);
 		tradeSlot.first->setID(tradeSlot.second->getTypeId().num);
 		tradeSlot.first->subtitle->setText(std::to_string(calcExpCost(tradeSlot.second->getTypeId())));
 	}

client/windows/CWindowWithArtifacts.cpp

@@ -188,6 +188,7 @@ void CWindowWithArtifacts::update()
 		if(auto artPlace = artSet->getArtPlace(GH.getCursorPosition()))
 			artPlace->hover(true);
 	}
+	redraw();
 }
 
 void CWindowWithArtifacts::markPossibleSlots() const

lib/IGameEventsReceiver.h

@@ -90,7 +90,7 @@ public:
 	virtual void artifactAssembled(const ArtifactLocation &al){};
 	virtual void artifactDisassembled(const ArtifactLocation &al){};
 	virtual void artifactMoved(const ArtifactLocation &src, const ArtifactLocation &dst){};
-	virtual void bulkArtMovementStart(size_t numOfArts) {};
+	virtual void bulkArtMovementStart(size_t totalNumOfArts, size_t possibleAssemblyNumOfArts) {};
 	virtual void askToAssembleArtifact(const ArtifactLocation & dst) {};
 
 	virtual void heroVisit(const CGHeroInstance *visitor, const CGObjectInstance *visitedObj, bool start){};

lib/MetaString.h

@@ -49,7 +49,7 @@ private:
 		REPLACE_TEXTID_STRING,
 		REPLACE_NUMBER,
 		REPLACE_POSITIVE_NUMBER,
-                APPEND_EOL
+		APPEND_EOL
 	};
 
 	std::vector<EMessage> message;

lib/networkPacks/PacksForClient.h

@@ -1058,7 +1058,6 @@ struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 		: interfaceOwner(PlayerColor::NEUTRAL)
 		, srcArtHolder(ObjectInstanceID::NONE)
 		, dstArtHolder(ObjectInstanceID::NONE)
-		, swap(false)
 		, srcCreature(std::nullopt)
 		, dstCreature(std::nullopt)
 	{
@@ -1067,7 +1066,6 @@ struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 		: interfaceOwner(interfaceOwner)
 		, srcArtHolder(srcArtHolder)
 		, dstArtHolder(dstArtHolder)
-		, swap(swap)
 		, srcCreature(std::nullopt)
 		, dstCreature(std::nullopt)
 	{
@@ -1077,7 +1075,6 @@ struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 
 	std::vector<LinkedSlots> artsPack0;
 	std::vector<LinkedSlots> artsPack1;
-	bool swap;
 
 	void visitTyped(ICPackVisitor & visitor) override;
 
@@ -1090,7 +1087,6 @@ struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 		h & dstArtHolder;
 		h & srcCreature;
 		h & dstCreature;
-		h & swap;
 	}
 };
 

server/CGameHandler.cpp

@@ -2774,7 +2774,6 @@ bool CGameHandler::moveArtifact(const PlayerColor & player, const ArtifactLocati
 		// Previous artifact must be swapped
 		COMPLAIN_RET_FALSE_IF(!dstArtifact->canBePutAt(srcArtSet, src.slot, true), "Cannot swap artifacts!");
 		ma.artsPack1.push_back(BulkMoveArtifacts::LinkedSlots(dstSlot, src.slot));
-		ma.swap = true;
 	}
 
 	auto hero = getHero(dst.artHolder);

server/battles/BattleResultProcessor.cpp

@@ -460,7 +460,7 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 			iw.player = finishingBattle->winnerHero->tempOwner;
 			iw.text.appendLocalString(EMetaText::GENERAL_TXT, 30); //You have captured enemy artifact
 
-			for(auto art : arts) //TODO; separate function to display loot for various objects?
+			for(const auto art : arts) //TODO; separate function to display loot for various objects?
 			{
 				if(art->isScroll())
 					iw.components.emplace_back(ComponentType::SPELL_SCROLL, art->getScrollSpellID());