* Fixed bug 101, placing artifacts in specific slots in the backpack. Cleaned up swapArtifacts() to use more descriptive names as well.

client/GUIClasses.cpp

@@ -3523,8 +3523,8 @@ void CArtPlace::clickLeft(tribool down, bool previousState)
 		}
 		else //perform artifact substitution
 		{
-			if(slotID >= 19)	//we are an backpack slot - remove active artifact and put it to the last free pos in backpack
-			{					//TODO: putting artifacts in the middle of backpack (pushing following arts)
+			if (slotID >= 19) // Backpack slot - Remove active artifact and insert it into the designated position in backpack.
+			{
 				const CArtifact *cur = ourOwner->commonInfo->activeArtPlace->ourArt;
 				assert(cur); //there is highlighted slot, it must contain an art
 				switch(cur->id)
@@ -3544,21 +3544,21 @@ void CArtPlace::clickLeft(tribool down, bool previousState)
 						ourOwner->commonInfo->activeArtPlace->ourOwner->curHero,
 						ourOwner->commonInfo->activeArtPlace->slotID,
 						ourOwner->curHero,
-						ourOwner->curHero->artifacts.size() + 19);
+						slotID);
 					break;
 				}
 			}
 			//check if swap is possible
 			else if(this->fitsHere(ourOwner->commonInfo->activeArtPlace->ourArt) && ourOwner->commonInfo->activeArtPlace->fitsHere(this->ourArt))
 			{
-				int destSlot = slotID,
-					srcSlot = ourOwner->commonInfo->activeArtPlace->slotID;
+				int srcSlot = ourOwner->commonInfo->activeArtPlace->slotID;
+				int destSlot = slotID;
 
 				LOCPLINT->cb->swapArtifacts(
-					ourOwner->curHero,
-					destSlot,
 					ourOwner->commonInfo->activeArtPlace->ourOwner->curHero,
-					srcSlot);
+					srcSlot,
+					ourOwner->curHero,
+					destSlot);
 
 				ourOwner->commonInfo->activeArtPlace->clicked = false;
 				ourOwner->commonInfo->activeArtPlace = NULL;

lib/NetPacksLib.cpp

@@ -451,17 +451,18 @@ DLL_EXPORT void SetHeroArtifacts::setArtAtPos(ui16 pos, int art)
 		if(pos<19)
 			artifWorn.erase(pos);
 		else
-			artifacts -= artifacts[pos-19];
+			artifacts.erase(artifacts.begin() + (pos - 19));
 	}
 	else
 	{
-		if(pos<19)
+		if (pos < 19) {
 			artifWorn[pos] = art;
-		else
-			if(pos-19 < artifacts.size())
-				artifacts[pos-19] = art;
+		} else { // Goes into the backpack.
+			if(pos - 19 < artifacts.size())
+				artifacts.insert(artifacts.begin() + (pos - 19), art);
 			else
 				artifacts.push_back(art);
+		}
 	}
 }
 

server/CGameHandler.cpp

@@ -2298,38 +2298,58 @@ bool CGameHandler::garrisonSwap( si32 tid )
 	}
 }
 
-bool CGameHandler::swapArtifacts( si32 hid1, si32 hid2, ui16 slot1, ui16 slot2 )
+bool CGameHandler::swapArtifacts(si32 srcHeroID, si32 destHeroID, ui16 srcSlot, ui16 destSlot)
 {
-	CGHeroInstance *h1 = gs->getHero(hid1), *h2 = gs->getHero(hid2);
-	if((distance(h1->pos,h2->pos) > 1.5)   ||   (h1->tempOwner != h2->tempOwner))
+	CGHeroInstance *srcHero = gs->getHero(srcHeroID);
+	CGHeroInstance *destHero = gs->getHero(destHeroID);
+
+	// Make sure exchange is even possible between the two heroes.
+	if ((distance(srcHero->pos,destHero->pos) > 1.5 )|| (srcHero->tempOwner != destHero->tempOwner))
 		return false;
 
-	const CArtifact *a1 = h1->getArt(slot1), 
-		*a2=h2->getArt(slot2);
+	const CArtifact *srcArtifact = srcHero->getArt(srcSlot); 
+	const CArtifact *destArtifact = destHero->getArt(destSlot);
 
-	//check if 
-	//	1) slots are appropriate for that artifacts 
-	//	2) they are not war machine
-	if((a1 && slot2<19 && !vstd::contains(a1->possibleSlots,slot2) || (a2 && slot1<19 && !vstd::contains(a2->possibleSlots,slot1))) && complain("Cannot swap artifacts!")
-		|| (slot1>=13 && slot1<=16 || slot2>=13 && slot2<=16) && complain("Cannot move war machine!")
-	)
+	// Check if src/dest slots are appropriate for the artifacts exchanged.
+	// Moving to the backpack is always allowed.
+	if ((!srcArtifact || destSlot < 19)
+		&& (((srcArtifact && !vstd::contains(srcArtifact->possibleSlots, destSlot))
+			|| (destArtifact && srcSlot < 19 && !vstd::contains(destArtifact->possibleSlots, srcSlot)))))
 	{
+		complain("Cannot swap artifacts!");
+		return false;
+	}
+
+	// Make sure the artifacts are not war machines.
+	if ((srcSlot>=13 && srcSlot<=16) || (destSlot>=13 && destSlot<=16)) {
+		complain("Cannot move war machine!");
 		return false;
 	}
 
+	// Perform the exchange.
 	SetHeroArtifacts sha;
-	sha.hid = hid1;
-	sha.artifacts = h1->artifacts;
-	sha.artifWorn = h1->artifWorn;
-	sha.setArtAtPos(slot1,h2->getArtAtPos(slot2));
-	if(h1 == h2) sha.setArtAtPos(slot2,h1->getArtAtPos(slot1));
+	sha.hid = srcHeroID;
+	sha.artifacts = srcHero->artifacts;
+	sha.artifWorn = srcHero->artifWorn;
+
+	sha.setArtAtPos(srcSlot, -1);
+	if (destSlot < 19 && (destArtifact || srcSlot < 19))
+		sha.setArtAtPos(srcSlot, destHero->getArtAtPos(destSlot));
+
+	// Correction for destination from removing source artifact in backpack.
+	if (srcSlot >= 19 && destSlot >= 19 && srcSlot < destSlot)
+		destSlot--;
+
+	// Internal hero artifact arrangement.
+	if(srcHero == destHero)
+		sha.setArtAtPos(destSlot, srcHero->getArtAtPos(srcSlot));
 	sendAndApply(&sha);
-	if(hid1 != hid2)
-	{
-		sha.hid = hid2;
-		sha.artifacts = h2->artifacts;
-		sha.artifWorn = h2->artifWorn;
-		sha.setArtAtPos(slot2, a1 ? a1->id : -1);
+	if (srcHeroID != destHeroID) {
+		// Exchange between two different heroes.
+		sha.hid = destHeroID;
+		sha.artifacts = destHero->artifacts;
+		sha.artifWorn = destHero->artifWorn;
+		sha.setArtAtPos(destSlot, srcArtifact ? srcArtifact->id : -1);
 		sendAndApply(&sha);
 	}
 

server/CGameHandler.h

@@ -154,7 +154,7 @@ public:
 	bool setFormation( si32 hid, ui8 formation );
 	bool tradeResources( ui32 val, ui8 player, ui32 id1, ui32 id2 );
 	bool buyArtifact( ui32 hid, si32 aid );
-	bool swapArtifacts( si32 hid1, si32 hid2, ui16 slot1, ui16 slot2 );
+	bool swapArtifacts(si32 srcHeroID, si32 destHeroID, ui16 srcSlot, ui16 destSlot);
 	bool garrisonSwap(si32 tid);
 	bool upgradeCreature( ui32 objid, ui8 pos, ui32 upgID );
 	bool recruitCreatures(si32 objid, ui32 crid, ui32 cram);