Unified war machine mechanics.

* it is possible to define new war machines
* added warMachine field to artifact configuration

client/widgets/CArtifactHolder.cpp

@@ -161,16 +161,18 @@ void CHeroArtPlace::clickLeft(tribool down, bool previousState)
 				{
 					const CArtifact * const cur = ourOwner->commonInfo->src.art->artType;
 
-					switch(cur->id)
+					if(cur->id == ArtifactID::CATAPULT)
 					{
-					case ArtifactID::CATAPULT:
 						//should not happen, catapult cannot be selected
-						assert(cur->id != ArtifactID::CATAPULT);
-						break;
-					case ArtifactID::BALLISTA: case ArtifactID::AMMO_CART: case ArtifactID::FIRST_AID_TENT: //war machines cannot go to backpack
+						logGlobal->error("Attempt to move Catapult");
+					}
+					else if (cur->isBig())
+					{
+						//war machines cannot go to backpack
 						LOCPLINT->showInfoDialog(boost::str(boost::format(CGI->generaltexth->allTexts[153]) % cur->Name()));
-						break;
-					default:
+					}
+					else
+					{
 						setMeAsDest();
 						vstd::amin(ourOwner->commonInfo->dst.slotID, ArtifactPosition(
 							ourOwner->curHero->artifactsInBackpack.size() + GameConstants::BACKPACK_START));
@@ -184,7 +186,6 @@ void CHeroArtPlace::clickLeft(tribool down, bool previousState)
 							deselect();
 						else
 							ourOwner->realizeCurrentTransaction();
-						break;
 					}
 				}
 			}
@@ -366,7 +367,7 @@ bool CHeroArtPlace::fitsHere(const CArtifactInstance * art) const
 
 	// Anything but War Machines can be placed in backpack.
 	if (slotID >= GameConstants::BACKPACK_START)
-		return !CGI->arth->isBigArtifact(art->artType->id);
+		return !art->artType->isBig();
 
 	return art->canBePutAt(ArtifactLocation(ourOwner->curHero, slotID), true);
 }

client/windows/CCastleInterface.cpp

@@ -754,7 +754,8 @@ void CCastleBuildings::enterBlacksmith(ArtifactID artifactID)
 	}
 	int price = CGI->arth->artifacts[artifactID]->price;
 	bool possible = LOCPLINT->cb->getResourceAmount(Res::GOLD) >= price && !hero->hasArt(artifactID);
-	GH.pushInt(new CBlacksmithDialog(possible, CArtHandler::machineIDToCreature(artifactID), artifactID, hero->id));
+	CreatureID cre = artifactID.toArtifact()->warMachine;
+	GH.pushInt(new CBlacksmithDialog(possible, cre, artifactID, hero->id));
 }
 
 void CCastleBuildings::enterBuilding(BuildingID building)

client/windows/GUIClasses.cpp

@@ -205,7 +205,7 @@ void CRecruitmentWindow::buy()
 	CreatureID crid =  selected->creature->idNumber;
 	SlotID dstslot = dst-> getSlotFor(crid);
 
-	if(!dstslot.validSlot() && !vstd::contains(CGI->arth->bigArtifacts,CGI->arth->creatureToMachineID(crid))) //no available slot
+	if(!dstslot.validSlot() && (selected->creature->warMachine == ArtifactID::NONE)) //no available slot
 	{
 		std::string txt;
 		if(dst->ID == Obj::HERO)

config/artifacts.json

@@ -17,22 +17,26 @@
 	"catapult":
 	{
 		"index" : 3,
-		"type" : ["HERO"]
+		"type" : ["HERO"],
+		"warMachine":"catapult"
 	},
 	"ballista":
 	{
 		"index" : 4,
-		"type" : ["HERO"]
+		"type" : ["HERO"],
+		"warMachine":"ballista"
 	},
 	"ammoCart":
 	{
 		"index" : 5,
-		"type" : ["HERO"]
+		"type" : ["HERO"],
+		"warMachine":"ammoCart"
 	},
 	"firstAidTent":
 	{
 		"index" : 6,
-		"type" : ["HERO"]
+		"type" : ["HERO"],
+		"warMachine":"firstAidTent"
 	},
 	"centaurAxe":
 	{
@@ -1333,7 +1337,7 @@
 				"subtype" : 1,
 				"val" : 0,
 				"valueType" : "BASE_NUMBER"
-			}			
+			}
 		],
 		"index" : 93,
 		"type" : ["HERO"]

config/schemas/artifact.json

@@ -4,7 +4,7 @@
 	"title" : "VCMI artifact format",
 	"description" : "Format used to define new artifacts in VCMI",
 	"required" : [ "class", "text", "type", "value" ],
-	
+
 	"definitions" : {
 		"growingBonusList" : {
 			"type" : "array",
@@ -117,6 +117,12 @@
 		"value": {
 			"type":"number",
 			"description": "Cost of this artifact, in gold"
+		},
+		"warMachine":
+		{
+			"type":"string",
+			"description": "Creature id to use on battle field. If set, this artifact is war machine"
 		}
+
 	}
 }

lib/BattleInfo.cpp

@@ -432,22 +432,36 @@ BattleInfo * BattleInfo::setupBattle( int3 tile, ETerrainType terrain, BFieldTyp
 	if(!creatureBank)
 	{
 		//Checks if hero has artifact and create appropriate stack
-		auto handleWarMachine= [&](int side, ArtifactPosition artslot, CreatureID cretype, BattleHex hex)
+		auto handleWarMachine= [&](int side, ArtifactPosition artslot, BattleHex hex)
 		{
-			if(heroes[side] && heroes[side]->getArt(artslot))
-				stacks.push_back(curB->generateNewStack(CStackBasicDescriptor(cretype, 1), !side, SlotID::WAR_MACHINES_SLOT, hex));
+			const CArtifactInstance * warMachineArt = heroes[side]->getArt(artslot);
+
+			if(nullptr != warMachineArt)
+			{
+				CreatureID cre = warMachineArt->artType->warMachine;
+
+				if(cre != CreatureID::NONE)
+					stacks.push_back(curB->generateNewStack(CStackBasicDescriptor(cre, 1), !side, SlotID::WAR_MACHINES_SLOT, hex));
+			}
 		};
 
-		handleWarMachine(0, ArtifactPosition::MACH1, CreatureID::BALLISTA, 52);
-		handleWarMachine(0, ArtifactPosition::MACH2, CreatureID::AMMO_CART, 18);
-		handleWarMachine(0, ArtifactPosition::MACH3, CreatureID::FIRST_AID_TENT, 154);
-		if(town && town->hasFort())
-			handleWarMachine(0, ArtifactPosition::MACH4, CreatureID::CATAPULT, 120);
+		if(heroes[0])
+		{
 
-		if(!town) //defending hero shouldn't receive ballista (bug #551)
-			handleWarMachine(1, ArtifactPosition::MACH1, CreatureID::BALLISTA, 66);
-		handleWarMachine(1, ArtifactPosition::MACH2, CreatureID::AMMO_CART, 32);
-		handleWarMachine(1, ArtifactPosition::MACH3, CreatureID::FIRST_AID_TENT, 168);
+			handleWarMachine(0, ArtifactPosition::MACH1, 52);
+			handleWarMachine(0, ArtifactPosition::MACH2, 18);
+			handleWarMachine(0, ArtifactPosition::MACH3, 154);
+			if(town && town->hasFort())
+				handleWarMachine(0, ArtifactPosition::MACH4, 120);
+		}
+
+		if(heroes[1])
+		{
+			if(!town) //defending hero shouldn't receive ballista (bug #551)
+				handleWarMachine(1, ArtifactPosition::MACH1, 66);
+			handleWarMachine(1, ArtifactPosition::MACH2, 32);
+			handleWarMachine(1, ArtifactPosition::MACH3, 168);
+		}
 	}
 	//war machines added
 

lib/CArtHandler.cpp

@@ -15,6 +15,7 @@
 #include "CGeneralTextHandler.h"
 #include "VCMI_Lib.h"
 #include "CModHandler.h"
+#include "CCreatureHandler.h"
 #include "spells/CSpellHandler.h"
 #include "mapObjects/MapObjects.h"
 #include "NetPacksBase.h"
@@ -61,14 +62,21 @@ const std::string & CArtifact::EventText() const
 	return eventText;
 }
 
-bool CArtifact::isBig () const
+bool CArtifact::isBig() const
 {
-	return VLC->arth->isBigArtifact(id);
+	return warMachine != CreatureID::NONE;
 }
 
-bool CArtifact::isTradable () const
+bool CArtifact::isTradable() const
 {
-	return VLC->arth->isTradableArtifact(id);
+	switch(id)
+	{
+	case ArtifactID::SPELLBOOK:
+	case ArtifactID::GRAIL:
+		return false;
+	default:
+		return !isBig();
+	}
 }
 
 CArtifact::CArtifact()
@@ -120,6 +128,26 @@ void CArtifact::addNewBonus(const std::shared_ptr<Bonus>& b)
 	CBonusSystemNode::addNewBonus(b);
 }
 
+void CArtifact::fillWarMachine()
+{
+	switch (id)
+	{
+	case ArtifactID::CATAPULT:
+		warMachine = CreatureID::CATAPULT;
+		break;
+	case ArtifactID::BALLISTA:
+		warMachine = CreatureID::BALLISTA;
+		break;
+	case ArtifactID::FIRST_AID_TENT:
+		warMachine = CreatureID::FIRST_AID_TENT;
+		break;
+	case ArtifactID::AMMO_CART:
+		warMachine = CreatureID::AMMO_CART;
+		break;
+	}
+	warMachine = CreatureID::NONE; //this artifact is not a creature
+}
+
 void CGrowingArtifact::levelUpArtifact (CArtifactInstance * art)
 {
 	auto b = std::make_shared<Bonus>();
@@ -146,11 +174,6 @@ void CGrowingArtifact::levelUpArtifact (CArtifactInstance * art)
 
 CArtHandler::CArtHandler()
 {
-	//VLC->arth = this;
-
-	// War machines are the default big artifacts.
-	for (ArtifactID i = ArtifactID::CATAPULT; i <= ArtifactID::FIRST_AID_TENT; i.advance(1))
-		bigArtifacts.insert(i);
 }
 
 CArtHandler::~CArtHandler()
@@ -310,6 +333,19 @@ CArtifact * CArtHandler::loadFromJson(const JsonNode & node, const std::string &
 		auto bonus = JsonUtils::parseBonus(b);
 		art->addNewBonus(bonus);
 	}
+
+	const JsonNode & warMachine = node["warMachine"];
+	if(warMachine.getType() == JsonNode::DATA_STRING && warMachine.String() != "")
+	{
+		VLC->modh->identifiers.requestIdentifier("creature", warMachine, [=](si32 id)
+		{
+			art->warMachine = CreatureID(id);
+
+			//this assumes that creature object is stored before registration
+			VLC->creh->creatures.at(id)->warMachine = art->id;
+		});
+	}
+
 	return art;
 }
 
@@ -453,47 +489,6 @@ void CArtHandler::loadGrowingArt(CGrowingArtifact * art, const JsonNode & node)
 	}
 }
 
-//TODO: use bimap
-ArtifactID CArtHandler::creatureToMachineID(CreatureID id)
-{
-	switch (id)
-	{
-	case CreatureID::CATAPULT: //Catapult
-		return ArtifactID::CATAPULT;
-		break;
-	case CreatureID::BALLISTA: //Ballista
-		return ArtifactID::BALLISTA;
-		break;
-	case CreatureID::FIRST_AID_TENT: //First Aid tent
-		return ArtifactID::FIRST_AID_TENT;
-		break;
-	case CreatureID::AMMO_CART: //Ammo cart
-		return ArtifactID::AMMO_CART;
-		break;
-	}
-	return ArtifactID::NONE; //this creature is not artifact
-}
-
-CreatureID CArtHandler::machineIDToCreature(ArtifactID id)
-{
-	switch (id)
-	{
-	case ArtifactID::CATAPULT:
-		return CreatureID::CATAPULT;
-		break;
-	case ArtifactID::BALLISTA:
-		return CreatureID::BALLISTA;
-		break;
-	case ArtifactID::FIRST_AID_TENT:
-		return CreatureID::FIRST_AID_TENT;
-		break;
-	case ArtifactID::AMMO_CART:
-		return CreatureID::AMMO_CART;
-		break;
-	}
-	return CreatureID::NONE; //this artifact is not a creature
-}
-
 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)
@@ -635,22 +630,6 @@ bool CArtHandler::legalArtifact(ArtifactID id)
 		art->aClass <= CArtifact::ART_RELIC);
 }
 
-bool CArtHandler::isTradableArtifact(ArtifactID id) const
-{
-	switch (id)
-	{
-	case ArtifactID::SPELLBOOK:
-	case ArtifactID::GRAIL:
-	case ArtifactID::CATAPULT:
-	case ArtifactID::BALLISTA:
-	case ArtifactID::AMMO_CART:
-	case ArtifactID::FIRST_AID_TENT:
-		return false;
-	default:
-		return true;
-	}
-}
-
 void CArtHandler::initAllowedArtifactsList(const std::vector<bool> &allowed)
 {
 	allowedArtifacts.clear();

lib/CArtHandler.h

@@ -61,6 +61,7 @@ public:
 	std::vector<CArtifact *> constituentOf; // Reverse map of constituents - combined arts that include this art
 	EartClass aClass;
 	ArtifactID id;
+	CreatureID warMachine;
 
 	const std::string &Name() const; //getter
 	const std::string &Description() const; //getter
@@ -84,12 +85,23 @@ public:
 		{
 			h & identifier;
 		}
+
+		if(version >= 771)
+		{
+			h & warMachine;
+		}
+		else if(!h.saving)
+		{
+			fillWarMachine();
+		}
 	}
 
 	CArtifact();
 	~CArtifact();
 
 	friend class CArtHandler;
+private:
+	void fillWarMachine();
 };
 
 class DLL_LINKAGE CGrowingArtifact : public CArtifact //for example commander artifacts getting bonuses after battle
@@ -213,7 +225,6 @@ public:
 
 	std::vector< ConstTransitivePtr<CArtifact> > artifacts;
 	std::vector<CArtifact *> allowedArtifacts;
-	std::set<ArtifactID> bigArtifacts; // Artifacts that cannot be moved to backpack, e.g. war machines.
 	std::set<ArtifactID> growingArtifacts;
 
 	void addBonuses(CArtifact *art, const JsonNode &bonusList);
@@ -231,13 +242,7 @@ public:
 	ArtifactID pickRandomArtifact(CRandomGenerator & rand, int flags, std::function<bool(ArtifactID)> accepts);
 
 	bool legalArtifact(ArtifactID id);
-	//void getAllowedArts(std::vector<ConstTransitivePtr<CArtifact> > &out, std::vector<CArtifact*> *arts, int flag);
-	//void getAllowed(std::vector<ConstTransitivePtr<CArtifact> > &out, int flags);
-	bool isBigArtifact (ArtifactID artID) const {return bigArtifacts.find(artID) != bigArtifacts.end();}
-	bool isTradableArtifact (ArtifactID id) const;
 	void initAllowedArtifactsList(const std::vector<bool> &allowed); //allowed[art_id] -> 0 if not allowed, 1 if allowed
-	static ArtifactID creatureToMachineID(CreatureID id);
-	static CreatureID machineIDToCreature(ArtifactID id);
 	void makeItCreatureArt (CArtifact * a, bool onlyCreature = true);
 	void makeItCreatureArt (ArtifactID aid, bool onlyCreature = true);
 	void makeItCommanderArt (CArtifact * a, bool onlyCommander = true);
@@ -264,7 +269,6 @@ public:
 	{
 		h & artifacts & allowedArtifacts & treasures & minors & majors & relics
 			& growingArtifacts;
-		//if(!h.saving) sortArts();
 	}
 
 private:

lib/CCreatureHandler.cpp

@@ -152,6 +152,26 @@ void CCreature::setId(CreatureID ID)
 	CBonusSystemNode::treeHasChanged();
 }
 
+void CCreature::fillWarMachine()
+{
+	switch (idNumber)
+	{
+	case CreatureID::CATAPULT: //Catapult
+		warMachine = ArtifactID::CATAPULT;
+		break;
+	case CreatureID::BALLISTA: //Ballista
+		warMachine = ArtifactID::BALLISTA;
+		break;
+	case CreatureID::FIRST_AID_TENT: //First Aid tent
+		warMachine = ArtifactID::FIRST_AID_TENT;
+		break;
+	case CreatureID::AMMO_CART: //Ammo cart
+		warMachine = ArtifactID::AMMO_CART;
+		break;
+	}
+	warMachine = ArtifactID::NONE; //this creature is not artifact
+}
+
 static void AddAbility(CCreature *cre, const JsonVector &ability_vec)
 {
 	auto nsf = std::make_shared<Bonus>();

lib/CCreatureHandler.h

@@ -98,6 +98,8 @@ public:
 		}
 	} sounds;
 
+	ArtifactID warMachine;
+
 	bool isItNativeTerrain(int terrain) const;
 	bool isDoubleWide() const; //returns true if unit is double wide on battlefield
 	bool isFlying() const; //returns true if it is a flying unit
@@ -142,9 +144,20 @@ public:
 		{
 			h & identifier;
 		}
+		if(version >= 771)
+		{
+			h & warMachine;
+		}
+		else if(!h.saving)
+		{
+			fillWarMachine();
+		}
 	}
 
 	CCreature();
+
+private:
+	void fillWarMachine();
 };
 
 class DLL_LINKAGE CCreatureHandler : public IHandlerBase

lib/CTownHandler.cpp

@@ -584,7 +584,7 @@ void CTownHandler::loadTown(CTown &town, const JsonNode & source)
 	VLC->modh->identifiers.requestIdentifier("creature", source["warMachine"],
 	[&town](si32 creature)
 	{
-		town.warMachine = CArtHandler::creatureToMachineID(CreatureID(creature));
+		town.warMachine = CreatureID(creature).toCreature()->warMachine;
 	});
 
 	town.moatDamage = source["moatDamage"].Float();

lib/mapObjects/CGHeroInstance.cpp

@@ -352,34 +352,43 @@ void CGHeroInstance::initArmy(CRandomGenerator & rand, IArmyDescriptor *dst /*=
 
 		int count = rand.nextInt(stack.minAmount, stack.maxAmount);
 
-		if(stack.creature >= CreatureID::CATAPULT &&
-		   stack.creature <= CreatureID::ARROW_TOWERS) //war machine
+		const CCreature * creature = stack.creature.toCreature();
+
+		if(creature == nullptr)
+		{
+			logGlobal->error("Hero %s has invalid creature with id %d in initial army", name, stack.creature.toEnum());
+			continue;
+		}
+
+		if(creature->warMachine != ArtifactID::NONE) //war machine
 		{
 			warMachinesGiven++;
 			if(dst != this)
 				continue;
 
 			int slot = -1;
-			ArtifactID aid = ArtifactID::NONE;
-			switch (stack.creature)
+			ArtifactID aid = creature->warMachine;
+			const CArtifact * art = aid.toArtifact();
+
+			if(art != nullptr && !art->possibleSlots.at(ArtBearer::HERO).empty())
 			{
-			case CreatureID::CATAPULT:
-				slot = ArtifactPosition::MACH4;
-				aid = ArtifactID::CATAPULT;
-				break;
-			default:
-				aid = CArtHandler::creatureToMachineID(stack.creature);
-				slot = 9 + aid;
-				break;
+				//TODO: should we try another possible slots?
+				ArtifactPosition slot = art->possibleSlots.at(ArtBearer::HERO).front();
+
+				if(!getArt(slot))
+					putArtifact(slot, CArtifactInstance::createNewArtifactInstance(aid));
+				else
+					logGlobal->warnStream() << "Hero " << name << " already has artifact at " << slot << ", omitting giving " << aid;
 			}
-			auto convSlot = ArtifactPosition(slot);
-			if(!getArt(convSlot))
-				putArtifact(convSlot, CArtifactInstance::createNewArtifactInstance(aid));
 			else
-				logGlobal->warnStream() << "Hero " << name << " already has artifact at " << slot << ", omitting giving " << aid;
+			{
+				logGlobal->error("Hero %s has invalid war machine in initial army", name);
+			}
 		}
 		else
+		{
 			dst->setCreature(SlotID(stackNo-warMachinesGiven), stack.creature, count);
+		}
 	}
 }
 

lib/mapping/MapFormatH3M.cpp

@@ -872,9 +872,17 @@ bool CMapLoaderH3M::loadArtifactToSlot(CGHeroInstance * hero, int slot)
 	bool isArt  =  aid != artmask;
 	if(isArt)
 	{
-		if(vstd::contains(VLC->arth->bigArtifacts, aid) && slot >= GameConstants::BACKPACK_START)
+		const CArtifact * art = ArtifactID(aid).toArtifact();
+
+		if(nullptr == art)
+		{
+			logGlobal->warnStream() << "Invalid artifact in hero's backpack, ignoring...";
+			return false;
+		}
+
+		if(art->isBig() && slot >= GameConstants::BACKPACK_START)
 		{
-			logGlobal->warnStream() << "Warning: A big artifact (war machine) in hero's backpack, ignoring...";
+			logGlobal->warnStream() << "A big artifact (war machine) in hero's backpack, ignoring...";
 			return false;
 		}
 		if(aid == 0 && slot == ArtifactPosition::MISC5)

lib/serializer/CSerializer.h

@@ -14,7 +14,7 @@
 #include "../ConstTransitivePtr.h"
 #include "../GameConstants.h"
 
-const ui32 SERIALIZATION_VERSION = 770;
+const ui32 SERIALIZATION_VERSION = 771;
 const ui32 MINIMAL_SERIALIZATION_VERSION = 753;
 const std::string SAVEGAME_MAGIC = "VCMISVG";
 

server/CGameHandler.cpp

@@ -3135,7 +3135,7 @@ bool CGameHandler::recruitCreatures(ObjectInstanceID objid, ObjectInstanceID dst
 	const CGDwelling * dw = static_cast<const CGDwelling *>(getObj(objid));
 	const CArmedInstance *dst = nullptr;
 	const CCreature *c = VLC->creh->creatures.at(crid);
-	bool warMachine = c->hasBonusOfType(Bonus::SIEGE_WEAPON);
+	const bool warMachine = c->warMachine != ArtifactID::NONE;
 
 	//TODO: test for owning
 	//TODO: check if dst can recruit objects (e.g. hero is actually visiting object, town and source are same, etc)
@@ -3186,24 +3186,18 @@ bool CGameHandler::recruitCreatures(ObjectInstanceID objid, ObjectInstanceID dst
 	if (warMachine)
 	{
 		const CGHeroInstance *h = dynamic_cast<const CGHeroInstance*>(dst);
-		if (!h)
-			COMPLAIN_RET("Only hero can buy war machines");
 
-		switch(crid)
-		{
-		case CreatureID::BALLISTA:
-			giveHeroNewArtifact(h, VLC->arth->artifacts[ArtifactID::BALLISTA], ArtifactPosition::MACH1);
-			break;
-		case CreatureID::FIRST_AID_TENT:
-			giveHeroNewArtifact(h, VLC->arth->artifacts[ArtifactID::FIRST_AID_TENT], ArtifactPosition::MACH3);
-			break;
-		case CreatureID::AMMO_CART:
-			giveHeroNewArtifact(h, VLC->arth->artifacts[ArtifactID::AMMO_CART], ArtifactPosition::MACH2);
-			break;
-		default:
-			complain("This war machine cannot be recruited!");
-			return false;
-		}
+		COMPLAIN_RET_FALSE_IF(!h, "Only hero can buy war machines");
+
+		ArtifactID artId = c->warMachine;
+
+		COMPLAIN_RET_FALSE_IF(artId == ArtifactID::CATAPULT, "Catapult cannot be recruited!");
+
+		const CArtifact * art = artId.toArtifact();
+
+		COMPLAIN_RET_FALSE_IF(nullptr == art, "Invalid war machine artifact");
+
+		return giveHeroNewArtifact(h, art);
 	}
 	else
 	{
@@ -3444,7 +3438,10 @@ bool CGameHandler::assembleArtifacts (ObjectInstanceID heroID, ArtifactPosition
 bool CGameHandler::buyArtifact(ObjectInstanceID hid, ArtifactID aid)
 {
 	const CGHeroInstance * hero = getHero(hid);
+	COMPLAIN_RET_FALSE_IF(nullptr == hero, "Invalid hero index");
 	const CGTownInstance * town = hero->visitedTown;
+	COMPLAIN_RET_FALSE_IF(nullptr == town, "Hero not in town");
+
 	if (aid==ArtifactID::SPELLBOOK)
 	{
 		if ((!town->hasBuilt(BuildingID::MAGES_GUILD_1) && complain("Cannot buy a spellbook, no mage guild in the town!"))
@@ -3459,26 +3456,24 @@ bool CGameHandler::buyArtifact(ObjectInstanceID hid, ArtifactID aid)
 		giveSpells(town,hero);
 		return true;
 	}
-	else if (aid < 7  &&  aid > 3) //war machine
+	else
 	{
-		int price = VLC->arth->artifacts[aid]->price;
+		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(hero->hasArt(aid),"Hero already has this machine!");
+		const int price = art->price;
+		COMPLAIN_RET_FALSE_IF(getPlayer(hero->getOwner())->resources.at(Res::GOLD) < price, "Not enough gold!");
 
-		if ((hero->getArt(ArtifactPosition(9+aid)) && complain("Hero already has this machine!"))
-		 || (getPlayer(hero->getOwner())->resources.at(Res::GOLD) < price && complain("Not enough gold!")))
-		{
-			return false;
-		}
 		if  ((town->hasBuilt(BuildingID::BLACKSMITH) && town->town->warMachine == aid)
 		 || ((town->hasBuilt(BuildingID::BALLISTA_YARD, ETownType::STRONGHOLD)) && aid == ArtifactID::BALLISTA))
 		{
 			giveResource(hero->getOwner(),Res::GOLD,-price);
-			giveHeroNewArtifact(hero, VLC->arth->artifacts[aid], ArtifactPosition(9+aid));
-			return true;
+			return giveHeroNewArtifact(hero, art);
 		}
 		else
 			COMPLAIN_RET("This machine is unavailable here!");
 	}
-	return false;
 }
 
 bool CGameHandler::buyArtifact(const IMarket *m, const CGHeroInstance *h, Res::ERes rid, ArtifactID aid)
@@ -6053,6 +6048,18 @@ void CGameHandler::putArtifact(const ArtifactLocation &al, const CArtifactInstan
 	sendAndApply(&pa);
 }
 
+bool CGameHandler::giveHeroNewArtifact(const CGHeroInstance* h, const CArtifact* art)
+{
+	COMPLAIN_RET_FALSE_IF(art->possibleSlots.at(ArtBearer::HERO).empty(),"Not a hero artifact!");
+
+	ArtifactPosition slot = art->possibleSlots.at(ArtBearer::HERO).front();
+
+	COMPLAIN_RET_FALSE_IF(nullptr != h->getArt(slot, false), "Hero already has artifact in slot");
+
+	giveHeroNewArtifact(h, art, slot);
+	return true;
+}
+
 void CGameHandler::giveHeroNewArtifact(const CGHeroInstance *h, const CArtifact *artType, ArtifactPosition pos)
 {
 	CArtifactInstance *a = nullptr;
@@ -6415,10 +6422,12 @@ CasualtiesAfterBattle::CasualtiesAfterBattle(const CArmedInstance * _army, Battl
 		}
 		else if (st->slot == SlotID::WAR_MACHINES_SLOT)
 		{
-			auto warMachine = VLC->arth->creatureToMachineID(st->type->idNumber);
+			auto warMachine = st->type->warMachine;
 
 			if (warMachine == ArtifactID::NONE)
+			{
 				logGlobal->error("Invalid creature in war machine virtual slot. Stack: %s", st->nodeName());
+			}
 			//catapult artifact remain even if "creature" killed in siege
 			else if (warMachine != ArtifactID::CATAPULT && !st->count)
 			{

server/CGameHandler.h

@@ -143,6 +143,7 @@ public:
 
 	void removeAfterVisit(const CGObjectInstance *object) override;
 
+	bool giveHeroNewArtifact(const CGHeroInstance *h, const CArtifact *art);
 	void giveHeroNewArtifact(const CGHeroInstance *h, const CArtifact *artType, ArtifactPosition pos) override;
 	void giveHeroArtifact(const CGHeroInstance *h, const CArtifactInstance *a, ArtifactPosition pos) override;
 	void putArtifact(const ArtifactLocation &al, const CArtifactInstance *a) override;