Merge pull request #5609 from SoundSSGood/end-of-battle-infowindows

End of battle infowindows refactoring

client/ArtifactsUIController.cpp

@@ -60,10 +60,9 @@ bool ArtifactsUIController::askToAssemble(const CGHeroInstance * hero, const Art
 	{
 		auto askThread = new std::thread([this, hero, art, slot, assemblyPossibilities, checkIgnored]() -> void
 			{
-				std::scoped_lock askLock(askAssembleArtifactMutex);
+				std::scoped_lock interfaceLock(ENGINE->interfaceMutex);
 				for(const auto combinedArt : assemblyPossibilities)
 				{
-					std::scoped_lock interfaceLock(ENGINE->interfaceMutex);
 					if(checkIgnored)
 					{
 						if(vstd::contains(ignoredArtifacts, combinedArt->getId()))

client/ArtifactsUIController.h

@@ -24,8 +24,6 @@ class ArtifactsUIController
 	size_t numOfArtsAskAssembleSession;
 	std::set<ArtifactID> ignoredArtifacts;
 
-	std::mutex askAssembleArtifactMutex;
-
 public:
 	ArtifactsUIController();
 	bool askToAssemble(const ArtifactLocation & al, const bool onlyEquipped = false, const bool checkIgnored = false);
@@ -39,4 +37,3 @@ public:
 	void artifactAssembled();
 	void artifactDisassembled();
 };
- 

client/CMakeLists.txt

@@ -191,6 +191,7 @@ set(vcmiclientcommon_SRCS
 	NetPacksClient.cpp
 	NetPacksLobbyClient.cpp
 	ServerRunner.cpp
+	UIHelper.cpp
 )
 
 set(vcmiclientcommon_HEADERS
@@ -410,6 +411,7 @@ set(vcmiclientcommon_HEADERS
 	LobbyClientNetPackVisitors.h
 	ServerRunner.h
 	resource.h
+	UIHelper.h
 )
 
 if(APPLE_IOS)

client/CPlayerInterface.cpp

@@ -1072,6 +1072,7 @@ void CPlayerInterface::showInfoDialogAndWait(std::vector<Component> & components
 
 void CPlayerInterface::showYesNoDialog(const std::string &text, CFunctionList<void()> onYes, CFunctionList<void()> onNo, const std::vector<std::shared_ptr<CComponent>> & components)
 {
+	waitWhileDialog();
 	movementController->requestMovementAbort();
 	GAME->interface()->showingDialog->setBusy();
 	CInfoWindow::showYesNoDialog(text, components, onYes, onNo, playerID);

client/Client.h

@@ -194,7 +194,7 @@ public:
 	void tryJoiningArmy(const CArmedInstance * src, const CArmedInstance * dst, bool removeObjWhenFinished, bool allowMerging) override {}
 	bool moveStack(const StackLocation & src, const StackLocation & dst, TQuantity count = -1) override {return false;}
 
-	void removeAfterVisit(const CGObjectInstance * object) override {};
+	void removeAfterVisit(const ObjectInstanceID & id) override {};
 	bool swapGarrisonOnSiege(ObjectInstanceID tid) override {return false;};
 	bool giveHeroNewArtifact(const CGHeroInstance * h, const ArtifactID & artId, const ArtifactPosition & pos) override {return false;};
 	bool giveHeroNewScroll(const CGHeroInstance * h, const SpellID & spellId, const ArtifactPosition & pos) override {return false;};

client/NetPacksClient.cpp

@@ -26,6 +26,7 @@
 #include "CMT.h"
 #include "GameChatHandler.h"
 #include "CServerHandler.h"
+#include "UIHelper.h"
 
 #include "../CCallback.h"
 #include "../lib/filesystem/Filesystem.h"
@@ -291,7 +292,7 @@ void ApplyClientNetPackVisitor::visitEraseArtifact(BulkEraseArtifacts & pack)
 void ApplyClientNetPackVisitor::visitBulkMoveArtifacts(BulkMoveArtifacts & pack)
 {
 	const auto dstOwner = cl.getOwner(pack.dstArtHolder);
-	const auto applyMove = [this, &pack, dstOwner](std::vector<BulkMoveArtifacts::LinkedSlots> & artsPack)
+	const auto applyMove = [this, &pack, dstOwner](const std::vector<MoveArtifactInfo> & artsPack)
 	{
 		for(const auto & slotToMove : artsPack)
 		{
@@ -851,8 +852,36 @@ void ApplyClientNetPackVisitor::visitStacksInjured(StacksInjured & pack)
 
 void ApplyClientNetPackVisitor::visitBattleResultsApplied(BattleResultsApplied & pack)
 {
-	callInterfaceIfPresent(cl, pack.player1, &IGameEventsReceiver::battleResultsApplied);
-	callInterfaceIfPresent(cl, pack.player2, &IGameEventsReceiver::battleResultsApplied);
+	if(!pack.learnedSpells.spells.empty())
+	{
+		const auto hero = GAME->interface()->cb->getHero(pack.learnedSpells.hid);
+		assert(hero);
+		callInterfaceIfPresent(cl, pack.victor, &CGameInterface::showInfoDialog, EInfoWindowMode::MODAL,
+			UIHelper::getEagleEyeInfoWindowText(*hero, pack.learnedSpells.spells), UIHelper::getSpellsComponents(pack.learnedSpells.spells), soundBase::soundID(0));
+	}
+
+	const auto artSet = GAME->interface()->cb->getArtSet(ArtifactLocation(pack.artifacts.front().dstArtHolder));
+	assert(artSet);
+	std::vector<Component> artComponents;
+	for(const auto & artPack : pack.artifacts)
+	{
+		auto packComponents = UIHelper::getArtifactsComponents(*artSet, artPack.artsPack0);
+		artComponents.insert(artComponents.end(), std::make_move_iterator(packComponents.begin()), std::make_move_iterator(packComponents.end()));
+	}
+	if(!artComponents.empty())
+		callInterfaceIfPresent(cl, pack.victor, &CGameInterface::showInfoDialog, EInfoWindowMode::MODAL, UIHelper::getArtifactsInfoWindowText(),
+			artComponents, soundBase::soundID(0));
+
+	for(auto & artPack : pack.artifacts)
+		visitBulkMoveArtifacts(artPack);
+
+	if(pack.raisedStack.getCreature())
+		callInterfaceIfPresent(cl, pack.victor, &CGameInterface::showInfoDialog, EInfoWindowMode::AUTO,
+			UIHelper::getNecromancyInfoWindowText(pack.raisedStack), std::vector<Component>{Component(ComponentType::CREATURE, pack.raisedStack.getId(),
+			pack.raisedStack.count)}, UIHelper::getNecromancyInfoWindowSound());
+
+	callInterfaceIfPresent(cl, pack.victor, &IGameEventsReceiver::battleResultsApplied);
+	callInterfaceIfPresent(cl, pack.loser, &IGameEventsReceiver::battleResultsApplied);
 	callInterfaceIfPresent(cl, PlayerColor::SPECTATOR, &IGameEventsReceiver::battleResultsApplied);
 }
 

client/UIHelper.cpp

@@ -0,0 +1,93 @@
+/*
+ * UIHelper.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 "UIHelper.h"
+
+#include "widgets/CComponent.h"
+
+#include "../lib/CArtHandler.h"
+#include "../lib/CArtifactInstance.h"
+#include "../lib/mapObjects/CGHeroInstance.h"
+#include "../lib/networkPacks/ArtifactLocation.h"
+#include "../lib/CRandomGenerator.h"
+#include "../lib/CCreatureSet.h"
+
+std::vector<Component> UIHelper::getArtifactsComponents(const CArtifactSet & artSet, const std::vector<MoveArtifactInfo> & movedPack)
+{
+    std::vector<Component> components;
+    for(const auto & artMoveInfo : movedPack)
+    {
+        const auto art = artSet.getArt(artMoveInfo.dstPos);
+        assert(art);
+
+        if(art->isScroll())
+            components.emplace_back(ComponentType::SPELL_SCROLL, art->getScrollSpellID());
+        else
+            components.emplace_back(ComponentType::ARTIFACT, art->getTypeId());
+    }
+    return components;
+}
+
+std::vector<Component> UIHelper::getSpellsComponents(const std::set<SpellID> & spells)
+{
+    std::vector<Component> components;
+    for(const auto & spell : spells)
+        components.emplace_back(ComponentType::SPELL, spell);
+    return components;
+}
+
+soundBase::soundID UIHelper::getNecromancyInfoWindowSound()
+{
+    return soundBase::soundID(soundBase::pickup01 + CRandomGenerator::getDefault().nextInt(6));
+}
+
+std::string UIHelper::getNecromancyInfoWindowText(const CStackBasicDescriptor & stack)
+{
+    MetaString text;
+    if(stack.count > 1) // Practicing the dark arts of necromancy, ... (plural)
+    {
+        text.appendLocalString(EMetaText::GENERAL_TXT, 145);
+        text.replaceNumber(stack.count);
+    }
+    else // Practicing the dark arts of necromancy, ... (singular)
+    {
+        text.appendLocalString(EMetaText::GENERAL_TXT, 146);
+    }
+    text.replaceName(stack);
+    return text.toString();
+}
+
+std::string UIHelper::getArtifactsInfoWindowText()
+{
+    MetaString text;
+    text.appendLocalString(EMetaText::GENERAL_TXT, 30);
+    return text.toString();
+}
+
+std::string UIHelper::getEagleEyeInfoWindowText(const CGHeroInstance & hero, const std::set<SpellID> & spells)
+{
+    MetaString text;
+    text.appendLocalString(EMetaText::GENERAL_TXT, 221); // Through eagle-eyed observation, %s is able to learn %s
+    text.replaceRawString(hero.getNameTranslated());
+
+    auto curSpell = spells.begin();
+    text.replaceName(*curSpell++);
+    for(int i = 1; i < spells.size(); i++, curSpell++)
+    {
+        if(i + 1 == spells.size())
+            text.appendLocalString(EMetaText::GENERAL_TXT, 141); // " and "
+        else
+            text.appendRawString(", ");
+        text.appendName(*curSpell);
+    }
+    text.appendRawString(".");
+    return text.toString();
+}

client/UIHelper.h

@@ -0,0 +1,35 @@
+/*
+ * UIHelper.h, part of VCMI engine
+ *
+ * Authors: listed in file AUTHORS in main folder
+ *
+ * License: GNU General Public License v2.0 or later
+ * Full text of license available in license.txt file, in main folder
+ *
+ */
+#pragma once
+
+#include "StdInc.h"
+
+#include "../lib/CSoundBase.h"
+#include "../lib/texts/MetaString.h"
+
+VCMI_LIB_NAMESPACE_BEGIN
+
+struct MoveArtifactInfo;
+struct Component;
+class CArtifactSet;
+class CGHeroInstance;
+class CStackBasicDescriptor;
+
+VCMI_LIB_NAMESPACE_END
+
+namespace UIHelper
+{
+    std::vector<Component> getArtifactsComponents(const CArtifactSet & artSet, const std::vector<MoveArtifactInfo> & movedPack);
+    std::vector<Component> getSpellsComponents(const std::set<SpellID> & spells);
+    soundBase::soundID getNecromancyInfoWindowSound();
+    std::string getNecromancyInfoWindowText(const CStackBasicDescriptor & stack);
+    std::string getArtifactsInfoWindowText();
+    std::string getEagleEyeInfoWindowText(const CGHeroInstance & hero, const std::set<SpellID> & spells);
+}

client/windows/CWindowWithArtifacts.cpp

@@ -12,7 +12,6 @@
 
 #include "CHeroWindow.h"
 #include "CSpellWindow.h"
-#include "CExchangeWindow.h"
 #include "CHeroBackpackWindow.h"
 
 #include "../GameEngine.h"

lib/ArtifactUtils.cpp

@@ -67,7 +67,7 @@ DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtBackpackPosition(const CArtifa
 	if(target->bearerType() == ArtBearer::HERO)
 		if(art->canBePutAt(target, ArtifactPosition::BACKPACK_START))
 		{
-			return ArtifactPosition::BACKPACK_START;
+			return ArtifactPosition::BACKPACK_START + target->artifactsInBackpack.size();
 		}
 	return ArtifactPosition::PRE_FIRST;
 }

lib/IGameCallback.h

@@ -118,7 +118,7 @@ public:
 	virtual void tryJoiningArmy(const CArmedInstance *src, const CArmedInstance *dst, bool removeObjWhenFinished, bool allowMerging) =0; //merges army from src do dst or opens a garrison window
 	virtual bool moveStack(const StackLocation &src, const StackLocation &dst, TQuantity count) = 0;
 
-	virtual void removeAfterVisit(const CGObjectInstance *object) = 0; //object will be destroyed when interaction is over. Do not call when interaction is not ongoing!
+	virtual void removeAfterVisit(const ObjectInstanceID & id) = 0; //object will be destroyed when interaction is over. Do not call when interaction is not ongoing!
 
 	virtual bool giveHeroNewArtifact(const CGHeroInstance * h, const ArtifactID & artId, const ArtifactPosition & pos) = 0;
 	virtual bool giveHeroNewScroll(const CGHeroInstance * h, const SpellID & spellId, const ArtifactPosition & pos) = 0;

lib/mapObjects/CGHeroInstance.cpp

@@ -1037,32 +1037,6 @@ CStackBasicDescriptor CGHeroInstance::calculateNecromancy (const BattleResult &b
 	return CStackBasicDescriptor();
 }
 
-/**
- * Show the necromancy dialog with information about units raised.
- * @param raisedStack Pair where the first element represents ID of the raised creature
- * and the second element the amount.
- */
-void CGHeroInstance::showNecromancyDialog(const CStackBasicDescriptor &raisedStack, vstd::RNG & rand) const
-{
-	InfoWindow iw;
-	iw.type = EInfoWindowMode::AUTO;
-	iw.soundID = soundBase::pickup01 + rand.nextInt(6);
-	iw.player = tempOwner;
-	iw.components.emplace_back(ComponentType::CREATURE, raisedStack.getId(), raisedStack.count);
-
-	if (raisedStack.count > 1) // Practicing the dark arts of necromancy, ... (plural)
-	{
-		iw.text.appendLocalString(EMetaText::GENERAL_TXT, 145);
-		iw.text.replaceNumber(raisedStack.count);
-	}
-	else // Practicing the dark arts of necromancy, ... (singular)
-	{
-		iw.text.appendLocalString(EMetaText::GENERAL_TXT, 146);
-	}
-	iw.text.replaceName(raisedStack);
-
-	cb->showInfoDialog(&iw);
-}
 /*
 int3 CGHeroInstance::getSightCenter() const
 {

lib/mapObjects/CGHeroInstance.h

@@ -241,7 +241,6 @@ public:
 	int getBasePrimarySkillValue(PrimarySkill which) const; //the value of a base-skill without items or temporary bonuses
 
 	CStackBasicDescriptor calculateNecromancy (const BattleResult &battleResult) const;
-	void showNecromancyDialog(const CStackBasicDescriptor &raisedStack, vstd::RNG & rand) const;
 	EDiggingStatus diggingStatus() const;
 
 	//////////////////////////////////////////////////////////////////////////

lib/networkPacks/ArtifactLocation.h

@@ -52,4 +52,31 @@ struct ArtifactLocation
 	}
 };
 
+struct MoveArtifactInfo
+{
+	ArtifactPosition srcPos;
+	ArtifactPosition dstPos;
+	bool askAssemble;
+
+	MoveArtifactInfo()
+		: srcPos(ArtifactPosition::PRE_FIRST)
+		, dstPos(ArtifactPosition::PRE_FIRST)
+		, askAssemble(false)
+	{
+	}
+	MoveArtifactInfo(const ArtifactPosition & srcPos, const ArtifactPosition & dstPos, bool askAssemble = false)
+		: srcPos(srcPos)
+		, dstPos(dstPos)
+		, askAssemble(askAssemble)
+	{
+	}
+
+	template <typename Handler> void serialize(Handler & h)
+	{
+		h & srcPos;
+		h & dstPos;
+		h & askAssemble;
+	}
+};
+
 VCMI_LIB_NAMESPACE_END

lib/networkPacks/NetPacksLib.cpp

@@ -1778,7 +1778,7 @@ void BulkEraseArtifacts::applyGs(CGameState *gs)
 
 void BulkMoveArtifacts::applyGs(CGameState *gs)
 {
-	const auto bulkArtsRemove = [gs](std::vector<LinkedSlots> & artsPack, CArtifactSet & artSet)
+	const auto bulkArtsRemove = [gs](std::vector<MoveArtifactInfo> & artsPack, CArtifactSet & artSet)
 	{
 		std::vector<ArtifactPosition> packToRemove;
 		for(const auto & slotsPair : artsPack)
@@ -1792,7 +1792,7 @@ void BulkMoveArtifacts::applyGs(CGameState *gs)
 			gs->getMap().removeArtifactInstance(artSet, slot);
 	};
 
-	const auto bulkArtsPut = [gs](std::vector<LinkedSlots> & artsPack, CArtifactSet & initArtSet, CArtifactSet & dstArtSet)
+	const auto bulkArtsPut = [gs](std::vector<MoveArtifactInfo> & artsPack, CArtifactSet & initArtSet, CArtifactSet & dstArtSet)
 	{
 		for(const auto & slotsPair : artsPack)
 		{
@@ -2118,25 +2118,22 @@ void BattleCancelled::applyGs(CGameState *gs)
 void BattleResultAccepted::applyGs(CGameState *gs)
 {
 	// Remove any "until next battle" bonuses
-	for(auto & res : heroResult)
-	{
-		if(res.hero)
-			res.hero->removeBonusesRecursive(Bonus::OneBattle);
-	}
+	if(const auto attackerHero = gs->getHero(heroResult[BattleSide::ATTACKER].heroId))
+		attackerHero->removeBonusesRecursive(Bonus::OneBattle);
+	if(const auto defenderHero = gs->getHero(heroResult[BattleSide::DEFENDER].heroId))
+		defenderHero->removeBonusesRecursive(Bonus::OneBattle);
 
 	if(winnerSide != BattleSide::NONE)
 	{
 		// Grow up growing artifacts
-		const auto hero = heroResult[winnerSide].hero;
-
-		if (hero)
+		if(const auto winnerHero = gs->getHero(heroResult[winnerSide].heroId))
 		{
-			if(hero->commander && hero->commander->alive)
+			if(winnerHero->commander && winnerHero->commander->alive)
 			{
-				for(auto & art : hero->commander->artifactsWorn)
+				for(auto & art : winnerHero->commander->artifactsWorn)
 					art.second.artifact->growingUp();
 			}
-			for(auto & art : hero->artifactsWorn)
+			for(auto & art : winnerHero->artifactsWorn)
 			{
 				art.second.artifact->growingUp();
 			}
@@ -2145,26 +2142,17 @@ void BattleResultAccepted::applyGs(CGameState *gs)
 
 	if(gs->getSettings().getBoolean(EGameSettings::MODULE_STACK_EXPERIENCE))
 	{
-		if(heroResult[BattleSide::ATTACKER].army)
+		if(const auto attackerArmy = gs->getArmyInstance(heroResult[BattleSide::ATTACKER].armyId))
 		{
-			heroResult[BattleSide::ATTACKER].army->giveStackExp(heroResult[BattleSide::ATTACKER].exp);
-			heroResult[BattleSide::ATTACKER].army->nodeHasChanged();
+			attackerArmy->giveStackExp(heroResult[BattleSide::ATTACKER].exp);
+			attackerArmy->nodeHasChanged();
 		}
-		if(heroResult[BattleSide::DEFENDER].army)
+		if(const auto defenderArmy = gs->getArmyInstance(heroResult[BattleSide::DEFENDER].armyId))
 		{
-			heroResult[BattleSide::DEFENDER].army->giveStackExp(heroResult[BattleSide::DEFENDER].exp);
-			heroResult[BattleSide::DEFENDER].army->nodeHasChanged();
+			defenderArmy->giveStackExp(heroResult[BattleSide::DEFENDER].exp);
+			defenderArmy->nodeHasChanged();
 		}
-
 	}
-
-	auto currentBattle = boost::range::find_if(gs->currentBattles, [&](const auto & battle)
-	{
-		return battle->battleID == battleID;
-	});
-
-	assert(currentBattle != gs->currentBattles.end());
-	gs->currentBattles.erase(currentBattle);
 }
 
 void BattleLogMessage::applyGs(CGameState *gs)
@@ -2328,6 +2316,23 @@ void BattleUnitsChanged::applyBattle(IBattleState * battleState)
 	}
 }
 
+void BattleResultsApplied::applyGs(CGameState * gs)
+{
+	learnedSpells.applyGs(gs);
+
+	for(auto & artPack : artifacts)
+		artPack.applyGs(gs);
+
+	const auto currentBattle = std::find_if(gs->currentBattles.begin(), gs->currentBattles.end(),
+		[this](const auto & battle)
+		{
+			return battle->battleID == battleID;
+		});
+
+	assert(currentBattle != gs->currentBattles.end());
+	gs->currentBattles.erase(currentBattle);
+}
+
 void BattleObstaclesChanged::applyGs(CGameState *gs)
 {
 	applyBattle(gs->getBattle(battleID));

lib/networkPacks/PacksForClient.h

@@ -1049,27 +1049,6 @@ struct DLL_LINKAGE BulkEraseArtifacts : CArtifactOperationPack
 
 struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 {
-	struct LinkedSlots
-	{
-		ArtifactPosition srcPos;
-		ArtifactPosition dstPos;
-		bool askAssemble;
-
-		LinkedSlots() = default;
-		LinkedSlots(const ArtifactPosition & srcPos, const ArtifactPosition & dstPos, bool askAssemble = false)
-			: srcPos(srcPos)
-			, dstPos(dstPos)
-			, askAssemble(askAssemble)
-		{
-		}
-		template <typename Handler> void serialize(Handler & h)
-		{
-			h & srcPos;
-			h & dstPos;
-			h & askAssemble;
-		}
-	};
-
 	PlayerColor interfaceOwner;
 	ObjectInstanceID srcArtHolder;
 	ObjectInstanceID dstArtHolder;
@@ -1095,8 +1074,8 @@ struct DLL_LINKAGE BulkMoveArtifacts : CArtifactOperationPack
 
 	void applyGs(CGameState * gs) override;
 
-	std::vector<LinkedSlots> artsPack0;
-	std::vector<LinkedSlots> artsPack1;
+	std::vector<MoveArtifactInfo> artsPack0;
+	std::vector<MoveArtifactInfo> artsPack1;
 
 	void visitTyped(ICPackVisitor & visitor) override;
 

lib/networkPacks/PacksForClientBattle.h

@@ -11,6 +11,7 @@
 
 #include "NetPacksBase.h"
 #include "BattleChanges.h"
+#include "PacksForClient.h"
 #include "../battle/BattleHexArray.h"
 #include "../battle/BattleAction.h"
 #include "../texts/MetaString.h"
@@ -95,16 +96,18 @@ struct DLL_LINKAGE BattleResultAccepted : public CPackForClient
 	struct HeroBattleResults
 	{
 		HeroBattleResults()
-			: hero(nullptr), army(nullptr), exp(0) {}
+			: heroId(ObjectInstanceID::NONE)
+			, armyId(ObjectInstanceID::NONE)
+			, exp(0) {}
 
-		CGHeroInstance * hero;
-		CArmedInstance * army;
+		ObjectInstanceID heroId;
+		ObjectInstanceID armyId;
 		TExpType exp;
 
 		template <typename Handler> void serialize(Handler & h)
 		{
-			h & hero;
-			h & army;
+			h & armyId;
+			h & heroId;
 			h & exp;
 		}
 	};
@@ -112,12 +115,14 @@ struct DLL_LINKAGE BattleResultAccepted : public CPackForClient
 	BattleID battleID = BattleID::NONE;
 	BattleSideArray<HeroBattleResults> heroResult;
 	BattleSide winnerSide;
+	std::vector<BulkMoveArtifacts> artifacts;
 
 	template <typename Handler> void serialize(Handler & h)
 	{
 		h & battleID;
 		h & heroResult;
 		h & winnerSide;
+		h & artifacts;
 		assert(battleID != BattleID::NONE);
 	}
 };
@@ -131,7 +136,6 @@ struct DLL_LINKAGE BattleResult : public Query
 	BattleSide winner = BattleSide::NONE; //0 - attacker, 1 - defender, [2 - draw (should be possible?)]
 	BattleSideArray<std::map<CreatureID, si32>> casualties; //first => casualties of attackers - map crid => number
 	BattleSideArray<TExpType> exp{0,0}; //exp for attacker and defender
-	std::set<ArtifactInstanceID> artifacts; //artifacts taken from loser to winner - currently unused
 
 	void visitTyped(ICPackVisitor & visitor) override;
 	void applyGs(CGameState *gs) override {}
@@ -144,7 +148,6 @@ struct DLL_LINKAGE BattleResult : public Query
 		h & winner;
 		h & casualties;
 		h & exp;
-		h & artifacts;
 		assert(battleID != BattleID::NONE);
 	}
 };
@@ -421,15 +424,22 @@ struct DLL_LINKAGE StacksInjured : public CPackForClient
 struct DLL_LINKAGE BattleResultsApplied : public CPackForClient
 {
 	BattleID battleID = BattleID::NONE;
-	PlayerColor player1, player2;
+	PlayerColor victor;
+	PlayerColor loser;
+	ChangeSpells learnedSpells;
+	std::vector<BulkMoveArtifacts> artifacts;
+	CStackBasicDescriptor raisedStack;
 	void visitTyped(ICPackVisitor & visitor) override;
-	void applyGs(CGameState *gs) override {}
+	void applyGs(CGameState *gs) override;
 
 	template <typename Handler> void serialize(Handler & h)
 	{
 		h & battleID;
-		h & player1;
-		h & player2;
+		h & victor;
+		h & loser;
+		h & learnedSpells;
+		h & artifacts;
+		h & raisedStack;
 		assert(battleID != BattleID::NONE);
 	}
 };

lib/rewardable/Interface.cpp

@@ -212,7 +212,7 @@ void Rewardable::Interface::grantRewardAfterLevelup(const Rewardable::VisitInfo
 
 	if(info.reward.removeObject)
 		if(auto * instance = dynamic_cast<const CGObjectInstance*>(this))
-			cb->removeAfterVisit(instance);
+			cb->removeAfterVisit(instance->id);
 }
 
 void Rewardable::Interface::serializeJson(JsonSerializeFormat & handler)

server/CGameHandler.cpp

@@ -2643,14 +2643,14 @@ bool CGameHandler::moveArtifact(const PlayerColor & player, const ArtifactLocati
 	{
 		// Previous artifact must be swapped
 		COMPLAIN_RET_FALSE_IF(!dstArtifact->canBePutAt(srcArtSet, src.slot, true), "Cannot swap artifacts!");
-		ma.artsPack1.push_back(BulkMoveArtifacts::LinkedSlots(dstSlot, src.slot));
+		ma.artsPack1.emplace_back(dstSlot, src.slot);
 	}
 
 	auto hero = getHero(dst.artHolder);
 	if(ArtifactUtils::checkSpellbookIsNeeded(hero, srcArtifact->getTypeId(), dstSlot))
 		giveHeroNewArtifact(hero, ArtifactID::SPELLBOOK, ArtifactPosition::SPELLBOOK);
 
-	ma.artsPack0.push_back(BulkMoveArtifacts::LinkedSlots(src.slot, dstSlot));
+	ma.artsPack0.emplace_back(src.slot, dstSlot);
 	if(src.artHolder != dst.artHolder && !isDstSlotBackpack)
 		ma.artsPack0.back().askAssemble = true;
 	sendAndApply(ma);
@@ -2676,14 +2676,14 @@ bool CGameHandler::bulkMoveArtifacts(const PlayerColor & player, ObjectInstanceI
 	CArtifactFittingSet artFittingSet(pdstSet->bearerType());
 
 	auto moveArtifact = [this, &artFittingSet, dstId](const CArtifactInstance * artifact,
-		ArtifactPosition srcSlot, std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
+		ArtifactPosition srcSlot, std::vector<MoveArtifactInfo> & slots) -> void
 	{
 		assert(artifact);
 		auto dstSlot = ArtifactUtils::getArtAnyPosition(&artFittingSet, artifact->getTypeId());
 		if(dstSlot != ArtifactPosition::PRE_FIRST)
 		{
 			artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
-			slots.push_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
+			slots.emplace_back(srcSlot, dstSlot);
 
 			// TODO Shouldn't be here. Possibly in callback after equipping the artifact
 			if(auto dstHero = getHero(dstId))
@@ -2696,7 +2696,7 @@ bool CGameHandler::bulkMoveArtifacts(const PlayerColor & player, ObjectInstanceI
 
 	if(swap)
 	{
-		auto moveArtsWorn = [moveArtifact](const CArtifactSet * srcArtSet, std::vector<BulkMoveArtifacts::LinkedSlots> & slots)
+		auto moveArtsWorn = [moveArtifact](const CArtifactSet * srcArtSet, std::vector<MoveArtifactInfo> & slots)
 		{
 			for(auto & artifact : srcArtSet->artifactsWorn)
 			{
@@ -2705,12 +2705,12 @@ bool CGameHandler::bulkMoveArtifacts(const PlayerColor & player, ObjectInstanceI
 			}
 		};
 		auto moveArtsInBackpack = [](const CArtifactSet * artSet,
-			std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
+			std::vector<MoveArtifactInfo> & slots) -> void
 		{
 			for(auto & slotInfo : artSet->artifactsInBackpack)
 			{
 				auto slot = artSet->getArtPos(slotInfo.artifact);
-				slots.push_back(BulkMoveArtifacts::LinkedSlots(slot, slot));
+				slots.emplace_back(slot, slot);
 			}
 		};
 		if(equipped)
@@ -2766,7 +2766,7 @@ bool CGameHandler::manageBackpackArtifacts(const PlayerColor & player, const Obj
 	BulkMoveArtifacts bma(player, heroID, heroID, false);
 	const auto makeSortBackpackRequest = [artSet, &bma](const std::function<int32_t(const ArtSlotInfo&)> & getSortId)
 	{
-		std::map<int32_t, std::vector<BulkMoveArtifacts::LinkedSlots>> packsSorted;
+		std::map<int32_t, std::vector<MoveArtifactInfo>> packsSorted;
 		ArtifactPosition backpackSlot = ArtifactPosition::BACKPACK_START;
 		for(const auto & backpackSlotInfo : artSet->artifactsInBackpack)
 			packsSorted.try_emplace(getSortId(backpackSlotInfo)).first->second.emplace_back(backpackSlot++, ArtifactPosition::PRE_FIRST);
@@ -2888,11 +2888,7 @@ bool CGameHandler::switchArtifactsCostume(const PlayerColor & player, const Obje
 		{
 			if(const auto slot = artFittingSet.getArtPos(artPos.second, false, false); slot != ArtifactPosition::PRE_FIRST)
 			{
-				bma.artsPack0.emplace_back(BulkMoveArtifacts::LinkedSlots
-					{
-						artSet->getArtPos(artFittingSet.getArt(slot)),
-						artPos.first
-					});
+				bma.artsPack0.emplace_back(artSet->getArtPos(artFittingSet.getArt(slot)), artPos.first);
 				artFittingSet.removeArtifact(slot);
 				if(ArtifactUtils::isSlotBackpack(slot))
 					estimateBackpackSize--;
@@ -2903,7 +2899,7 @@ bool CGameHandler::switchArtifactsCostume(const PlayerColor & player, const Obje
 		for(const auto & slot : ArtifactUtils::commonWornSlots())
 			if(artFittingSet.getArt(slot))
 			{
-				bma.artsPack0.emplace_back(BulkMoveArtifacts::LinkedSlots{slot, ArtifactPosition::BACKPACK_START});
+				bma.artsPack0.emplace_back(slot, ArtifactPosition::BACKPACK_START);
 				estimateBackpackSize++;
 			}
 		
@@ -3842,7 +3838,7 @@ bool CGameHandler::addToSlot(const StackLocation &sl, const CCreature *c, TQuant
 void CGameHandler::tryJoiningArmy(const CArmedInstance *src, const CArmedInstance *dst, bool removeObjWhenFinished, bool allowMerging)
 {
 	if (removeObjWhenFinished)
-		removeAfterVisit(src);
+		removeAfterVisit(src->id);
 
 	if (!src->canBeMergedWith(*dst, allowMerging))
 	{
@@ -4090,14 +4086,14 @@ bool CGameHandler::isBlockedByQueries(const CPackForServer *pack, PlayerColor pl
 	return false;
 }
 
-void CGameHandler::removeAfterVisit(const CGObjectInstance *object)
+void CGameHandler::removeAfterVisit(const ObjectInstanceID & id)
 {
 	//If the object is being visited, there must be a matching query
 	for (const auto &query : queries->allQueries())
 	{
 		if (auto someVistQuery = std::dynamic_pointer_cast<MapObjectVisitQuery>(query))
 		{
-			if (someVistQuery->visitedObject == object)
+			if(someVistQuery->visitedObject->id == id)
 			{
 				someVistQuery->removeObjectAfterVisit = true;
 				return;

server/CGameHandler.h

@@ -132,7 +132,7 @@ public:
 	void tryJoiningArmy(const CArmedInstance *src, const CArmedInstance *dst, bool removeObjWhenFinished, bool allowMerging) override;
 	bool moveStack(const StackLocation &src, const StackLocation &dst, TQuantity count = -1) override;
 
-	void removeAfterVisit(const CGObjectInstance *object) override;
+	void removeAfterVisit(const ObjectInstanceID & id) override;
 
 	bool giveHeroNewArtifact(const CGHeroInstance * h, const CArtifact * artType, const SpellID & spellId, const ArtifactPosition & pos);
 	bool giveHeroNewArtifact(const CGHeroInstance * h, const ArtifactID & artId, const ArtifactPosition & pos) override;

server/battles/BattleProcessor.cpp

@@ -306,7 +306,7 @@ void BattleProcessor::endBattleConfirm(const BattleID & battleID)
 	resultProcessor->endBattleConfirm(*battle);
 }
 
-void BattleProcessor::battleAfterLevelUp(const BattleID & battleID, const BattleResult &result)
+void BattleProcessor::battleFinalize(const BattleID & battleID, const BattleResult &result)
 {
-	resultProcessor->battleAfterLevelUp(battleID, result);
+	resultProcessor->battleFinalize(battleID, result);
 }

server/battles/BattleProcessor.h

@@ -69,7 +69,7 @@ public:
 	/// Applies results of a battle once player agrees to them
 	void endBattleConfirm(const BattleID & battleID);
 	/// Applies results of a battle after potential levelup
-	void battleAfterLevelUp(const BattleID & battleID, const BattleResult & result);
+	void battleFinalize(const BattleID & battleID, const BattleResult & result);
 
 	template <typename Handler> void serialize(Handler &h)
 	{

server/battles/BattleResultProcessor.cpp

@@ -9,6 +9,7 @@
  */
 #include "StdInc.h"
 #include "BattleResultProcessor.h"
+#include "battle/BattleInfo.h"
 
 #include "../CGameHandler.h"
 #include "../TurnTimerHandler.h"
@@ -20,17 +21,12 @@
 #include "../../lib/CStack.h"
 #include "../../lib/CPlayerState.h"
 #include "../../lib/IGameSettings.h"
-#include "../../lib/battle/CBattleInfoCallback.h"
-#include "../../lib/battle/IBattleState.h"
 #include "../../lib/battle/SideInBattle.h"
 #include "../../lib/gameState/CGameState.h"
 #include "../../lib/mapObjects/CGTownInstance.h"
 #include "../../lib/networkPacks/PacksForClientBattle.h"
-#include "../../lib/networkPacks/PacksForClient.h"
 #include "../../lib/spells/CSpellHandler.h"
 
-#include <vstd/RNG.h>
-
 #include <boost/lexical_cast.hpp>
 
 BattleResultProcessor::BattleResultProcessor(BattleProcessor * owner, CGameHandler * newGameHandler)
@@ -186,17 +182,19 @@ void CasualtiesAfterBattle::updateArmy(CGameHandler *gh)
 
 FinishingBattleHelper::FinishingBattleHelper(const CBattleInfoCallback & info, const BattleResult & result, int remainingBattleQueriesCount)
 {
+	const auto attackerHero = info.getBattle()->getSideHero(BattleSide::ATTACKER);
+	const auto defenderHero = info.getBattle()->getSideHero(BattleSide::DEFENDER);
 	if (result.winner == BattleSide::ATTACKER)
 	{
-		winnerHero = info.getBattle()->getSideHero(BattleSide::ATTACKER);
-		loserHero = info.getBattle()->getSideHero(BattleSide::DEFENDER);
+		winnerId = attackerHero ? attackerHero->id : ObjectInstanceID::NONE;
+		loserId = defenderHero ? defenderHero->id : ObjectInstanceID::NONE;
 		victor = info.getBattle()->getSidePlayer(BattleSide::ATTACKER);
 		loser = info.getBattle()->getSidePlayer(BattleSide::DEFENDER);
 	}
 	else
 	{
-		winnerHero = info.getBattle()->getSideHero(BattleSide::DEFENDER);
-		loserHero = info.getBattle()->getSideHero(BattleSide::ATTACKER);
+		winnerId = defenderHero ? defenderHero->id : ObjectInstanceID::NONE;
+		loserId = attackerHero ? attackerHero->id : ObjectInstanceID::NONE;
 		victor = info.getBattle()->getSidePlayer(BattleSide::DEFENDER);
 		loser = info.getBattle()->getSidePlayer(BattleSide::ATTACKER);
 	}
@@ -313,8 +311,6 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 	auto * battleResult = battleResults.at(battle.getBattle()->getBattleID()).get();
 	auto * finishingBattle = finishingBattles.at(battle.getBattle()->getBattleID()).get();
 
-	const EBattleResult result = battleResult->result;
-
 	//calculate casualties before deleting battle
 	CasualtiesAfterBattle cab1(battle, BattleSide::ATTACKER);
 	CasualtiesAfterBattle cab2(battle, BattleSide::DEFENDER);
@@ -322,280 +318,209 @@ void BattleResultProcessor::endBattleConfirm(const CBattleInfoCallback & battle)
 	cab1.updateArmy(gameHandler);
 	cab2.updateArmy(gameHandler); //take casualties after battle is deleted
 
+	const auto winnerHero = battle.battleGetFightingHero(finishingBattle->winnerSide);
+	const auto loserHero = battle.battleGetFightingHero(CBattleInfoEssentials::otherSide(finishingBattle->winnerSide));
+
 	if(battleResult->winner == BattleSide::DEFENDER
-	   && finishingBattle->winnerHero
-	   && finishingBattle->winnerHero->visitedTown
-	   && !finishingBattle->winnerHero->inTownGarrison
-	   && finishingBattle->winnerHero->visitedTown->garrisonHero == finishingBattle->winnerHero)
+	   && winnerHero
+	   && winnerHero->visitedTown
+	   && !winnerHero->inTownGarrison
+	   && winnerHero->visitedTown->garrisonHero == winnerHero)
 	{
-		gameHandler->swapGarrisonOnSiege(finishingBattle->winnerHero->visitedTown->id); //return defending visitor from garrison to its rightful place
+		gameHandler->swapGarrisonOnSiege(winnerHero->visitedTown->id); //return defending visitor from garrison to its rightful place
 	}
 	//give exp
-	if(!finishingBattle->isDraw() && battleResult->exp[finishingBattle->winnerSide] && finishingBattle->winnerHero)
-		gameHandler->giveExperience(finishingBattle->winnerHero, battleResult->exp[finishingBattle->winnerSide]);
+	if(!finishingBattle->isDraw() && battleResult->exp[finishingBattle->winnerSide] && winnerHero)
+		gameHandler->giveExperience(winnerHero, battleResult->exp[finishingBattle->winnerSide]);
 
-	// Eagle Eye handling
-	if(!finishingBattle->isDraw() && finishingBattle->winnerHero)
+	// Add statistics
+	if(loserHero && !finishingBattle->isDraw())
+	{
+		ConstTransitivePtr<CGHeroInstance> strongestHero = nullptr;
+		for(auto & hero : gameHandler->gameState()->getPlayerState(finishingBattle->loser)->getHeroes())
+			if(!strongestHero || hero->exp > strongestHero->exp)
+				strongestHero = hero;
+		if(strongestHero->id == finishingBattle->loserId && strongestHero->level > 5)
+			gameHandler->gameState()->statistic.accumulatedValues[finishingBattle->victor].lastDefeatedStrongestHeroDay = gameHandler->gameState()->getDate(Date::DAY);
+	}
+	if(battle.sideToPlayer(BattleSide::ATTACKER) == PlayerColor::NEUTRAL || battle.sideToPlayer(BattleSide::DEFENDER) == PlayerColor::NEUTRAL)
+	{
+		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::ATTACKER)].numBattlesNeutral++;
+		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::DEFENDER)].numBattlesNeutral++;
+		if(!finishingBattle->isDraw())
+			gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(finishingBattle->winnerSide)].numWinBattlesNeutral++;
+	}
+	else
 	{
-		ChangeSpells spells;
+		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::ATTACKER)].numBattlesPlayer++;
+		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::DEFENDER)].numBattlesPlayer++;
+		if(!finishingBattle->isDraw())
+			gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(finishingBattle->winnerSide)].numWinBattlesPlayer++;
+	}
 
-		if(auto eagleEyeLevel = finishingBattle->winnerHero->valOfBonuses(BonusType::LEARN_BATTLE_SPELL_LEVEL_LIMIT))
-		{
-			auto eagleEyeChance = finishingBattle->winnerHero->valOfBonuses(BonusType::LEARN_BATTLE_SPELL_CHANCE);
-			for(auto & spellId : battle.getBattle()->getUsedSpells(battle.otherSide(battleResult->winner)))
-			{
-				auto spell = spellId.toEntity(LIBRARY->spells());
-				if(spell
-					&& spell->getLevel() <= eagleEyeLevel
-					&& !finishingBattle->winnerHero->spellbookContainsSpell(spell->getId())
-					&& gameHandler->getRandomGenerator().nextInt(99) < eagleEyeChance)
-				{
-					spells.spells.insert(spell->getId());
-				}
-			}
-		}
+	BattleResultAccepted raccepted;
+	raccepted.battleID = battle.getBattle()->getBattleID();
+	raccepted.heroResult[finishingBattle->winnerSide].heroId = winnerHero ? winnerHero->id : ObjectInstanceID::NONE;
+	raccepted.heroResult[CBattleInfoEssentials::otherSide(finishingBattle->winnerSide)].heroId = loserHero ? loserHero->id : ObjectInstanceID::NONE;
+	raccepted.heroResult[BattleSide::ATTACKER].armyId = battle.battleGetArmyObject(BattleSide::ATTACKER)->id;
+	raccepted.heroResult[BattleSide::DEFENDER].armyId = battle.battleGetArmyObject(BattleSide::DEFENDER)->id;
+	raccepted.heroResult[BattleSide::ATTACKER].exp = battleResult->exp[BattleSide::ATTACKER];
+	raccepted.heroResult[BattleSide::DEFENDER].exp = battleResult->exp[BattleSide::DEFENDER];
+	raccepted.winnerSide = finishingBattle->winnerSide;
+	gameHandler->sendAndApply(raccepted);
 
-		if(!spells.spells.empty())
-		{
-			spells.learn = 1;
-			spells.hid = finishingBattle->winnerHero->id;
+	//--> continuation (battleFinalize) occurs after level-up gameHandler->queries are handled or on removing query
+}
+
+void BattleResultProcessor::battleFinalize(const BattleID & battleID, const BattleResult & result)
+{
+	LOG_TRACE(logGlobal);
 
-			InfoWindow iw;
-			iw.player = finishingBattle->winnerHero->tempOwner;
-			iw.text.appendLocalString(EMetaText::GENERAL_TXT, 221); //Through eagle-eyed observation, %s is able to learn %s
-			iw.text.replaceRawString(finishingBattle->winnerHero->getNameTranslated());
+	assert(finishingBattles.count(battleID) != 0);
+	if(finishingBattles.count(battleID) == 0)
+		return;
 
-			std::ostringstream names;
-			for(int i = 0; i < spells.spells.size(); i++)
-			{
-				names << "%s";
-				if(i < spells.spells.size() - 2)
-					names << ", ";
-				else if(i < spells.spells.size() - 1)
-					names << "%s";
-			}
-			names << ".";
+	auto & finishingBattle = finishingBattles[battleID];
+
+	finishingBattle->remainingBattleQueriesCount--;
+	logGlobal->trace("Decremented gameHandler->queries count to %d", finishingBattle->remainingBattleQueriesCount);
+
+	if (finishingBattle->remainingBattleQueriesCount > 0)
+		//Battle results will be handled when all battle gameHandler->queries are closed
+		return;
+
+	//TODO consider if we really want it to work like above. ATM each player as unblocked as soon as possible
+	// but the battle consequences are applied after final player is unblocked. Hard to abuse...
+	// Still, it looks like a hole.
 
-			iw.text.replaceRawString(names.str());
+	const auto battle = std::find_if(gameHandler->gameState()->currentBattles.begin(), gameHandler->gameState()->currentBattles.end(),
+		[battleID](const auto & desiredBattle)
+		{
+			return desiredBattle->battleID == battleID;
+		});
+	assert(battle != gameHandler->gameState()->currentBattles.end());
 
-			auto it = spells.spells.begin();
-			for(int i = 0; i < spells.spells.size(); i++, it++)
+	const auto winnerHero = (*battle)->battleGetFightingHero(finishingBattle->winnerSide);
+	const auto loserHero = (*battle)->battleGetFightingHero(CBattleInfoEssentials::otherSide(finishingBattle->winnerSide));
+	BattleResultsApplied resultsApplied;
+
+	// Eagle Eye handling
+	if(!finishingBattle->isDraw() && winnerHero)
+	{
+		if(auto eagleEyeLevel = winnerHero->valOfBonuses(BonusType::LEARN_BATTLE_SPELL_LEVEL_LIMIT))
+		{
+			resultsApplied.learnedSpells.learn = 1;
+			resultsApplied.learnedSpells.hid = finishingBattle->winnerId;
+			for(const auto & spellId : (*battle)->getUsedSpells(CBattleInfoEssentials::otherSide(result.winner)))
 			{
-				iw.text.replaceName(*it);
-				if(i == spells.spells.size() - 2) //we just added pre-last name
-					iw.text.replaceLocalString(EMetaText::GENERAL_TXT, 141); // " and "
-				iw.components.emplace_back(ComponentType::SPELL, *it);
+				const auto spell = spellId.toEntity(LIBRARY->spells());
+				if(spell
+					&& spell->getLevel() <= eagleEyeLevel
+					&& !winnerHero->spellbookContainsSpell(spell->getId())
+					&& gameHandler->getRandomGenerator().nextInt(99) < winnerHero->valOfBonuses(BonusType::LEARN_BATTLE_SPELL_CHANCE))
+				{
+					resultsApplied.learnedSpells.spells.insert(spell->getId());
+				}
 			}
-			gameHandler->sendAndApply(iw);
-			gameHandler->sendAndApply(spells);
 		}
 	}
+
 	// Artifacts handling
-	if(result == EBattleResult::NORMAL && !finishingBattle->isDraw() && finishingBattle->winnerHero)
+	if(result.result == EBattleResult::NORMAL && !finishingBattle->isDraw() && winnerHero)
 	{
-		std::vector<const CArtifactInstance*> arts; // display them in window
-		CArtifactFittingSet artFittingSet(*finishingBattle->winnerHero);
-
-		const auto addArtifactToTransfer = [&artFittingSet, &arts](BulkMoveArtifacts & pack, const ArtifactPosition & srcSlot, const CArtifactInstance * art)
+		CArtifactFittingSet artFittingSet(*winnerHero);
+		const auto addArtifactToTransfer = [&artFittingSet](BulkMoveArtifacts & pack, const ArtifactPosition & srcSlot, const CArtifactInstance * art)
 		{
 			assert(art);
 			const auto dstSlot = ArtifactUtils::getArtAnyPosition(&artFittingSet, art->getTypeId());
 			if(dstSlot != ArtifactPosition::PRE_FIRST)
 			{
-				pack.artsPack0.emplace_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
+				pack.artsPack0.emplace_back(MoveArtifactInfo(srcSlot, dstSlot));
 				if(ArtifactUtils::isSlotEquipment(dstSlot))
 					pack.artsPack0.back().askAssemble = true;
-				arts.emplace_back(art);
 				artFittingSet.putArtifact(dstSlot, const_cast<CArtifactInstance*>(art));
 			}
 		};
-		const auto sendArtifacts = [this](BulkMoveArtifacts & bma)
-		{
-			if(!bma.artsPack0.empty())
-				gameHandler->sendAndApply(bma);
-		};
 
-		BulkMoveArtifacts packHero(finishingBattle->winnerHero->getOwner(), ObjectInstanceID::NONE, finishingBattle->winnerHero->id, false);
-		if(finishingBattle->loserHero)
+		if(loserHero)
 		{
-			packHero.srcArtHolder = finishingBattle->loserHero->id;
+			auto & packHero = resultsApplied.artifacts.emplace_back(finishingBattle->victor, finishingBattle->loserId, finishingBattle->winnerId, false);
+			packHero.srcArtHolder = finishingBattle->loserId;
 			for(const auto & slot : ArtifactUtils::commonWornSlots())
 			{
-				if(const auto artSlot = finishingBattle->loserHero->artifactsWorn.find(slot);
-					artSlot != finishingBattle->loserHero->artifactsWorn.end() && ArtifactUtils::isArtRemovable(*artSlot))
+				if(const auto artSlot = loserHero->artifactsWorn.find(slot); artSlot != loserHero->artifactsWorn.end() && ArtifactUtils::isArtRemovable(*artSlot))
 				{
 					addArtifactToTransfer(packHero, artSlot->first, artSlot->second.getArt());
 				}
 			}
-			for(const auto & artSlot : finishingBattle->loserHero->artifactsInBackpack)
+			for(const auto & artSlot : loserHero->artifactsInBackpack)
 			{
 				if(const auto art = artSlot.getArt(); art->getTypeId() != ArtifactID::GRAIL)
-					addArtifactToTransfer(packHero, finishingBattle->loserHero->getArtPos(art), art);
+					addArtifactToTransfer(packHero, loserHero->getArtPos(art), art);
 			}
 
-			if(finishingBattle->loserHero->commander)
+			if(loserHero->commander)
 			{
-				BulkMoveArtifacts packCommander(finishingBattle->winnerHero->getOwner(), finishingBattle->loserHero->id, finishingBattle->winnerHero->id, false);
-				packCommander.srcCreature = finishingBattle->loserHero->findStack(finishingBattle->loserHero->commander);
-				for(const auto & artSlot : finishingBattle->loserHero->commander->artifactsWorn)
+				auto & packCommander = resultsApplied.artifacts.emplace_back(finishingBattle->victor, finishingBattle->loserId, finishingBattle->winnerId, false);
+				packCommander.srcCreature = loserHero->findStack(loserHero->commander);
+				for(const auto & artSlot : loserHero->commander->artifactsWorn)
 					addArtifactToTransfer(packCommander, artSlot.first, artSlot.second.getArt());
-				sendArtifacts(packCommander);
 			}
-			auto armyObj = battle.battleGetArmyObject(battle.otherSide(battleResult->winner));
+			auto armyObj = dynamic_cast<const CArmedInstance*>(gameHandler->getObj(finishingBattle->loserId));
 			for(const auto & armySlot : armyObj->stacks)
 			{
-				BulkMoveArtifacts packsArmy(finishingBattle->winnerHero->getOwner(), finishingBattle->loserHero->id, finishingBattle->winnerHero->id, false);
+				auto & packsArmy = resultsApplied.artifacts.emplace_back(finishingBattle->victor, finishingBattle->loserId, finishingBattle->winnerId, false);
 				packsArmy.srcArtHolder = armyObj->id;
 				packsArmy.srcCreature = armySlot.first;
 				for(const auto & artSlot : armySlot.second->artifactsWorn)
 					addArtifactToTransfer(packsArmy, artSlot.first, armySlot.second->getArt(artSlot.first));
-				sendArtifacts(packsArmy);
 			}
 		}
-		// Display loot
-		if(!arts.empty())
-		{
-			InfoWindow iw;
-			iw.player = finishingBattle->winnerHero->tempOwner;
-			iw.text.appendLocalString(EMetaText::GENERAL_TXT, 30); //You have captured enemy artifact
-
-			for(const auto art : arts) //TODO; separate function to display loot for various objects?
-			{
-				if(art->isScroll())
-					iw.components.emplace_back(ComponentType::SPELL_SCROLL, art->getScrollSpellID());
-				else
-					iw.components.emplace_back(ComponentType::ARTIFACT, art->getTypeId());
-
-				if(iw.components.size() >= GameConstants::INFO_WINDOW_ARTIFACTS_MAX_ITEMS)
-				{
-					gameHandler->sendAndApply(iw);
-					iw.components.clear();
-				}
-			}
-			gameHandler->sendAndApply(iw);
-		}
-		if(!packHero.artsPack0.empty())
-			sendArtifacts(packHero);
-	}
-
-	// Remove beaten hero
-	if(finishingBattle->loserHero)
-	{
-		//add statistics
-		if(!finishingBattle->isDraw())
-		{
-			ConstTransitivePtr<CGHeroInstance> strongestHero = nullptr;
-			for(auto & hero : gameHandler->gameState()->getPlayerState(finishingBattle->loser)->getHeroes())
-				if(!strongestHero || hero->exp > strongestHero->exp)
-					strongestHero = hero;
-			if(strongestHero->id == finishingBattle->loserHero->id && strongestHero->level > 5)
-				gameHandler->gameState()->statistic.accumulatedValues[finishingBattle->victor].lastDefeatedStrongestHeroDay = gameHandler->gameState()->getDate(Date::DAY);
-		}
-
-		RemoveObject ro(finishingBattle->loserHero->id, finishingBattle->victor);
-		gameHandler->sendAndApply(ro);
-	}
-	// For draw case both heroes should be removed
-	if(finishingBattle->isDraw() && finishingBattle->winnerHero)
-	{
-		RemoveObject ro(finishingBattle->winnerHero->id, finishingBattle->loser);
-		gameHandler->sendAndApply(ro);
 	}
 
-	// add statistic
-	if(battle.sideToPlayer(BattleSide::ATTACKER) == PlayerColor::NEUTRAL || battle.sideToPlayer(BattleSide::DEFENDER) == PlayerColor::NEUTRAL)
-	{
-		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::ATTACKER)].numBattlesNeutral++;
-		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::DEFENDER)].numBattlesNeutral++;
-		if(!finishingBattle->isDraw())
-			gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(finishingBattle->winnerSide)].numWinBattlesNeutral++;
-	}
-	else
+	// Necromancy handling
+	// Give raised units to winner, if any were raised, units will be given after casualties are taken
+	if(winnerHero)
 	{
-		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::ATTACKER)].numBattlesPlayer++;
-		gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(BattleSide::DEFENDER)].numBattlesPlayer++;
-		if(!finishingBattle->isDraw())
-			gameHandler->gameState()->statistic.accumulatedValues[battle.sideToPlayer(finishingBattle->winnerSide)].numWinBattlesPlayer++;
+		resultsApplied.raisedStack = winnerHero->calculateNecromancy(result);
+		const SlotID necroSlot = resultsApplied.raisedStack.getCreature() ? winnerHero->getSlotFor(resultsApplied.raisedStack.getCreature()) : SlotID();
+		if(necroSlot != SlotID() && !finishingBattle->isDraw())
+			gameHandler->addToSlot(StackLocation(finishingBattle->winnerId, necroSlot), resultsApplied.raisedStack.getCreature(), resultsApplied.raisedStack.count);
 	}
 
-	BattleResultAccepted raccepted;
-	raccepted.battleID = battle.getBattle()->getBattleID();
-	raccepted.heroResult[BattleSide::ATTACKER].army = const_cast<CArmedInstance*>(battle.battleGetArmyObject(BattleSide::ATTACKER));
-	raccepted.heroResult[BattleSide::DEFENDER].army = const_cast<CArmedInstance*>(battle.battleGetArmyObject(BattleSide::DEFENDER));
-	raccepted.heroResult[BattleSide::ATTACKER].hero = const_cast<CGHeroInstance*>(battle.battleGetFightingHero(BattleSide::ATTACKER));
-	raccepted.heroResult[BattleSide::DEFENDER].hero = const_cast<CGHeroInstance*>(battle.battleGetFightingHero(BattleSide::DEFENDER));
-	raccepted.heroResult[BattleSide::ATTACKER].exp = battleResult->exp[BattleSide::ATTACKER];
-	raccepted.heroResult[BattleSide::DEFENDER].exp = battleResult->exp[BattleSide::DEFENDER];
-	raccepted.winnerSide = finishingBattle->winnerSide;
-	gameHandler->sendAndApply(raccepted);
-
-	gameHandler->queries->popIfTop(battleQuery);
-	//--> continuation (battleAfterLevelUp) occurs after level-up gameHandler->queries are handled or on removing query
-}
-
-void BattleResultProcessor::battleAfterLevelUp(const BattleID & battleID, const BattleResult & result)
-{
-	LOG_TRACE(logGlobal);
-
-	assert(finishingBattles.count(battleID) != 0);
-	if(finishingBattles.count(battleID) == 0)
-		return;
-
-	auto & finishingBattle = finishingBattles[battleID];
-
-	finishingBattle->remainingBattleQueriesCount--;
-	logGlobal->trace("Decremented gameHandler->queries count to %d", finishingBattle->remainingBattleQueriesCount);
-
-	if (finishingBattle->remainingBattleQueriesCount > 0)
-		//Battle results will be handled when all battle gameHandler->queries are closed
-		return;
-
-	//TODO consider if we really want it to work like above. ATM each player as unblocked as soon as possible
-	// but the battle consequences are applied after final player is unblocked. Hard to abuse...
-	// Still, it looks like a hole.
-
-	// Necromancy if applicable.
-	const CStackBasicDescriptor raisedStack = finishingBattle->winnerHero ? finishingBattle->winnerHero->calculateNecromancy(result) : CStackBasicDescriptor();
-	// Give raised units to winner and show dialog, if any were raised,
-	// units will be given after casualties are taken
-	const SlotID necroSlot = raisedStack.getCreature() ? finishingBattle->winnerHero->getSlotFor(raisedStack.getCreature()) : SlotID();
-
-	if (necroSlot != SlotID() && !finishingBattle->isDraw())
-	{
-		finishingBattle->winnerHero->showNecromancyDialog(raisedStack, gameHandler->getRandomGenerator());
-		gameHandler->addToSlot(StackLocation(finishingBattle->winnerHero->id, necroSlot), raisedStack.getCreature(), raisedStack.count);
-	}
-
-	BattleResultsApplied resultsApplied;
 	resultsApplied.battleID = battleID;
-	resultsApplied.player1 = finishingBattle->victor;
-	resultsApplied.player2 = finishingBattle->loser;
+	resultsApplied.victor = finishingBattle->victor;
+	resultsApplied.loser = finishingBattle->loser;
 	gameHandler->sendAndApply(resultsApplied);
 
 	//handle victory/loss of engaged players
-	std::set<PlayerColor> playerColors = {finishingBattle->loser, finishingBattle->victor};
-	gameHandler->checkVictoryLossConditions(playerColors);
+	gameHandler->checkVictoryLossConditions({finishingBattle->loser, finishingBattle->victor});
 
 	if (result.result == EBattleResult::SURRENDER)
 	{
 		gameHandler->gameState()->statistic.accumulatedValues[finishingBattle->loser].numHeroSurrendered++;
-		gameHandler->heroPool->onHeroSurrendered(finishingBattle->loser, finishingBattle->loserHero);
+		gameHandler->heroPool->onHeroSurrendered(finishingBattle->loser, loserHero);
 	}
 
 	if (result.result == EBattleResult::ESCAPE)
 	{
 		gameHandler->gameState()->statistic.accumulatedValues[finishingBattle->loser].numHeroEscaped++;
-		gameHandler->heroPool->onHeroEscaped(finishingBattle->loser, finishingBattle->loserHero);
+		gameHandler->heroPool->onHeroEscaped(finishingBattle->loser, loserHero);
 	}
 
-	if (result.winner != BattleSide::NONE && finishingBattle->winnerHero && finishingBattle->winnerHero->stacks.empty()
-		&& (!finishingBattle->winnerHero->commander || !finishingBattle->winnerHero->commander->alive))
+	// Remove beaten hero
+	if(loserHero)
 	{
-		RemoveObject ro(finishingBattle->winnerHero->id, finishingBattle->winnerHero->getOwner());
+		RemoveObject ro(loserHero->id, finishingBattle->victor);
 		gameHandler->sendAndApply(ro);
-
-		if (gameHandler->getSettings().getBoolean(EGameSettings::HEROES_RETREAT_ON_WIN_WITHOUT_TROOPS))
-			gameHandler->heroPool->onHeroEscaped(finishingBattle->victor, finishingBattle->winnerHero);
+	}
+	// For draw case both heroes should be removed
+	if(finishingBattle->isDraw() && winnerHero)
+	{
+		RemoveObject ro(winnerHero->id, finishingBattle->loser);
+		gameHandler->sendAndApply(ro);
+		if(gameHandler->getSettings().getBoolean(EGameSettings::HEROES_RETREAT_ON_WIN_WITHOUT_TROOPS))
+			gameHandler->heroPool->onHeroEscaped(finishingBattle->victor, winnerHero);
 	}
 
 	finishingBattles.erase(battleID);

server/battles/BattleResultProcessor.h

@@ -46,7 +46,8 @@ struct FinishingBattleHelper
 
 	inline bool isDraw() const {return winnerSide == BattleSide::NONE;}
 
-	const CGHeroInstance *winnerHero, *loserHero;
+	ObjectInstanceID winnerId;
+	ObjectInstanceID loserId;
 	PlayerColor victor, loser;
 	BattleSide winnerSide;
 
@@ -54,8 +55,8 @@ struct FinishingBattleHelper
 
 	template <typename Handler> void serialize(Handler &h)
 	{
-		h & winnerHero;
-		h & loserHero;
+		h & winnerId;
+		h & loserId;
 		h & victor;
 		h & loser;
 		h & winnerSide;
@@ -79,5 +80,5 @@ public:
 	void setBattleResult(const CBattleInfoCallback & battle, EBattleResult resultType, BattleSide victoriusSide);
 	void endBattle(const CBattleInfoCallback & battle); //ends battle
 	void endBattleConfirm(const CBattleInfoCallback & battle);
-	void battleAfterLevelUp(const BattleID & battleID, const BattleResult & result);
+	void battleFinalize(const BattleID & battleID, const BattleResult & result);
 };

server/queries/BattleQueries.cpp

@@ -65,7 +65,7 @@ void CBattleQuery::onRemoval(PlayerColor color)
 	assert(result);
 
 	if(result)
-		gh->battles->battleAfterLevelUp(battleID, *result);
+		gh->battles->battleFinalize(battleID, *result);
 }
 
 void CBattleQuery::onExposure(QueryPtr topQuery)

test/mock/mock_IGameCallback.h

@@ -68,7 +68,7 @@ public:
 	void tryJoiningArmy(const CArmedInstance *src, const CArmedInstance *dst, bool removeObjWhenFinished, bool allowMerging) override {} //merges army from src do dst or opens a garrison window
 	bool moveStack(const StackLocation &src, const StackLocation &dst, TQuantity count) override {return false;}
 
-	void removeAfterVisit(const CGObjectInstance *object) override {} //object will be destroyed when interaction is over. Do not call when interaction is not ongoing!
+	void removeAfterVisit(const ObjectInstanceID & id) override {} //object will be destroyed when interaction is over. Do not call when interaction is not ongoing!
 
 	bool giveHeroNewArtifact(const CGHeroInstance * h, const ArtifactID & artId, const ArtifactPosition & pos) override {return false;}
 	bool giveHeroNewScroll(const CGHeroInstance * h, const SpellID & spellId, const ArtifactPosition & pos) override {return false;}