Merge pull request #1717 from SoundSSGood/backpack-limit-size

Introducing backpack size limit functionality

client/CPlayerInterface.cpp

@@ -1769,6 +1769,9 @@ void CPlayerInterface::tryDiggging(const CGHeroInstance * h)
 	case EDiggingStatus::WRONG_TERRAIN:
 		msgToShow = 60; ////Try looking on land!
 		break;
+	case EDiggingStatus::BACKPACK_IS_FULL:
+		msgToShow = 247; //Searching for the Grail is fruitless...
+		break;
 	default:
 		assert(0);
 	}
@@ -1894,7 +1897,7 @@ void CPlayerInterface::askToAssembleArtifact(const ArtifactLocation &al)
 							 al.slot.num);
 			return;
 		}
-		CHeroArtPlace::askToAssemble(art, al.slot, hero);
+		CHeroArtPlace::askToAssemble(hero, al.slot);
 	}
 }
 

client/Client.h

@@ -211,8 +211,8 @@ public:
 
 	void removeAfterVisit(const CGObjectInstance * object) override {};
 	bool swapGarrisonOnSiege(ObjectInstanceID tid) override {return false;};
-	void giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos) override {};
-	void giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) override {};
+	bool giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos) override {return false;}
+	bool giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) override {return false;}
 	void putArtifact(const ArtifactLocation & al, const CArtifactInstance * a) override {};
 	void removeArtifact(const ArtifactLocation & al) override {};
 	bool moveArtifact(const ArtifactLocation & al1, const ArtifactLocation & al2) override {return false;};

client/widgets/CArtifactHolder.cpp

@@ -101,9 +101,9 @@ void CHeroArtPlace::selectSlot(bool on)
 void CHeroArtPlace::clickLeft(tribool down, bool previousState)
 {
 	//LRClickableAreaWTextComp::clickLeft(down);
-	bool inBackpack = slotID >= GameConstants::BACKPACK_START,
-		srcInBackpack = ourOwner->commonInfo->src.slotID >= GameConstants::BACKPACK_START,
-		srcInSameHero = ourOwner->commonInfo->src.AOH == ourOwner;
+	bool inBackpack = ArtifactUtils::isSlotBackpack(slotID);
+	bool srcInBackpack = ArtifactUtils::isSlotBackpack(ourOwner->commonInfo->src.slotID);
+	bool srcInSameHero = ourOwner->commonInfo->src.AOH == ourOwner;
 
 	if(ourOwner->highlightModeCallback && ourArt)
 	{
@@ -171,16 +171,16 @@ void CHeroArtPlace::clickLeft(tribool down, bool previousState)
 					else
 					{
 						setMeAsDest();
-						vstd::amin(ourOwner->commonInfo->dst.slotID, ArtifactPosition(
-							(si32)ourOwner->curHero->artifactsInBackpack.size() + GameConstants::BACKPACK_START));
-						if(srcInBackpack && srcInSameHero)
+						vstd::amin(ourOwner->commonInfo->dst.slotID, ourOwner->curHero->artifactsInBackpack.size() + GameConstants::BACKPACK_START);
+						if(ArtifactUtils::isBackpackFreeSlots(ourOwner->curHero))
 						{
-							if(!ourArt								//cannot move from backpack to AFTER backpack -> combined with vstd::amin above it will guarantee that dest is at most the last artifact
-								|| ourOwner->commonInfo->src.slotID < ourOwner->commonInfo->dst.slotID) //rearranging arts in backpack after taking src artifact, the dest id will be shifted
-								vstd::advance(ourOwner->commonInfo->dst.slotID, -1);
+							if(!srcInSameHero || ourOwner->commonInfo->dst.slotID != ourOwner->commonInfo->src.slotID)
+								ourOwner->realizeCurrentTransaction();
+						}
+						else
+						{
+							LOCPLINT->showInfoDialog(CGI->generaltexth->translate("core.genrltxt.152"));
 						}
-						if(!srcInSameHero || ourOwner->commonInfo->dst.slotID != ourOwner->commonInfo->src.slotID)
-							ourOwner->realizeCurrentTransaction();
 					}
 				}
 			}
@@ -195,13 +195,12 @@ void CHeroArtPlace::clickLeft(tribool down, bool previousState)
 	}
 }
 
-bool CHeroArtPlace::askToAssemble(const CArtifactInstance *art, ArtifactPosition slot,
-                              const CGHeroInstance *hero)
+bool CHeroArtPlace::askToAssemble(const CGHeroInstance * hero, ArtifactPosition slot)
 {
-	assert(art);
 	assert(hero);
-	bool assembleEqipped = !ArtifactUtils::isSlotBackpack(slot);
-	auto assemblyPossibilities = art->assemblyPossibilities(hero, assembleEqipped);
+	const auto art = hero->getArt(slot);
+	assert(art);
+	auto assemblyPossibilities = art->assemblyPossibilities(hero, ArtifactUtils::isSlotEquipment(slot));
 
 	// If the artifact can be assembled, display dialog.
 	for(const auto * combination : assemblyPossibilities)
@@ -218,6 +217,26 @@ bool CHeroArtPlace::askToAssemble(const CArtifactInstance *art, ArtifactPosition
 	return false;
 }
 
+bool CHeroArtPlace::askToDisassemble(const CGHeroInstance * hero, ArtifactPosition slot)
+{
+	assert(hero);
+	const auto art = hero->getArt(slot);
+	assert(art);
+
+	if(art->canBeDisassembled())
+	{
+		if(ArtifactUtils::isSlotBackpack(slot) && !ArtifactUtils::isBackpackFreeSlots(hero, art->artType->constituents->size() - 1))
+			return false;
+		
+		LOCPLINT->showArtifactAssemblyDialog(
+			art->artType,
+			nullptr,
+			std::bind(&CCallback::assembleArtifacts, LOCPLINT->cb.get(), hero, slot, false, ArtifactID()));
+		return true;
+	}
+	return false;
+}
+
 void CHeroArtPlace::clickRight(tribool down, bool previousState)
 {
 	if(ourArt && down && !locked && text.size() && !picked)  //if there is no description or it's a lock, do nothing ;]
@@ -225,18 +244,12 @@ void CHeroArtPlace::clickRight(tribool down, bool previousState)
 		if(ourOwner->allowedAssembling)
 		{
 			// If the artifact can be assembled, display dialog.
-			if(askToAssemble(ourArt, slotID, ourOwner->curHero))
+			if(askToAssemble(ourOwner->curHero, slotID))
 			{
 				return;
 			}
-
-			// Otherwise if the artifact can be diasassembled, display dialog.
-			if(ourArt->canBeDisassembled())
+			if(askToDisassemble(ourOwner->curHero, slotID))
 			{
-				LOCPLINT->showArtifactAssemblyDialog(
-					ourArt->artType,
-					nullptr,
-					std::bind(&CCallback::assembleArtifacts, LOCPLINT->cb.get(), ourOwner->curHero, slotID, false, ArtifactID()));
 				return;
 			}
 		}
@@ -633,11 +646,17 @@ CArtifactsOfHero::~CArtifactsOfHero()
 	if(!curHero->artifactsTransitionPos.empty())
 	{
 		auto artPlace = getArtPlace(
-			ArtifactUtils::getArtifactDstPosition(curHero->artifactsTransitionPos.begin()->artifact, curHero));
-		assert(artPlace);
-		assert(artPlace->ourOwner);
-		artPlace->setMeAsDest();
-		artPlace->ourOwner->realizeCurrentTransaction();
+			ArtifactUtils::getArtAnyPosition(curHero, curHero->artifactsTransitionPos.begin()->artifact->getTypeId()));
+		if(artPlace)
+		{
+			assert(artPlace->ourOwner);
+			artPlace->setMeAsDest();
+			artPlace->ourOwner->realizeCurrentTransaction();
+		}
+		else
+		{
+			//TODO remove artifact
+		}
 	}
 }
 
@@ -765,11 +784,7 @@ void CArtifactsOfHero::artifactRemoved(const ArtifactLocation &al)
 
 CArtifactsOfHero::ArtPlacePtr CArtifactsOfHero::getArtPlace(ArtifactPosition slot)
 {
-	if(slot == ArtifactPosition::TRANSITION_POS)
-	{
-		return nullptr;
-	}
-	if(slot < GameConstants::BACKPACK_START)
+	if(ArtifactUtils::isSlotEquipment(slot))
 	{
 		if(artWorn.find(slot) == artWorn.end())
 		{
@@ -777,15 +792,19 @@ CArtifactsOfHero::ArtPlacePtr CArtifactsOfHero::getArtPlace(ArtifactPosition slo
 			return nullptr;
 		}
 
-		return artWorn[ArtifactPosition(slot)];
+		return artWorn[slot];
 	}
-	else
+	if(ArtifactUtils::isSlotBackpack(slot))
 	{
 		for(ArtPlacePtr ap : backpack)
 			if(ap->slotID == slot)
 				return ap;
 		return nullptr;
 	}
+	else
+	{
+		return nullptr;
+	}
 }
 
 void CArtifactsOfHero::artifactUpdateSlots(const ArtifactLocation & al)
@@ -986,16 +1005,22 @@ void CCommanderArtPlace::createImage()
 void CCommanderArtPlace::returnArtToHeroCallback()
 {
 	ArtifactPosition artifactPos = commanderSlotID;
-	ArtifactPosition freeSlot = ourArt->firstBackpackSlot(commanderOwner);
-
-	ArtifactLocation src(commanderOwner->commander.get(), artifactPos);
-	ArtifactLocation dst(commanderOwner, freeSlot);
-
-	if (ourArt->canBePutAt(dst, true))
+	ArtifactPosition freeSlot = ArtifactUtils::getArtBackpackPosition(commanderOwner, ourArt->getTypeId());
+	if(freeSlot == ArtifactPosition::PRE_FIRST)
+	{
+		LOCPLINT->showInfoDialog(CGI->generaltexth->translate("core.genrltxt.152"));
+	}
+	else
 	{
-		LOCPLINT->cb->swapArtifacts(src, dst);
-		setArtifact(nullptr);
-		parent->redraw();
+		ArtifactLocation src(commanderOwner->commander.get(), artifactPos);
+		ArtifactLocation dst(commanderOwner, freeSlot);
+
+		if(ourArt->canBePutAt(dst, true))
+		{
+			LOCPLINT->cb->swapArtifacts(src, dst);
+			setArtifact(nullptr);
+			parent->redraw();
+		}
 	}
 }
 

client/widgets/CArtifactHolder.h

@@ -95,8 +95,8 @@ public:
 
 	void setMeAsDest(bool backpackAsVoid = true);
 	void setArtifact(const CArtifactInstance *art) override;
-	static bool askToAssemble(const CArtifactInstance *art, ArtifactPosition slot,
-	                          const CGHeroInstance *hero);
+	static bool askToAssemble(const CGHeroInstance * hero, ArtifactPosition slot);
+	static bool askToDisassemble(const CGHeroInstance * hero, ArtifactPosition slot);
 };
 
 /// Contains artifacts of hero. Distincts which artifacts are worn or backpacked

client/widgets/CGarrisonInt.cpp

@@ -204,7 +204,8 @@ bool CGarrisonSlot::highlightOrDropArtifact()
 					{
 						//creature can wear only one active artifact
 						//if we are placing a new one, the old one will be returned to the hero's backpack
-						LOCPLINT->cb->swapArtifacts(dst, ArtifactLocation(srcHero, dst.getArt()->firstBackpackSlot(srcHero)));
+						LOCPLINT->cb->swapArtifacts(dst, ArtifactLocation(srcHero,
+							ArtifactUtils::getArtBackpackPosition(srcHero, dst.getArt()->getTypeId())));
 					}
 					LOCPLINT->cb->swapArtifacts(src, dst);
 				}

client/windows/CCastleInterface.cpp

@@ -805,7 +805,7 @@ void CCastleBuildings::enterBlacksmith(ArtifactID artifactID)
 		LOCPLINT->showInfoDialog(boost::str(boost::format(CGI->generaltexth->allTexts[273]) % town->town->buildings.find(BuildingID::BLACKSMITH)->second->getNameTranslated()));
 		return;
 	}
-	auto art = dynamic_cast<const CArtifact*>(artifactID.toArtifact(CGI->artifacts()));
+	auto art = artifactID.toArtifact();
 
 	int price = art->getPrice();
 	bool possible = LOCPLINT->cb->getResourceAmount(Res::GOLD) >= price;

client/windows/CCreatureWindow.cpp

@@ -948,10 +948,14 @@ void CStackWindow::removeStackArtifact(ArtifactPosition pos)
 		logGlobal->error("Attempt to remove missing artifact");
 		return;
 	}
-	LOCPLINT->cb->swapArtifacts(ArtifactLocation(info->stackNode, pos), ArtifactLocation(info->owner, art->firstBackpackSlot(info->owner)));
-	stackArtifactButton.reset();
-	stackArtifactHelp.reset();
-	stackArtifactIcon.reset();
-	redraw();
+	const auto slot = ArtifactUtils::getArtBackpackPosition(info->owner, art->getTypeId());
+	if(slot != ArtifactPosition::PRE_FIRST)
+	{
+		LOCPLINT->cb->swapArtifacts(ArtifactLocation(info->stackNode, pos), ArtifactLocation(info->owner, slot));
+		stackArtifactButton.reset();
+		stackArtifactHelp.reset();
+		stackArtifactIcon.reset();
+		redraw();
+	}
 }
 

client/windows/CHeroWindow.cpp

@@ -358,7 +358,7 @@ void CHeroWindow::commanderWindow()
 	{
 		const CGHeroInstance *srcHero = commonInfo->src.AOH->getHero();
 		//artSelected = true;
-		ArtifactPosition freeSlot = art->firstAvailableSlot (curHero->commander);
+		const auto freeSlot = ArtifactUtils::getArtAnyPosition(curHero->commander, art->artType->getId());
 		if(freeSlot < ArtifactPosition::COMMANDER_AFTER_LAST) //we don't want to put it in commander's backpack!
 		{
 			ArtifactLocation src(srcHero, commonInfo->src.slotID);
@@ -368,7 +368,8 @@ void CHeroWindow::commanderWindow()
 			{	//equip clicked stack
 				if(dst.getArt())
 				{
-					LOCPLINT->cb->swapArtifacts (dst, ArtifactLocation(srcHero, dst.getArt()->firstBackpackSlot(srcHero)));
+					LOCPLINT->cb->swapArtifacts (dst, ArtifactLocation(srcHero,
+						ArtifactUtils::getArtBackpackPosition(srcHero, art->getTypeId())));
 				}
 				LOCPLINT->cb->swapArtifacts(src, dst);
 			}

client/windows/CTradeWindow.cpp

@@ -806,7 +806,8 @@ void CMarketplaceWindow::makeDeal()
 	if(!sliderValue)
 		return;
 
-	int leftIdToSend = -1;
+	bool allowDeal = true;
+	int leftIdToSend = hLeft->id;
 	switch (mode)
 	{
 		case EMarketMode::CREATURE_RESOURCE:
@@ -815,22 +816,31 @@ void CMarketplaceWindow::makeDeal()
 		case EMarketMode::ARTIFACT_RESOURCE:
 			leftIdToSend = hLeft->getArtInstance()->id.getNum();
 			break;
+		case EMarketMode::RESOURCE_ARTIFACT:
+			if(!ArtifactID(hRight->id).toArtifact()->canBePutAt(hero))
+			{
+				LOCPLINT->showInfoDialog(CGI->generaltexth->translate("core.genrltxt.326"));
+				allowDeal = false;
+			}
+			break;
 		default:
-			leftIdToSend = hLeft->id;
 			break;
 	}
 
-	if(slider)
-	{
-		LOCPLINT->cb->trade(market->o, mode, leftIdToSend, hRight->id, slider->getValue()*r1, hero);
-		slider->moveTo(0);
-	}
-	else
+	if(allowDeal)
 	{
-		LOCPLINT->cb->trade(market->o, mode, leftIdToSend, hRight->id, r2, hero);
+		if(slider)
+		{
+			LOCPLINT->cb->trade(market->o, mode, leftIdToSend, hRight->id, slider->getValue() * r1, hero);
+			slider->moveTo(0);
+		}
+		else
+		{
+			LOCPLINT->cb->trade(market->o, mode, leftIdToSend, hRight->id, r2, hero);
+		}
 	}
-	madeTransaction = true;
 
+	madeTransaction = true;
 	hLeft = nullptr;
 	hRight = nullptr;
 	selectionChanged(true);

client/windows/GUIClasses.cpp

@@ -843,7 +843,7 @@ void CExchangeController::moveArtifact(
 {
 	auto srcLocation = ArtifactLocation(source, srcPosition);
 	auto dstLocation = ArtifactLocation(target,
-		ArtifactUtils::getArtifactDstPosition(source->getArt(srcPosition), target));
+		ArtifactUtils::getArtAnyPosition(target, source->getArt(srcPosition)->getTypeId()));
 
 	cb->swapArtifacts(srcLocation, dstLocation);
 }

config/gameConfig.json

@@ -133,7 +133,9 @@
 			// if enabled, hero that wins a battle without any non-summoned troops left will retreat and become available in tavern instead of being lost
 			"retreatOnWinWithoutTroops" : true,
 			// Chances for a hero with default army to receive corresponding stack out of his predefined starting troops
-			"startingStackChances": [ 100, 88, 25]
+			"startingStackChances": [ 100, 88, 25],
+			// number of artifacts that can fit in a backpack. -1 is unlimited.
+			"backpackSize"		: -1
 		},
 
 		"towns":

lib/CArtHandler.cpp

@@ -139,6 +139,84 @@ bool CArtifact::isTradable() const
 	}
 }
 
+bool CArtifact::canBeDisassembled() const
+{
+	return !(constituents == nullptr);
+}
+
+bool CArtifact::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, bool assumeDestRemoved) const
+{
+	auto simpleArtCanBePutAt = [this](const CArtifactSet * artSet, ArtifactPosition slot, bool assumeDestRemoved) -> bool
+	{
+		if(ArtifactUtils::isSlotBackpack(slot))
+		{
+			if(isBig() || !ArtifactUtils::isBackpackFreeSlots(artSet))
+				return false;
+			return true;
+		}
+
+		if(!vstd::contains(possibleSlots.at(artSet->bearerType()), slot))
+			return false;
+
+		return artSet->isPositionFree(slot, assumeDestRemoved);
+	};
+
+	auto artCanBePutAt = [this, simpleArtCanBePutAt](const CArtifactSet * artSet, ArtifactPosition slot, bool assumeDestRemoved) -> bool
+	{
+		if(canBeDisassembled())
+		{
+			if(!simpleArtCanBePutAt(artSet, slot, assumeDestRemoved))
+				return false;
+			if(ArtifactUtils::isSlotBackpack(slot))
+				return true;
+
+			CArtifactFittingSet fittingSet(artSet->bearerType());
+			fittingSet.artifactsWorn = artSet->artifactsWorn;
+			if(assumeDestRemoved)
+				fittingSet.removeArtifact(slot);
+			assert(constituents);
+			for(const auto art : *constituents)
+			{
+				auto possibleSlot = ArtifactUtils::getArtAnyPosition(&fittingSet, art->getId());
+				if(ArtifactUtils::isSlotEquipment(possibleSlot))
+				{
+					fittingSet.setNewArtSlot(possibleSlot, nullptr, true);
+				}
+				else
+				{
+					return false;
+				}
+			}
+			return true;
+		}
+		else
+		{
+			return simpleArtCanBePutAt(artSet, slot, assumeDestRemoved);
+		}
+	};
+
+	if(slot == ArtifactPosition::TRANSITION_POS)
+		return true;
+
+	if(slot == ArtifactPosition::FIRST_AVAILABLE)
+	{
+		for(const auto & slot : possibleSlots.at(artSet->bearerType()))
+		{
+			if(artCanBePutAt(artSet, slot, assumeDestRemoved))
+				return true;
+		}
+		return artCanBePutAt(artSet, GameConstants::BACKPACK_START, assumeDestRemoved);
+	}
+	else if(ArtifactUtils::isSlotBackpack(slot))
+	{
+		return artCanBePutAt(artSet, GameConstants::BACKPACK_START, assumeDestRemoved);
+	}
+	else
+	{
+		return artCanBePutAt(artSet, slot, assumeDestRemoved);
+	}
+}
+
 CArtifact::CArtifact()
 {
 	setNodeType(ARTIFACT);
@@ -800,61 +878,14 @@ std::string CArtifactInstance::getEffectiveDescription(const CGHeroInstance * he
 	return text;
 }
 
-ArtifactPosition CArtifactInstance::firstAvailableSlot(const CArtifactSet *h) const
-{
-	for(const auto & slot : artType->possibleSlots.at(h->bearerType()))
-	{
-		if(canBePutAt(h, slot)) //if(artType->fitsAt(h->artifWorn, slot))
-		{
-			//we've found a free suitable slot.
-			return slot;
-		}
-	}
-
-	//if haven't find proper slot, use backpack
-	return firstBackpackSlot(h);
-}
-
-ArtifactPosition CArtifactInstance::firstBackpackSlot(const CArtifactSet *h) const
+ArtifactID CArtifactInstance::getTypeId() const
 {
-	if(!artType->isBig()) //discard big artifact
-		return ArtifactPosition(GameConstants::BACKPACK_START + static_cast<si32>(h->artifactsInBackpack.size()));
-
-	return ArtifactPosition::PRE_FIRST;
+	return artType->getId();
 }
 
 bool CArtifactInstance::canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved) const
 {
-	return canBePutAt(al.getHolderArtSet(), al.slot, assumeDestRemoved);
-}
-
-bool CArtifactInstance::canBePutAt(const CArtifactSet *artSet, ArtifactPosition slot, bool assumeDestRemoved) const
-{
-	if(slot == ArtifactPosition::TRANSITION_POS)
-	{
-		return true;
-	}
-
-	if(ArtifactUtils::isSlotBackpack(slot))
-	{
-		if(artType->isBig())
-			return false;
-
-		//TODO backpack limit
-		return true;
-	}
-
- 	auto possibleSlots = artType->possibleSlots.find(artSet->bearerType());
- 	if(possibleSlots == artType->possibleSlots.end())
- 	{
-		logMod->warn("Warning: artifact %s doesn't have defined allowed slots for bearer of type %s", artType->getNameTranslated(), artSet->bearerType());
-		return false;
-	}
-
-	if(!vstd::contains(possibleSlots->second, slot))
-		return false;
-
-	return artSet->isPositionFree(slot, assumeDestRemoved);
+	return artType->canBePutAt(al.getHolderArtSet(), al.slot, assumeDestRemoved);
 }
 
 void CArtifactInstance::putAt(ArtifactLocation al)
@@ -876,7 +907,7 @@ void CArtifactInstance::removeFrom(ArtifactLocation al)
 
 bool CArtifactInstance::canBeDisassembled() const
 {
-	return bool(artType->constituents);
+	return artType->canBeDisassembled();
 }
 
 std::vector<const CArtifact *> CArtifactInstance::assemblyPossibilities(const CArtifactSet * h, bool equipped) const
@@ -1007,53 +1038,6 @@ bool CArtifactInstance::isPart(const CArtifactInstance *supposedPart) const
 	return supposedPart == this;
 }
 
-bool CCombinedArtifactInstance::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, bool assumeDestRemoved) const
-{
-	if(slot == ArtifactPosition::TRANSITION_POS)
-		return true;
-	if(!CArtifactInstance::canBePutAt(artSet, slot, assumeDestRemoved))
-		return false;
-	if(ArtifactUtils::isSlotBackpack(slot))
-		return true; //we can always remove combined art to the backapck
-
-	CArtifactFittingSet fittingSet(artSet->bearerType());
-	fittingSet.artifactsWorn = artSet->artifactsWorn;
-	auto artToRemove = fittingSet.getArt(slot);
-	if(assumeDestRemoved && artToRemove)
-	{
-		if(artToRemove->canBeDisassembled())
-		{
-			auto combinedArtToRemove = dynamic_cast<CCombinedArtifactInstance*>(artToRemove);
-			for(auto & part : combinedArtToRemove->constituentsInfo)
-			{
-				if(ArtifactUtils::isSlotEquipment(part.slot))
-				{
-					fittingSet.eraseArtSlot(part.slot);
-				}
-			}
-		}
-		fittingSet.eraseArtSlot(slot);
-	}
-	for(auto & art : constituentsInfo)
-	{
-		auto possibleSlot = art.art->firstAvailableSlot(&fittingSet);
-		if(ArtifactUtils::isSlotEquipment(possibleSlot))
-		{
-			fittingSet.setNewArtSlot(possibleSlot, nullptr, true);
-		}
-		else
-		{
-			return false;
-		}
-	}
-	return true;
-}
-
-bool CCombinedArtifactInstance::canBeDisassembled() const
-{
-	return true;
-}
-
 CCombinedArtifactInstance::CCombinedArtifactInstance(CArtifact *Art)
 	: CArtifactInstance(Art) //TODO: seems unused, but need to be written
 {
@@ -1097,7 +1081,7 @@ void CCombinedArtifactInstance::putAt(ArtifactLocation al)
 		CArtifactInstance *mainConstituent = figureMainConstituent(al); //it'll be replaced with combined artifact, not a lock
 		CArtifactInstance::putAt(al); //puts combined art (this)
 
-		for(ConstituentInfo &ci : constituentsInfo)
+		for(ConstituentInfo & ci : constituentsInfo)
 		{
 			if(ci.art != mainConstituent)
 			{
@@ -1105,14 +1089,11 @@ void CCombinedArtifactInstance::putAt(ArtifactLocation al)
 				const bool inActiveSlot = vstd::isbetween(ci.slot, 0, GameConstants::BACKPACK_START);
 				const bool suggestedPosValid = ci.art->canBePutAt(suggestedPos);
 
-				ArtifactPosition pos = ArtifactPosition::PRE_FIRST;
-				if(inActiveSlot  &&  suggestedPosValid) //there is a valid suggestion where to place lock
-					pos = ci.slot;
-				else
-					ci.slot = pos = ci.art->firstAvailableSlot(al.getHolderArtSet());
+				if(!(inActiveSlot && suggestedPosValid)) //there is a valid suggestion where to place lock
+					ci.slot = ArtifactUtils::getArtAnyPosition(al.getHolderArtSet(), ci.art->getTypeId());
 
-				assert(!ArtifactUtils::isSlotBackpack(pos));
-				al.getHolderArtSet()->setNewArtSlot(pos, ci.art, true); //sets as lock
+				assert(ArtifactUtils::isSlotEquipment(ci.slot));
+				al.getHolderArtSet()->setNewArtSlot(ci.slot, ci.art, true); //sets as lock
 			}
 			else
 			{
@@ -1232,7 +1213,7 @@ std::vector<ArtifactPosition> CArtifactSet::getAllArtPositions(const ArtifactID
 {
 	std::vector<ArtifactPosition> result;
 	for(const auto & slotInfo : artifactsWorn)
-		if(slotInfo.second.artifact->artType->getId() == aid && (allowLocked || !slotInfo.second.locked))
+		if(slotInfo.second.artifact->getTypeId() == aid && (allowLocked || !slotInfo.second.locked))
 			result.push_back(slotInfo.first);
 
 	if(onlyWorn)
@@ -1318,7 +1299,7 @@ std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *> CArtifac
 			auto * ass = dynamic_cast<CCombinedArtifactInstance *>(art.get());
 			for(auto& ci : ass->constituentsInfo)
 			{
-				if(ci.art->artType->getId() == aid)
+				if(ci.art->getTypeId() == aid)
 				{
 					return {ass, ci.art};
 				}
@@ -1470,7 +1451,7 @@ void CArtifactSet::serializeJsonHero(JsonSerializeFormat & handler, CMap * map)
 	{
 		backpackTemp.reserve(artifactsInBackpack.size());
 		for(const ArtSlotInfo & info : artifactsInBackpack)
-			backpackTemp.push_back(info.artifact->artType->getId());
+			backpackTemp.push_back(info.artifact->getTypeId());
 	}
 	handler.serializeIdArray(NArtifactPosition::backpack, backpackTemp);
 	if(!handler.saving)
@@ -1478,8 +1459,8 @@ void CArtifactSet::serializeJsonHero(JsonSerializeFormat & handler, CMap * map)
 		for(const ArtifactID & artifactID : backpackTemp)
 		{
 			auto * artifact = CArtifactInstance::createArtifact(map, artifactID.toEnum());
-			auto slot = ArtifactPosition(GameConstants::BACKPACK_START + static_cast<si32>(artifactsInBackpack.size()));
-			if(artifact->canBePutAt(this, slot))
+			auto slot = ArtifactPosition(GameConstants::BACKPACK_START + (si32)artifactsInBackpack.size());
+			if(artifact->artType->canBePutAt(this, slot))
 				putArtifact(slot, artifact);
 		}
 	}
@@ -1505,7 +1486,7 @@ void CArtifactSet::serializeJsonSlot(JsonSerializeFormat & handler, const Artifa
 
 		if(info != nullptr && !info->locked)
 		{
-			artifactID = info->artifact->artType->getId();
+			artifactID = info->artifact->getTypeId();
 			handler.serializeId(NArtifactPosition::namesHero[slot.num], artifactID, ArtifactID::NONE);
 		}
 	}
@@ -1517,7 +1498,7 @@ void CArtifactSet::serializeJsonSlot(JsonSerializeFormat & handler, const Artifa
 		{
 			auto * artifact = CArtifactInstance::createArtifact(map, artifactID.toEnum());
 
-			if(artifact->canBePutAt(this, slot))
+			if(artifact->artType->canBePutAt(this, slot))
 			{
 				putArtifact(slot, artifact);
 			}
@@ -1540,8 +1521,10 @@ void CArtifactFittingSet::putArtifact(ArtifactPosition pos, CArtifactInstance *
 	{
 		for(auto & part : dynamic_cast<CCombinedArtifactInstance*>(art)->constituentsInfo)
 		{
+			const auto slot = ArtifactUtils::getArtAnyPosition(this, part.art->getTypeId());
+			assert(slot != ArtifactPosition::PRE_FIRST);
 			// For the ArtFittingSet is no needed to do figureMainConstituent, just lock slots
-			this->setNewArtSlot(part.art->firstAvailableSlot(this), part.art, true);
+			this->setNewArtSlot(slot, part.art, true);
 		}
 	}
 	else
@@ -1550,25 +1533,50 @@ void CArtifactFittingSet::putArtifact(ArtifactPosition pos, CArtifactInstance *
 	}
 }
 
+void CArtifactFittingSet::removeArtifact(ArtifactPosition pos)
+{
+	// Removes the art from the CartifactSet, but does not remove it from art->constituentsInfo.
+	// removeArtifact is for CArtifactFittingSet only. Do not move it to the parent class.
+
+	auto art = getArt(pos);
+	if(art == nullptr)
+		return;
+	if(art->canBeDisassembled())
+	{
+		auto combinedArt = dynamic_cast<CCombinedArtifactInstance*>(art);
+		for(const auto & part : combinedArt->constituentsInfo)
+		{
+			if(ArtifactUtils::isSlotEquipment(part.slot))
+				eraseArtSlot(part.slot);
+		}
+	}
+	eraseArtSlot(pos);
+}
+
 ArtBearer::ArtBearer CArtifactFittingSet::bearerType() const
 {
 	return this->Bearer;
 }
 
-DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtifactDstPosition(const CArtifactInstance * artifact,
-	const CArtifactSet * target)
+DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtAnyPosition(const CArtifactSet * target, const ArtifactID & aid)
 {
-	for(const auto & slot : artifact->artType->possibleSlots.at(target->bearerType()))
+	const auto * art = aid.toArtifact();
+	for(const auto & slot : art->possibleSlots.at(target->bearerType()))
 	{
-		const auto * existingArtInfo = target->getSlot(slot);
-
-		if((!existingArtInfo || !existingArtInfo->locked)
-			&& artifact->canBePutAt(target, slot))
-		{
+		if(art->canBePutAt(target, slot))
 			return slot;
-		}
 	}
-	return ArtifactPosition(GameConstants::BACKPACK_START);
+	return getArtBackpackPosition(target, aid);
+}
+
+DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtBackpackPosition(const CArtifactSet * target, const ArtifactID & aid)
+{
+	const auto * art = aid.toArtifact();
+	if(art->canBePutAt(target, GameConstants::BACKPACK_START))
+	{
+		return GameConstants::BACKPACK_START;
+	}
+	return ArtifactPosition::PRE_FIRST;
 }
 
 DLL_LINKAGE const std::vector<ArtifactPosition::EArtifactPosition> & ArtifactUtils::unmovableSlots()
@@ -1637,4 +1645,13 @@ DLL_LINKAGE bool ArtifactUtils::isSlotEquipment(const ArtifactPosition & slot)
 	return slot < GameConstants::BACKPACK_START && slot >= 0;
 }
 
+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;
+	else
+		return target->artifactsInBackpack.size() + reqSlots <= backpackCap;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/CArtHandler.h

@@ -90,6 +90,9 @@ public:
 
 	virtual void levelUpArtifact (CArtifactInstance * art){};
 
+	virtual bool canBeDisassembled() const;
+	virtual bool canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot = ArtifactPosition::FIRST_AVAILABLE,
+		bool assumeDestRemoved = false) const;
 	void updateFrom(const JsonNode & data);
 	void serializeJson(JsonSerializeFormat & handler);
 
@@ -151,11 +154,9 @@ public:
 	void setType(CArtifact *Art);
 
 	std::string getEffectiveDescription(const CGHeroInstance *hero = nullptr) const;
-	ArtifactPosition firstAvailableSlot(const CArtifactSet *h) const;
-	ArtifactPosition firstBackpackSlot(const CArtifactSet *h) const;
 	SpellID getGivenSpellID() const; //to be used with scrolls (and similar arts), -1 if none
 
-	virtual bool canBePutAt(const CArtifactSet *artSet, ArtifactPosition slot, bool assumeDestRemoved = false) const;
+	ArtifactID getTypeId() const;
 	bool canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved = false) const;  //forwards to the above one
 	virtual bool canBeDisassembled() const;
 	virtual void putAt(ArtifactLocation al);
@@ -209,8 +210,6 @@ public:
 
 	std::vector<ConstituentInfo> constituentsInfo;
 
-	bool canBePutAt(const CArtifactSet *artSet, ArtifactPosition slot, bool assumeDestRemoved = false) const override;
-	bool canBeDisassembled() const override;
 	void putAt(ArtifactLocation al) override;
 	void removeFrom(ArtifactLocation al) override;
 	bool isPart(const CArtifactInstance *supposedPart) const override;
@@ -371,6 +370,7 @@ class DLL_LINKAGE CArtifactFittingSet : public CArtifactSet
 public:
 	CArtifactFittingSet(ArtBearer::ArtBearer Bearer);
 	void putArtifact(ArtifactPosition pos, CArtifactInstance * art) override;
+	void removeArtifact(ArtifactPosition pos);
 	ArtBearer::ArtBearer bearerType() const override;
 
 protected:
@@ -380,8 +380,8 @@ protected:
 namespace ArtifactUtils
 {
 	// Calculates where an artifact gets placed when it gets transferred from one hero to another.
-	DLL_LINKAGE ArtifactPosition getArtifactDstPosition(const CArtifactInstance * artifact, 
-		const CArtifactSet * target);
+	DLL_LINKAGE ArtifactPosition getArtAnyPosition(const CArtifactSet * target, const ArtifactID & aid);
+	DLL_LINKAGE ArtifactPosition getArtBackpackPosition(const CArtifactSet * target, const ArtifactID & aid);
 	// TODO: Make this constexpr when the toolset is upgraded
 	DLL_LINKAGE const std::vector<ArtifactPosition::EArtifactPosition> & unmovableSlots();
 	DLL_LINKAGE const std::vector<ArtifactPosition::EArtifactPosition> & constituentWornSlots();
@@ -389,6 +389,7 @@ namespace ArtifactUtils
 	DLL_LINKAGE bool checkSpellbookIsNeeded(const CGHeroInstance * heroPtr, const ArtifactID & artID, const ArtifactPosition & slot);
 	DLL_LINKAGE bool isSlotBackpack(const ArtifactPosition & slot);
 	DLL_LINKAGE bool isSlotEquipment(const ArtifactPosition & slot);
+	DLL_LINKAGE bool isBackpackFreeSlots(const CArtifactSet * target, const size_t reqSlots = 1);
 }
 
 VCMI_LIB_NAMESPACE_END

lib/CGameState.cpp

@@ -1588,12 +1588,17 @@ void CGameState::giveCampaignBonusToHero(CGHeroInstance * hero)
 			}
 			break;
 		case CScenarioTravel::STravelBonus::ARTIFACT:
-			gs->giveHeroArtifact(hero, static_cast<ArtifactID>(curBonus->info2));
+			if(!gs->giveHeroArtifact(hero, static_cast<ArtifactID>(curBonus->info2)))
+				logGlobal->error("Cannot give starting artifact - no free slots!");
 			break;
 		case CScenarioTravel::STravelBonus::SPELL_SCROLL:
 			{
 				CArtifactInstance * scroll = CArtifactInstance::createScroll(SpellID(curBonus->info2));
-				scroll->putAt(ArtifactLocation(hero, scroll->firstAvailableSlot(hero)));
+				const auto slot = ArtifactUtils::getArtAnyPosition(hero, scroll->getTypeId());
+				if(ArtifactUtils::isSlotEquipment(slot) || ArtifactUtils::isSlotBackpack(slot))
+					scroll->putAt(ArtifactLocation(hero, slot));
+				else
+					logGlobal->error("Cannot give starting scroll - no free slots!");
 			}
 			break;
 		case CScenarioTravel::STravelBonus::PRIMARY_SKILL:
@@ -1686,7 +1691,8 @@ void CGameState::initStartingBonus()
 				const Artifact * toGive = VLC->arth->pickRandomArtifact(getRandomGenerator(), CArtifact::ART_TREASURE).toArtifact(VLC->artifacts());
 
 				CGHeroInstance *hero = elem.second.heroes[0];
-				giveHeroArtifact(hero, toGive->getId());
+				if(!giveHeroArtifact(hero, toGive->getId()))
+					logGlobal->error("Cannot give starting artifact - no free slots!");
 			}
 			break;
 		}
@@ -2805,12 +2811,21 @@ void CGameState::attachArmedObjects()
 	}
 }
 
-void CGameState::giveHeroArtifact(CGHeroInstance * h, const ArtifactID & aid)
+bool CGameState::giveHeroArtifact(CGHeroInstance * h, const ArtifactID & aid)
 {
 	 CArtifact * const artifact = VLC->arth->objects[aid]; //pointer to constant object
-	 CArtifactInstance *ai = CArtifactInstance::createNewArtifactInstance(artifact);
+	 CArtifactInstance * ai = CArtifactInstance::createNewArtifactInstance(artifact);
 	 map->addNewArtifactInstance(ai);
-	 ai->putAt(ArtifactLocation(h, ai->firstAvailableSlot(h)));
+	 auto slot = ArtifactUtils::getArtAnyPosition(h, aid);
+	 if(ArtifactUtils::isSlotEquipment(slot) || ArtifactUtils::isSlotBackpack(slot))
+	 {
+		 ai->putAt(ArtifactLocation(h, slot));
+		 return true;
+	 }
+	 else
+	 {
+		 return false;
+	 }
 }
 
 std::set<HeroTypeID> CGameState::getUnusedAllowedHeroes(bool alsoIncludeNotAllowed) const

lib/CGameState.h

@@ -178,7 +178,7 @@ public:
 
 	void updateEntity(Metatype metatype, int32_t index, const JsonNode & data) override;
 
-	void giveHeroArtifact(CGHeroInstance * h, const ArtifactID & aid);
+	bool giveHeroArtifact(CGHeroInstance * h, const ArtifactID & aid);
 
 	void apply(CPack *pack);
 	BattleField battleGetBattlefieldType(int3 tile, CRandomGenerator & rand);

lib/GameConstants.h

@@ -47,7 +47,6 @@ namespace GameConstants
 
 	const int ALL_PLAYERS = 255; //bitfield
 
-	const ui16 BACKPACK_START = 19;
 	const int CREATURES_PER_TOWN = 7; //without upgrades
 	const int SPELL_LEVELS = 5;
 	const int SPELL_SCHOOL_LEVELS = 4;
@@ -968,7 +967,8 @@ public:
 		CAN_DIG = 0,
 		LACK_OF_MOVEMENT,
 		WRONG_TERRAIN,
-		TILE_OCCUPIED
+		TILE_OCCUPIED,
+		BACKPACK_IS_FULL
 	};
 
 	EDiggingStatus(EEDiggingStatus _num = UNKNOWN) : num(_num)
@@ -1040,10 +1040,24 @@ public:
 	ID_LIKE_CLASS_COMMON(ArtifactPosition, EArtifactPosition)
 
 	EArtifactPosition num;
+
+        STRONG_INLINE EArtifactPosition operator+(const int arg)
+	{
+		return EArtifactPosition(static_cast<int>(num) + arg);
+	}
+	STRONG_INLINE EArtifactPosition operator+(const EArtifactPosition & arg)
+	{
+		return EArtifactPosition(static_cast<int>(num) + static_cast<int>(arg));
+	}
 };
 
 ID_LIKE_OPERATORS(ArtifactPosition, ArtifactPosition::EArtifactPosition)
 
+namespace GameConstants
+{
+	const auto BACKPACK_START = ArtifactPosition::AFTER_LAST;
+}
+
 class ArtifactID
 {
 public:

lib/GameSettings.cpp

@@ -69,6 +69,7 @@ void GameSettings::load(const JsonNode & input)
 		{EGameSettings::HEROES_PER_PLAYER_TOTAL_CAP,            "heroes",    "perPlayerTotalCap"          },
 		{EGameSettings::HEROES_RETREAT_ON_WIN_WITHOUT_TROOPS,   "heroes",    "retreatOnWinWithoutTroops"  },
 		{EGameSettings::HEROES_STARTING_STACKS_CHANCES,         "heroes",    "startingStackChances"       },
+		{EGameSettings::HEROES_BACKPACK_CAP,                    "heroes",    "backpackSize"               },
 		{EGameSettings::MARKETS_BLACK_MARKET_RESTOCK_PERIOD,    "markets",   "blackMarketRestockPeriod"   },
 		{EGameSettings::MODULE_COMMANDERS,                      "modules",   "commanders"                 },
 		{EGameSettings::MODULE_STACK_ARTIFACT,                  "modules",   "stackArtifact"              },

lib/GameSettings.h

@@ -36,6 +36,7 @@ enum class EGameSettings
 	HEROES_PER_PLAYER_TOTAL_CAP,
 	HEROES_RETREAT_ON_WIN_WITHOUT_TROOPS,
 	HEROES_STARTING_STACKS_CHANCES,
+	HEROES_BACKPACK_CAP,
 	MARKETS_BLACK_MARKET_RESTOCK_PERIOD,
 	MODULE_COMMANDERS,
 	MODULE_STACK_ARTIFACT,

lib/IGameCallback.h

@@ -109,8 +109,8 @@ public:
 
 	virtual void removeAfterVisit(const CGObjectInstance *object) = 0; //object will be destroyed when interaction is over. Do not call when interaction is not ongoing!
 
-	virtual void giveHeroNewArtifact(const CGHeroInstance *h, const CArtifact *artType, ArtifactPosition pos) = 0;
-	virtual void giveHeroArtifact(const CGHeroInstance *h, const CArtifactInstance *a, ArtifactPosition pos) = 0; //pos==-1 - first free slot in backpack=0; pos==-2 - default if available or backpack
+	virtual bool giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos) = 0;
+	virtual bool giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) = 0;
 	virtual void putArtifact(const ArtifactLocation &al, const CArtifactInstance *a) = 0;
 	virtual void removeArtifact(const ArtifactLocation &al) = 0;
 	virtual bool moveArtifact(const ArtifactLocation &al1, const ArtifactLocation &al2) = 0;

lib/NetPacksLib.cpp

@@ -1693,14 +1693,18 @@ void RebalanceStacks::applyGs(CGameState * gs)
 				if (alDest.getArt())
 				{
 					auto * hero = dynamic_cast<CGHeroInstance *>(src.army.get());
-					if (hero)
+					auto dstSlot = ArtifactUtils::getArtBackpackPosition(hero, alDest.getArt()->getTypeId());
+					if(hero && dstSlot != ArtifactPosition::PRE_FIRST)
 					{
-						artDest->move (alDest, ArtifactLocation (hero, alDest.getArt()->firstBackpackSlot (hero)));
+						artDest->move (alDest, ArtifactLocation (hero, dstSlot));
 					}
 					//else - artifact cna be lost :/
 					else
 					{
-						logNetwork->warn("Artifact is present at destination slot!");
+						EraseArtifact ea;
+						ea.al = alDest;
+						ea.applyGs(gs);
+						logNetwork->warn("Cannot move artifact! No free slots");
 					}
 					artHere->move (alHere, alDest);
 					//TODO: choose from dialog

lib/mapObjects/CGHeroInstance.cpp

@@ -1023,11 +1023,6 @@ void CGHeroInstance::putArtifact(ArtifactPosition pos, CArtifactInstance *art)
 	art->putAt(ArtifactLocation(this, pos));
 }
 
-void CGHeroInstance::putInBackpack(CArtifactInstance *art)
-{
-	putArtifact(art->firstBackpackSlot(this), art);
-}
-
 bool CGHeroInstance::hasSpellbook() const
 {
 	return getArt(ArtifactPosition::SPELLBOOK);
@@ -1126,7 +1121,8 @@ EDiggingStatus CGHeroInstance::diggingStatus() const
 {
 	if(static_cast<int>(movement) < maxMovePoints(true))
 		return EDiggingStatus::LACK_OF_MOVEMENT;
-
+	if(!VLC->arth->objects[ArtifactID::GRAIL]->canBePutAt(this))
+		return EDiggingStatus::BACKPACK_IS_FULL;
 	return cb->getTileDigStatus(visitablePos());
 }
 

lib/mapObjects/CGHeroInstance.h

@@ -234,7 +234,6 @@ public:
 	void initHero(CRandomGenerator & rand, const HeroTypeID & SUBID);
 
 	void putArtifact(ArtifactPosition pos, CArtifactInstance * art) override;
-	void putInBackpack(CArtifactInstance *art);
 	void initExp(CRandomGenerator & rand);
 	void initArmy(CRandomGenerator & rand, IArmyDescriptor *dst = nullptr);
 	//void giveArtifact (ui32 aid);

lib/mapObjects/CQuest.cpp

@@ -140,18 +140,26 @@ bool CQuest::checkQuest(const CGHeroInstance * h) const
 				return true;
 			return false;
 		case MISSION_ART:
+		{
 			// if the object was deserialized
 			if(artifactsRequirements.empty())
 				for(const auto & id : m5arts)
 					++artifactsRequirements[id];
 
+			size_t reqSlots = 0;
 			for(const auto & elem : artifactsRequirements)
 			{
 				// check required amount of artifacts
 				if(h->getArtPosCount(elem.first, false, true, true) < elem.second)
 					return false;
+				if(!h->hasArt(elem.first))
+					reqSlots += h->getAssemblyByConstituent(elem.first)->constituentsInfo.size() - 2;
 			}
-			return true;
+			if(ArtifactUtils::isBackpackFreeSlots(h, reqSlots))
+				return true;
+			else
+				return false;
+		}
 		case MISSION_ARMY:
 			return checkMissionArmy(this, h);
 		case MISSION_RESOURCES:
@@ -786,21 +794,28 @@ void CGSeerHut::finishQuest(const CGHeroInstance * h, ui32 accept) const
 		switch (quest->missionType)
 		{
 			case CQuest::MISSION_ART:
-				for (auto & elem : quest->m5arts)
+				for(auto & elem : quest->m5arts)
 				{
-					if(!h->hasArt(elem))
+					if(h->hasArt(elem))
+					{
+						cb->removeArtifact(ArtifactLocation(h, h->getArtPos(elem, false)));
+					}
+					else
 					{
-						// first we need to disassemble this backpack artifact
 						const auto * assembly = h->getAssemblyByConstituent(elem);
 						assert(assembly);
-						for(const auto & ci : assembly->constituentsInfo)
+						auto parts = assembly->constituentsInfo;
+
+						// Remove the assembly
+						cb->removeArtifact(ArtifactLocation(h, h->getArtPos(assembly)));
+
+						// Disassemble this backpack artifact
+						for(const auto & ci : parts)
 						{
-							cb->giveHeroNewArtifact(h, ci.art->artType, ArtifactPosition::PRE_FIRST);
+							if(ci.art->getTypeId() != elem)
+								cb->giveHeroNewArtifact(h, ci.art->artType, GameConstants::BACKPACK_START);
 						}
-						// remove the assembly
-						cb->removeArtifact(ArtifactLocation(h, h->getArtPos(assembly)));
 					}
-					cb->removeArtifact(ArtifactLocation(h, h->getArtPos(elem, false)));
 				}
 				break;
 			case CQuest::MISSION_ARMY:

lib/mapObjects/MiscObjects.cpp

@@ -1300,9 +1300,12 @@ void CGArtifact::onHeroVisit(const CGHeroInstance * h) const
 		InfoWindow iw;
 		iw.type = EInfoWindowMode::AUTO;
 		iw.player = h->tempOwner;
-		switch(ID)
+
+		if(storedArtifact->artType->canBePutAt(h))
 		{
-		case Obj::ARTIFACT:
+			switch (ID)
+			{
+			case Obj::ARTIFACT:
 			{
 				iw.components.emplace_back(Component::EComponentType::ARTIFACT, subID, 0, 0);
 				if(message.length())
@@ -1311,7 +1314,7 @@ void CGArtifact::onHeroVisit(const CGHeroInstance * h) const
 					iw.text.addTxt(MetaString::ART_EVNTS, subID);
 			}
 			break;
-		case Obj::SPELL_SCROLL:
+			case Obj::SPELL_SCROLL:
 			{
 				int spellID = storedArtifact->getGivenSpellID();
 				iw.components.emplace_back(Component::EComponentType::SPELL, spellID, 0, 0);
@@ -1324,6 +1327,11 @@ void CGArtifact::onHeroVisit(const CGHeroInstance * h) const
 				}
 			}
 			break;
+			}
+		}
+		else
+		{
+			iw.text.addTxt(MetaString::ADVOB_TXT, 2);
 		}
 		cb->showInfoDialog(&iw);
 		pick(h);
@@ -1368,8 +1376,8 @@ void CGArtifact::onHeroVisit(const CGHeroInstance * h) const
 
 void CGArtifact::pick(const CGHeroInstance * h) const
 {
-	cb->giveHeroArtifact(h, storedArtifact, ArtifactPosition::FIRST_AVAILABLE);
-	cb->removeObject(this);
+	if(cb->giveHeroArtifact(h, storedArtifact, ArtifactPosition::FIRST_AVAILABLE))
+		cb->removeObject(this);
 }
 
 BattleField CGArtifact::getBattlefield() const

server/CGameHandler.cpp

@@ -758,12 +758,15 @@ void CGameHandler::endBattle(int3 tile, const CGHeroInstance * heroAttacker, con
 	{
 		auto sendMoveArtifact = [&](const CArtifactInstance *art, MoveArtifact *ma)
 		{
-			arts.push_back(art);
-			auto slot = art->firstAvailableSlot(finishingBattle->winnerHero);
-			ma->dst = ArtifactLocation(finishingBattle->winnerHero, slot);
-			if(ArtifactUtils::isSlotBackpack(slot))
-				ma->askAssemble = false;
-			sendAndApply(ma);
+			const auto slot = ArtifactUtils::getArtAnyPosition(finishingBattle->winnerHero, art->getTypeId());
+			if(slot != ArtifactPosition::PRE_FIRST)
+			{
+				arts.push_back(art);
+				ma->dst = ArtifactLocation(finishingBattle->winnerHero, slot);
+				if(ArtifactUtils::isSlotBackpack(slot))
+					ma->askAssemble = false;
+				sendAndApply(ma);
+			}
 		};
 
 		if (finishingBattle->loserHero)
@@ -3914,36 +3917,41 @@ bool CGameHandler::moveArtifact(const ArtifactLocation &al1, const ArtifactLocat
 	const CArmedInstance *srcObj = src.relatedObj(), *dstObj = dst.relatedObj();
 
 	// Make sure exchange is even possible between the two heroes.
-	if (!isAllowedExchange(srcObj->id, dstObj->id))
+	if(!isAllowedExchange(srcObj->id, dstObj->id))
 		COMPLAIN_RET("That heroes cannot make any exchange!");
 
 	const CArtifactInstance *srcArtifact = src.getArt();
 	const CArtifactInstance *destArtifact = dst.getArt();
+	const bool isDstSlotBackpack = ArtifactUtils::isSlotBackpack(dst.slot);
 
-	if (srcArtifact == nullptr)
+	if(srcArtifact == nullptr)
 		COMPLAIN_RET("No artifact to move!");
-	if (destArtifact && srcPlayer != dstPlayer)
+	if(destArtifact && srcPlayer != dstPlayer)
 		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 || !ArtifactUtils::isSlotBackpack(dst.slot))
+	if((!srcArtifact || !isDstSlotBackpack)
 		&& srcArtifact && !srcArtifact->canBePutAt(dst, true))
 		COMPLAIN_RET("Cannot move artifact!");
 
 	auto srcSlot = src.getSlot();
 	auto dstSlot = dst.getSlot();
 
-	if ((srcSlot && srcSlot->locked) || (dstSlot && dstSlot->locked))
+	if((srcSlot && srcSlot->locked) || (dstSlot && dstSlot->locked))
 		COMPLAIN_RET("Cannot move artifact locks.");
 
-	if (dst.slot >= GameConstants::BACKPACK_START && srcArtifact->artType->isBig())
+	if(isDstSlotBackpack && srcArtifact->artType->isBig())
 		COMPLAIN_RET("Cannot put big artifacts in backpack!");
-	if (src.slot == ArtifactPosition::MACH4 || dst.slot == ArtifactPosition::MACH4)
+	if(src.slot == ArtifactPosition::MACH4 || dst.slot == ArtifactPosition::MACH4)
 		COMPLAIN_RET("Cannot move catapult!");
 
-	if(ArtifactUtils::isSlotBackpack(dst.slot))
-		vstd::amin(dst.slot, ArtifactPosition(GameConstants::BACKPACK_START + (si32)dst.getHolderArtSet()->artifactsInBackpack.size()));
+	if(isDstSlotBackpack)
+	{
+		if(!ArtifactUtils::isBackpackFreeSlots(dst.getHolderArtSet()))
+			COMPLAIN_RET("Backpack is full!");
+		vstd::amin(dst.slot, GameConstants::BACKPACK_START + dst.getHolderArtSet()->artifactsInBackpack.size());
+	}
 
 	if(!(src.slot == ArtifactPosition::TRANSITION_POS && dst.slot == ArtifactPosition::TRANSITION_POS))
 	{
@@ -3951,7 +3959,7 @@ bool CGameHandler::moveArtifact(const ArtifactLocation &al1, const ArtifactLocat
 			COMPLAIN_RET("Won't move artifact: Dest same as source!");
 
 		// Check if dst slot is occupied
-		if(!ArtifactUtils::isSlotBackpack(dst.slot) && destArtifact)
+		if(!isDstSlotBackpack && destArtifact)
 		{
 			// Previous artifact must be removed first
 			moveArtifact(dst, ArtifactLocation(dst.artHolder, ArtifactPosition::TRANSITION_POS));
@@ -4000,12 +4008,15 @@ bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID
 		std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
 	{
 		assert(artifact);
-		auto dstSlot = ArtifactUtils::getArtifactDstPosition(artifact, &artFittingSet);
-		artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
-		slots.push_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
+		auto dstSlot = ArtifactUtils::getArtAnyPosition(&artFittingSet, artifact->getTypeId());
+		if(dstSlot != ArtifactPosition::PRE_FIRST)
+		{
+			artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
+			slots.push_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
 
-		if(ArtifactUtils::checkSpellbookIsNeeded(dstHero, artifact->artType->getId(), dstSlot))
-			giveHeroNewArtifact(dstHero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
+			if(ArtifactUtils::checkSpellbookIsNeeded(dstHero, artifact->getTypeId(), dstSlot))
+				giveHeroNewArtifact(dstHero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
+		}
 	};
 
 	if(swap)
@@ -4073,18 +4084,17 @@ bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID
 bool CGameHandler::assembleArtifacts (ObjectInstanceID heroID, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo)
 {
 	const CGHeroInstance * hero = getHero(heroID);
-	const CArtifactInstance *destArtifact = hero->getArt(artifactSlot);
+	const CArtifactInstance * destArtifact = hero->getArt(artifactSlot);
 
-	if (!destArtifact)
+	if(!destArtifact)
 		COMPLAIN_RET("assembleArtifacts: there is no such artifact instance!");
 
 	if(assemble)
 	{
-		CArtifact *combinedArt = VLC->arth->objects[assembleTo];
+		CArtifact * combinedArt = VLC->arth->objects[assembleTo];
 		if(!combinedArt->constituents)
 			COMPLAIN_RET("assembleArtifacts: Artifact being attempted to assemble is not a combined artifacts!");
-		bool combineEquipped = !ArtifactUtils::isSlotBackpack(artifactSlot);
-		if(!vstd::contains(destArtifact->assemblyPossibilities(hero, combineEquipped), combinedArt))
+		if(!vstd::contains(destArtifact->assemblyPossibilities(hero, ArtifactUtils::isSlotEquipment(artifactSlot)), combinedArt))
 			COMPLAIN_RET("assembleArtifacts: It's impossible to assemble requested artifact!");
 
 		
@@ -4098,9 +4108,13 @@ bool CGameHandler::assembleArtifacts (ObjectInstanceID heroID, ArtifactPosition
 	}
 	else
 	{
-		if (!destArtifact->artType->constituents)
+		if(!destArtifact->canBeDisassembled())
 			COMPLAIN_RET("assembleArtifacts: Artifact being attempted to disassemble is not a combined artifact!");
 
+		if(ArtifactUtils::isSlotBackpack(artifactSlot)
+			&& !ArtifactUtils::isBackpackFreeSlots(hero, destArtifact->artType->constituents->size() - 1))
+			COMPLAIN_RET("assembleArtifacts: Artifact being attempted to disassemble but backpack is full!");
+
 		DisassembledArtifact da;
 		da.al = ArtifactLocation(hero, artifactSlot);
 		sendAndApply(&da);
@@ -4139,7 +4153,7 @@ bool CGameHandler::buyArtifact(ObjectInstanceID hid, ArtifactID aid)
 		const int price = art->price;
 		COMPLAIN_RET_FALSE_IF(getPlayerState(hero->getOwner())->resources.at(Res::GOLD) < price, "Not enough gold!");
 
-		if  ((town->hasBuilt(BuildingID::BLACKSMITH) && town->town->warMachine == aid)
+		if ((town->hasBuilt(BuildingID::BLACKSMITH) && town->town->warMachine == aid)
 		 || (town->hasBuilt(BuildingSubID::BALLISTA_YARD) && aid == ArtifactID::BALLISTA))
 		{
 			giveResource(hero->getOwner(),Res::GOLD,-price);
@@ -4195,7 +4209,6 @@ bool CGameHandler::buyArtifact(const IMarket *m, const CGHeroInstance *h, Res::E
 		COMPLAIN_RET("Cannot find selected artifact on the list");
 
 	sendAndApply(&saa);
-
 	giveHeroNewArtifact(h, VLC->arth->objects[aid], ArtifactPosition::FIRST_AVAILABLE);
 	return true;
 }
@@ -6805,35 +6818,32 @@ bool CGameHandler::makeAutomaticAction(const CStack *stack, BattleAction &ba)
 	return ret;
 }
 
-void CGameHandler::giveHeroArtifact(const CGHeroInstance *h, const CArtifactInstance *a, ArtifactPosition pos)
+bool CGameHandler::giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos)
 {
 	assert(a->artType);
-	ArtifactLocation al;
-	al.artHolder = const_cast<CGHeroInstance*>(h);
+	ArtifactLocation al(h, ArtifactPosition::PRE_FIRST);
 
-	ArtifactPosition slot = ArtifactPosition::PRE_FIRST;
-	if (pos < 0)
+	if(pos == ArtifactPosition::FIRST_AVAILABLE)
 	{
-		if (pos == ArtifactPosition::FIRST_AVAILABLE)
-			slot = a->firstAvailableSlot(h);
-		else
-			slot = a->firstBackpackSlot(h);
+		al.slot = ArtifactUtils::getArtAnyPosition(h, a->getTypeId());
 	}
-	else
+	else if(ArtifactUtils::isSlotBackpack(pos))
 	{
-		slot = pos;
+		al.slot = ArtifactUtils::getArtBackpackPosition(h, a->getTypeId());
 	}
-
-	al.slot = slot;
-
-	if (slot < 0 || !a->canBePutAt(al))
+	else
 	{
-		complain("Cannot put artifact in that slot!");
-		return;
+		al.slot = pos;
 	}
 
-	putArtifact(al, a);
+	if(a->canBePutAt(al))
+		putArtifact(al, a);
+	else
+		return false;
+
+	return true;
 }
+
 void CGameHandler::putArtifact(const ArtifactLocation &al, const CArtifactInstance *a)
 {
 	PutArtifact pa;
@@ -6842,36 +6852,31 @@ void CGameHandler::putArtifact(const ArtifactLocation &al, const CArtifactInstan
 	sendAndApply(&pa);
 }
 
-bool CGameHandler::giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * art)
+bool CGameHandler::giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos)
 {
-	COMPLAIN_RET_FALSE_IF(art->possibleSlots.at(ArtBearer::HERO).empty(), "Not a hero artifact!");
-
-	ArtifactPosition slot = art->possibleSlots.at(ArtBearer::HERO).front();
+	assert(artType);
+	if(pos != ArtifactPosition::FIRST_AVAILABLE && !ArtifactUtils::isSlotBackpack(pos))
+		COMPLAIN_RET_FALSE_IF(!artType->canBePutAt(h, pos, false), "Cannot put artifact in that slot!");
 
-	COMPLAIN_RET_FALSE_IF(nullptr != h->getArt(slot, false), "Hero already has artifact in slot");
-
-	giveHeroNewArtifact(h, art, slot);
-	return true;
-}
+	CArtifactInstance * newArtInst = nullptr;
+	if(artType->canBeDisassembled())
+		newArtInst = new CCombinedArtifactInstance();
+	else
+		newArtInst = new CArtifactInstance();
 
-void CGameHandler::giveHeroNewArtifact(const CGHeroInstance *h, const CArtifact *artType, ArtifactPosition pos)
-{
-	CArtifactInstance *a = nullptr;
-	if (!artType->constituents)
+	newArtInst->artType = artType; // *NOT* via settype -> all bonus-related stuff must be done by NewArtifact apply
+	if(giveHeroArtifact(h, newArtInst, pos))
 	{
-		a = new CArtifactInstance();
+		NewArtifact na;
+		na.art = newArtInst;
+		sendAndApply(&na); // -> updates a!!!, will create a on other machines
+		return true;
 	}
 	else
 	{
-		a = new CCombinedArtifactInstance();
+		delete newArtInst;
+		return false;
 	}
-	a->artType = artType; //*NOT* via settype -> all bonus-related stuff must be done by NewArtifact apply
-
-	NewArtifact na;
-	na.art = a;
-	sendAndApply(&na); // -> updates a!!!, will create a on other machines
-
-	giveHeroArtifact(h, a, pos);
 }
 
 void CGameHandler::setBattleResult(BattleResult::EResult resultType, int victoriusSide)
@@ -7021,8 +7026,11 @@ void CGameHandler::handleCheatCode(std::string & cheat, PlayerColor player, cons
 		cheated = true;
 		if (!hero) return;
 		///Give hero all artifacts except war machines, spell scrolls and spell book
-		for (int g = 7; g < VLC->arth->objects.size(); ++g) //including artifacts from mods
-			giveHeroNewArtifact(hero, VLC->arth->objects[g], ArtifactPosition::PRE_FIRST);
+		for(int g = 7; g < VLC->arth->objects.size(); ++g) //including artifacts from mods
+		{
+			if(VLC->arth->objects[g]->canBePutAt(hero))
+				giveHeroNewArtifact(hero, VLC->arth->objects[g], ArtifactPosition::FIRST_AVAILABLE);
+		}
 	}
 	else if (cheat == "vcmiglorfindel" || cheat == "vcmilevel")
 	{

server/CGameHandler.h

@@ -175,9 +175,8 @@ public:
 
 	void removeAfterVisit(const CGObjectInstance *object) override;
 
-	bool giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * art);
-	void giveHeroNewArtifact(const CGHeroInstance *h, const CArtifact *artType, ArtifactPosition pos) override;
-	void giveHeroArtifact(const CGHeroInstance *h, const CArtifactInstance *a, ArtifactPosition pos) override;
+	bool giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos = ArtifactPosition::FIRST_AVAILABLE) override;
+	bool giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) override;
 	void putArtifact(const ArtifactLocation &al, const CArtifactInstance *a) override;
 	void removeArtifact(const ArtifactLocation &al) override;
 	bool moveArtifact(const ArtifactLocation & al1, const ArtifactLocation & al2) override;

test/mock/mock_IGameCallback.h

@@ -64,8 +64,8 @@ public:
 
 	void removeAfterVisit(const CGObjectInstance *object) override {} //object will be destroyed when interaction is over. Do not call when interaction is not ongoing!
 
-	void giveHeroNewArtifact(const CGHeroInstance *h, const CArtifact *artType, ArtifactPosition pos) override {}
-	void giveHeroArtifact(const CGHeroInstance *h, const CArtifactInstance *a, ArtifactPosition pos) override {} //pos==-1 - first free slot in backpack=0; pos==-2 - default if available or backpack
+	bool giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, ArtifactPosition pos) override {return false;}
+	bool giveHeroArtifact(const CGHeroInstance * h, const CArtifactInstance * a, ArtifactPosition pos) override {return false;}
 	void putArtifact(const ArtifactLocation &al, const CArtifactInstance *a) override {}
 	void removeArtifact(const ArtifactLocation &al) override {}
 	bool moveArtifact(const ArtifactLocation &al1, const ArtifactLocation &al2) override {return false;}