Merge pull request #2268 from SoundSSGood/CArtifactInstance-rework

CArtifact CArtifactInstance refactoring

AI/Nullkiller/AIGateway.cpp

@@ -1006,7 +1006,7 @@ void AIGateway::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance
 				//FIXME: why are the above possible to be null?
 
 				bool emptySlotFound = false;
-				for(auto slot : artifact->artType->possibleSlots.at(target->bearerType()))
+				for(auto slot : artifact->artType->getPossibleSlots().at(target->bearerType()))
 				{
 					ArtifactLocation destLocation(target, slot);
 					if(target->isPositionFree(slot) && artifact->canBePutAt(destLocation, true)) //combined artifacts are not always allowed to move
@@ -1019,7 +1019,7 @@ void AIGateway::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance
 				}
 				if(!emptySlotFound) //try to put that atifact in already occupied slot
 				{
-					for(auto slot : artifact->artType->possibleSlots.at(target->bearerType()))
+					for(auto slot : artifact->artType->getPossibleSlots().at(target->bearerType()))
 					{
 						auto otherSlot = target->getSlot(slot);
 						if(otherSlot && otherSlot->artifact) //we need to exchange artifact for better one

AI/Nullkiller/AIUtility.cpp

@@ -306,10 +306,10 @@ bool compareArtifacts(const CArtifactInstance * a1, const CArtifactInstance * a2
 	auto art1 = a1->artType;
 	auto art2 = a2->artType;
 
-	if(art1->price == art2->price)
+	if(art1->getPrice() == art2->getPrice())
 		return art1->valOfBonuses(BonusType::PRIMARY_SKILL) > art2->valOfBonuses(BonusType::PRIMARY_SKILL);
 	else
-		return art1->price > art2->price;
+		return art1->getPrice() > art2->getPrice();
 }
 
 bool isWeeklyRevisitable(const CGObjectInstance * obj)

AI/VCAI/AIUtility.cpp

@@ -256,8 +256,8 @@ bool compareArtifacts(const CArtifactInstance * a1, const CArtifactInstance * a2
 	auto art1 = a1->artType;
 	auto art2 = a2->artType;
 
-	if(art1->price == art2->price)
+	if(art1->getPrice() == art2->getPrice())
 		return art1->valOfBonuses(BonusType::PRIMARY_SKILL) > art2->valOfBonuses(BonusType::PRIMARY_SKILL);
 	else
-		return art1->price > art2->price;
+		return art1->getPrice() > art2->getPrice();
 }

AI/VCAI/VCAI.cpp

@@ -1192,7 +1192,7 @@ void VCAI::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance * ot
 				//FIXME: why are the above possible to be null?
 
 				bool emptySlotFound = false;
-				for(auto slot : artifact->artType->possibleSlots.at(target->bearerType()))
+				for(auto slot : artifact->artType->getPossibleSlots().at(target->bearerType()))
 				{
 					ArtifactLocation destLocation(target, slot);
 					if(target->isPositionFree(slot) && artifact->canBePutAt(destLocation, true)) //combined artifacts are not always allowed to move
@@ -1205,7 +1205,7 @@ void VCAI::pickBestArtifacts(const CGHeroInstance * h, const CGHeroInstance * ot
 				}
 				if(!emptySlotFound) //try to put that atifact in already occupied slot
 				{
-					for(auto slot : artifact->artType->possibleSlots.at(target->bearerType()))
+					for(auto slot : artifact->artType->getPossibleSlots().at(target->bearerType()))
 					{
 						auto otherSlot = target->getSlot(slot);
 						if(otherSlot && otherSlot->artifact) //we need to exchange artifact for better one

client/widgets/CArtifactHolder.cpp

@@ -236,11 +236,11 @@ void CHeroArtPlace::addCombinedArtInfo(std::map<const CArtifact*, int> & arts)
 		text += "{" + combinedArt.first->getNameTranslated() + "}";
 		if(arts.size() == 1)
 		{
-			for(const auto part : *combinedArt.first->constituents)
+			for(const auto part : combinedArt.first->getConstituents())
 				artList += "\n" + part->getNameTranslated();
 		}
 		text += " (" + boost::str(boost::format("%d") % combinedArt.second) + " / " +
-			boost::str(boost::format("%d") % combinedArt.first->constituents->size()) + ")" + artList;
+			boost::str(boost::format("%d") % combinedArt.first->getConstituents().size()) + ")" + artList;
 	}
 }
 
@@ -290,9 +290,9 @@ bool ArtifactUtilsClient::askToDisassemble(const CGHeroInstance * hero, const Ar
 	const auto art = hero->getArt(slot);
 	assert(art);
 
-	if(art->canBeDisassembled())
+	if(art->isCombined())
 	{
-		if(ArtifactUtils::isSlotBackpack(slot) && !ArtifactUtils::isBackpackFreeSlots(hero, art->artType->constituents->size() - 1))
+		if(ArtifactUtils::isSlotBackpack(slot) && !ArtifactUtils::isBackpackFreeSlots(hero, art->artType->getConstituents().size() - 1))
 			return false;
 
 		LOCPLINT->showArtifactAssemblyDialog(

client/widgets/CArtifactsOfHeroAltar.cpp

@@ -98,13 +98,10 @@ void CArtifactsOfHeroAltar::deleteFromVisible(const CArtifactInstance * artInst)
 	}
 	else
 	{
-		if(artInst->canBeDisassembled())
+		for(const auto & part : artInst->getPartsInfo())
 		{
-			for(const auto & part : dynamic_cast<const CCombinedArtifactInstance*>(artInst)->constituentsInfo)
-			{
-				if(part.slot != ArtifactPosition::PRE_FIRST)
-					getArtPlace(part.slot)->setArtifact(nullptr);
-			}
+			if(part.slot != ArtifactPosition::PRE_FIRST)
+				getArtPlace(part.slot)->setArtifact(nullptr);
 		}
 	}
 }

client/widgets/CArtifactsOfHeroBase.cpp

@@ -257,14 +257,14 @@ void CArtifactsOfHeroBase::setSlotData(ArtPlacePtr artPlace, const ArtifactPosit
 	{
 		artPlace->lockSlot(slotInfo->locked);
 		artPlace->setArtifact(slotInfo->artifact);
-		if(!slotInfo->artifact->canBeDisassembled())
+		if(!slotInfo->artifact->isCombined())
 		{
 			// If the artifact is part of at least one combined artifact, add additional information
 			std::map<const CArtifact*, int> arts;
-			for(const auto combinedArt : slotInfo->artifact->artType->constituentOf)
+			for(const auto combinedArt : slotInfo->artifact->artType->getPartOf())
 			{
 				arts.insert(std::pair(combinedArt, 0));
-				for(const auto part : *combinedArt->constituents)
+				for(const auto part : combinedArt->getConstituents())
 					if(artSet.hasArt(part->getId(), true))
 						arts.at(combinedArt)++;
 			}

client/widgets/CComponent.cpp

@@ -34,6 +34,7 @@
 #include "../../lib/CGeneralTextHandler.h"
 #include "../../lib/NetPacksBase.h"
 #include "../../lib/CArtHandler.h"
+#include "../../lib/CArtifactInstance.h"
 
 CComponent::CComponent(Etype Type, int Subtype, int Val, ESize imageSize, EFonts font):
 	perDay(false)
@@ -169,16 +170,12 @@ std::string CComponent::getDescription()
 	case artifact:
 	{
 		auto artID = ArtifactID(subtype);
-		std::unique_ptr<CArtifactInstance> art;
-		if (artID != ArtifactID::SPELL_SCROLL)
+		auto description = VLC->arth->objects[artID]->getDescriptionTranslated();
+		if(artID == ArtifactID::SPELL_SCROLL)
 		{
-			art.reset(ArtifactUtils::createNewArtifactInstance(artID));
+			ArtifactUtils::insertScrrollSpellName(description, SpellID(val));
 		}
-		else
-		{
-			art.reset(ArtifactUtils::createScroll(SpellID(val)));
-		}
-		return art->getDescription();
+		return description;
 	}
 	case experience: return CGI->generaltexth->allTexts[241];
 	case spell:      return (*CGI->spellh)[subtype]->getDescriptionTranslated(val);

client/windows/CCastleInterface.cpp

@@ -800,7 +800,7 @@ void CCastleBuildings::enterBlacksmith(ArtifactID artifactID)
 	bool possible = LOCPLINT->cb->getResourceAmount(EGameResID::GOLD) >= price;
 	if(possible)
 	{
-		for(auto slot : art->possibleSlots.at(ArtBearer::HERO))
+		for(auto slot : art->getPossibleSlots().at(ArtBearer::HERO))
 		{
 			if(hero->getArt(slot) == nullptr)
 			{

client/windows/CCreatureWindow.cpp

@@ -588,7 +588,7 @@ CStackWindow::MainSection::MainSection(CStackWindow * owner, int yOffset, bool s
 		auto art = parent->info->stackNode->getArt(ArtifactPosition::CREATURE_SLOT);
 		if(art)
 		{
-			parent->stackArtifactIcon = std::make_shared<CAnimImage>("ARTIFACT", art->artType->iconIndex, 0, pos.x, pos.y);
+			parent->stackArtifactIcon = std::make_shared<CAnimImage>("ARTIFACT", art->artType->getIconIndex(), 0, pos.x, pos.y);
 			parent->stackArtifactHelp = std::make_shared<LRClickableAreaWTextComp>(Rect(pos, Point(44, 44)), CComponent::artifact);
 			parent->stackArtifactHelp->type = art->artType->getId();
 

client/windows/CTradeWindow.cpp

@@ -803,7 +803,7 @@ void CMarketplaceWindow::makeDeal()
 			leftIdToSend = hLeft->serial;
 			break;
 		case EMarketMode::ARTIFACT_RESOURCE:
-			leftIdToSend = hLeft->getArtInstance()->id.getNum();
+			leftIdToSend = hLeft->getArtInstance()->getId().getNum();
 			break;
 		case EMarketMode::RESOURCE_ARTIFACT:
 			if(!ArtifactID(hRight->id).toArtifact()->canBePutAt(hero))

cmake_modules/VCMI_lib.cmake

@@ -220,6 +220,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/BattleFieldHandler.cpp
 		${MAIN_LIB_DIR}/CAndroidVMHelper.cpp
 		${MAIN_LIB_DIR}/CArtHandler.cpp
+		${MAIN_LIB_DIR}/CArtifactInstance.cpp
 		${MAIN_LIB_DIR}/CBonusTypeHandler.cpp
 		${MAIN_LIB_DIR}/CBuildingHandler.cpp
 		${MAIN_LIB_DIR}/CConfigHandler.cpp
@@ -550,6 +551,7 @@ macro(add_main_lib TARGET_NAME LIBRARY_TYPE)
 		${MAIN_LIB_DIR}/BattleFieldHandler.h
 		${MAIN_LIB_DIR}/CAndroidVMHelper.h
 		${MAIN_LIB_DIR}/CArtHandler.h
+		${MAIN_LIB_DIR}/CArtifactInstance.h
 		${MAIN_LIB_DIR}/CBonusTypeHandler.h
 		${MAIN_LIB_DIR}/CBuildingHandler.h
 		${MAIN_LIB_DIR}/CConfigHandler.h

lib/ArtifactUtils.cpp

@@ -12,6 +12,7 @@
 
 #include "CArtHandler.h"
 #include "GameSettings.h"
+#include "spells/CSpellHandler.h"
 
 #include "mapping/CMap.h"
 #include "mapObjects/CGHeroInstance.h"
@@ -21,7 +22,7 @@ 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()))
+	for(const auto & slot : art->getPossibleSlots().at(target->bearerType()))
 	{
 		if(art->canBePutAt(target, slot))
 			return slot;
@@ -119,15 +120,15 @@ DLL_LINKAGE std::vector<const CArtifact*> ArtifactUtils::assemblyPossibilities(
 {
 	std::vector<const CArtifact*> arts;
 	const auto * art = aid.toArtifact();
-	if(art->canBeDisassembled())
+	if(art->isCombined())
 		return arts;
 
-	for(const auto artifact : art->constituentOf)
+	for(const auto artifact : art->getPartOf())
 	{
 		assert(artifact->constituents);
 		bool possible = true;
 
-		for(const auto constituent : *artifact->constituents) //check if all constituents are available
+		for(const auto constituent : artifact->getConstituents()) //check if all constituents are available
 		{
 			if(equipped)
 			{
@@ -165,24 +166,23 @@ DLL_LINKAGE CArtifactInstance * ArtifactUtils::createScroll(const SpellID & sid)
 
 DLL_LINKAGE CArtifactInstance * ArtifactUtils::createNewArtifactInstance(CArtifact * art)
 {
-	if(art->canBeDisassembled())
+	assert(art);
+
+	auto * artInst = new CArtifactInstance(art);
+	if(art->isCombined())
 	{
-		auto * ret = new CCombinedArtifactInstance(art);
-		ret->createConstituents();
-		return ret;
+		assert(art->constituents);
+		for(const auto & part : art->getConstituents())
+			artInst->addPart(ArtifactUtils::createNewArtifactInstance(part), ArtifactPosition::PRE_FIRST);
 	}
-	else
+	if(art->isGrowing())
 	{
-		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;
+		auto bonus = std::make_shared<Bonus>();
+		bonus->type = BonusType::LEVEL_COUNTER;
+		bonus->val = 0;
+		artInst->addNewBonus(bonus);
 	}
+	return artInst;
 }
 
 DLL_LINKAGE CArtifactInstance * ArtifactUtils::createNewArtifactInstance(const ArtifactID & aid)
@@ -209,15 +209,28 @@ DLL_LINKAGE CArtifactInstance * ArtifactUtils::createArtifact(CMap * map, const
 		art = new CArtifactInstance(); // random, empty
 	}
 	map->addNewArtifactInstance(art);
-	if(art->artType && art->canBeDisassembled())
+	if(art->artType && art->isCombined())
 	{
-		auto * combined = dynamic_cast<CCombinedArtifactInstance*>(art);
-		for(CCombinedArtifactInstance::ConstituentInfo & ci : combined->constituentsInfo)
+		for(auto & part : art->getPartsInfo())
 		{
-			map->addNewArtifactInstance(ci.art);
+			map->addNewArtifactInstance(part.art);
 		}
 	}
 	return art;
 }
 
+DLL_LINKAGE void ArtifactUtils::insertScrrollSpellName(std::string & description, const SpellID & sid)
+{
+	// We expect scroll description to be like this: This scroll contains the [spell name] spell which is added
+	// into spell book for as long as hero carries the scroll. So we want to replace text in [...] with a spell name.
+	// However other language versions don't have name placeholder at all, so we have to be careful
+	auto nameStart = description.find_first_of('[');
+	auto nameEnd = description.find_first_of(']', nameStart);
+	if(sid.getNum() >= 0)
+	{
+		if(nameStart != std::string::npos && nameEnd != std::string::npos)
+			description = description.replace(nameStart, nameEnd - nameStart + 1, sid.toSpell(VLC->spells())->getNameTranslated());
+	}
+}
+
 VCMI_LIB_NAMESPACE_END

lib/ArtifactUtils.h

@@ -41,6 +41,7 @@ namespace ArtifactUtils
 	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);
+	DLL_LINKAGE void insertScrrollSpellName(std::string & description, const SpellID & sid);
 }
 
 VCMI_LIB_NAMESPACE_END

lib/CArtHandler.cpp

@@ -15,9 +15,7 @@
 #include "CGeneralTextHandler.h"
 #include "CModHandler.h"
 #include "GameSettings.h"
-#include "spells/CSpellHandler.h"
 #include "mapObjects/MapObjects.h"
-#include "NetPacksBase.h"
 #include "StringConstants.h"
 
 #include "mapObjectConstructors/AObjectTypeHandler.h"
@@ -48,6 +46,51 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
+bool CCombinedArtifact::isCombined() const
+{
+	return !(constituents.empty());
+}
+
+const std::vector<CArtifact*> & CCombinedArtifact::getConstituents() const
+{
+	return constituents;
+}
+
+const std::vector<CArtifact*> & CCombinedArtifact::getPartOf() const
+{
+	return partOf;
+}
+
+bool CScrollArtifact::isScroll() const
+{
+	return static_cast<const CArtifact*>(this)->getId() == ArtifactID::SPELL_SCROLL;
+}
+
+bool CGrowingArtifact::isGrowing() const
+{
+	return !bonusesPerLevel.empty() || !thresholdBonuses.empty();
+}
+
+std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getBonusesPerLevel()
+{
+	return bonusesPerLevel;
+}
+
+const std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getBonusesPerLevel() const
+{
+	return bonusesPerLevel;
+}
+
+std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getThresholdBonuses()
+{
+	return thresholdBonuses;
+}
+
+const std::vector <std::pair<ui16, Bonus>> & CGrowingArtifact::getThresholdBonuses() const
+{
+	return thresholdBonuses;
+}
+
 int32_t CArtifact::getIndex() const
 {
 	return id.toEnum();
@@ -136,11 +179,6 @@ 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
@@ -160,7 +198,7 @@ bool CArtifact::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, b
 
 	auto artCanBePutAt = [this, simpleArtCanBePutAt](const CArtifactSet * artSet, ArtifactPosition slot, bool assumeDestRemoved) -> bool
 	{
-		if(canBeDisassembled())
+		if(isCombined())
 		{
 			if(!simpleArtCanBePutAt(artSet, slot, assumeDestRemoved))
 				return false;
@@ -172,7 +210,7 @@ bool CArtifact::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, b
 			if(assumeDestRemoved)
 				fittingSet.removeArtifact(slot);
 			assert(constituents);
-			for(const auto art : *constituents)
+			for(const auto art : constituents)
 			{
 				auto possibleSlot = ArtifactUtils::getArtAnyPosition(&fittingSet, art->getId());
 				if(ArtifactUtils::isSlotEquipment(possibleSlot))
@@ -215,6 +253,8 @@ bool CArtifact::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, b
 }
 
 CArtifact::CArtifact()
+	: iconIndex(ArtifactID::NONE),
+	price(0)
 {
 	setNodeType(ARTIFACT);
 	possibleSlots[ArtBearer::HERO]; //we want to generate map entry even if it will be empty
@@ -259,38 +299,21 @@ void CArtifact::addNewBonus(const std::shared_ptr<Bonus>& b)
 	CBonusSystemNode::addNewBonus(b);
 }
 
-void CArtifact::updateFrom(const JsonNode& data)
+const std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition>> & CArtifact::getPossibleSlots() const
 {
-	//TODO:CArtifact::updateFrom
+	return possibleSlots;
 }
 
-void CArtifact::serializeJson(JsonSerializeFormat & handler)
+void CArtifact::updateFrom(const JsonNode& data)
 {
-
+	//TODO:CArtifact::updateFrom
 }
 
-void CGrowingArtifact::levelUpArtifact (CArtifactInstance * art)
+void CArtifact::setImage(int32_t iconIndex, std::string image, std::string large)
 {
-	auto b = std::make_shared<Bonus>();
-	b->type = BonusType::LEVEL_COUNTER;
-	b->val = 1;
-	b->duration = BonusDuration::COMMANDER_KILLED;
-	art->accumulateBonus(b);
-
-	for(const auto & bonus : bonusesPerLevel)
-	{
-		if (art->valOfBonuses(BonusType::LEVEL_COUNTER) % bonus.first == 0) //every n levels
-		{
-			art->accumulateBonus(std::make_shared<Bonus>(bonus.second));
-		}
-	}
-	for(const auto & bonus : thresholdBonuses)
-	{
-		if (art->valOfBonuses(BonusType::LEVEL_COUNTER) == bonus.first) //every n levels
-		{
-			art->addNewBonus(std::make_shared<Bonus>(bonus.second));
-		}
-	}
+	this->iconIndex = iconIndex;
+	this->image = image;
+	this->large = large;
 }
 
 CArtHandler::~CArtHandler() = default;
@@ -376,17 +399,19 @@ CArtifact * CArtHandler::loadFromJson(const std::string & scope, const JsonNode
 	assert(identifier.find(':') == std::string::npos);
 	assert(!scope.empty());
 
-	CArtifact * art = nullptr;
-
-	if(!VLC->settings()->getBoolean(EGameSettings::MODULE_COMMANDERS) || node["growing"].isNull())
+	CArtifact * art = new CArtifact();
+	if(!node["growing"].isNull())
 	{
-		art = new CArtifact();
-	}
-	else
-	{
-		auto * growing = new CGrowingArtifact();
-		loadGrowingArt(growing, node);
-		art = growing;
+		for(auto bonus : node["growing"]["bonusesPerLevel"].Vector())
+		{
+			art->bonusesPerLevel.emplace_back(static_cast<ui16>(bonus["level"].Float()), Bonus());
+			JsonUtils::parseBonus(bonus["bonus"], &art->bonusesPerLevel.back().second);
+		}
+		for(auto bonus : node["growing"]["thresholdBonuses"].Vector())
+		{
+			art->thresholdBonuses.emplace_back(static_cast<ui16>(bonus["level"].Float()), Bonus());
+			JsonUtils::parseBonus(bonus["bonus"], &art->thresholdBonuses.back().second);
+		}
 	}
 	art->id = ArtifactID(index);
 	art->identifier = identifier;
@@ -571,34 +596,19 @@ void CArtHandler::loadComponents(CArtifact * art, const JsonNode & node)
 {
 	if (!node["components"].isNull())
 	{
-		art->constituents = std::make_unique<std::vector<CArtifact *>>();
 		for(const auto & component : node["components"].Vector())
 		{
 			VLC->modh->identifiers.requestIdentifier("artifact", component, [=](si32 id)
 			{
 				// when this code is called both combinational art as well as component are loaded
 				// so it is safe to access any of them
-				art->constituents->push_back(objects[id]);
-				objects[id]->constituentOf.push_back(art);
+				art->constituents.push_back(objects[id]);
+				objects[id]->partOf.push_back(art);
 			});
 		}
 	}
 }
 
-void CArtHandler::loadGrowingArt(CGrowingArtifact * art, const JsonNode & node) const
-{
-	for (auto b : node["growing"]["bonusesPerLevel"].Vector())
-	{
-		art->bonusesPerLevel.emplace_back(static_cast<ui16>(b["level"].Float()), Bonus());
-		JsonUtils::parseBonus(b["bonus"], &art->bonusesPerLevel.back().second);
-	}
-	for (auto b : node["growing"]["thresholdBonuses"].Vector())
-	{
-		art->thresholdBonuses.emplace_back(static_cast<ui16>(b["level"].Float()), Bonus());
-		JsonUtils::parseBonus(b["bonus"], &art->thresholdBonuses.back().second);
-	}
-}
-
 ArtifactID CArtHandler::pickRandomArtifact(CRandomGenerator & rand, int flags, std::function<bool(ArtifactID)> accepts)
 {
 	auto getAllowedArts = [&](std::vector<ConstTransitivePtr<CArtifact> > &out, std::vector<CArtifact*> *arts, CArtifact::EartClass flag)
@@ -683,7 +693,7 @@ bool CArtHandler::legalArtifact(const ArtifactID & id)
 	auto art = objects[id];
 	//assert ( (!art->constituents) || art->constituents->size() ); //artifacts is not combined or has some components
 
-	if(art->constituents)
+	if(art->isCombined())
 		return false; //no combo artifacts spawning
 
 	if(art->aClass < CArtifact::ART_TREASURE || art->aClass > CArtifact::ART_RELIC)
@@ -787,179 +797,11 @@ void CArtHandler::afterLoadFinalization()
 	CBonusSystemNode::treeHasChanged();
 }
 
-CArtifactInstance::CArtifactInstance()
-{
-	init();
-}
-
-CArtifactInstance::CArtifactInstance( CArtifact *Art)
-{
-	init();
-	setType(Art);
-}
-
-void CArtifactInstance::setType( CArtifact *Art )
-{
-	artType = Art;
-	attachTo(*Art);
-}
-
-std::string CArtifactInstance::nodeName() const
-{
-	return "Artifact instance of " + (artType ? artType->getJsonKey() : std::string("uninitialized")) + " type";
-}
-
-void CArtifactInstance::init()
-{
-	id = ArtifactInstanceID();
-	id = static_cast<ArtifactInstanceID>(ArtifactID::NONE); //to be randomized
-	setNodeType(ARTIFACT_INSTANCE);
-}
-
-std::string CArtifactInstance::getDescription() const
-{
-	std::string text = artType->getDescriptionTranslated();
-	if(artType->getId() == ArtifactID::SPELL_SCROLL)
-	{
-		// we expect scroll description to be like this: This scroll contains the [spell name] spell which is added into your spell book for as long as you carry the scroll.
-		// so we want to replace text in [...] with a spell name
-		// however other language versions don't have name placeholder at all, so we have to be careful
-		SpellID spellID = getScrollSpellID();
-		size_t nameStart = text.find_first_of('[');
-		size_t nameEnd = text.find_first_of(']', nameStart);
-		if(spellID.getNum() >= 0)
-		{
-			if(nameStart != std::string::npos  &&  nameEnd != std::string::npos)
-				text = text.replace(nameStart, nameEnd - nameStart + 1, spellID.toSpell(VLC->spells())->getNameTranslated());
-		}
-	}
-	return text;
-}
-
-ArtifactID CArtifactInstance::getTypeId() const
-{
-	return artType->getId();
-}
-
-bool CArtifactInstance::canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved) const
-{
-	return artType->canBePutAt(al.getHolderArtSet(), al.slot, assumeDestRemoved);
-}
-
-void CArtifactInstance::putAt(const ArtifactLocation & al)
-{
-	al.getHolderArtSet()->putArtifact(al.slot, this);
-}
-
-void CArtifactInstance::removeFrom(const ArtifactLocation & al)
-{
-	al.getHolderArtSet()->removeArtifact(al.slot);
-}
-
-bool CArtifactInstance::canBeDisassembled() const
-{
-	return artType->canBeDisassembled();
-}
-
-void CArtifactInstance::move(const ArtifactLocation & src, const ArtifactLocation & dst)
-{
-	removeFrom(src);
-	putAt(dst);
-}
-
-void CArtifactInstance::deserializationFix()
-{
-	setType(artType);
-}
-
-SpellID CArtifactInstance::getScrollSpellID() const
-{
-	const auto b = getBonusLocalFirst(Selector::type()(BonusType::SPELL));
-	if(!b)
-	{
-		logMod->warn("Warning: %s doesn't bear any spell!", nodeName());
-		return SpellID::NONE;
-	}
-	return SpellID(b->subtype);
-}
-
-bool CArtifactInstance::isPart(const CArtifactInstance *supposedPart) const
-{
-	return supposedPart == this;
-}
-
-CCombinedArtifactInstance::CCombinedArtifactInstance(CArtifact *Art)
-	: CArtifactInstance(Art) //TODO: seems unused, but need to be written
-{
-}
-
-void CCombinedArtifactInstance::createConstituents()
-{
-	assert(artType);
-	assert(artType->constituents);
-
-	for(const CArtifact * art : *artType->constituents)
-	{
-		addAsConstituent(ArtifactUtils::createNewArtifactInstance(art->getId()), ArtifactPosition::PRE_FIRST);
-	}
-}
-
-void CCombinedArtifactInstance::addAsConstituent(CArtifactInstance * art, const ArtifactPosition & slot)
-{
-	assert(vstd::contains_if(*artType->constituents, [=](const CArtifact * constituent){
-		return constituent->getId() == art->artType->getId();
-	}));
-	assert(art->getParentNodes().size() == 1  &&  art->getParentNodes().front() == art->artType);
-	constituentsInfo.emplace_back(art, slot);
-	attachTo(*art);
-}
-
-void CCombinedArtifactInstance::removeFrom(const ArtifactLocation & al)
-{
-	CArtifactInstance::removeFrom(al);
-	for(auto & part : constituentsInfo)
-	{
-		if(part.slot != ArtifactPosition::PRE_FIRST)
-			part.slot = ArtifactPosition::PRE_FIRST;
-	}
-}
-
-void CCombinedArtifactInstance::deserializationFix()
-{
-	for(ConstituentInfo &ci : constituentsInfo)
-		attachTo(*ci.art);
-}
-
-bool CCombinedArtifactInstance::isPart(const CArtifactInstance *supposedPart) const
-{
-	bool me = CArtifactInstance::isPart(supposedPart);
-	if(me)
-		return true;
-
-	//check for constituents
-	for(const ConstituentInfo &constituent : constituentsInfo)
-		if(constituent.art == supposedPart)
-			return true;
-
-	return false;
-}
-
-CCombinedArtifactInstance::ConstituentInfo::ConstituentInfo(CArtifactInstance * Art, const ArtifactPosition & Slot):
-	art(Art),
-	slot(Slot)
-{
-}
-
-bool CCombinedArtifactInstance::ConstituentInfo::operator==(const ConstituentInfo &rhs) const
-{
-	return art == rhs.art && slot == rhs.slot;
-}
-
 CArtifactSet::~CArtifactSet() = default;
 
 const CArtifactInstance * CArtifactSet::getArt(const ArtifactPosition & pos, bool excludeLocked) const
 {
-	if(const ArtSlotInfo *si = getSlot(pos))
+	if(const ArtSlotInfo * si = getSlot(pos))
 	{
 		if(si->artifact && (!excludeLocked || !si->locked))
 			return si->artifact;
@@ -1033,11 +875,11 @@ ArtifactPosition CArtifactSet::getArtPos(const CArtifactInstance *art) const
 const CArtifactInstance * CArtifactSet::getArtByInstanceId(const ArtifactInstanceID & artInstId) const
 {
 	for(auto i : artifactsWorn)
-		if(i.second.artifact->id == artInstId)
+		if(i.second.artifact->getId() == artInstId)
 			return i.second.artifact;
 
 	for(auto i : artifactsInBackpack)
-		if(i.artifact->id == artInstId)
+		if(i.artifact->getId() == artInstId)
 			return i.artifact;
 
 	return nullptr;
@@ -1047,7 +889,7 @@ const ArtifactPosition CArtifactSet::getSlotByInstance(const CArtifactInstance *
 {
 	if(artInst)
 	{
-		for(auto & slot : artInst->artType->possibleSlots.at(bearerType()))
+		for(const auto & slot : artInst->artType->getPossibleSlots().at(bearerType()))
 			if(getArt(slot) == artInst)
 				return slot;
 
@@ -1087,19 +929,18 @@ unsigned CArtifactSet::getArtPosCount(const ArtifactID & aid, bool onlyWorn, boo
 void CArtifactSet::putArtifact(ArtifactPosition slot, CArtifactInstance * art)
 {
 	setNewArtSlot(slot, art, false);
-	if(art->artType->canBeDisassembled() && ArtifactUtils::isSlotEquipment(slot))
+	if(art->artType->isCombined() && 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)
+		for(const auto & part : art->getPartsInfo())
+			if(vstd::contains(part.art->artType->getPossibleSlots().at(bearerType()), slot)
 				&& (part.slot == ArtifactPosition::PRE_FIRST))
 			{
 				mainPart = part.art;
 				break;
 			}
 
-		for(auto & part : parts)
+		for(auto & part : art->getPartsInfo())
 		{
 			if(part.art != mainPart)
 			{
@@ -1118,10 +959,9 @@ void CArtifactSet::removeArtifact(ArtifactPosition slot)
 	auto art = getArt(slot, false);
 	if(art)
 	{
-		if(art->canBeDisassembled())
+		if(art->isCombined())
 		{
-			auto combinedArt = dynamic_cast<CCombinedArtifactInstance*>(art);
-			for(auto & part : combinedArt->constituentsInfo)
+			for(auto & part : art->getPartsInfo())
 			{
 				if(getArt(part.slot, false))
 					eraseArtSlot(part.slot);
@@ -1131,19 +971,18 @@ void CArtifactSet::removeArtifact(ArtifactPosition slot)
 	}
 }
 
-std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *> CArtifactSet::searchForConstituent(const ArtifactID & aid) const
+std::pair<const CArtifactInstance *, const CArtifactInstance *> CArtifactSet::searchForConstituent(const ArtifactID & aid) const
 {
 	for(const auto & slot : artifactsInBackpack)
 	{
 		auto art = slot.artifact;
-		if(art->canBeDisassembled())
+		if(art->isCombined())
 		{
-			auto * ass = dynamic_cast<CCombinedArtifactInstance *>(art.get());
-			for(auto& ci : ass->constituentsInfo)
+			for(auto & ci : art->getPartsInfo())
 			{
 				if(ci.art->getTypeId() == aid)
 				{
-					return {ass, ci.art};
+					return {art, ci.art};
 				}
 			}
 		}
@@ -1156,7 +995,7 @@ const CArtifactInstance * CArtifactSet::getHiddenArt(const ArtifactID & aid) con
 	return searchForConstituent(aid).second;
 }
 
-const CCombinedArtifactInstance * CArtifactSet::getAssemblyByConstituent(const ArtifactID & aid) const
+const CArtifactInstance * CArtifactSet::getAssemblyByConstituent(const ArtifactID & aid) const
 {
 	return searchForConstituent(aid).first;
 }

lib/CArtHandler.h

@@ -20,9 +20,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 class CArtHandler;
-class CArtifact;
 class CGHeroInstance;
-struct ArtifactLocation;
 class CArtifactSet;
 class CArtifactInstance;
 class CRandomGenerator;
@@ -44,26 +42,75 @@ namespace ArtBearer
 	};
 }
 
-class DLL_LINKAGE CArtifact : public Artifact, public CBonusSystemNode //container for artifacts
+class DLL_LINKAGE CCombinedArtifact
 {
-	ArtifactID id;
+protected:
+	CCombinedArtifact() = default;
+
+	std::vector<CArtifact*> constituents; // Artifacts IDs a combined artifact consists of, or nullptr.
+	std::vector<CArtifact*> partOf; // Reverse map of constituents - combined arts that include this art
+public:
+	bool isCombined() const;
+	const std::vector<CArtifact*> & getConstituents() const;
+	const std::vector<CArtifact*> & getPartOf() const;
+
+	template <typename Handler> void serialize(Handler & h, const int version)
+	{
+		h & constituents;
+		h & partOf;
+	}
+};
+
+class DLL_LINKAGE CScrollArtifact
+{
+protected:
+	CScrollArtifact() = default;
+public:
+	bool isScroll() const;
+};
+
+class DLL_LINKAGE CGrowingArtifact
+{
+protected:
+	CGrowingArtifact() = default;
+
+	std::vector <std::pair<ui16, Bonus>> bonusesPerLevel; // Bonus given each n levels
+	std::vector <std::pair<ui16, Bonus>> thresholdBonuses; // After certain level they will be added once
+public:
+	bool isGrowing() const;
+
+	std::vector <std::pair<ui16, Bonus>> & getBonusesPerLevel();
+	const std::vector <std::pair<ui16, Bonus>> & getBonusesPerLevel() const;
+	std::vector <std::pair<ui16, Bonus>> & getThresholdBonuses();
+	const std::vector <std::pair<ui16, Bonus>> & getThresholdBonuses() const;
 
+	template <typename Handler> void serialize(Handler & h, const int version)
+	{
+		h & bonusesPerLevel;
+		h & thresholdBonuses;
+	}
+};
+
+// Container for artifacts. Not for instances.
+class DLL_LINKAGE CArtifact
+	: public Artifact, public CBonusSystemNode, public CCombinedArtifact, public CScrollArtifact, public CGrowingArtifact
+{
+	ArtifactID id;
+	std::string image;
+	std::string large; // big image for custom artifacts, used in drag & drop
+	std::string advMapDef; // used for adventure map object
 	std::string modScope;
 	std::string identifier;
+	int32_t iconIndex;
+	uint32_t price;
+	CreatureID warMachine;
+	// Bearer Type => ids of slots where artifact can be placed
+	std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition>> possibleSlots;
 
 public:
 	enum EartClass {ART_SPECIAL=1, ART_TREASURE=2, ART_MINOR=4, ART_MAJOR=8, ART_RELIC=16}; //artifact classes
 
-	std::string image;
-	std::string large; // big image for custom artifacts, used in drag & drop
-	std::string advMapDef; //used for adventure map object
-	si32 iconIndex = ArtifactID::NONE;
-	ui32 price = 0;
-	std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition> > possibleSlots; //Bearer Type => ids of slots where artifact can be placed
-	std::unique_ptr<std::vector<CArtifact *> > constituents; // Artifacts IDs a combined artifact consists of, or nullptr.
-	std::vector<CArtifact *> constituentOf; // Reverse map of constituents - combined arts that include this art
 	EartClass aClass = ART_SPECIAL;
-	CreatureID warMachine;
 
 	int32_t getIndex() const override;
 	int32_t getIconIndex() const override;
@@ -88,26 +135,25 @@ public:
 	int getArtClassSerial() const; //0 - treasure, 1 - minor, 2 - major, 3 - relic, 4 - spell scroll, 5 - other
 	std::string nodeName() const override;
 	void addNewBonus(const std::shared_ptr<Bonus>& b) override;
+	const std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition>> & getPossibleSlots() const;
 
-	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);
+	// Is used for testing purposes only
+	void setImage(int32_t iconIndex, std::string image, std::string large);
 
-	template <typename Handler> void serialize(Handler &h, const int version)
+	template <typename Handler> void serialize(Handler & h, const int version)
 	{
 		h & static_cast<CBonusSystemNode&>(*this);
+		h & static_cast<CCombinedArtifact&>(*this);
+		h & static_cast<CGrowingArtifact&>(*this);
 		h & image;
 		h & large;
 		h & advMapDef;
 		h & iconIndex;
 		h & price;
 		h & possibleSlots;
-		h & constituents;
-		h & constituentOf;
 		h & aClass;
 		h & id;
 		h & modScope;
@@ -121,99 +167,6 @@ public:
 	friend class CArtHandler;
 };
 
-class DLL_LINKAGE CGrowingArtifact : public CArtifact //for example commander artifacts getting bonuses after battle
-{
-public:
-	std::vector <std::pair <ui16, Bonus> > bonusesPerLevel; //bonus given each n levels
-	std::vector <std::pair <ui16, Bonus> > thresholdBonuses; //after certain level they will be added once
-
-	void levelUpArtifact(CArtifactInstance * art) override;
-
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & static_cast<CArtifact&>(*this);
-		h & bonusesPerLevel;
-		h & thresholdBonuses;
-	}
-};
-
-class DLL_LINKAGE CArtifactInstance : public CBonusSystemNode
-{
-protected:
-	void init();
-public:
-	CArtifactInstance(CArtifact * Art);
-	CArtifactInstance();
-
-	ConstTransitivePtr<CArtifact> artType;
-	ArtifactInstanceID id;
-
-	std::string nodeName() const override;
-	void deserializationFix();
-	void setType(CArtifact *Art);
-
-	std::string getDescription() const;
-	SpellID getScrollSpellID() const; //to be used with scrolls (and similar arts), -1 if none
-
-	ArtifactID getTypeId() const;
-	bool canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved = false) const;  //forwards to the above one
-	virtual bool canBeDisassembled() const;
-	/// 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;
-
-	virtual void putAt(const ArtifactLocation & al);
-	virtual void removeFrom(const ArtifactLocation & al);
-	virtual void move(const ArtifactLocation & src, const ArtifactLocation & dst);
-
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & static_cast<CBonusSystemNode&>(*this);
-		h & artType;
-		h & id;
-		BONUS_TREE_DESERIALIZATION_FIX
-	}
-};
-
-class DLL_LINKAGE CCombinedArtifactInstance : public CArtifactInstance
-{
-public:
-	CCombinedArtifactInstance(CArtifact * Art);
-	struct ConstituentInfo
-	{
-		ConstTransitivePtr<CArtifactInstance> art;
-		ArtifactPosition slot;
-		template <typename Handler> void serialize(Handler &h, const int version)
-		{
-			h & art;
-			h & slot;
-		}
-
-		bool operator==(const ConstituentInfo &rhs) const;
-		ConstituentInfo(CArtifactInstance * art = nullptr, const ArtifactPosition & slot = ArtifactPosition::PRE_FIRST);
-	};
-
-	std::vector<ConstituentInfo> constituentsInfo;
-
-	bool isPart(const CArtifactInstance *supposedPart) const override;
-	void createConstituents();
-	void addAsConstituent(CArtifactInstance * art, const ArtifactPosition & slot);
-	void removeFrom(const ArtifactLocation & al) override;
-
-	CCombinedArtifactInstance() = default;
-
-	void deserializationFix();
-
-	friend class CArtifactInstance;
-	friend struct AssembledArtifact;
-	template <typename Handler> void serialize(Handler &h, const int version)
-	{
-		h & static_cast<CArtifactInstance&>(*this);
-		h & constituentsInfo;
-		BONUS_TREE_DESERIALIZATION_FIX
-	}
-};
-
 class DLL_LINKAGE CArtHandler : public CHandlerBase<ArtifactID, Artifact, CArtifact, ArtifactService>
 {
 public:
@@ -269,7 +222,6 @@ private:
 	void loadClass(CArtifact * art, const JsonNode & node) const;
 	void loadType(CArtifact * art, const JsonNode & node) const;
 	void loadComponents(CArtifact * art, const JsonNode & node);
-	void loadGrowingArt(CGrowingArtifact * art, const JsonNode & node) const;
 
 	void erasePickedArt(const ArtifactID & id);
 };
@@ -313,7 +265,7 @@ public:
 	const ArtifactPosition getSlotByInstance(const CArtifactInstance * artInst) const;
 	/// Search for constituents of assemblies in backpack which do not have an ArtifactPosition
 	const CArtifactInstance * getHiddenArt(const ArtifactID & aid) const;
-	const CCombinedArtifactInstance * getAssemblyByConstituent(const ArtifactID & aid) const;
+	const CArtifactInstance * getAssemblyByConstituent(const ArtifactID & aid) const;
 	/// Checks if hero possess artifact of given id (either in backack or worn)
 	bool hasArt(const ArtifactID & aid, bool onlyWorn = false, bool searchBackpackAssemblies = false, bool allowLocked = true) const;
 	bool hasArtBackpack(const ArtifactID & aid) const;
@@ -335,7 +287,7 @@ public:
 
 	void serializeJsonArtifacts(JsonSerializeFormat & handler, const std::string & fieldName, CMap * map);
 protected:
-	std::pair<const CCombinedArtifactInstance *, const CArtifactInstance *> searchForConstituent(const ArtifactID & aid) const;
+	std::pair<const CArtifactInstance *, const CArtifactInstance *> searchForConstituent(const ArtifactID & aid) const;
 
 private:
 	void serializeJsonHero(JsonSerializeFormat & handler, CMap * map);

lib/CArtifactInstance.cpp

@@ -0,0 +1,192 @@
+/*
+ * CArtifactInstance.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 "StdInc.h"
+#include "CArtifactInstance.h"
+
+#include "ArtifactUtils.h"
+#include "CArtHandler.h"
+#include "NetPacksBase.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+void CCombinedArtifactInstance::addPart(CArtifactInstance * art, const ArtifactPosition & slot)
+{
+	auto artInst = static_cast<CArtifactInstance*>(this);
+	assert(vstd::contains_if(*artInst->artType->constituents,
+		[=](const CArtifact * partType)
+		{
+			return partType->getId() == art->getTypeId();
+		}));
+	assert(art->getParentNodes().size() == 1  &&  art->getParentNodes().front() == art->artType);
+	partsInfo.emplace_back(art, slot);
+	artInst->attachTo(*art);
+}
+
+bool CCombinedArtifactInstance::isPart(const CArtifactInstance * supposedPart) const
+{
+	if(supposedPart == this)
+		return true;
+
+	for(const PartInfo & constituent : partsInfo)
+	{
+		if(constituent.art == supposedPart)
+			return true;
+	}
+
+	return false;
+}
+
+std::vector<CCombinedArtifactInstance::PartInfo> & CCombinedArtifactInstance::getPartsInfo()
+{
+	// TODO romove this func. encapsulation violation
+	return partsInfo;
+}
+
+const std::vector<CCombinedArtifactInstance::PartInfo> & CCombinedArtifactInstance::getPartsInfo() const
+{
+	return partsInfo;
+}
+
+SpellID CScrollArtifactInstance::getScrollSpellID() const
+{
+	auto artInst = static_cast<const CArtifactInstance*>(this);
+	const auto bonus = artInst->getBonusLocalFirst(Selector::type()(BonusType::SPELL));
+	if(!bonus)
+	{
+		logMod->warn("Warning: %s doesn't bear any spell!", artInst->nodeName());
+		return SpellID::NONE;
+	}
+	return SpellID(bonus->subtype);
+}
+
+void CGrowingArtifactInstance::growingUp()
+{
+	auto artInst = static_cast<CArtifactInstance*>(this);
+	
+	if(artInst->artType->isGrowing())
+	{
+
+		auto bonus = std::make_shared<Bonus>();
+		bonus->type = BonusType::LEVEL_COUNTER;
+		bonus->val = 1;
+		bonus->duration = BonusDuration::COMMANDER_KILLED;
+		artInst->accumulateBonus(bonus);
+
+		for(const auto & bonus : artInst->artType->getBonusesPerLevel())
+		{
+			// Every n levels
+			if(artInst->valOfBonuses(BonusType::LEVEL_COUNTER) % bonus.first == 0)
+			{
+				artInst->accumulateBonus(std::make_shared<Bonus>(bonus.second));
+			}
+		}
+		for(const auto & bonus : artInst->artType->getThresholdBonuses())
+		{
+			// At n level
+			if(artInst->valOfBonuses(BonusType::LEVEL_COUNTER) == bonus.first)
+			{
+				artInst->addNewBonus(std::make_shared<Bonus>(bonus.second));
+			}
+		}
+	}
+}
+
+void CArtifactInstance::init()
+{
+	// Artifact to be randomized
+	id = static_cast<ArtifactInstanceID>(ArtifactID::NONE);
+	setNodeType(ARTIFACT_INSTANCE);
+}
+
+CArtifactInstance::CArtifactInstance(CArtifact * art)
+{
+	init();
+	setType(art);
+}
+
+CArtifactInstance::CArtifactInstance()
+{
+	init();
+}
+
+void CArtifactInstance::setType(CArtifact * art)
+{
+	artType = art;
+	attachTo(*art);
+}
+
+std::string CArtifactInstance::nodeName() const
+{
+	return "Artifact instance of " + (artType ? artType->getJsonKey() : std::string("uninitialized")) + " type";
+}
+
+std::string CArtifactInstance::getDescription() const
+{
+	std::string text = artType->getDescriptionTranslated();
+	if(artType->isScroll())
+		ArtifactUtils::insertScrrollSpellName(text, getScrollSpellID());
+	return text;
+}
+
+ArtifactID CArtifactInstance::getTypeId() const
+{
+	return artType->getId();
+}
+
+ArtifactInstanceID CArtifactInstance::getId() const
+{
+	return id;
+}
+
+void CArtifactInstance::setId(ArtifactInstanceID id)
+{
+	this->id = id;
+}
+
+bool CArtifactInstance::canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved) const
+{
+	return artType->canBePutAt(al.getHolderArtSet(), al.slot, assumeDestRemoved);
+}
+
+bool CArtifactInstance::isCombined() const
+{
+	return artType->isCombined();
+}
+
+void CArtifactInstance::putAt(const ArtifactLocation & al)
+{
+	al.getHolderArtSet()->putArtifact(al.slot, this);
+}
+
+void CArtifactInstance::removeFrom(const ArtifactLocation & al)
+{
+	al.getHolderArtSet()->removeArtifact(al.slot);
+	for(auto & part : partsInfo)
+	{
+		if(part.slot != ArtifactPosition::PRE_FIRST)
+			part.slot = ArtifactPosition::PRE_FIRST;
+	}
+}
+
+void CArtifactInstance::move(const ArtifactLocation & src, const ArtifactLocation & dst)
+{
+	removeFrom(src);
+	putAt(dst);
+}
+
+void CArtifactInstance::deserializationFix()
+{
+	setType(artType);
+	for(PartInfo & part : partsInfo)
+		attachTo(*part.art);
+}
+
+VCMI_LIB_NAMESPACE_END

lib/CArtifactInstance.h

@@ -0,0 +1,102 @@
+/*
+ * CArtifactInstance.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 "bonuses/CBonusSystemNode.h"
+#include "GameConstants.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+struct ArtifactLocation;
+
+class DLL_LINKAGE CCombinedArtifactInstance
+{
+protected:
+	CCombinedArtifactInstance() = default;
+public:
+	struct PartInfo
+	{
+		ConstTransitivePtr<CArtifactInstance> art;
+		ArtifactPosition slot;
+		template <typename Handler> void serialize(Handler & h, const int version)
+		{
+			h & art;
+			h & slot;
+		}
+		PartInfo(CArtifactInstance * art = nullptr, const ArtifactPosition & slot = ArtifactPosition::PRE_FIRST)
+			: art(art), slot(slot) {};
+	};
+	void addPart(CArtifactInstance * art, const ArtifactPosition & slot);
+	// Checks if supposed part inst is part of this combined art inst
+	bool isPart(const CArtifactInstance * supposedPart) const;
+	std::vector<PartInfo> & getPartsInfo();
+	const std::vector<PartInfo> & getPartsInfo() const;
+
+	template <typename Handler> void serialize(Handler & h, const int version)
+	{
+		h & partsInfo;
+	}
+protected:
+	std::vector<PartInfo> partsInfo;
+};
+
+class DLL_LINKAGE CScrollArtifactInstance
+{
+protected:
+	CScrollArtifactInstance() = default;
+public:
+	SpellID getScrollSpellID() const;
+};
+
+class DLL_LINKAGE CGrowingArtifactInstance
+{
+protected:
+	CGrowingArtifactInstance() = default;
+public:
+	void growingUp();
+};
+
+class DLL_LINKAGE CArtifactInstance
+	: public CBonusSystemNode, public CCombinedArtifactInstance, public CScrollArtifactInstance, public CGrowingArtifactInstance
+{
+protected:
+	void init();
+
+	ArtifactInstanceID id;
+public:
+	ConstTransitivePtr<CArtifact> artType;
+
+	CArtifactInstance(CArtifact * art);
+	CArtifactInstance();
+	void setType(CArtifact * art);
+	std::string nodeName() const override;
+	std::string getDescription() const;
+	ArtifactID getTypeId() const;
+	ArtifactInstanceID getId() const;
+	void setId(ArtifactInstanceID id);
+
+	bool canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved = false) const;
+	bool isCombined() const;
+	void putAt(const ArtifactLocation & al);
+	void removeFrom(const ArtifactLocation & al);
+	void move(const ArtifactLocation & src, const ArtifactLocation & dst);
+	
+	void deserializationFix();
+	template <typename Handler> void serialize(Handler & h, const int version)
+	{
+		h & static_cast<CBonusSystemNode&>(*this);
+		h & static_cast<CCombinedArtifactInstance&>(*this);
+		h & artType;
+		h & id;
+		BONUS_TREE_DESERIALIZATION_FIX
+	}
+};
+
+VCMI_LIB_NAMESPACE_END

lib/CCreatureSet.h

@@ -13,6 +13,7 @@
 #include "bonuses/CBonusSystemNode.h"
 #include "GameConstants.h"
 #include "CArtHandler.h"
+#include "CArtifactInstance.h"
 #include "CCreatureHandler.h"
 
 #include <vcmi/Entity.h>

lib/JsonRandom.cpp

@@ -206,7 +206,7 @@ namespace JsonRandom
 		{
 			CArtifact * art = VLC->arth->objects[artID];
 
-			if(!vstd::iswithin(art->price, minValue, maxValue))
+			if(!vstd::iswithin(art->getPrice(), minValue, maxValue))
 				return false;
 
 			if(!allowedClasses.empty() && !allowedClasses.count(art->aClass))
@@ -217,7 +217,7 @@ namespace JsonRandom
 
 			if(!allowedPositions.empty())
 			{
-				for(const auto & pos : art->possibleSlots[ArtBearer::HERO])
+				for(const auto & pos : art->getPossibleSlots().at(ArtBearer::HERO))
 				{
 					if(allowedPositions.count(pos))
 						return true;

lib/NetPacks.h

@@ -1491,20 +1491,30 @@ struct DLL_LINKAGE BattleSetActiveStack : public CPackForClient
 struct DLL_LINKAGE BattleResultAccepted : public CPackForClient
 {
 	void applyGs(CGameState * gs) const;
-	
-	CGHeroInstance * hero1 = nullptr;
-	CGHeroInstance * hero2 = nullptr;
-	CArmedInstance * army1 = nullptr;
-	CArmedInstance * army2 = nullptr;
-	TExpType exp[2];
 
-	template <typename Handler> void serialize(Handler &h, const int version)
+	struct HeroBattleResults
 	{
-		h & hero1;
-		h & hero2;
-		h & army1;
-		h & army2;
-		h & exp;
+		HeroBattleResults()
+			: hero(nullptr), army(nullptr), exp(0) {}
+
+		CGHeroInstance * hero;
+		CArmedInstance * army;
+		TExpType exp;
+
+		template <typename Handler> void serialize(Handler & h, const int version)
+		{
+			h & hero;
+			h & army;
+			h & exp;
+		}
+	};
+	std::array<HeroBattleResults, 2> heroResult;
+	ui8 winnerSide;
+
+	template <typename Handler> void serialize(Handler & h, const int version)
+	{
+		h & heroResult;
+		h & winnerSide;
 	}
 };
 

lib/NetPacksLib.cpp

@@ -1524,12 +1524,17 @@ void NewObject::applyGs(CGameState *gs)
 void NewArtifact::applyGs(CGameState *gs)
 {
 	assert(!vstd::contains(gs->map->artInstances, art));
-	gs->map->addNewArtifactInstance(art);
-
 	assert(!art->getParentNodes().size());
+	assert(art->artType);
+
 	art->setType(art->artType);
-	if(auto * cart = dynamic_cast<CCombinedArtifactInstance *>(art.get()))
-		cart->createConstituents();
+	if(art->isCombined())
+	{
+		assert(art->artType->getConstituents());
+		for(const auto & part : art->artType->getConstituents())
+			art->addPart(ArtifactUtils::createNewArtifactInstance(part), ArtifactPosition::PRE_FIRST);
+	}
+	gs->map->addNewArtifactInstance(art);
 }
 
 const CStackInstance * StackLocation::getStack()
@@ -1822,7 +1827,7 @@ void EraseArtifact::applyGs(CGameState *gs)
 		for(auto& p : aset->artifactsWorn)
 		{
 			auto art = p.second.artifact;
-			if(art->canBeDisassembled() && art->isPart(slot->artifact))
+			if(art->isCombined() && art->isPart(slot->artifact))
 			{
 				dis.al.slot = aset->getArtPos(art);
 				#ifndef NDEBUG
@@ -1930,10 +1935,10 @@ void AssembledArtifact::applyGs(CGameState *gs)
 			return art->getId() == builtArt->getId();
 		}));
 
-	auto * combinedArt = new CCombinedArtifactInstance(builtArt);
+	auto * combinedArt = new CArtifactInstance(builtArt);
 	gs->map->addNewArtifactInstance(combinedArt);
 	// Retrieve all constituents
-	for(const CArtifact * constituent : *builtArt->constituents)
+	for(const CArtifact * constituent : builtArt->getConstituents())
 	{
 		ArtifactPosition pos = combineEquipped ? artSet->getArtPos(constituent->getId(), true, false) :
 			artSet->getArtBackpackPos(constituent->getId());
@@ -1944,8 +1949,8 @@ void AssembledArtifact::applyGs(CGameState *gs)
 		constituentInstance->removeFrom(ArtifactLocation(al.artHolder, pos));
 		if(combineEquipped)
 		{
-			if(!vstd::contains(combinedArt->artType->possibleSlots[artSet->bearerType()], al.slot)
-				&& vstd::contains(combinedArt->artType->possibleSlots[artSet->bearerType()], pos))
+			if(!vstd::contains(combinedArt->artType->getPossibleSlots().at(artSet->bearerType()), al.slot)
+				&& vstd::contains(combinedArt->artType->getPossibleSlots().at(artSet->bearerType()), pos))
 				al.slot = pos;
 			if(al.slot == pos)
 				pos = ArtifactPosition::PRE_FIRST;
@@ -1955,7 +1960,7 @@ void AssembledArtifact::applyGs(CGameState *gs)
 			al.slot = std::min(al.slot, pos);
 			pos = ArtifactPosition::PRE_FIRST;
 		}
-		combinedArt->addAsConstituent(constituentInstance, pos);
+		combinedArt->addPart(constituentInstance, pos);
 	}
 
 	//put new combined artifacts
@@ -1964,19 +1969,19 @@ void AssembledArtifact::applyGs(CGameState *gs)
 
 void DisassembledArtifact::applyGs(CGameState *gs)
 {
-	auto * disassembled = dynamic_cast<CCombinedArtifactInstance *>(al.getArt());
+	auto * disassembled = al.getArt();
 	assert(disassembled);
 
-	std::vector<CCombinedArtifactInstance::ConstituentInfo> constituents = disassembled->constituentsInfo;
+	auto parts = disassembled->getPartsInfo();
 	disassembled->removeFrom(al);
-	for(CCombinedArtifactInstance::ConstituentInfo &ci : constituents)
+	for(auto & part : parts)
 	{
-		ArtifactLocation constituentLoc = al;
-		constituentLoc.slot = (ci.slot >= 0 ? ci.slot : al.slot); //-1 is slot of main constituent -> it'll replace combined artifact in its pos
-		disassembled->detachFrom(*ci.art);
-		ci.art->putAt(constituentLoc);
+		ArtifactLocation partLoc = al;
+		// ArtifactPosition::PRE_FIRST is value of main part slot -> it'll replace combined artifact in its pos
+		partLoc.slot = (ArtifactUtils::isSlotEquipment(part.slot) ? part.slot : al.slot);
+		disassembled->detachFrom(*part.art);
+		part.art->putAt(partLoc);
 	}
-
 	gs->map->eraseArtifactInstance(disassembled);
 }
 
@@ -2209,32 +2214,32 @@ void BattleUpdateGateState::applyGs(CGameState * gs) const
 
 void BattleResultAccepted::applyGs(CGameState * gs) const
 {
-	for(auto * h : {hero1, hero2})
+	// Remove any "until next battle" bonuses
+	for(auto & res : heroResult)
 	{
-		if(h)
-		{
-			h->removeBonusesRecursive(Bonus::OneBattle); 	//remove any "until next battle" bonuses
-			if (h->commander && h->commander->alive)
-			{
-				for (auto art : h->commander->artifactsWorn) //increment bonuses for commander artifacts
-				{
-					art.second.artifact->artType->levelUpArtifact (art.second.artifact);
-				}
-			}
-		}
+		if(res.hero)
+			res.hero->removeBonusesRecursive(Bonus::OneBattle);
 	}
 
-	if(VLC->settings()->getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE))
+	// Grow up growing artifacts
+	if(const auto hero = heroResult[winnerSide].hero)
 	{
-		for(int i = 0; i < 2; i++)
+		if(hero->commander && hero->commander->alive)
 		{
-			if(exp[i])
-			{
-				if(auto * army = (i == 0 ? army1 : army2))
-					army->giveStackExp(exp[i]);
-			}
+			for(auto & art : hero->commander->artifactsWorn)
+				art.second.artifact->growingUp();
 		}
-
+		for(auto & art : hero->artifactsWorn)
+		{
+			art.second.artifact->growingUp();
+		}
+	}
+	if(VLC->settings()->getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE))
+	{
+		if(heroResult[0].army)
+			heroResult[0].army->giveStackExp(heroResult[0].exp);
+		if(heroResult[1].army)
+			heroResult[1].army->giveStackExp(heroResult[1].exp);
 		CBonusSystemNode::treeHasChanged();
 	}
 

lib/battle/BattleInfo.cpp

@@ -378,7 +378,7 @@ BattleInfo * BattleInfo::setupBattle(const int3 & tile, TerrainId terrain, const
 
 			if(nullptr != warMachineArt)
 			{
-				CreatureID cre = warMachineArt->artType->warMachine;
+				CreatureID cre = warMachineArt->artType->getWarMachine();
 
 				if(cre != CreatureID::NONE)
 					curB->generateNewStack(curB->nextUnitId(), CStackBasicDescriptor(cre, 1), side, SlotID::WAR_MACHINES_SLOT, hex);

lib/mapObjects/CGHeroInstance.cpp

@@ -406,10 +406,10 @@ void CGHeroInstance::initArmy(CRandomGenerator & rand, IArmyDescriptor * dst)
 			ArtifactID aid = creature->warMachine;
 			const CArtifact * art = aid.toArtifact();
 
-			if(art != nullptr && !art->possibleSlots.at(ArtBearer::HERO).empty())
+			if(art != nullptr && !art->getPossibleSlots().at(ArtBearer::HERO).empty())
 			{
 				//TODO: should we try another possible slots?
-				ArtifactPosition slot = art->possibleSlots.at(ArtBearer::HERO).front();
+				ArtifactPosition slot = art->getPossibleSlots().at(ArtBearer::HERO).front();
 
 				if(!getArt(slot))
 					putArtifact(slot, ArtifactUtils::createNewArtifactInstance(aid));

lib/mapObjects/CQuest.cpp

@@ -153,7 +153,7 @@ bool CQuest::checkQuest(const CGHeroInstance * h) const
 				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;
+					reqSlots += h->getAssemblyByConstituent(elem.first)->getPartsInfo().size() - 2;
 			}
 			if(ArtifactUtils::isBackpackFreeSlots(h, reqSlots))
 				return true;
@@ -804,7 +804,7 @@ void CGSeerHut::finishQuest(const CGHeroInstance * h, ui32 accept) const
 					{
 						const auto * assembly = h->getAssemblyByConstituent(elem);
 						assert(assembly);
-						auto parts = assembly->constituentsInfo;
+						auto parts = assembly->getPartsInfo();
 
 						// Remove the assembly
 						cb->removeArtifact(ArtifactLocation(h, h->getArtPos(assembly)));

lib/mapObjects/MiscObjects.cpp

@@ -820,7 +820,7 @@ void CGArtifact::afterAddToMap(CMap * map)
 	//Artifacts from map objects are never removed
 	//FIXME: This should be revertible in map editor
 
-	if(ID == Obj::SPELL_SCROLL && storedArtifact && storedArtifact->id.getNum() < 0)
+	if(ID == Obj::SPELL_SCROLL && storedArtifact && storedArtifact->getId().getNum() < 0)
         map->addNewArtifactInstance(storedArtifact);
 }
 

lib/mapping/CMap.cpp

@@ -466,7 +466,7 @@ void CMap::checkForObjectives()
 
 void CMap::addNewArtifactInstance(CArtifactInstance * art)
 {
-	art->id = ArtifactInstanceID(static_cast<si32>(artInstances.size()));
+	art->setId(static_cast<ArtifactInstanceID>(artInstances.size()));
 	artInstances.emplace_back(art);
 }
 
@@ -474,7 +474,7 @@ void CMap::eraseArtifactInstance(CArtifactInstance * art)
 {
 	//TODO: handle for artifacts removed in map editor
 	assert(artInstances[art->id.getNum()] == art);
-	artInstances[art->id.getNum()].dellNull();
+	artInstances[art->getId().getNum()].dellNull();
 }
 
 void CMap::addNewQuestInstance(CQuest* quest)

lib/mapping/MapFormatH3M.cpp

@@ -746,7 +746,7 @@ void CMapLoaderH3M::readAllowedArtifacts()
 	if(!features.levelSOD)
 	{
 		for(CArtifact * artifact : VLC->arth->objects)
-			if(artifact->constituents)
+			if(artifact->isCombined())
 				map->allowedArtifact[artifact->getId()] = false;
 	}
 

lib/registerTypes/RegisterTypes.h

@@ -206,7 +206,6 @@ void registerTypesMapObjects2(Serializer &s)
 
 //	s.template registerType<CBonusSystemNode>();
 	s.template registerType<CBonusSystemNode, CArtifact>();
-	s.template registerType<CArtifact, CGrowingArtifact>();
 	s.template registerType<CBonusSystemNode, CCreature>();
 	s.template registerType<CBonusSystemNode, CStackInstance>();
 	s.template registerType<CStackInstance, CCommanderInstance>();
@@ -218,7 +217,6 @@ void registerTypesMapObjects2(Serializer &s)
 	s.template registerType<CBonusSystemNode, BattleInfo>();
 	//s.template registerType<QuestInfo>();
 	s.template registerType<CBonusSystemNode, CArtifactInstance>();
-	s.template registerType<CArtifactInstance, CCombinedArtifactInstance>();
 
 	//s.template registerType<CObstacleInstance>();
 		s.template registerType<CObstacleInstance, SpellCreatedObstacle>();

lib/rmg/CMapGenerator.cpp

@@ -113,7 +113,7 @@ void CMapGenerator::initQuestArtsRemaining()
 	for (auto art : VLC->arth->objects)
 	{
 		//Don't use parts of combined artifacts
-		if (art->aClass == CArtifact::ART_TREASURE && VLC->arth->legalArtifact(art->getId()) && art->constituentOf.empty())
+		if (art->aClass == CArtifact::ART_TREASURE && VLC->arth->legalArtifact(art->getId()) && art->getPartOf().empty())
 			questArtifacts.push_back(art->getId());
 	}
 }

lib/serializer/CSerializer.cpp

@@ -33,7 +33,7 @@ void CSerializer::addStdVecItems(CGameState *gs, LibClasses *lib)
 	registerVectoredType<CArtifact, ArtifactID>(&lib->arth->objects,
 		[](const CArtifact &art){ return art.getId(); });
 	registerVectoredType<CArtifactInstance, ArtifactInstanceID>(&gs->map->artInstances,
-		[](const CArtifactInstance &artInst){ return artInst.id; });
+		[](const CArtifactInstance &artInst){ return artInst.getId(); });
 	registerVectoredType<CQuest, si32>(&gs->map->quests,
 		[](const CQuest &q){ return q.qid; });
 

mapeditor/validator.cpp

@@ -140,7 +140,7 @@ std::list<Validator::Issue> Validator::validate(const CMap * map)
 				{
 					if(ins->storedArtifact)
 					{
-						if(!map->allowedSpell[ins->storedArtifact->id.getNum()])
+						if(!map->allowedSpell[ins->storedArtifact->getId().getNum()])
 							issues.emplace_back(QString("Spell scroll %1 is prohibited by map settings").arg(ins->getObjectName().c_str()), false);
 					}
 					else

server/CGameHandler.cpp

@@ -807,12 +807,13 @@ void CGameHandler::endBattleConfirm(const BattleInfo * battleInfo)
 		changePrimSkill(finishingBattle->winnerHero, PrimarySkill::EXPERIENCE, battleResult.data->exp[finishingBattle->winnerSide]);
 	
 	BattleResultAccepted raccepted;
-	raccepted.army1 = const_cast<CArmedInstance*>(bEndArmy1);
-	raccepted.army2 = const_cast<CArmedInstance*>(bEndArmy2);
-	raccepted.hero1 = const_cast<CGHeroInstance*>(battleInfo->sides.at(0).hero);
-	raccepted.hero2 = const_cast<CGHeroInstance*>(battleInfo->sides.at(1).hero);
-	raccepted.exp[0] = battleResult.data->exp[0];
-	raccepted.exp[1] = battleResult.data->exp[1];
+	raccepted.heroResult[0].army = const_cast<CArmedInstance*>(bEndArmy1);
+	raccepted.heroResult[1].army = const_cast<CArmedInstance*>(bEndArmy2);
+	raccepted.heroResult[0].hero = const_cast<CGHeroInstance*>(battleInfo->sides.at(0).hero);
+	raccepted.heroResult[1].hero = const_cast<CGHeroInstance*>(battleInfo->sides.at(1).hero);
+	raccepted.heroResult[0].exp = battleResult.data->exp[0];
+	raccepted.heroResult[1].exp = battleResult.data->exp[1];
+	raccepted.winnerSide = finishingBattle->winnerSide;
 	sendAndApply(&raccepted);
 
 	queries.popIfTop(battleQuery);
@@ -4083,7 +4084,7 @@ bool CGameHandler::assembleArtifacts (ObjectInstanceID heroID, ArtifactPosition
 	if(assemble)
 	{
 		CArtifact * combinedArt = VLC->arth->objects[assembleTo];
-		if(!combinedArt->constituents)
+		if(!combinedArt->isCombined())
 			COMPLAIN_RET("assembleArtifacts: Artifact being attempted to assemble is not a combined artifacts!");
 		if (!vstd::contains(ArtifactUtils::assemblyPossibilities(hero, destArtifact->getTypeId(),
 			ArtifactUtils::isSlotEquipment(artifactSlot)), combinedArt))
@@ -4102,11 +4103,11 @@ bool CGameHandler::assembleArtifacts (ObjectInstanceID heroID, ArtifactPosition
 	}
 	else
 	{
-		if(!destArtifact->canBeDisassembled())
+		if(!destArtifact->isCombined())
 			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))
+			&& !ArtifactUtils::isBackpackFreeSlots(hero, destArtifact->artType->getConstituents().size() - 1))
 			COMPLAIN_RET("assembleArtifacts: Artifact being attempted to disassemble but backpack is full!");
 
 		DisassembledArtifact da;
@@ -4159,9 +4160,9 @@ bool CGameHandler::buyArtifact(ObjectInstanceID hid, ArtifactID aid)
 	{
 		const CArtifact * art = aid.toArtifact();
 		COMPLAIN_RET_FALSE_IF(nullptr == art, "Invalid artifact index to buy");
-		COMPLAIN_RET_FALSE_IF(art->warMachine == CreatureID::NONE, "War machine artifact required");
+		COMPLAIN_RET_FALSE_IF(art->getWarMachine() == CreatureID::NONE, "War machine artifact required");
 		COMPLAIN_RET_FALSE_IF(hero->hasArt(aid),"Hero already has this machine!");
-		const int price = art->price;
+		const int price = art->getPrice();
 		COMPLAIN_RET_FALSE_IF(getPlayerState(hero->getOwner())->resources[EGameResID::GOLD] < price, "Not enough gold!");
 
 		if ((town->hasBuilt(BuildingID::BLACKSMITH) && town->town->warMachine == aid)
@@ -6808,17 +6809,12 @@ bool CGameHandler::giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact
 		COMPLAIN_RET_FALSE_IF(!artType->canBePutAt(h, pos, false), "Cannot put artifact in that slot!");
 	}
 
-	CArtifactInstance * newArtInst = nullptr;
-	if(artType->canBeDisassembled())
-		newArtInst = new CCombinedArtifactInstance();
-	else
-		newArtInst = new CArtifactInstance();
-
+	auto * newArtInst = new CArtifactInstance();
 	newArtInst->artType = artType; // *NOT* via settype -> all bonus-related stuff must be done by NewArtifact apply
 
 	NewArtifact na;
 	na.art = newArtInst;
-	sendAndApply(&na); // -> updates a!!!, will create a on other machines
+	sendAndApply(&na); // -> updates newArtInst!!!
 
 	if(giveHeroArtifact(h, newArtInst, pos))
 		return true;

test/entity/CArtifactTest.cpp

@@ -32,9 +32,7 @@ protected:
 
 TEST_F(CArtifactTest, RegistersIcons)
 {
-	subject->iconIndex = 4242;
-	subject->image = "Test1";
-	subject->large = "Test2";
+        subject-> setImage(4242, "Test1", "Test2");
 
 	auto cb = [this](auto && PH1, auto && PH2, auto && PH3, auto && PH4) 
 	{