Merge remote-tracking branch 'soundsgood/CArtHandler-rework' into develop

client/widgets/CArtifactHolder.cpp

@@ -23,6 +23,7 @@
 
 #include "../../CCallback.h"
 #include "../../lib/CGeneralTextHandler.h"
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 
 void CArtPlace::setInternals(const CArtifactInstance * artInst)

client/widgets/CArtifactsOfHeroAltar.cpp

@@ -14,6 +14,7 @@
 
 #include "../../CCallback.h"
 
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 
 CArtifactsOfHeroAltar::CArtifactsOfHeroAltar(const Point & position)
@@ -81,7 +82,7 @@ void CArtifactsOfHeroAltar::pickedArtMoveToAltar(const ArtifactPosition & slot)
 {
 	if(ArtifactUtils::isSlotBackpack(slot) || ArtifactUtils::isSlotEquipment(slot) || slot == ArtifactPosition::TRANSITION_POS)
 	{
-		assert(!curHero->getSlot(pickedArtFromSlot)->getArt());
+		assert(curHero->getSlot(slot)->getArt());
 		LOCPLINT->cb->swapArtifacts(ArtifactLocation(curHero, slot), ArtifactLocation(curHero, pickedArtFromSlot));
 		pickedArtFromSlot = ArtifactPosition::PRE_FIRST;
 	}

client/widgets/CArtifactsOfHeroBase.cpp

@@ -21,6 +21,7 @@
 
 #include "../../CCallback.h"
 
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 
 CArtifactsOfHeroBase::CArtifactsOfHeroBase()

client/widgets/CComponent.cpp

@@ -26,6 +26,7 @@
 #include "../widgets/TextControls.h"
 #include "../CGameInfo.h"
 
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/CTownHandler.h"
 #include "../../lib/spells/CSpellHandler.h"
 #include "../../lib/CCreatureHandler.h"
@@ -171,11 +172,11 @@ std::string CComponent::getDescription()
 		std::unique_ptr<CArtifactInstance> art;
 		if (artID != ArtifactID::SPELL_SCROLL)
 		{
-			art.reset(CArtifactInstance::createNewArtifactInstance(artID));
+			art.reset(ArtifactUtils::createNewArtifactInstance(artID));
 		}
 		else
 		{
-			art.reset(CArtifactInstance::createScroll(SpellID(val)));
+			art.reset(ArtifactUtils::createScroll(SpellID(val)));
 		}
 		return art->getDescription();
 	}

client/widgets/CGarrisonInt.cpp

@@ -22,6 +22,7 @@
 
 #include "../../CCallback.h"
 
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/CGeneralTextHandler.h"
 #include "../../lib/CCreatureHandler.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"

client/widgets/CWindowWithArtifacts.cpp

@@ -22,6 +22,7 @@
 #include "../CPlayerInterface.h"
 #include "../CGameInfo.h"
 
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/CGeneralTextHandler.h"
 #include "../../lib/mapObjects/CGHeroInstance.h"
 

client/windows/CCreatureWindow.cpp

@@ -25,6 +25,7 @@
 #include "../gui/Shortcut.h"
 
 #include "../../CCallback.h"
+#include "../../lib/ArtifactUtils.h"
 #include "../../lib/CStack.h"
 #include "../../lib/CBonusTypeHandler.h"
 #include "../../lib/CGeneralTextHandler.h"

client/windows/CHeroWindow.cpp

@@ -29,6 +29,7 @@
 
 #include "../../CCallback.h"
 
+#include "../lib/ArtifactUtils.h"
 #include "../lib/CArtHandler.h"
 #include "../lib/CConfigHandler.h"
 #include "../lib/CGeneralTextHandler.h"

client/windows/GUIClasses.cpp

@@ -47,6 +47,7 @@
 
 #include "../lib/mapObjects/CGHeroInstance.h"
 #include "../lib/mapObjects/CGMarket.h"
+#include "../lib/ArtifactUtils.h"
 #include "../lib/mapObjects/CGTownInstance.h"
 #include "../lib/mapObjects/ObjectTemplate.h"
 #include "../lib/CArtHandler.h"

cmake_modules/VCMI_lib.cmake

@@ -196,6 +196,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 
 		${MAIN_LIB_DIR}/vstd/StringUtils.cpp
 
+		${MAIN_LIB_DIR}/ArtifactUtils.cpp
 		${MAIN_LIB_DIR}/BasicTypes.cpp
 		${MAIN_LIB_DIR}/BattleFieldHandler.cpp
 		${MAIN_LIB_DIR}/CAndroidVMHelper.cpp
@@ -500,6 +501,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/spells/effects/Sacrifice.h
 
 		${MAIN_LIB_DIR}/AI_Base.h
+		${MAIN_LIB_DIR}/ArtifactUtils.h
 		${MAIN_LIB_DIR}/BattleFieldHandler.h
 		${MAIN_LIB_DIR}/CAndroidVMHelper.h
 		${MAIN_LIB_DIR}/CArtHandler.h

lib/ArtifactUtils.cpp

@@ -0,0 +1,223 @@
+/*
+ * ArtifactUtils.cpp, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+
+#include "ArtifactUtils.h"
+
+#include "CArtHandler.h"
+#include "GameSettings.h"
+
+#include "mapping/CMap.h"
+#include "mapObjects/CGHeroInstance.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtAnyPosition(const CArtifactSet * target, const ArtifactID & aid)
+{
+	const auto * art = aid.toArtifact();
+	for(const auto & slot : art->possibleSlots.at(target->bearerType()))
+	{
+		if(art->canBePutAt(target, slot))
+			return slot;
+	}
+	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()
+{
+	static const std::vector<ArtifactPosition::EArtifactPosition> positions =
+	{
+		ArtifactPosition::SPELLBOOK,
+		ArtifactPosition::MACH4
+	};
+
+	return positions;
+}
+
+DLL_LINKAGE const std::vector<ArtifactPosition::EArtifactPosition> & ArtifactUtils::constituentWornSlots()
+{
+	static const std::vector<ArtifactPosition::EArtifactPosition> positions =
+	{
+		ArtifactPosition::HEAD,
+		ArtifactPosition::SHOULDERS,
+		ArtifactPosition::NECK,
+		ArtifactPosition::RIGHT_HAND,
+		ArtifactPosition::LEFT_HAND,
+		ArtifactPosition::TORSO,
+		ArtifactPosition::RIGHT_RING,
+		ArtifactPosition::LEFT_RING,
+		ArtifactPosition::FEET,
+		ArtifactPosition::MISC1,
+		ArtifactPosition::MISC2,
+		ArtifactPosition::MISC3,
+		ArtifactPosition::MISC4,
+		ArtifactPosition::MISC5,
+	};
+
+	return positions;
+}
+
+DLL_LINKAGE bool ArtifactUtils::isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot)
+{
+	return slot.second.artifact
+		&& !slot.second.locked
+		&& !vstd::contains(unmovableSlots(), slot.first);
+}
+
+DLL_LINKAGE bool ArtifactUtils::checkSpellbookIsNeeded(const CGHeroInstance * heroPtr, const ArtifactID & artID, const ArtifactPosition & slot)
+{
+	// TODO what'll happen if Titan's thunder is equipped by pickin git up or the start of game?
+	// Titan's Thunder creates new spellbook on equip
+	if(artID == ArtifactID::TITANS_THUNDER && slot == ArtifactPosition::RIGHT_HAND)
+	{
+		if(heroPtr)
+		{
+			if(heroPtr && !heroPtr->hasSpellbook())
+				return true;
+		}
+	}
+	return false;
+}
+
+DLL_LINKAGE bool ArtifactUtils::isSlotBackpack(const ArtifactPosition & slot)
+{
+	return slot >= GameConstants::BACKPACK_START;
+}
+
+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;
+}
+
+DLL_LINKAGE std::vector<const CArtifact*> ArtifactUtils::assemblyPossibilities(
+	const CArtifactSet * artSet, const ArtifactID & aid, bool equipped)
+{
+	std::vector<const CArtifact*> arts;
+	const auto * art = aid.toArtifact();
+	if(art->canBeDisassembled())
+		return arts;
+
+	for(const auto artifact : art->constituentOf)
+	{
+		assert(artifact->constituents);
+		bool possible = true;
+
+		for(const auto constituent : *artifact->constituents) //check if all constituents are available
+		{
+			if(equipped)
+			{
+				// Search for equipped arts
+				if(!artSet->hasArt(constituent->getId(), true, false, false))
+				{
+					possible = false;
+					break;
+				}
+			}
+			else
+			{
+				// Search in backpack
+				if(!artSet->hasArtBackpack(constituent->getId()))
+				{
+					possible = false;
+					break;
+				}
+			}
+		}
+		if(possible)
+			arts.push_back(artifact);
+	}
+	return arts;
+}
+
+DLL_LINKAGE CArtifactInstance * ArtifactUtils::createScroll(const SpellID & sid)
+{
+	auto ret = new CArtifactInstance(VLC->arth->objects[ArtifactID::SPELL_SCROLL]);
+	auto bonus = std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::SPELL,
+		BonusSource::ARTIFACT_INSTANCE, -1, ArtifactID::SPELL_SCROLL, sid);
+	ret->addNewBonus(bonus);
+	return ret;
+}
+
+DLL_LINKAGE CArtifactInstance * ArtifactUtils::createNewArtifactInstance(CArtifact * art)
+{
+	if(art->canBeDisassembled())
+	{
+		auto * ret = new CCombinedArtifactInstance(art);
+		ret->createConstituents();
+		return ret;
+	}
+	else
+	{
+		auto * ret = new CArtifactInstance(art);
+		if(dynamic_cast<CGrowingArtifact*>(art))
+		{
+			auto bonus = std::make_shared<Bonus>();
+			bonus->type = BonusType::LEVEL_COUNTER;
+			bonus->val = 0;
+			ret->addNewBonus(bonus);
+		}
+		return ret;
+	}
+}
+
+DLL_LINKAGE CArtifactInstance * ArtifactUtils::createNewArtifactInstance(const ArtifactID & aid)
+{
+	return ArtifactUtils::createNewArtifactInstance(VLC->arth->objects[aid]);
+}
+
+DLL_LINKAGE CArtifactInstance * ArtifactUtils::createArtifact(CMap * map, const ArtifactID & aid, int spellID)
+{
+	CArtifactInstance * art = nullptr;
+	if(aid >= 0)
+	{
+		if(spellID < 0)
+		{
+			art = ArtifactUtils::createNewArtifactInstance(aid);
+		}
+		else
+		{
+			art = ArtifactUtils::createScroll(SpellID(spellID));
+		}
+	}
+	else //TODO: create combined artifact instance for random artifacts, just in case
+	{
+		art = new CArtifactInstance(); // random, empty
+	}
+	map->addNewArtifactInstance(art);
+	if(art->artType && art->canBeDisassembled())
+	{
+		auto * combined = dynamic_cast<CCombinedArtifactInstance*>(art);
+		for(CCombinedArtifactInstance::ConstituentInfo & ci : combined->constituentsInfo)
+		{
+			map->addNewArtifactInstance(ci.art);
+		}
+	}
+	return art;
+}
+
+VCMI_LIB_NAMESPACE_END

lib/ArtifactUtils.h

@@ -0,0 +1,46 @@
+/*
+ * ArtifactUtils.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+#include "StdInc.h"
+
+#include "GameConstants.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+class CArtHandler;
+class CArtifact;
+class CGHeroInstance;
+class CArtifactSet;
+class CArtifactInstance;
+struct ArtSlotInfo;
+class CMap;
+
+namespace ArtifactUtils
+{
+	// Calculates where an artifact gets placed when it gets transferred from one hero to another.
+	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();
+	DLL_LINKAGE bool isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot);
+	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);
+	DLL_LINKAGE std::vector<const CArtifact*> assemblyPossibilities(const CArtifactSet * artSet, const ArtifactID & aid, bool equipped);
+	DLL_LINKAGE CArtifactInstance * createScroll(const SpellID & sid);
+	DLL_LINKAGE CArtifactInstance * createNewArtifactInstance(CArtifact * art);
+	DLL_LINKAGE CArtifactInstance * createNewArtifactInstance(const ArtifactID & aid);
+	DLL_LINKAGE CArtifactInstance * createArtifact(CMap * map, const ArtifactID & aid, int spellID = -1);
+}
+
+VCMI_LIB_NAMESPACE_END

lib/CArtHandler.cpp

@@ -11,21 +11,17 @@
 #include "StdInc.h"
 #include "CArtHandler.h"
 
-#include "filesystem/Filesystem.h"
+#include "ArtifactUtils.h"
 #include "CGeneralTextHandler.h"
-#include "VCMI_Lib.h"
 #include "CModHandler.h"
 #include "GameSettings.h"
-#include "CCreatureHandler.h"
 #include "spells/CSpellHandler.h"
 #include "mapObjects/MapObjects.h"
 #include "NetPacksBase.h"
 #include "StringConstants.h"
-#include "CRandomGenerator.h"
 
 #include "mapObjectConstructors/AObjectTypeHandler.h"
 #include "mapObjectConstructors/CObjectClassesHandler.h"
-#include "mapping/CMap.h"
 #include "serializer/JsonSerializeFormat.h"
 
 // Note: list must match entries in ArtTraits.txt
@@ -820,14 +816,6 @@ std::string CArtifactInstance::nodeName() const
 	return "Artifact instance of " + (artType ? artType->getJsonKey() : std::string("uninitialized")) + " type";
 }
 
-CArtifactInstance * CArtifactInstance::createScroll(const SpellID & sid)
-{
-	auto * ret = new CArtifactInstance(VLC->arth->objects[ArtifactID::SPELL_SCROLL]);
-	auto b = std::make_shared<Bonus>(BonusDuration::PERMANENT, BonusType::SPELL, BonusSource::ARTIFACT_INSTANCE, -1, ArtifactID::SPELL_SCROLL, sid);
-	ret->addNewBonus(b);
-	return ret;
-}
-
 void CArtifactInstance::init()
 {
 	id = ArtifactInstanceID();
@@ -870,19 +858,12 @@ bool CArtifactInstance::canBePutAt(const ArtifactLocation & al, bool assumeDestR
 
 void CArtifactInstance::putAt(ArtifactLocation al)
 {
-	assert(canBePutAt(al));
-
-	al.getHolderArtSet()->setNewArtSlot(al.slot, this, false);
-	if(ArtifactUtils::isSlotEquipment(al.slot))
-		al.getHolderNode()->attachTo(*this);
+	al.getHolderArtSet()->putArtifact(al.slot, this);
 }
 
 void CArtifactInstance::removeFrom(ArtifactLocation al)
 {
-	assert(al.getHolderArtSet()->getArt(al.slot) == this);
-	al.getHolderArtSet()->eraseArtSlot(al.slot);
-	if(ArtifactUtils::isSlotEquipment(al.slot))
-		al.getHolderNode()->detachFrom(*this);
+	al.getHolderArtSet()->removeArtifact(al.slot);
 }
 
 bool CArtifactInstance::canBeDisassembled() const
@@ -896,67 +877,6 @@ void CArtifactInstance::move(const ArtifactLocation & src, const ArtifactLocatio
 	putAt(dst);
 }
 
-CArtifactInstance * CArtifactInstance::createNewArtifactInstance(CArtifact *Art)
-{
-	if(!Art->constituents)
-	{
-		auto * ret = new CArtifactInstance(Art);
-		if (dynamic_cast<CGrowingArtifact *>(Art))
-		{
-			auto bonus = std::make_shared<Bonus>();
-			bonus->type = BonusType::LEVEL_COUNTER;
-			bonus->val = 0;
-			ret->addNewBonus (bonus);
-		}
-		return ret;
-	}
-	else
-	{
-		auto * ret = new CCombinedArtifactInstance(Art);
-		ret->createConstituents();
-		return ret;
-	}
-}
-
-CArtifactInstance * CArtifactInstance::createNewArtifactInstance(const ArtifactID & aid)
-{
-	return createNewArtifactInstance(VLC->arth->objects[aid]);
-}
-
-CArtifactInstance * CArtifactInstance::createArtifact(CMap * map, const ArtifactID & aid, int spellID)
-{
-	CArtifactInstance * a = nullptr;
-	if(aid >= 0)
-	{
-		if(spellID < 0)
-		{
-			a = CArtifactInstance::createNewArtifactInstance(aid);
-		}
-		else
-		{
-			a = CArtifactInstance::createScroll(SpellID(spellID));
-		}
-	}
-	else //FIXME: create combined artifact instance for random combined artifacts, just in case
-	{
-		a = new CArtifactInstance(); //random, empty
-	}
-
-	map->addNewArtifactInstance(a);
-
-	//TODO make it nicer
-	if(a->artType && (!!a->artType->constituents))
-	{
-		auto * comb = dynamic_cast<CCombinedArtifactInstance *>(a);
-		for(CCombinedArtifactInstance::ConstituentInfo & ci : comb->constituentsInfo)
-		{
-			map->addNewArtifactInstance(ci.art);
-		}
-	}
-	return a;
-}
-
-
 void CArtifactInstance::deserializationFix()
 {
 	setType(artType);
@@ -990,7 +910,7 @@ void CCombinedArtifactInstance::createConstituents()
 
 	for(const CArtifact * art : *artType->constituents)
 	{
-		addAsConstituent(CArtifactInstance::createNewArtifactInstance(art->getId()), ArtifactPosition::PRE_FIRST);
+		addAsConstituent(ArtifactUtils::createNewArtifactInstance(art->getId()), ArtifactPosition::PRE_FIRST);
 	}
 }
 
@@ -1004,90 +924,6 @@ void CCombinedArtifactInstance::addAsConstituent(CArtifactInstance * art, const
 	attachTo(*art);
 }
 
-void CCombinedArtifactInstance::putAt(ArtifactLocation al)
-{
-	if(al.slot == ArtifactPosition::TRANSITION_POS)
-	{
-		CArtifactInstance::putAt(al);
-	}
-	else if(ArtifactUtils::isSlotBackpack(al.slot))
-	{
-		CArtifactInstance::putAt(al);
-		for(ConstituentInfo &ci : constituentsInfo)
-			ci.slot = ArtifactPosition::PRE_FIRST;
-	}
-	else
-	{
-		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)
-		{
-			if(ci.art != mainConstituent)
-			{
-				const ArtifactLocation suggestedPos(al.artHolder, ci.slot);
-				const bool inActiveSlot = vstd::isbetween(ci.slot, 0, GameConstants::BACKPACK_START);
-				const bool suggestedPosValid = ci.art->canBePutAt(suggestedPos);
-
-				if(!(inActiveSlot && suggestedPosValid)) //there is a valid suggestion where to place lock
-					ci.slot = ArtifactUtils::getArtAnyPosition(al.getHolderArtSet(), ci.art->getTypeId());
-
-				assert(ArtifactUtils::isSlotEquipment(ci.slot));
-				al.getHolderArtSet()->setNewArtSlot(ci.slot, ci.art, true); //sets as lock
-			}
-			else
-			{
-				ci.slot = ArtifactPosition::PRE_FIRST;
-			}
-		}
-	}
-}
-
-void CCombinedArtifactInstance::removeFrom(ArtifactLocation al)
-{
-	if(ArtifactUtils::isSlotBackpack(al.slot) || al.slot == ArtifactPosition::TRANSITION_POS)
-	{
-		CArtifactInstance::removeFrom(al);
-	}
-	else
-	{
-		for(ConstituentInfo &ci : constituentsInfo)
-		{
-			if(ci.slot >= 0)
-			{
-				al.getHolderArtSet()->eraseArtSlot(ci.slot);
-				ci.slot = ArtifactPosition::PRE_FIRST;
-			}
-			else
-			{
-				//main constituent
-				CArtifactInstance::removeFrom(al);
-			}
-		}
-	}
-}
-
-CArtifactInstance * CCombinedArtifactInstance::figureMainConstituent(const ArtifactLocation & al)
-{
-	CArtifactInstance *mainConstituent = nullptr; //it'll be replaced with combined artifact, not a lock
-	for(ConstituentInfo &ci : constituentsInfo)
-		if(ci.slot == al.slot)
-			mainConstituent = ci.art;
-
-	if(!mainConstituent)
-	{
-		for(ConstituentInfo &ci : constituentsInfo)
-		{
-			if(vstd::contains(ci.art->artType->possibleSlots[al.getHolderArtSet()->bearerType()], al.slot))
-			{
-				mainConstituent = ci.art;
-			}
-		}
-	}
-
-	return mainConstituent;
-}
-
 void CCombinedArtifactInstance::deserializationFix()
 {
 	for(ConstituentInfo &ci : constituentsInfo)
@@ -1248,6 +1084,52 @@ unsigned CArtifactSet::getArtPosCount(const ArtifactID & aid, bool onlyWorn, boo
 	return 0;
 }
 
+void CArtifactSet::putArtifact(ArtifactPosition slot, CArtifactInstance * art)
+{
+	setNewArtSlot(slot, art, false);
+	if(art->artType->canBeDisassembled() && ArtifactUtils::isSlotEquipment(slot))
+	{
+		const CArtifactInstance * mainPart = nullptr;
+		auto & parts = dynamic_cast<CCombinedArtifactInstance*>(art)->constituentsInfo;
+		for(const auto & part : parts)
+			if(vstd::contains(part.art->artType->possibleSlots.at(bearerType()), slot))
+			{
+				mainPart = part.art;
+				break;
+			}
+
+		for(auto & part : parts)
+		{
+			if(part.art != mainPart)
+			{
+				if(!part.art->artType->canBePutAt(this, part.slot))
+					part.slot = ArtifactUtils::getArtAnyPosition(this, part.art->getTypeId());
+
+				assert(ArtifactUtils::isSlotEquipment(part.slot));
+				setNewArtSlot(part.slot, art, true);
+			}
+		}
+	}
+}
+
+void CArtifactSet::removeArtifact(ArtifactPosition slot)
+{
+	auto art = getArt(slot, false);
+	if(art)
+	{
+		if(art->canBeDisassembled())
+		{
+			auto combinedArt = dynamic_cast<CCombinedArtifactInstance*>(art);
+			for(auto & part : combinedArt->constituentsInfo)
+			{
+				if(getArt(part.slot, false))
+					eraseArtSlot(part.slot);
+			}
+		}
+		eraseArtSlot(slot);
+	}
+}
+
 std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *> CArtifactSet::searchForConstituent(const ArtifactID & aid) const
 {
 	for(const auto & slot : artifactsInBackpack)
@@ -1310,32 +1192,28 @@ bool CArtifactSet::isPositionFree(const ArtifactPosition & pos, bool onlyLockChe
 	return true; //no slot means not used
 }
 
-ArtSlotInfo & CArtifactSet::retrieveNewArtSlot(const ArtifactPosition & slot)
+void CArtifactSet::setNewArtSlot(const ArtifactPosition & slot, CArtifactInstance * art, bool locked)
 {
 	assert(!vstd::contains(artifactsWorn, slot));
 
+	ArtSlotInfo * slotInfo;
 	if(slot == ArtifactPosition::TRANSITION_POS)
 	{
 		// Always add to the end. Always take from the beginning.
 		artifactsTransitionPos.emplace_back();
-		return artifactsTransitionPos.back();
+		slotInfo = &artifactsTransitionPos.back();
 	}
-	if(!ArtifactUtils::isSlotBackpack(slot))
-		return artifactsWorn[slot];
-
-	ArtSlotInfo newSlot;
-	size_t index  = slot - GameConstants::BACKPACK_START;
-	auto position = artifactsInBackpack.begin() + index;
-	auto inserted = artifactsInBackpack.insert(position, newSlot);
-
-	return *inserted;
-}
-
-void CArtifactSet::setNewArtSlot(const ArtifactPosition & slot, CArtifactInstance * art, bool locked)
-{
-	ArtSlotInfo & asi = retrieveNewArtSlot(slot);
-	asi.artifact = art;
-	asi.locked = locked;
+	else if(ArtifactUtils::isSlotEquipment(slot))
+	{
+		slotInfo =  &artifactsWorn[slot];
+	}
+	else
+	{
+		auto position = artifactsInBackpack.begin() + slot - GameConstants::BACKPACK_START;
+		slotInfo = &(*artifactsInBackpack.emplace(position, ArtSlotInfo()));
+	}
+	slotInfo->artifact = art;
+	slotInfo->locked = locked;
 }
 
 void CArtifactSet::eraseArtSlot(const ArtifactPosition & slot)
@@ -1417,7 +1295,7 @@ void CArtifactSet::serializeJsonHero(JsonSerializeFormat & handler, CMap * map)
 	{
 		for(const ArtifactID & artifactID : backpackTemp)
 		{
-			auto * artifact = CArtifactInstance::createArtifact(map, artifactID.toEnum());
+			auto * artifact = ArtifactUtils::createArtifact(map, artifactID.toEnum());
 			auto slot = ArtifactPosition(GameConstants::BACKPACK_START + (si32)artifactsInBackpack.size());
 			if(artifact->artType->canBePutAt(this, slot))
 				putArtifact(slot, artifact);
@@ -1455,7 +1333,7 @@ void CArtifactSet::serializeJsonSlot(JsonSerializeFormat & handler, const Artifa
 
 		if(artifactID != ArtifactID::NONE)
 		{
-			auto * artifact = CArtifactInstance::createArtifact(map, artifactID.toEnum());
+			auto * artifact = ArtifactUtils::createArtifact(map, artifactID.toEnum());
 
 			if(artifact->artType->canBePutAt(this, slot))
 			{
@@ -1474,183 +1352,9 @@ CArtifactFittingSet::CArtifactFittingSet(ArtBearer::ArtBearer Bearer):
 {
 }
 
-void CArtifactFittingSet::putArtifact(ArtifactPosition pos, CArtifactInstance * art)
-{
-	if(art && art->canBeDisassembled() && ArtifactUtils::isSlotEquipment(pos))
-	{
-		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(slot, part.art, true);
-		}
-	}
-	else
-	{
-		this->setNewArtSlot(pos, art, false);
-	}
-}
-
-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::getArtAnyPosition(const CArtifactSet * target, const ArtifactID & aid)
-{
-	const auto * art = aid.toArtifact();
-	for(const auto & slot : art->possibleSlots.at(target->bearerType()))
-	{
-		if(art->canBePutAt(target, slot))
-			return slot;
-	}
-	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()
-{
-	static const std::vector<ArtifactPosition::EArtifactPosition> positions =
-	{
-		ArtifactPosition::SPELLBOOK,
-		ArtifactPosition::MACH4
-	};
-
-	return positions;
-}
-
-DLL_LINKAGE const std::vector<ArtifactPosition::EArtifactPosition> & ArtifactUtils::constituentWornSlots()
-{
-	static const std::vector<ArtifactPosition::EArtifactPosition> positions =
-	{
-		ArtifactPosition::HEAD,
-		ArtifactPosition::SHOULDERS,
-		ArtifactPosition::NECK,
-		ArtifactPosition::RIGHT_HAND,
-		ArtifactPosition::LEFT_HAND,
-		ArtifactPosition::TORSO,
-		ArtifactPosition::RIGHT_RING,
-		ArtifactPosition::LEFT_RING,
-		ArtifactPosition::FEET,
-		ArtifactPosition::MISC1,
-		ArtifactPosition::MISC2,
-		ArtifactPosition::MISC3,
-		ArtifactPosition::MISC4,
-		ArtifactPosition::MISC5,
-	};
-
-	return positions;
-}
-
-DLL_LINKAGE bool ArtifactUtils::isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot)
-{
-	return slot.second.artifact
-		&& !slot.second.locked
-		&& !vstd::contains(unmovableSlots(), slot.first);
-}
-
-DLL_LINKAGE bool ArtifactUtils::checkSpellbookIsNeeded(const CGHeroInstance * heroPtr, const ArtifactID & artID, const ArtifactPosition & slot)
-{
-	// TODO what'll happen if Titan's thunder is equipped by pickin git up or the start of game?
-	// Titan's Thunder creates new spellbook on equip
-	if(artID == ArtifactID::TITANS_THUNDER && slot == ArtifactPosition::RIGHT_HAND)
-	{
-		if(heroPtr)
-		{
-			if(heroPtr && !heroPtr->hasSpellbook())
-				return true;
-		}
-	}
-	return false;
-}
-
-DLL_LINKAGE bool ArtifactUtils::isSlotBackpack(const ArtifactPosition & slot)
-{
-	return slot >= GameConstants::BACKPACK_START;
-}
-
-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;
-}
-
-DLL_LINKAGE std::vector<const CArtifact*> ArtifactUtils::assemblyPossibilities(
-	const CArtifactSet * artSet, const ArtifactID & aid, bool equipped)
-{
-	std::vector<const CArtifact*> arts;
-	const auto * art = aid.toArtifact();
-	if(art->canBeDisassembled())
-		return arts;
-
-	for(const auto artifact : art->constituentOf)
-	{
-		assert(artifact->constituents);
-		bool possible = true;
-
-		for(const auto constituent : *artifact->constituents) //check if all constituents are available
-		{
-			if(equipped)
-			{
-				// Search for equipped arts
-				if(!artSet->hasArt(constituent->getId(), true, false, false))
-				{
-					possible = false;
-					break;
-				}
-			}
-			else
-			{
-				// Search in backpack
-				if(!artSet->hasArtBackpack(constituent->getId()))
-				{
-					possible = false;
-					break;
-				}
-			}
-		}
-		if(possible)
-			arts.push_back(artifact);
-	}
-	return arts;
-}
-
 VCMI_LIB_NAMESPACE_END

lib/CArtHandler.h

@@ -141,15 +141,13 @@ class DLL_LINKAGE CArtifactInstance : public CBonusSystemNode
 {
 protected:
 	void init();
-	CArtifactInstance(CArtifact *Art);
 public:
+	CArtifactInstance(CArtifact * Art);
 	CArtifactInstance();
 
 	ConstTransitivePtr<CArtifact> artType;
 	ArtifactInstanceID id;
 
-	//CArtifactInstance(int aid);
-
 	std::string nodeName() const override;
 	void deserializationFix();
 	void setType(CArtifact *Art);
@@ -160,12 +158,12 @@ public:
 	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);
-	virtual void removeFrom(ArtifactLocation al);
 	/// Checks if this a part of this artifact: artifact instance is a part
 	/// of itself, additionally truth is returned for constituents of combined arts
 	virtual bool isPart(const CArtifactInstance *supposedPart) const;
 
+	void putAt(ArtifactLocation al);
+	void removeFrom(ArtifactLocation al);
 	void move(const ArtifactLocation & src,const ArtifactLocation & dst);
 
 	template <typename Handler> void serialize(Handler &h, const int version)
@@ -175,25 +173,12 @@ public:
 		h & id;
 		BONUS_TREE_DESERIALIZATION_FIX
 	}
-
-	static CArtifactInstance * createScroll(const SpellID & sid);
-	static CArtifactInstance *createNewArtifactInstance(CArtifact *Art);
-	static CArtifactInstance * createNewArtifactInstance(const ArtifactID & aid);
-
-	/**
-	 * Creates an artifact instance.
-	 *
-	 * @param aid the id of the artifact
-	 * @param spellID optional. the id of a spell if a spell scroll object should be created
-	 * @return the created artifact instance
-	 */
-	static CArtifactInstance * createArtifact(CMap * map, const ArtifactID & aid, int spellID = -1);
 };
 
 class DLL_LINKAGE CCombinedArtifactInstance : public CArtifactInstance
 {
-	CCombinedArtifactInstance(CArtifact *Art);
 public:
+	CCombinedArtifactInstance(CArtifact * Art);
 	struct ConstituentInfo
 	{
 		ConstTransitivePtr<CArtifactInstance> art;
@@ -210,13 +195,9 @@ public:
 
 	std::vector<ConstituentInfo> constituentsInfo;
 
-	void putAt(ArtifactLocation al) override;
-	void removeFrom(ArtifactLocation al) override;
 	bool isPart(const CArtifactInstance *supposedPart) const override;
-
 	void createConstituents();
 	void addAsConstituent(CArtifactInstance * art, const ArtifactPosition & slot);
-	CArtifactInstance * figureMainConstituent(const ArtifactLocation & al); //main constituent is replaced with us (combined art), not lock
 
 	CCombinedArtifactInstance() = default;
 
@@ -314,7 +295,6 @@ public:
 	std::map<ArtifactPosition, ArtSlotInfo> artifactsWorn; //map<position,artifact_id>; positions: 0 - head; 1 - shoulders; 2 - neck; 3 - right hand; 4 - left hand; 5 - torso; 6 - right ring; 7 - left ring; 8 - feet; 9 - misc1; 10 - misc2; 11 - misc3; 12 - misc4; 13 - mach1; 14 - mach2; 15 - mach3; 16 - mach4; 17 - spellbook; 18 - misc5
 	std::vector<ArtSlotInfo> artifactsTransitionPos; // Used as transition position for dragAndDrop artifact exchange
 
-	ArtSlotInfo & retrieveNewArtSlot(const ArtifactPosition & slot);
 	void setNewArtSlot(const ArtifactPosition & slot, CArtifactInstance * art, bool locked);
 	void eraseArtSlot(const ArtifactPosition & slot);
 
@@ -340,7 +320,8 @@ public:
 	unsigned getArtPosCount(const ArtifactID & aid, bool onlyWorn = true, bool searchBackpackAssemblies = true, bool allowLocked = true) const;
 
 	virtual ArtBearer::ArtBearer bearerType() const = 0;
-	virtual void putArtifact(ArtifactPosition pos, CArtifactInstance * art) = 0;
+	virtual void putArtifact(ArtifactPosition slot, CArtifactInstance * art);
+	virtual void removeArtifact(ArtifactPosition slot);
 	virtual ~CArtifactSet();
 
 	template <typename Handler> void serialize(Handler &h, const int version)
@@ -368,28 +349,10 @@ 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:
 	ArtBearer::ArtBearer Bearer;
 };
 
-namespace ArtifactUtils
-{
-	// Calculates where an artifact gets placed when it gets transferred from one hero to another.
-	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();
-	DLL_LINKAGE bool isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot);
-	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);
-	DLL_LINKAGE std::vector<const CArtifact*> assemblyPossibilities(const CArtifactSet * artSet, const ArtifactID & aid, bool equipped);
-}
-
 VCMI_LIB_NAMESPACE_END

lib/CCreatureSet.cpp

@@ -10,6 +10,7 @@
 #include "StdInc.h"
 #include "CCreatureSet.h"
 
+#include "ArtifactUtils.h"
 #include "CConfigHandler.h"
 #include "CCreatureHandler.h"
 #include "VCMI_Lib.h"
@@ -870,7 +871,20 @@ ArtBearer::ArtBearer CStackInstance::bearerType() const
 void CStackInstance::putArtifact(ArtifactPosition pos, CArtifactInstance * art)
 {
 	assert(!getArt(pos));
-	art->putAt(ArtifactLocation(this, pos));
+	assert(art->artType->canBePutAt(this, pos));
+
+	CArtifactSet::putArtifact(pos, art);
+	if(ArtifactUtils::isSlotEquipment(pos))
+		attachTo(*art);
+}
+
+void CStackInstance::removeArtifact(ArtifactPosition pos)
+{
+	assert(getArt(pos));
+
+	CArtifactSet::removeArtifact(pos);
+	if(ArtifactUtils::isSlotEquipment(pos))
+		detachFrom(*getArt(pos));
 }
 
 void CStackInstance::serializeJson(JsonSerializeFormat & handler)

lib/CCreatureSet.h

@@ -121,6 +121,7 @@ public:
 	virtual void giveStackExp(TExpType exp);
 	bool valid(bool allowUnrandomized) const;
 	void putArtifact(ArtifactPosition pos, CArtifactInstance * art) override;//from CArtifactSet
+	void removeArtifact(ArtifactPosition pos) override;
 	ArtBearer::ArtBearer bearerType() const override; //from CArtifactSet
 	virtual std::string nodeName() const override; //from CBonusSystemnode
 	void deserializationFix();

lib/CGameState.cpp

@@ -11,6 +11,7 @@
 #include "CGameState.h"
 
 #include "mapping/CCampaignHandler.h"
+#include "ArtifactUtils.h"
 #include "CArtHandler.h"
 #include "CBuildingHandler.h"
 #include "CGeneralTextHandler.h"
@@ -1593,7 +1594,7 @@ void CGameState::giveCampaignBonusToHero(CGHeroInstance * hero)
 			break;
 		case CScenarioTravel::STravelBonus::SPELL_SCROLL:
 			{
-				CArtifactInstance * scroll = CArtifactInstance::createScroll(SpellID(curBonus->info2));
+				CArtifactInstance * scroll = ArtifactUtils::createScroll(SpellID(curBonus->info2));
 				const auto slot = ArtifactUtils::getArtAnyPosition(hero, scroll->getTypeId());
 				if(ArtifactUtils::isSlotEquipment(slot) || ArtifactUtils::isSlotBackpack(slot))
 					scroll->putAt(ArtifactLocation(hero, slot));
@@ -2826,7 +2827,7 @@ void CGameState::attachArmedObjects()
 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 = ArtifactUtils::createNewArtifactInstance(artifact);
 	 map->addNewArtifactInstance(ai);
 	 auto slot = ArtifactUtils::getArtAnyPosition(h, aid);
 	 if(ArtifactUtils::isSlotEquipment(slot) || ArtifactUtils::isSlotBackpack(slot))

lib/NetPacksLib.cpp

@@ -8,6 +8,7 @@
  *
  */
 #include "StdInc.h"
+#include "ArtifactUtils.h"
 #include "NetPacks.h"
 #include "NetPackVisitor.h"
 #include "CGeneralTextHandler.h"
@@ -1798,7 +1799,6 @@ void PutArtifact::applyGs(CGameState *gs)
 	assert(vstd::contains(gs->map->artInstances, art));
 	assert(!art->getParentNodes().empty());
 	art->putAt(al);
-	//al.hero->putArtifact(al.slot, art);
 }
 
 void EraseArtifact::applyGs(CGameState *gs)
@@ -1936,7 +1936,6 @@ void AssembledArtifact::applyGs(CGameState *gs)
 
 		//move constituent from hero to be part of new, combined artifact
 		constituentInstance->removeFrom(ArtifactLocation(al.artHolder, pos));
-		combinedArt->addAsConstituent(constituentInstance, pos);
 		if(combineEquipped)
 		{
 			if(!vstd::contains(combinedArt->artType->possibleSlots[artSet->bearerType()], al.slot)
@@ -1947,6 +1946,9 @@ void AssembledArtifact::applyGs(CGameState *gs)
 		{
 			al.slot = std::min(al.slot, pos);
 		}
+		if(al.slot == pos)
+			pos = ArtifactPosition::PRE_FIRST;
+		combinedArt->addAsConstituent(constituentInstance, pos);
 	}
 
 	//put new combined artifacts

lib/mapObjects/CGHeroInstance.cpp

@@ -16,6 +16,7 @@
 
 #include "../NetPacks.h"
 #include "../CGeneralTextHandler.h"
+#include "../ArtifactUtils.h"
 #include "../CHeroHandler.h"
 #include "../TerrainHandler.h"
 #include "../RoadHandler.h"
@@ -280,10 +281,10 @@ void CGHeroInstance::initHero(CRandomGenerator & rand)
 		spells -= SpellID::PRESET;
 
 	if(!getArt(ArtifactPosition::MACH4) && !getArt(ArtifactPosition::SPELLBOOK) && type->haveSpellBook) //no catapult means we haven't read pre-existent set -> use default rules for spellbook
-		putArtifact(ArtifactPosition::SPELLBOOK, CArtifactInstance::createNewArtifactInstance(ArtifactID::SPELLBOOK));
+		putArtifact(ArtifactPosition::SPELLBOOK, ArtifactUtils::createNewArtifactInstance(ArtifactID::SPELLBOOK));
 
 	if(!getArt(ArtifactPosition::MACH4))
-		putArtifact(ArtifactPosition::MACH4, CArtifactInstance::createNewArtifactInstance(ArtifactID::CATAPULT)); //everyone has a catapult
+		putArtifact(ArtifactPosition::MACH4, ArtifactUtils::createNewArtifactInstance(ArtifactID::CATAPULT)); //everyone has a catapult
 
 	if(portrait < 0 || portrait == 255)
 		portrait = type->imageIndex;
@@ -389,7 +390,7 @@ void CGHeroInstance::initArmy(CRandomGenerator & rand, IArmyDescriptor * dst)
 				ArtifactPosition slot = art->possibleSlots.at(ArtBearer::HERO).front();
 
 				if(!getArt(slot))
-					putArtifact(slot, CArtifactInstance::createNewArtifactInstance(aid));
+					putArtifact(slot, ArtifactUtils::createNewArtifactInstance(aid));
 				else
 					logGlobal->warn("Hero %s already has artifact at %d, omitting giving artifact %d", getNameTranslated(), slot.toEnum(), aid.toEnum());
 			}
@@ -1031,7 +1032,21 @@ std::string CGHeroInstance::getBiographyTextID() const
 void CGHeroInstance::putArtifact(ArtifactPosition pos, CArtifactInstance *art)
 {
 	assert(!getArt(pos));
-	art->putAt(ArtifactLocation(this, pos));
+	assert(art->artType->canBePutAt(this, pos));
+
+	CArtifactSet::putArtifact(pos, art);
+	if(ArtifactUtils::isSlotEquipment(pos))
+		attachTo(*art);
+}
+
+void CGHeroInstance::removeArtifact(ArtifactPosition pos)
+{
+	auto art = getArt(pos);
+	assert(art);
+
+	CArtifactSet::removeArtifact(pos);
+	if(ArtifactUtils::isSlotEquipment(pos))
+		detachFrom(*art);
 }
 
 bool CGHeroInstance::hasSpellbook() const

lib/mapObjects/CGHeroInstance.h

@@ -221,9 +221,9 @@ public:
 	void initHero(CRandomGenerator & rand, const HeroTypeID & SUBID);
 
 	void putArtifact(ArtifactPosition pos, CArtifactInstance * art) override;
+	void removeArtifact(ArtifactPosition pos) override;
 	void initExp(CRandomGenerator & rand);
 	void initArmy(CRandomGenerator & rand, IArmyDescriptor *dst = nullptr);
-	//void giveArtifact (ui32 aid);
 	void pushPrimSkill(PrimarySkill::PrimarySkill which, int val);
 	ui8 maxlevelsToMagicSchool() const;
 	ui8 maxlevelsToWisdom() const;

lib/mapObjects/CQuest.cpp

@@ -13,6 +13,7 @@
 
 #include <vcmi/spells/Spell.h>
 
+#include "../ArtifactUtils.h"
 #include "../NetPacks.h"
 #include "../CSoundBase.h"
 #include "../CGeneralTextHandler.h"

lib/mapping/MapFormatH3M.cpp

@@ -15,6 +15,7 @@
 #include "MapReaderH3M.h"
 #include "MapFormat.h"
 
+#include "../ArtifactUtils.h"
 #include "../CCreatureHandler.h"
 #include "../CGeneralTextHandler.h"
 #include "../CHeroHandler.h"
@@ -870,7 +871,7 @@ bool CMapLoaderH3M::loadArtifactToSlot(CGHeroInstance * hero, int slot)
 	// H3 bug workaround - Enemy hero on 3rd scenario of Good1.h3c campaign ("Long Live The Queen")
 	// He has Shackles of War (normally - MISC slot artifact) in LEFT_HAND slot set in editor
 	// Artifact seems to be missing in game, so skip artifacts that don't fit target slot
-	auto * artifact = CArtifactInstance::createArtifact(map, artifactID);
+	auto * artifact = ArtifactUtils::createArtifact(map, artifactID);
 	auto artifactPos = ArtifactPosition(slot);
 	if(artifact->canBePutAt(ArtifactLocation(hero, artifactPos)))
 	{
@@ -1119,7 +1120,7 @@ CGObjectInstance * CMapLoaderH3M::readArtifact(const int3 & mapPosition, std::sh
 		artID = ArtifactID(objectTemplate->subid);
 	}
 
-	object->storedArtifact = CArtifactInstance::createArtifact(map, artID, spellID);
+	object->storedArtifact = ArtifactUtils::createArtifact(map, artID, spellID);
 	return object;
 }
 

lib/mapping/MapFormatJson.cpp

@@ -16,6 +16,7 @@
 #include "../JsonDetail.h"
 #include "CMap.h"
 #include "MapFormat.h"
+#include "../ArtifactUtils.h"
 #include "../CModHandler.h"
 #include "../CHeroHandler.h"
 #include "../CTownHandler.h"
@@ -1205,7 +1206,7 @@ void CMapLoaderJson::MapObjectLoader::configure()
 			artID = ArtifactID(art->subID);
 		}
 
-		art->storedArtifact = CArtifactInstance::createArtifact(owner->map, artID, spellID);
+		art->storedArtifact = ArtifactUtils::createArtifact(owner->map, artID, spellID);
 	}
 
 	if(auto * hero = dynamic_cast<CGHeroInstance *>(instance))

lib/rmg/modificators/TreasurePlacer.cpp

@@ -19,6 +19,7 @@
 #include "../TileInfo.h"
 #include "../CZonePlacer.h"
 #include "QuestArtifactPlacer.h"
+#include "../../ArtifactUtils.h"
 #include "../../mapObjectConstructors/AObjectTypeHandler.h"
 #include "../../mapObjectConstructors/CObjectClassesHandler.h"
 #include "../../mapObjectConstructors/CommonConstructors.h"
@@ -210,7 +211,7 @@ void TreasurePlacer::addAllPossibleObjects()
 					out.push_back(spell->id);
 				}
 			}
-			auto * a = CArtifactInstance::createScroll(*RandomGeneratorUtil::nextItem(out, zone.getRand()));
+			auto * a = ArtifactUtils::createScroll(*RandomGeneratorUtil::nextItem(out, zone.getRand()));
 			obj->storedArtifact = a;
 			return obj;
 		};

mapeditor/inspector/inspector.cpp

@@ -9,6 +9,7 @@
  */
 #include "StdInc.h"
 #include "inspector.h"
+#include "../lib/ArtifactUtils.h"
 #include "../lib/CArtHandler.h"
 #include "../lib/spells/CSpellHandler.h"
 #include "../lib/CHeroHandler.h"
@@ -173,7 +174,7 @@ void Initializer::initialize(CGArtifact * o)
 				out.push_back(spell->id);
 			}
 		}
-		auto a = CArtifactInstance::createScroll(*RandomGeneratorUtil::nextItem(out, CRandomGenerator::getDefault()));
+		auto a = ArtifactUtils::createScroll(*RandomGeneratorUtil::nextItem(out, CRandomGenerator::getDefault()));
 		o->storedArtifact = a;
 	}
 }
@@ -530,7 +531,7 @@ void Inspector::setProperty(CGArtifact * o, const QString & key, const QVariant
 		{
 			if(spell->getJsonKey() == value.toString().toStdString())
 			{
-				o->storedArtifact = CArtifactInstance::createScroll(spell->getId());
+				o->storedArtifact = ArtifactUtils::createScroll(spell->getId());
 				break;
 			}
 		}

mapeditor/mapcontroller.cpp

@@ -10,6 +10,7 @@
 
 #include "mapcontroller.h"
 
+#include "../lib/ArtifactUtils.h"
 #include "../lib/GameConstants.h"
 #include "../lib/mapObjectConstructors/AObjectTypeHandler.h"
 #include "../lib/mapObjectConstructors/CObjectClassesHandler.h"
@@ -188,7 +189,7 @@ void MapController::repairMap()
 						out.push_back(spell->id);
 					}
 				}
-				auto a = CArtifactInstance::createScroll(*RandomGeneratorUtil::nextItem(out, CRandomGenerator::getDefault()));
+				auto a = ArtifactUtils::createScroll(*RandomGeneratorUtil::nextItem(out, CRandomGenerator::getDefault()));
 				art->storedArtifact = a;
 			}
 			else

server/CGameHandler.cpp

@@ -13,6 +13,7 @@
 #include "../lib/filesystem/FileInfo.h"
 #include "../lib/int3.h"
 #include "../lib/mapping/CCampaignHandler.h"
+#include "../lib/ArtifactUtils.h"
 #include "../lib/StartInfo.h"
 #include "../lib/CModHandler.h"
 #include "../lib/CArtHandler.h"