Merge pull request #2749 from SoundSSGood/artifacts-related-fixes

Artifacts related fixes

CCallback.cpp

@@ -168,14 +168,10 @@ 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.
  */
-bool CCallback::assembleArtifacts (const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)
+void CCallback::assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)
 {
-	if (player != hero->tempOwner)
-		return false;
-
 	AssembleArtifacts aa(hero->id, artifactSlot, assemble, assembleTo);
 	sendRequest(&aa);
-	return true;
 }
 
 void CCallback::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap)

CCallback.h

@@ -89,7 +89,7 @@ public:
 	virtual int splitStack(const CArmedInstance *s1, const CArmedInstance *s2, SlotID p1, SlotID p2, int val)=0;//split creatures from the first stack
 	//virtual bool swapArtifacts(const CGHeroInstance * hero1, ui16 pos1, const CGHeroInstance * hero2, ui16 pos2)=0; //swaps artifacts between two given heroes
 	virtual bool swapArtifacts(const ArtifactLocation &l1, const ArtifactLocation &l2)=0;
-	virtual bool assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)=0;
+	virtual void assembleArtifacts(const CGHeroInstance * hero, 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;
@@ -170,7 +170,7 @@ public:
 	int bulkMergeStacks(ObjectInstanceID armyId, SlotID srcSlot) override;
 	bool dismissHero(const CGHeroInstance * hero) override;
 	bool swapArtifacts(const ArtifactLocation &l1, const ArtifactLocation &l2) override;
-	bool assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo) override;
+	void assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo) override;
 	void bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap) override;
 	void eraseArtifactByClient(const ArtifactLocation & al) override;
 	bool buildBuilding(const CGTownInstance *town, BuildingID buildingID) override;

client/CPlayerInterface.cpp

@@ -1288,7 +1288,7 @@ void CPlayerInterface::showGarrisonDialog( const CArmedInstance *up, const CGHer
  * into a combinational one on an artifact screen. Does not require the combination of
  * artifacts to be legal.
  */
-void CPlayerInterface::showArtifactAssemblyDialog(const Artifact * artifact, const Artifact * assembledArtifact, CFunctionList<bool()> onYes)
+void CPlayerInterface::showArtifactAssemblyDialog(const Artifact * artifact, const Artifact * assembledArtifact, CFunctionList<void()> onYes)
 {
 	std::string text = artifact->getDescriptionTranslated();
 	text += "\n\n";

client/CPlayerInterface.h

@@ -185,7 +185,7 @@ public: // public interface for use by client via LOCPLINT access
 	void showShipyardDialog(const IShipyard *obj) override; //obj may be town or shipyard;
 
 	void showHeroExchange(ObjectInstanceID hero1, ObjectInstanceID hero2);
-	void showArtifactAssemblyDialog(const Artifact * artifact, const Artifact * assembledArtifact, CFunctionList<bool()> onYes);
+	void showArtifactAssemblyDialog(const Artifact * artifact, const Artifact * assembledArtifact, CFunctionList<void()> onYes);
 	void waitWhileDialog(bool unlockPim = true);
 	void waitForAllDialogs(bool unlockPim = true);
 	void openTownWindow(const CGTownInstance * town); //shows townscreen

client/widgets/CArtifactHolder.cpp

@@ -260,17 +260,28 @@ bool ArtifactUtilsClient::askToAssemble(const CGHeroInstance * hero, const Artif
 	assert(hero);
 	const auto art = hero->getArt(slot);
 	assert(art);
-	auto assemblyPossibilities = ArtifactUtils::assemblyPossibilities(hero, art->getTypeId(), ArtifactUtils::isSlotEquipment(slot));
 
-	for(const auto combinedArt : assemblyPossibilities)
-	{
-		LOCPLINT->showArtifactAssemblyDialog(
-			art->artType,
-			combinedArt,
-			std::bind(&CCallback::assembleArtifacts, LOCPLINT->cb.get(), hero, slot, true, combinedArt->getId()));
+	if(hero->tempOwner != LOCPLINT->playerID)
+		return false;
 
-		if(assemblyPossibilities.size() > 2)
-			logGlobal->warn("More than one possibility of assembling on %s... taking only first", art->artType->getNameTranslated());
+	auto assemblyPossibilities = ArtifactUtils::assemblyPossibilities(hero, art->getTypeId(), ArtifactUtils::isSlotEquipment(slot));
+	if(!assemblyPossibilities.empty())
+	{
+		auto askThread = new boost::thread([hero, art, slot, assemblyPossibilities]() -> void
+			{
+				for(const auto combinedArt : assemblyPossibilities)
+				{
+					bool assembleConfirmed = false;
+					CFunctionList<void()> onYesHandlers([&assembleConfirmed]() -> void {assembleConfirmed = true; });
+					onYesHandlers += std::bind(&CCallback::assembleArtifacts, LOCPLINT->cb.get(), hero, slot, true, combinedArt->getId());
+
+					LOCPLINT->showArtifactAssemblyDialog(art->artType, combinedArt, onYesHandlers);
+					LOCPLINT->waitWhileDialog(false);
+					if(assembleConfirmed)
+						break;
+				}
+			});
+		askThread->detach();
 		return true;
 	}
 	return false;
@@ -282,6 +293,9 @@ bool ArtifactUtilsClient::askToDisassemble(const CGHeroInstance * hero, const Ar
 	const auto art = hero->getArt(slot);
 	assert(art);
 
+	if(hero->tempOwner != LOCPLINT->playerID)
+		return false;
+
 	if(art->isCombined())
 	{
 		if(ArtifactUtils::isSlotBackpack(slot) && !ArtifactUtils::isBackpackFreeSlots(hero, art->artType->getConstituents().size() - 1))

client/widgets/CArtifactsOfHeroAltar.cpp

@@ -58,7 +58,7 @@ void CArtifactsOfHeroAltar::updateBackpackSlots()
 void CArtifactsOfHeroAltar::scrollBackpack(int offset)
 {
 	CArtifactsOfHeroBase::scrollBackpackForArtSet(offset, visibleArtSet);
-	safeRedraw();
+	redraw();
 }
 
 void CArtifactsOfHeroAltar::pickUpArtifact(CHeroArtPlace & artPlace)

client/widgets/CArtifactsOfHeroBase.cpp

@@ -90,6 +90,8 @@ void CArtifactsOfHeroBase::init(
 	rightBackpackRoll = std::make_shared<CButton>(Point(632, 364), AnimationPath::builtin("hsbtns5.def"), CButton::tooltip(), [scrollHandler]() { scrollHandler(+1); }, EShortcut::MOVE_RIGHT);
 	leftBackpackRoll->block(true);
 	rightBackpackRoll->block(true);
+
+	setRedrawParent(true);
 }
 
 void CArtifactsOfHeroBase::leftClickArtPlace(CHeroArtPlace & artPlace)
@@ -127,7 +129,7 @@ const CGHeroInstance * CArtifactsOfHeroBase::getHero() const
 void CArtifactsOfHeroBase::scrollBackpack(int offset)
 {
 	scrollBackpackForArtSet(offset, *curHero);
-	safeRedraw();
+	redraw();
 }
 
 void CArtifactsOfHeroBase::scrollBackpackForArtSet(int offset, const CArtifactSet & artSet)
@@ -172,17 +174,6 @@ void CArtifactsOfHeroBase::scrollBackpackForArtSet(int offset, const CArtifactSe
 		rightBackpackRoll->block(!scrollingPossible);
 }
 
-void CArtifactsOfHeroBase::safeRedraw()
-{
-	if(isActive())
-	{
-		if(parent)
-			parent->redraw();
-		else
-			redraw();
-	}
-}
-
 void CArtifactsOfHeroBase::markPossibleSlots(const CArtifactInstance * art, bool assumeDestRemoved)
 {
 	for(auto artPlace : artWorn)

client/widgets/CArtifactsOfHeroBase.h

@@ -33,7 +33,6 @@ public:
 	virtual void setHero(const CGHeroInstance * hero);
 	virtual const CGHeroInstance * getHero() const;
 	virtual void scrollBackpack(int offset);
-	virtual void safeRedraw();
 	virtual void markPossibleSlots(const CArtifactInstance * art, bool assumeDestRemoved = true);
 	virtual void unmarkSlots();
 	virtual ArtPlacePtr getArtPlace(const ArtifactPosition & slot);

client/widgets/CArtifactsOfHeroKingdom.cpp

@@ -39,6 +39,8 @@ CArtifactsOfHeroKingdom::CArtifactsOfHeroKingdom(ArtPlaceMap ArtWorn, std::vecto
 	}
 	leftBackpackRoll->addCallback(std::bind(&CArtifactsOfHeroBase::scrollBackpack, this, -1));
 	rightBackpackRoll->addCallback(std::bind(&CArtifactsOfHeroBase::scrollBackpack, this, +1));
+
+	setRedrawParent(true);
 }
 
 CArtifactsOfHeroKingdom::~CArtifactsOfHeroKingdom()

client/widgets/CArtifactsOfHeroMarket.cpp

@@ -37,5 +37,5 @@ void CArtifactsOfHeroMarket::scrollBackpack(int offset)
 			}
 		}
 	}
-	safeRedraw();
+	redraw();
 }

client/widgets/CWindowWithArtifacts.cpp

@@ -261,7 +261,7 @@ void CWindowWithArtifacts::artifactMoved(const ArtifactLocation & srcLoc, const
 	auto pickedArtInst = std::get<const CArtifactInstance*>(curState.value());
 	assert(!pickedArtInst || destLoc.isHolder(std::get<const CGHeroInstance*>(curState.value())));
 
-	auto artifactMovedBody = [this, withRedraw, &srcLoc, &destLoc, &pickedArtInst](auto artSetWeak) -> void
+	auto artifactMovedBody = [this, withRedraw, &destLoc, &pickedArtInst](auto artSetWeak) -> void
 	{
 		auto artSetPtr = artSetWeak.lock();
 		if(artSetPtr)
@@ -269,12 +269,8 @@ void CWindowWithArtifacts::artifactMoved(const ArtifactLocation & srcLoc, const
 			const auto hero = artSetPtr->getHero();
 			if(pickedArtInst)
 			{
-				if(artSetPtr->isActive())
-				{
-					CCS->curh->dragAndDropCursor(AnimationPath::builtin("artifact"), pickedArtInst->artType->getIconIndex());
-					if(srcLoc.isHolder(hero) || !std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroKingdom>>)
-						artSetPtr->markPossibleSlots(pickedArtInst, hero->tempOwner == LOCPLINT->playerID);
-				}
+				markPossibleSlots();
+				CCS->curh->dragAndDropCursor(AnimationPath::builtin("artifact"), pickedArtInst->artType->getIconIndex());
 			}
 			else
 			{
@@ -296,7 +292,7 @@ void CWindowWithArtifacts::artifactMoved(const ArtifactLocation & srcLoc, const
 				{
 					cew->updateWidgets();
 				}
-				artSetPtr->safeRedraw();
+				artSetPtr->redraw();
 			}
 
 			// Make sure the status bar is updated so it does not display old text
@@ -319,6 +315,7 @@ void CWindowWithArtifacts::artifactDisassembled(const ArtifactLocation & artLoc)
 
 void CWindowWithArtifacts::artifactAssembled(const ArtifactLocation & artLoc)
 {
+	markPossibleSlots();
 	updateSlots(artLoc.slot);
 }
 
@@ -333,7 +330,7 @@ void CWindowWithArtifacts::updateSlots(const ArtifactPosition & slot)
 			else if(ArtifactUtils::isSlotBackpack(slot))
 				artSetPtr->updateBackpackSlots();
 
-			artSetPtr->safeRedraw();
+			artSetPtr->redraw();
 		}
 	};
 
@@ -395,3 +392,26 @@ std::optional<CWindowWithArtifacts::CArtifactsOfHeroPtr> CWindowWithArtifacts::f
 	}
 	return res;
 }
+
+void CWindowWithArtifacts::markPossibleSlots()
+{
+	if(const auto pickedArtInst = getPickedArtifact())
+	{
+		const auto heroArtOwner = getHeroPickedArtifact();
+		auto artifactAssembledBody = [&pickedArtInst, &heroArtOwner](auto artSetWeak) -> void
+		{
+			if(auto artSetPtr = artSetWeak.lock())
+			{
+				if(artSetPtr->isActive())
+				{
+					const auto hero = artSetPtr->getHero();
+					if(heroArtOwner == hero || !std::is_same_v<decltype(artSetWeak), std::weak_ptr<CArtifactsOfHeroKingdom>>)
+						artSetPtr->markPossibleSlots(pickedArtInst, hero->tempOwner == LOCPLINT->playerID);
+				}
+			}
+		};
+
+		for(auto artSetWeak : artSets)
+			std::visit(artifactAssembledBody, artSetWeak);
+	}
+}

client/widgets/CWindowWithArtifacts.h

@@ -46,4 +46,5 @@ private:
 	void updateSlots(const ArtifactPosition & slot);
 	std::optional<std::tuple<const CGHeroInstance*, const CArtifactInstance*>> getState();
 	std::optional<CArtifactsOfHeroPtr> findAOHbyRef(CArtifactsOfHeroBase & artsInst);
+	void markPossibleSlots();
 };

client/widgets/MiscWidgets.cpp

@@ -114,9 +114,10 @@ void LRClickableAreaWTextComp::showPopupWindow(const Point & cursorPosition)
 	LRClickableAreaWText::showPopupWindow(cursorPosition); //only if with-component variant not occurred
 }
 
-CHeroArea::CHeroArea(int x, int y, const CGHeroInstance * _hero)
+CHeroArea::CHeroArea(int x, int y, const CGHeroInstance * hero)
 	: CIntObject(LCLICK | HOVER),
-	hero(_hero)
+	hero(hero),
+	clickFunctor(nullptr)
 {
 	OBJECT_CONSTRUCTION_CAPTURING(255-DISPOSE);
 
@@ -126,13 +127,24 @@ CHeroArea::CHeroArea(int x, int y, const CGHeroInstance * _hero)
 	pos.h = 64;
 
 	if(hero)
+	{
 		portrait = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), hero->portrait);
+		clickFunctor = [hero]() -> void
+		{
+			LOCPLINT->openHeroWindow(hero);
+		};
+	}
+}
+
+void CHeroArea::addClickCallback(ClickFunctor callback)
+{
+	clickFunctor = callback;
 }
 
 void CHeroArea::clickPressed(const Point & cursorPosition)
 {
-	if(hero)
-		LOCPLINT->openHeroWindow(hero);
+	if(clickFunctor)
+		clickFunctor();
 }
 
 void CHeroArea::hover(bool on)

client/widgets/MiscWidgets.h

@@ -164,17 +164,21 @@ public:
 	~CMinorResDataBar();
 };
 
-/// Opens hero window by left-clicking on it
+/// Performs an action by left-clicking on it. Opens hero window by default
 class CHeroArea: public CIntObject
 {
-	const CGHeroInstance * hero;
-	std::shared_ptr<CAnimImage> portrait;
-
 public:
-	CHeroArea(int x, int y, const CGHeroInstance * _hero);
+	using ClickFunctor = std::function<void()>;
 
+	CHeroArea(int x, int y, const CGHeroInstance * hero);
+	void addClickCallback(ClickFunctor callback);
 	void clickPressed(const Point & cursorPosition) override;
 	void hover(bool on) override;
+private:
+	const CGHeroInstance * hero;
+	std::shared_ptr<CAnimImage> portrait;
+	ClickFunctor clickFunctor;
+	ClickFunctor showPopupHandler;
 };
 
 /// Can interact on left and right mouse clicks

client/windows/GUIClasses.cpp

@@ -884,9 +884,6 @@ CExchangeWindow::CExchangeWindow(ObjectInstanceID hero1, ObjectInstanceID hero2,
 		manaValues[leftRight] = std::make_shared<CLabel>(155 + 490 * leftRight, qeLayout ? 66 : 71, FONT_SMALL, ETextAlignment::CENTER, Colors::WHITE);
 	}
 
-	portraits[0] = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), heroInst[0]->portrait, 0, 257, 13);
-	portraits[1] = std::make_shared<CAnimImage>(AnimationPath::builtin("PortraitsLarge"), heroInst[1]->portrait, 0, 485, 13);
-
 	artifs[0] = std::make_shared<CArtifactsOfHeroMain>(Point(-334, 150));
 	artifs[0]->setHero(heroInst[0]);
 	artifs[1] = std::make_shared<CArtifactsOfHeroMain>(Point(98, 150));
@@ -933,7 +930,12 @@ CExchangeWindow::CExchangeWindow(ObjectInstanceID hero1, ObjectInstanceID hero2,
 			boost::algorithm::replace_first(secSkillAreas[b][g]->hoverText, "%s", CGI->skillh->getByIndex(skill)->getNameTranslated());
 		}
 
-		heroAreas[b] = std::make_shared<CHeroArea>(257 + 228*b, 13, hero);
+		heroAreas[b] = std::make_shared<CHeroArea>(257 + 228 * b, 13, hero);
+		heroAreas[b]->addClickCallback([this, hero]() -> void
+			{
+				if(getPickedArtifact() == nullptr)
+					LOCPLINT->openHeroWindow(hero);
+			});
 
 		specialtyAreas[b] = std::make_shared<LRClickableAreaWText>();
 		specialtyAreas[b]->pos = Rect(Point(pos.x + 69 + 490 * b, pos.y + (qeLayout ? 41 : 45)), Point(32, 32));

client/windows/GUIClasses.h

@@ -286,7 +286,6 @@ class CExchangeWindow : public CStatusbarWindow, public IGarrisonHolder, public
 	std::array<std::shared_ptr<CLabel>, 2> expValues;
 	std::array<std::shared_ptr<CAnimImage>, 2> manaImages;
 	std::array<std::shared_ptr<CLabel>, 2> manaValues;
-	std::array<std::shared_ptr<CAnimImage>, 2> portraits;
 
 	std::vector<std::shared_ptr<LRClickableAreaWTextComp>> primSkillAreas;
 	std::array<std::vector<std::shared_ptr<LRClickableAreaWTextComp>>, 2> secSkillAreas;