Correctly sacrifice many stacks or many atrifacts, fixes #2607

CCallback.cpp

@@ -218,9 +218,21 @@ void CCallback::trade(const CGObjectInstance *market, EMarketMode::EMarketMode m
 	pack.market = market;
 	pack.hero = hero;
 	pack.mode = mode;
-	pack.r1 = id1;
-	pack.r2 = id2;
-	pack.val = val1;
+	pack.r1 = std::vector<ui32>{id1};
+	pack.r2 = std::vector<ui32>{id2};
+	pack.val = std::vector<ui32>{val1};
+	sendRequest(&pack);
+}
+
+void CCallback::trade(const CGObjectInstance *market, EMarketMode::EMarketMode mode, std::vector<int> id1, std::vector<int> id2, std::vector<int> val1, const CGHeroInstance *hero)
+{
+	TradeOnMarketplace pack;
+	pack.market = market;
+	pack.hero = hero;
+	pack.mode = mode;
+	pack.r1 = std::vector<ui32>(id1.begin(), id1.end());
+	pack.r2 = std::vector<ui32>(id2.begin(), id2.end());
+	pack.val = std::vector<ui32>(val1.begin(), val1.end());
 	sendRequest(&pack);
 }
 

CCallback.h

@@ -57,6 +57,7 @@ public:
 	virtual void swapGarrisonHero(const CGTownInstance *town)=0;
 
 	virtual void trade(const CGObjectInstance *market, EMarketMode::EMarketMode mode, int id1, int id2, int val1, const CGHeroInstance *hero = nullptr)=0; //mode==0: sell val1 units of id1 resource for id2 resiurce
+	virtual void trade(const CGObjectInstance *market, EMarketMode::EMarketMode mode, std::vector<int> id1, std::vector<int> id2, std::vector<int> val1, const CGHeroInstance *hero = nullptr)=0;
 
 	virtual int selectionMade(int selection, QueryID queryID) =0;
 	virtual int sendQueryReply(const JsonNode & reply, QueryID queryID) =0;
@@ -139,6 +140,7 @@ public:
 	void swapGarrisonHero(const CGTownInstance *town) override;
 	void buyArtifact(const CGHeroInstance *hero, ArtifactID aid) override;
 	void trade(const CGObjectInstance *market, EMarketMode::EMarketMode mode, int id1, int id2, int val1, const CGHeroInstance *hero = nullptr) override;
+	void trade(const CGObjectInstance *market, EMarketMode::EMarketMode mode, std::vector<int> id1, std::vector<int> id2, std::vector<int> val1, const CGHeroInstance *hero = nullptr) override;
 	void setFormation(const CGHeroInstance * hero, bool tight) override;
 	void recruitHero(const CGObjectInstance *townOrTavern, const CGHeroInstance *hero) override;
 	void save(const std::string &fname) override;

client/windows/CTradeWindow.cpp

@@ -1231,13 +1231,20 @@ void CAltarWindow::makeDeal()
 		blockTrade();
 		slider->moveTo(0);
 
-		std::vector<int> toSacrifice = sacrificedUnits;
-		for (int i = 0; i < toSacrifice.size(); i++)
+		std::vector<int> ids;
+		std::vector<int> toSacrifice;
+
+		for (int i = 0; i < sacrificedUnits.size(); i++)
 		{
-			if(toSacrifice[i])
-				LOCPLINT->cb->trade(market->o, mode, i, 0, toSacrifice[i], hero);
+			if(sacrificedUnits[i])
+			{
+				ids.push_back(i);
+				toSacrifice.push_back(sacrificedUnits[i]);
+			}
 		}
 
+		LOCPLINT->cb->trade(market->o, mode, ids, {}, toSacrifice, hero);
+
 		for(int& val : sacrificedUnits)
 			val = 0;
 
@@ -1249,10 +1256,13 @@ void CAltarWindow::makeDeal()
 	}
 	else
 	{
+		std::vector<int> positions;
 		for(const CArtifactInstance *art : arts->artifactsOnAltar) //sacrifice each artifact on the list
 		{
-			LOCPLINT->cb->trade(market->o, mode, hero->getArtPos(art), -1, 1, hero);
+			positions.push_back(hero->getArtPos(art));
 		}
+
+		LOCPLINT->cb->trade(market->o, mode, positions, {}, {}, hero);
 		arts->artifactsOnAltar.clear();
 
 		for(CTradeableItem *t : items[0])

lib/NetPacks.h

@@ -2196,14 +2196,14 @@ struct BuyArtifact : public CPackForServer
 struct TradeOnMarketplace : public CPackForServer
 {
 	TradeOnMarketplace()
-		:market(nullptr), hero(nullptr), mode(EMarketMode::RESOURCE_RESOURCE), r1(0), r2(0), val(0)
+		:market(nullptr), hero(nullptr), mode(EMarketMode::RESOURCE_RESOURCE)
 	{};
 
 	const CGObjectInstance *market; //todo: replace with ObjectInstanceID
 	const CGHeroInstance *hero; //needed when trading artifacts / creatures
 	EMarketMode::EMarketMode mode;
-	ui32 r1, r2; //mode 0: r1 - sold resource, r2 - bought res (exception: when sacrificing art r1 is art id [todo: make r2 preferred slot?]
-	ui32 val; //units of sold resource
+	std::vector<ui32> r1, r2; //mode 0: r1 - sold resource, r2 - bought res (exception: when sacrificing art r1 is art id [todo: make r2 preferred slot?]
+	std::vector<ui32> val; //units of sold resource
 
 	bool applyGh(CGameHandler *gh);
 	template <typename Handler> void serialize(Handler &h, const int version)

server/CGameHandler.cpp

@@ -5418,50 +5418,89 @@ void CGameHandler::visitObjectOnTile(const TerrainTile &t, const CGHeroInstance
 	}
 }
 
-bool CGameHandler::sacrificeCreatures(const IMarket *market, const CGHeroInstance *hero, SlotID slot, ui32 count)
+bool CGameHandler::sacrificeCreatures(const IMarket *market, const CGHeroInstance *hero, std::vector<SlotID> slot, std::vector<ui32> count)
 {
 	if (!hero)
 		COMPLAIN_RET("You need hero to sacrifice creature!");
 
-	int oldCount = hero->getStackCount(slot);
+	int expSum = 0;
+	auto finish = [this, &hero, &expSum]()
+	{
+		changePrimSkill(hero, PrimarySkill::EXPERIENCE, hero->calculateXp(expSum));
+	};
+
+	for (int i = 0; i < slot.size(); ++i)
+	{
+		int oldCount = hero->getStackCount(slot[i]);
+
+		if (oldCount < count[i])
+		{
+			finish();
+			COMPLAIN_RET("Not enough creatures to sacrifice!")
+		}
+		else if (oldCount == count[i] && hero->stacksCount() == 1 && hero->needsLastStack())
+		{
+			finish();
+			COMPLAIN_RET("Cannot sacrifice last creature!");
+		}
 
-	if (oldCount < count)
-		COMPLAIN_RET("Not enough creatures to sacrifice!")
-	else if (oldCount == count && hero->stacksCount() == 1 && hero->needsLastStack())
-		COMPLAIN_RET("Cannot sacrifice last creature!");
+		int crid = hero->getStack(slot[i]).type->idNumber;
 
-	int crid = hero->getStack(slot).type->idNumber;
+		changeStackCount(StackLocation(hero, slot[i]), -count[i]);
 
-	changeStackCount(StackLocation(hero, slot), -count);
+		int dump, exp;
+		market->getOffer(crid, 0, dump, exp, EMarketMode::CREATURE_EXP);
+		exp *= count[i];
+		expSum += exp;
+	}
 
-	int dump, exp;
-	market->getOffer(crid, 0, dump, exp, EMarketMode::CREATURE_EXP);
-	exp *= count;
-	changePrimSkill(hero, PrimarySkill::EXPERIENCE, hero->calculateXp(exp));
+	finish();
 
 	return true;
 }
 
-bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, ArtifactPosition slot)
+bool CGameHandler::sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, std::vector<ArtifactPosition> slot)
 {
 	if (!hero)
 		COMPLAIN_RET("You need hero to sacrifice artifact!");
 
-	ArtifactLocation al(hero, slot);
-	const CArtifactInstance *a = al.getArt();
+	int expSum = 0;
+	auto finish = [this, &hero, &expSum]()
+	{
+		changePrimSkill(hero, PrimarySkill::EXPERIENCE, expSum);
+	};
+
+	for (int i = 0; i < slot.size(); ++i)
+	{
+		ArtifactLocation al(hero, slot[i]);
+		const CArtifactInstance *a = al.getArt();
+
+		if (!a)
+		{
+			finish();
+			COMPLAIN_RET("Cannot find artifact to sacrifice!");
+		}
+
+		const CArtifactInstance * art = hero->getArt(slot[i]);
+
+		if (!art)
+		{
+			finish();
+			COMPLAIN_RET("No artifact at position to sacrifice!");
+		}
+
+		si32 typId = art->artType->id;
+		int dmp, expToGive;
 
-	COMPLAIN_RET_FALSE_IF(!a,"Cannot find artifact to sacrifice!");
+		m->getOffer(typId, 0, dmp, expToGive, EMarketMode::ARTIFACT_EXP);
 
-	int dmp, expToGive;
-	const CArtifactInstance * art = hero->getArt(slot);
-	COMPLAIN_RET_FALSE_IF((!art), "No artifact at position to sacrifice!");
+		expSum += expToGive;
 
-	si32 typId = art->artType->id;
+		removeArtifact(al);
+	}
 
-	m->getOffer(typId, 0, dmp, expToGive, EMarketMode::ARTIFACT_EXP);
+	finish();
 
-	removeArtifact(al);
-	changePrimSkill(hero, PrimarySkill::EXPERIENCE, expToGive);
 	return true;
 }
 

server/CGameHandler.h

@@ -204,7 +204,7 @@ public:
 	bool buildBoat( ObjectInstanceID objid );
 	bool setFormation( ObjectInstanceID hid, ui8 formation );
 	bool tradeResources(const IMarket *market, ui32 val, PlayerColor player, ui32 id1, ui32 id2);
-	bool sacrificeCreatures(const IMarket *market, const CGHeroInstance *hero, SlotID slot, ui32 count);
+	bool sacrificeCreatures(const IMarket *market, const CGHeroInstance *hero, std::vector<SlotID> slot, std::vector<ui32> count);
 	bool sendResources(ui32 val, PlayerColor player, Res::ERes r1, PlayerColor r2);
 	bool sellCreatures(ui32 count, const IMarket *market, const CGHeroInstance * hero, SlotID slot, Res::ERes resourceID);
 	bool transformInUndead(const IMarket *market, const CGHeroInstance * hero, SlotID slot);
@@ -287,7 +287,7 @@ public:
 	void handleAttackBeforeCasting(BattleAttack *bat);
 	void handleAfterAttackCasting (const BattleAttack & bat);
 	void attackCasting(const BattleAttack & bat, Bonus::BonusType attackMode, const CStack * attacker);
-	bool sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, ArtifactPosition slot);
+	bool sacrificeArtifact(const IMarket * m, const CGHeroInstance * hero, std::vector<ArtifactPosition> slot);
 	void spawnWanderingMonsters(CreatureID creatureID);
 	void handleCheatCode(std::string & cheat, PlayerColor player, const CGHeroInstance * hero, const CGTownInstance * town, bool & cheated);
 	friend class CVCMIServer;

server/NetPacksServer.cpp

@@ -178,29 +178,54 @@ bool TradeOnMarketplace::applyGh( CGameHandler *gh )
 
 	ERROR_IF_NOT(player);
 
+	bool result = true;
+
 	switch(mode)
 	{
 	case EMarketMode::RESOURCE_RESOURCE:
-		return gh->tradeResources(m, val, player, r1, r2);
+		for (int i = 0; i < r1.size(); ++i)
+			result &= gh->tradeResources(m, val[i], player, r1[i], r2[i]);
+		break;
 	case EMarketMode::RESOURCE_PLAYER:
-		return gh->sendResources(val, player, static_cast<Res::ERes>(r1), PlayerColor(r2));
+		for (int i = 0; i < r1.size(); ++i)
+			result &= gh->sendResources(val[i], player, static_cast<Res::ERes>(r1[i]), PlayerColor(r2[i]));
+		break;
 	case EMarketMode::CREATURE_RESOURCE:
-		return gh->sellCreatures(val, m, hero, SlotID(r1), static_cast<Res::ERes>(r2));
+		for (int i = 0; i < r1.size(); ++i)
+			result &= gh->sellCreatures(val[i], m, hero, SlotID(r1[i]), static_cast<Res::ERes>(r2[i]));
+		break;
 	case EMarketMode::RESOURCE_ARTIFACT:
-		return gh->buyArtifact(m, hero, static_cast<Res::ERes>(r1), ArtifactID(r2));
+		for (int i = 0; i < r1.size(); ++i)
+			result &= gh->buyArtifact(m, hero, static_cast<Res::ERes>(r1[i]), ArtifactID(r2[i]));
+		break;
 	case EMarketMode::ARTIFACT_RESOURCE:
-		return gh->sellArtifact(m, hero, ArtifactInstanceID(r1), static_cast<Res::ERes>(r2));
+		for (int i = 0; i < r1.size(); ++i)
+			result &= gh->sellArtifact(m, hero, ArtifactInstanceID(r1[i]), static_cast<Res::ERes>(r2[i]));
+		break;
 	case EMarketMode::CREATURE_UNDEAD:
-		return gh->transformInUndead(m, hero, SlotID(r1));
+		for (int i = 0; i < r1.size(); ++i)
+			result &= gh->transformInUndead(m, hero, SlotID(r1[i]));
+		break;
 	case EMarketMode::RESOURCE_SKILL:
-		return gh->buySecSkill(m, hero, SecondarySkill(r2));
+		for (int i = 0; i < r2.size(); ++i)
+			result &= gh->buySecSkill(m, hero, SecondarySkill(r2[i]));
+		break;
 	case EMarketMode::CREATURE_EXP:
-		return gh->sacrificeCreatures(m, hero, SlotID(r1), val);
+	{
+		std::vector<SlotID> slotIDs(r1.begin(), r1.end());
+		std::vector<ui32> count(val.begin(), val.end());
+		return gh->sacrificeCreatures(m, hero, slotIDs, count);
+	}
 	case EMarketMode::ARTIFACT_EXP:
-		return gh->sacrificeArtifact(m, hero, ArtifactPosition(r1));
+	{
+		std::vector<ArtifactPosition> positions(r1.begin(), r1.end());
+		return gh->sacrificeArtifact(m, hero, positions);
+	}
 	default:
 		COMPLAIN_AND_RETURN("Unknown exchange mode!");
 	}
+
+	return result;
 }
 
 bool SetFormation::applyGh( CGameHandler *gh )