Merge remote-tracking branch 'upstream/develop' into battle-dialog

AI/Nullkiller/Analyzers/BuildAnalyzer.cpp

@@ -334,7 +334,7 @@ BuildingInfo::BuildingInfo()
 	buildCost = 0;
 	buildCostWithPrerequisits = 0;
 	prerequisitesCount = 0;
-	name = "";
+	name.clear();
 	armyStrength = 0;
 }
 

AI/Nullkiller/Behaviors/BuyArmyBehavior.cpp

@@ -57,8 +57,7 @@ Goals::TGoalVec BuyArmyBehavior::decompose() const
 				continue;
 			}
 
-			if(ai->nullkiller->heroManager->getHeroRole(targetHero) == HeroRole::MAIN
-				&& targetHero->getArmyStrength() >= 300)
+			if(ai->nullkiller->heroManager->getHeroRole(targetHero) == HeroRole::MAIN)
 			{
 				auto reinforcement = ai->nullkiller->armyManager->howManyReinforcementsCanGet(
 					targetHero,

CCallback.cpp

@@ -181,6 +181,12 @@ bool CCallback::assembleArtifacts (const CGHeroInstance * hero, ArtifactPosition
 	return true;
 }
 
+void CCallback::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap)
+{
+	BulkExchangeArtifacts bma(srcHero, dstHero, swap);
+	sendRequest(&bma);
+}
+
 bool CCallback::buildBuilding(const CGTownInstance *town, BuildingID buildingID)
 {
 	if(town->tempOwner!=player)

CCallback.h

@@ -94,6 +94,10 @@ public:
 	virtual int bulkSplitStack(ObjectInstanceID armyId, SlotID srcSlot, int howMany = 1) = 0;
 	virtual int bulkSmartSplitStack(ObjectInstanceID armyId, SlotID srcSlot) = 0;
 	virtual int bulkMergeStacks(ObjectInstanceID armyId, SlotID srcSlot) = 0;
+	
+	
+	// Moves all artifacts from one hero to another
+	virtual void bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap) = 0;
 };
 
 class CBattleCallback : public IBattleCallback, public CPlayerBattleCallback
@@ -151,6 +155,7 @@ public:
 	bool dismissHero(const CGHeroInstance * hero) override;
 	bool swapArtifacts(const ArtifactLocation &l1, const ArtifactLocation &l2) override;
 	bool assembleArtifacts(const CGHeroInstance * hero, ArtifactPosition artifactSlot, bool assemble, ArtifactID assembleTo) override;
+	void bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap) override;
 	bool buildBuilding(const CGTownInstance *town, BuildingID buildingID) override;
 	void recruitCreatures(const CGDwelling * obj, const CArmedInstance * dst, CreatureID ID, ui32 amount, si32 level=-1) override;
 	bool dismissCreature(const CArmedInstance *obj, SlotID stackPos) override;

client/CMusicHandler.cpp

@@ -9,6 +9,7 @@
  */
 #include "StdInc.h"
 #include <SDL_mixer.h>
+#include <SDL.h>
 
 #include "CMusicHandler.h"
 #include "CGameInfo.h"
@@ -410,15 +411,15 @@ void CMusicHandler::release()
 	CAudioBase::release();
 }
 
-void CMusicHandler::playMusic(const std::string & musicURI, bool loop)
+void CMusicHandler::playMusic(const std::string & musicURI, bool loop, bool fromStart)
 {
 	if (current && current->isTrack(musicURI))
 		return;
 
-	queueNext(this, "", musicURI, loop);
+	queueNext(this, "", musicURI, loop, fromStart);
 }
 
-void CMusicHandler::playMusicFromSet(const std::string & whichSet, bool loop)
+void CMusicHandler::playMusicFromSet(const std::string & whichSet, bool loop, bool fromStart)
 {
 	auto selectedSet = musicsSet.find(whichSet);
 	if (selectedSet == musicsSet.end())
@@ -431,10 +432,10 @@ void CMusicHandler::playMusicFromSet(const std::string & whichSet, bool loop)
 		return;
 
 	// in this mode - play random track from set
-	queueNext(this, whichSet, "", loop);
+	queueNext(this, whichSet, "", loop, fromStart);
 }
 
-void CMusicHandler::playMusicFromSet(const std::string & whichSet, const std::string & entryID, bool loop)
+void CMusicHandler::playMusicFromSet(const std::string & whichSet, const std::string & entryID, bool loop,  bool fromStart)
 {
 	auto selectedSet = musicsSet.find(whichSet);
 	if (selectedSet == musicsSet.end())
@@ -454,7 +455,7 @@ void CMusicHandler::playMusicFromSet(const std::string & whichSet, const std::st
 		return;
 
 	// in this mode - play specific track from set
-	queueNext(this, "", selectedEntry->second, loop);
+	queueNext(this, "", selectedEntry->second, loop, fromStart);
 }
 
 void CMusicHandler::queueNext(std::unique_ptr<MusicEntry> queued)
@@ -473,11 +474,11 @@ void CMusicHandler::queueNext(std::unique_ptr<MusicEntry> queued)
 	}
 }
 
-void CMusicHandler::queueNext(CMusicHandler *owner, const std::string & setName, const std::string & musicURI, bool looped)
+void CMusicHandler::queueNext(CMusicHandler *owner, const std::string & setName, const std::string & musicURI, bool looped, bool fromStart)
 {
 	try
 	{
-		queueNext(make_unique<MusicEntry>(owner, setName, musicURI, looped));
+		queueNext(make_unique<MusicEntry>(owner, setName, musicURI, looped, fromStart));
 	}
 	catch(std::exception &e)
 	{
@@ -526,10 +527,13 @@ void CMusicHandler::musicFinishedCallback()
 	}
 }
 
-MusicEntry::MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped):
+MusicEntry::MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped, bool fromStart):
 	owner(owner),
 	music(nullptr),
+	startTime(uint32_t(-1)),
+	startPosition(0),
 	loop(looped ? -1 : 1),
+	fromStart(fromStart),
 	setName(std::move(setName))
 {
 	if (!musicURI.empty())
@@ -578,11 +582,25 @@ bool MusicEntry::play()
 	}
 
 	logGlobal->trace("Playing music file %s", currentName);
-	if(Mix_PlayMusic(music, 1) == -1)
+
+	if ( !fromStart && owner->trackPositions.count(currentName) > 0 && owner->trackPositions[currentName] > 0)
+	{
+		float timeToStart = owner->trackPositions[currentName];
+		startPosition = std::round(timeToStart * 1000);
+
+		if (Mix_FadeInMusicPos(music, 1, 1000, timeToStart) == -1)
+		{
+			logGlobal->error("Unable to play music (%s)", Mix_GetError());
+			return false;
+		}
+	}
+	else if(Mix_PlayMusic(music, 1) == -1)
 	{
 		logGlobal->error("Unable to play music (%s)", Mix_GetError());
 		return false;
 	}
+
+	startTime = SDL_GetTicks();
 	return true;
 }
 
@@ -590,8 +608,13 @@ bool MusicEntry::stop(int fade_ms)
 {
 	if (Mix_PlayingMusic())
 	{
-		logGlobal->trace("Stopping music file %s", currentName);
 		loop = 0;
+		uint32_t endTime = SDL_GetTicks();
+		assert(startTime != uint32_t(-1));
+		float playDuration = (endTime - startTime + startPosition) / 1000.f;
+		owner->trackPositions[currentName] = playDuration;
+		logGlobal->info("Stopping music file %s at %f", currentName, playDuration);
+
 		Mix_FadeOutMusic(fade_ms);
 		return true;
 	}

client/CMusicHandler.h

@@ -99,6 +99,9 @@ class MusicEntry
 	Mix_Music *music;
 
 	int loop; // -1 = indefinite
+	bool fromStart;
+	uint32_t startTime;
+	uint32_t startPosition;
 	//if not null - set from which music will be randomly selected
 	std::string setName;
 	std::string currentName;
@@ -110,7 +113,7 @@ public:
 	bool isSet(std::string setName);
 	bool isTrack(std::string trackName);
 
-	MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped);
+	MusicEntry(CMusicHandler *owner, std::string setName, std::string musicURI, bool looped, bool fromStart);
 	~MusicEntry();
 
 	bool play();
@@ -128,10 +131,11 @@ private:
 	std::unique_ptr<MusicEntry> current;
 	std::unique_ptr<MusicEntry> next;
 
-	void queueNext(CMusicHandler *owner, const std::string & setName, const std::string & musicURI, bool looped);
+	void queueNext(CMusicHandler *owner, const std::string & setName, const std::string & musicURI, bool looped, bool fromStart);
 	void queueNext(std::unique_ptr<MusicEntry> queued);
 
 	std::map<std::string, std::map<std::string, std::string>> musicsSet;
+	std::map<std::string, float> trackPositions;
 public:
 	
 	CMusicHandler();
@@ -145,11 +149,11 @@ public:
 	void setVolume(ui32 percent) override;
 
 	/// play track by URI, if loop = true music will be looped
-	void playMusic(const std::string & musicURI, bool loop);
+	void playMusic(const std::string & musicURI, bool loop, bool fromStart);
 	/// play random track from this set
-	void playMusicFromSet(const std::string & musicSet, bool loop);
+	void playMusicFromSet(const std::string & musicSet, bool loop, bool fromStart);
 	/// play specific track from set
-	void playMusicFromSet(const std::string & musicSet, const std::string & entryID, bool loop);
+	void playMusicFromSet(const std::string & musicSet, const std::string & entryID, bool loop, bool fromStart);
 	void stopMusic(int fade_ms=1000);
 	void musicFinishedCallback();
 

client/CPlayerInterface.cpp

@@ -276,7 +276,7 @@ void CPlayerInterface::heroMoved(const TryMoveHero & details, bool verbose)
 	{
 		updateAmbientSounds();
 		//We may need to change music - select new track, music handler will change it if needed
-		CCS->musich->playMusicFromSet("terrain", LOCPLINT->cb->getTile(hero->visitablePos())->terType->name, true);
+		CCS->musich->playMusicFromSet("terrain", LOCPLINT->cb->getTile(hero->visitablePos())->terType->name, true, false);
 
 		if(details.result == TryMoveHero::TELEPORTATION)
 		{
@@ -2348,10 +2348,13 @@ void CPlayerInterface::acceptTurn()
 		while(CInfoWindow *iw = dynamic_cast<CInfoWindow *>(GH.topInt().get()))
 			iw->close();
 	}
-	waitWhileDialog();
 
 	if(CSH->howManyPlayerInterfaces() > 1)
+	{
+		waitWhileDialog(); // wait for player to accept turn in hot-seat mode
+
 		adventureInt->startTurn();
+	}
 
 	adventureInt->heroList.update();
 	adventureInt->townList.update();
@@ -2601,6 +2604,12 @@ void CPlayerInterface::artifactMoved(const ArtifactLocation &src, const Artifact
 		if (artWin)
 			artWin->artifactMoved(src, dst);
 	}
+	if(!GH.objsToBlit.empty())
+		GH.objsToBlit.back()->redraw();
+}
+
+void CPlayerInterface::artifactPossibleAssembling(const ArtifactLocation & dst)
+{
 	askToAssembleArtifact(dst);
 }
 

client/CPlayerInterface.h

@@ -134,6 +134,7 @@ public:
 	void artifactRemoved(const ArtifactLocation &al) override;
 	void artifactMoved(const ArtifactLocation &src, const ArtifactLocation &dst) override;
 	void artifactAssembled(const ArtifactLocation &al) override;
+	void artifactPossibleAssembling(const ArtifactLocation & dst) override;
 	void artifactDisassembled(const ArtifactLocation &al) override;
 
 	void heroVisit(const CGHeroInstance * visitor, const CGObjectInstance * visitedObj, bool start) override;

client/CServerHandler.cpp

@@ -489,7 +489,7 @@ void CServerHandler::sendMessage(const std::string & txt) const
 		std::string connectedId, playerColorId;
 		readed >> connectedId;
 		readed >> playerColorId;
-		if(connectedId.length(), playerColorId.length()) // BUG https://bugs.vcmi.eu/view.php?id=3144
+		if(connectedId.length() && playerColorId.length())
 		{
 			ui8 connected = boost::lexical_cast<int>(connectedId);
 			auto color = PlayerColor(boost::lexical_cast<int>(playerColorId));

client/CVideoHandler.cpp

@@ -381,7 +381,7 @@ void CVideoPlayer::update( int x, int y, SDL_Surface *dst, bool forceRedraw, boo
 
 void CVideoPlayer::close()
 {
-	fname = "";
+	fname.clear();
 	if (sws)
 	{
 		sws_freeContext(sws);

client/NetPacksClient.cpp

@@ -275,8 +275,31 @@ void EraseArtifact::applyCl(CClient *cl)
 void MoveArtifact::applyCl(CClient *cl)
 {
 	callInterfaceIfPresent(cl, src.owningPlayer(), &IGameEventsReceiver::artifactMoved, src, dst);
+	callInterfaceIfPresent(cl, src.owningPlayer(), &IGameEventsReceiver::artifactPossibleAssembling, dst);
 	if(src.owningPlayer() != dst.owningPlayer())
+	{
 		callInterfaceIfPresent(cl, dst.owningPlayer(), &IGameEventsReceiver::artifactMoved, src, dst);
+		callInterfaceIfPresent(cl, dst.owningPlayer(), &IGameEventsReceiver::artifactPossibleAssembling, dst);
+	}
+}
+
+void BulkMoveArtifacts::applyCl(CClient * cl)
+{
+	auto applyMove = [this, cl](std::vector<LinkedSlots> & artsPack) -> void
+	{
+		for(auto & slotToMove : artsPack)
+		{
+			auto srcLoc = ArtifactLocation(srcArtHolder, slotToMove.srcPos);
+			auto dstLoc = ArtifactLocation(dstArtHolder, slotToMove.dstPos);
+			callInterfaceIfPresent(cl, srcLoc.owningPlayer(), &IGameEventsReceiver::artifactMoved, srcLoc, dstLoc);
+			if(srcLoc.owningPlayer() != dstLoc.owningPlayer())
+				callInterfaceIfPresent(cl, dstLoc.owningPlayer(), &IGameEventsReceiver::artifactMoved, srcLoc, dstLoc);
+		}
+	};
+
+	applyMove(artsPack0);
+	if(swap)
+		applyMove(artsPack1);
 }
 
 void AssembledArtifact::applyCl(CClient *cl)

client/battle/CBattleInterface.cpp

@@ -411,7 +411,7 @@ CBattleInterface::CBattleInterface(const CCreatureSet *army1, const CCreatureSet
 	{
 		if(LOCPLINT->battleInt)
 		{
-			CCS->musich->playMusicFromSet("battle", true);
+			CCS->musich->playMusicFromSet("battle", true, true);
 			battleActionsStarted = true;
 			blockUI(settings["session"]["spectate"].Bool());
 			battleIntroSoundChannel = -1;
@@ -457,7 +457,7 @@ CBattleInterface::~CBattleInterface()
 	if (adventureInt && adventureInt->selection)
 	{
 		const auto & terrain = *(LOCPLINT->cb->getTile(adventureInt->selection->visitablePos())->terType);
-		CCS->musich->playMusicFromSet("terrain", terrain.name, true);
+		CCS->musich->playMusicFromSet("terrain", terrain.name, true, false);
 	}
 	animsAreDisplayed.setn(false);
 }

client/battle/CBattleInterfaceClasses.cpp

@@ -512,7 +512,7 @@ CBattleResultWindow::CBattleResultWindow(const BattleResult & br, CPlayerInterfa
 			break;
 		}
 
-		CCS->musich->playMusic("Music/Win Battle", false);
+		CCS->musich->playMusic("Music/Win Battle", false, true);
 		CCS->videoh->open("WIN3.BIK");
 		std::string str = CGI->generaltexth->allTexts[text];
 
@@ -549,7 +549,7 @@ CBattleResultWindow::CBattleResultWindow(const BattleResult & br, CPlayerInterfa
 			logGlobal->error("Invalid battle result code %d. Assumed normal.", static_cast<int>(br.result));
 			break;
 		}
-		CCS->musich->playMusic(musicName, false);
+		CCS->musich->playMusic(musicName, false, true);
 		CCS->videoh->open(videoName);
 
 		labels.push_back(std::make_shared<CLabel>(235, 235, FONT_SMALL, CENTER, Colors::WHITE, CGI->generaltexth->allTexts[text]));

client/mainmenu/CMainMenu.cpp

@@ -115,7 +115,7 @@ void CMenuScreen::show(SDL_Surface * to)
 
 void CMenuScreen::activate()
 {
-	CCS->musich->playMusic("Music/MainMenu", true);
+	CCS->musich->playMusic("Music/MainMenu", true, true);
 	if(!config["video"].isNull())
 		CCS->videoh->open(config["video"]["name"].String());
 	CIntObject::activate();

client/mainmenu/CPrologEpilogVideo.cpp

@@ -29,7 +29,7 @@ CPrologEpilogVideo::CPrologEpilogVideo(CCampaignScenario::SScenarioPrologEpilog
 	updateShadow();
 
 	CCS->videoh->open(CCampaignHandler::prologVideoName(spe.prologVideo));
-	CCS->musich->playMusic("Music/" + CCampaignHandler::prologMusicName(spe.prologMusic), true);
+	CCS->musich->playMusic("Music/" + CCampaignHandler::prologMusicName(spe.prologMusic), true, true);
 	// MPTODO: Custom campaign crashing on this?
 //	voiceSoundHandle = CCS->soundh->playSound(CCampaignHandler::prologVoiceName(spe.prologVideo));
 

client/widgets/AdventureMapClasses.cpp

@@ -1158,7 +1158,7 @@ void CInGameConsole::endEnteringText(bool printEnteredText)
 		previouslyEntered.push_back(txt);
 		//print(txt);
 	}
-	enteredText = "";
+	enteredText.clear();
 	if(GH.topInt() == adventureInt)
 	{
 		GH.statusbar->alignment = CENTER;

client/widgets/CArtifactHolder.cpp

@@ -367,7 +367,7 @@ void CHeroArtPlace::setArtifact(const CArtifactInstance *art)
 	if(!art)
 	{
 		image->disable();
-		text = std::string();
+		text.clear();
 		hoverText = CGI->generaltexth->allTexts[507];
 		return;
 	}
@@ -1034,7 +1034,7 @@ void CCommanderArtPlace::setArtifact(const CArtifactInstance * art)
 	if (!art)
 	{
 		image->disable();
-		text = std::string();
+		text.clear();
 		return;
 	}
 

client/widgets/TextControls.cpp

@@ -577,7 +577,7 @@ void CTextInput::textInputed(const SDL_TextInputEvent & event)
 		redraw();
 		cb(text);
 	}
-	newText = "";
+	newText.clear();
 
 #ifdef VCMI_ANDROID
 	notifyAndroidTextInputChanged(text);

client/windows/CAdvmapInterface.cpp

@@ -573,7 +573,7 @@ CAdvMapInt::CAdvMapInt():
 	strongInterest = true; // handle all mouse move events to prevent dead mouse move space in fullscreen mode
 	townList.onSelect = std::bind(&CAdvMapInt::selectionChanged,this);
 	bg = BitmapHandler::loadBitmap(ADVOPT.mainGraphic);
-	if (ADVOPT.worldViewGraphic != "")
+	if(!ADVOPT.worldViewGraphic.empty())
 	{
 		bgWorldView = BitmapHandler::loadBitmap(ADVOPT.worldViewGraphic);
 	}
@@ -1413,7 +1413,7 @@ void CAdvMapInt::select(const CArmedInstance *sel, bool centerView)
 		auto pos = sel->visitablePos();
 		auto tile = LOCPLINT->cb->getTile(pos);
 		if(tile)
-			CCS->musich->playMusicFromSet("terrain", tile->terType->name, true);
+			CCS->musich->playMusicFromSet("terrain", tile->terType->name, true, false);
 	}
 	if(centerView)
 		centerOn(sel);
@@ -1863,7 +1863,7 @@ void CAdvMapInt::aiTurnStarted()
 		return;
 
 	adjustActiveness(true);
-	CCS->musich->playMusicFromSet("enemy-turn", true);
+	CCS->musich->playMusicFromSet("enemy-turn", true, false);
 	adventureInt->minimap.setAIRadar(true);
 	adventureInt->infoBar.startEnemyTurn(LOCPLINT->cb->getCurrentPlayer());
 	adventureInt->infoBar.showAll(screen);//force refresh on inactive object

client/windows/CCastleInterface.cpp

@@ -1171,7 +1171,7 @@ CCastleInterface::CCastleInterface(const CGTownInstance * Town, const CGTownInst
 	townlist->onSelect = std::bind(&CCastleInterface::townChange, this);
 
 	recreateIcons();
-	CCS->musich->playMusic(town->town->clientInfo.musicTheme, true);
+	CCS->musich->playMusic(town->town->clientInfo.musicTheme, true, false);
 }
 
 CCastleInterface::~CCastleInterface()

client/windows/CSpellWindow.cpp

@@ -593,7 +593,7 @@ void CSpellWindow::SpellArea::clickRight(tribool down, bool previousState)
 		std::string dmgInfo;
 		auto causedDmg = owner->myInt->cb->estimateSpellDamage(mySpell, owner->myHero);
 		if(causedDmg == 0 || mySpell->id == SpellID::TITANS_LIGHTNING_BOLT) //Titan's Lightning Bolt already has damage info included
-			dmgInfo = "";
+			dmgInfo.clear();
 		else
 		{
 			dmgInfo = CGI->generaltexth->allTexts[343];

client/windows/CTradeWindow.cpp

@@ -192,7 +192,7 @@ void CTradeWindow::CTradeableItem::clickLeft(tribool down, bool previousState)
 
 				aw->arts->artifactsOnAltar.erase(art);
 				setID(-1);
-				subtitle = "";
+				subtitle.clear();
 				aw->deal->block(!aw->arts->artifactsOnAltar.size());
 			}
 

client/windows/GUIClasses.cpp

@@ -872,41 +872,6 @@ std::function<void()> CExchangeController::onMoveArmyToRight()
 	return [&]() { moveArmy(true); };
 }
 
-void CExchangeController::swapArtifacts(ArtifactPosition slot)
-{
-	bool leftHasArt = !left->isPositionFree(slot);
-	bool rightHasArt = !right->isPositionFree(slot);
-
-	if(!leftHasArt && !rightHasArt)
-		return;
-
-	ArtifactLocation leftLocation = ArtifactLocation(left, slot);
-	ArtifactLocation rightLocation = ArtifactLocation(right, slot);
-
-	if(leftHasArt && !left->artifactsWorn.at(slot).artifact->canBePutAt(rightLocation, true))
-		return;
-
-	if(rightHasArt && !right->artifactsWorn.at(slot).artifact->canBePutAt(leftLocation, true))
-		return;
-
-	if(leftHasArt)
-	{
-		if(rightHasArt)
-		{
-			auto art = right->getArt(slot);
-
-			cb->swapArtifacts(leftLocation, rightLocation);
-			cb->swapArtifacts(ArtifactLocation(right, right->getArtPos(art)), leftLocation);
-		}
-		else
-			cb->swapArtifacts(leftLocation, rightLocation);
-	}
-	else
-	{
-		cb->swapArtifacts(rightLocation, leftLocation);
-	}
-}
-
 std::vector<CArtifactInstance *> getBackpackArts(const CGHeroInstance * hero)
 {
 	std::vector<CArtifactInstance *> result;
@@ -919,92 +884,13 @@ std::vector<CArtifactInstance *> getBackpackArts(const CGHeroInstance * hero)
 	return result;
 }
 
-const std::vector<ArtifactPosition> unmovablePositions = {ArtifactPosition::SPELLBOOK, ArtifactPosition::MACH4};
-
-bool isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot)
-{
-	return slot.second.artifact
-		&& !slot.second.locked
-		&& !vstd::contains(unmovablePositions, slot.first);
-}
-
-// Puts all composite arts to backpack and returns their previous location
-std::vector<HeroArtifact> CExchangeController::moveCompositeArtsToBackpack()
-{
-	std::vector<const CGHeroInstance *> sides = {left, right};
-	std::vector<HeroArtifact> artPositions;
-
-	for(auto hero : sides)
-	{
-		for(int i = ArtifactPosition::HEAD; i < ArtifactPosition::AFTER_LAST; i++)
-		{
-			auto artPosition = ArtifactPosition(i);
-			auto art = hero->getArt(artPosition);
-
-			if(art && art->canBeDisassembled())
-			{
-				cb->swapArtifacts(
-					ArtifactLocation(hero, artPosition),
-					ArtifactLocation(hero, ArtifactPosition(GameConstants::BACKPACK_START)));
-
-				artPositions.push_back(HeroArtifact(hero, art, artPosition));
-			}
-		}
-	}
-
-	return artPositions;
-}
-
-void CExchangeController::swapArtifacts()
-{
-	for(int i = ArtifactPosition::HEAD; i < ArtifactPosition::AFTER_LAST; i++)
-	{
-		if(vstd::contains(unmovablePositions, i))
-			continue;
-
-		swapArtifacts(ArtifactPosition(i));
-	}
-
-	auto leftHeroBackpack = getBackpackArts(left);
-	auto rightHeroBackpack = getBackpackArts(right);
-
-	for(auto leftArt : leftHeroBackpack)
-	{
-		cb->swapArtifacts(
-			ArtifactLocation(left, left->getArtPos(leftArt)),
-			ArtifactLocation(right, ArtifactPosition(GameConstants::BACKPACK_START)));
-	}
-
-	for(auto rightArt : rightHeroBackpack)
-	{
-		cb->swapArtifacts(
-			ArtifactLocation(right, right->getArtPos(rightArt)),
-			ArtifactLocation(left, ArtifactPosition(GameConstants::BACKPACK_START)));
-	}
-}
-
 std::function<void()> CExchangeController::onSwapArtifacts()
 {
 	return [&]()
 	{
 		GsThread::run([=]
 		{
-			// it is not possible directly exchange composite artifacts like Angelic Alliance and Armor of Damned
-			auto compositeArtLocations = moveCompositeArtsToBackpack();
-
-			swapArtifacts();
-
-			for(HeroArtifact artLocation : compositeArtLocations)
-			{
-				auto target = artLocation.hero == left ? right : left;
-				auto currentPos = target->getArtPos(artLocation.artifact);
-
-				cb->swapArtifacts(
-					ArtifactLocation(target, currentPos),
-					ArtifactLocation(target, artLocation.artPosition));
-			}
-
-			view->redraw();
+			cb->bulkMoveArtifacts(left->id, right->id, true);
 		});
 	};
 }
@@ -1160,20 +1046,8 @@ void CExchangeController::moveArtifacts(bool leftToRight)
 	}
 
 	GsThread::run([=]
-	{	
-		while(vstd::contains_if(source->artifactsWorn, isArtRemovable))
-		{
-			auto art = std::find_if(source->artifactsWorn.begin(), source->artifactsWorn.end(), isArtRemovable);
-
-			moveArtifact(source, target, art->first);
-		}
-
-		while(!source->artifactsInBackpack.empty())
-		{
-			moveArtifact(source, target, source->getArtPos(source->artifactsInBackpack.begin()->artifact));
-		}
-
-		view->redraw();
+	{
+		cb->bulkMoveArtifacts(source->id, target->id, false);
 	});
 }
 
@@ -1182,26 +1056,11 @@ void CExchangeController::moveArtifact(
 	const CGHeroInstance * target,
 	ArtifactPosition srcPosition)
 {
-	auto artifact = source->getArt(srcPosition);
 	auto srcLocation = ArtifactLocation(source, srcPosition);
+	auto dstLocation = ArtifactLocation(target,
+		ArtifactUtils::getArtifactDstPosition(source->getArt(srcPosition), target, target->bearerType()));
 
-	for(auto slot : artifact->artType->possibleSlots.at(target->bearerType()))
-	{
-		auto existingArtifact = target->getArt(slot);
-		auto existingArtInfo = target->getSlot(slot);
-		ArtifactLocation destLocation(target, slot);
-
-		if(!existingArtifact
-			&& (!existingArtInfo || !existingArtInfo->locked)
-			&& artifact->canBePutAt(destLocation))
-		{
-			cb->swapArtifacts(srcLocation, ArtifactLocation(target, slot));
-			
-			return;
-		}
-	}
-
-	cb->swapArtifacts(srcLocation, ArtifactLocation(target, ArtifactPosition(GameConstants::BACKPACK_START)));
+	cb->swapArtifacts(srcLocation, dstLocation);
 }
 
 CExchangeWindow::CExchangeWindow(ObjectInstanceID hero1, ObjectInstanceID hero2, QueryID queryID)
@@ -1700,9 +1559,7 @@ int CUniversityWindow::CItem::state()
 {
 	if(parent->hero->getSecSkillLevel(SecondarySkill(ID)))//hero know this skill
 		return 1;
-	if(!parent->hero->canLearnSkill())//can't learn more skills
-		return 0;
-	if(parent->hero->type->heroClass->secSkillProbability[ID]==0)//can't learn this skill (like necromancy for most of non-necros)
+	if(!parent->hero->canLearnSkill(SecondarySkill(ID)))//can't learn more skills
 		return 0;
 	return 2;
 }

client/windows/GUIClasses.h

@@ -324,9 +324,6 @@ private:
 	void moveArtifacts(bool leftToRight);
 	void moveArtifact(const CGHeroInstance * source, const CGHeroInstance * target, ArtifactPosition srcPosition);
 	void moveStack(const CGHeroInstance * source, const CGHeroInstance * target, SlotID sourceSlot);
-	void swapArtifacts(ArtifactPosition artPosition);
-	std::vector<HeroArtifact> moveCompositeArtsToBackpack();
-	void swapArtifacts();
 };
 
 class CExchangeWindow : public CStatusbarWindow, public CGarrisonHolder, public CWindowWithArtifacts

lib/CArtHandler.cpp

@@ -352,7 +352,7 @@ CArtifact * CArtHandler::loadFromJson(const std::string & scope, const JsonNode
 	}
 
 	const JsonNode & warMachine = node["warMachine"];
-	if(warMachine.getType() == JsonNode::JsonType::DATA_STRING && warMachine.String() != "")
+	if(warMachine.getType() == JsonNode::JsonType::DATA_STRING && !warMachine.String().empty())
 	{
 		VLC->modh->identifiers.requestIdentifier("creature", warMachine, [=](si32 id)
 		{
@@ -1457,4 +1457,83 @@ void CArtifactSet::serializeJsonSlot(JsonSerializeFormat & handler, const Artifa
 	}
 }
 
+CArtifactFittingSet::CArtifactFittingSet(ArtBearer::ArtBearer Bearer)
+{
+	this->Bearer = Bearer;
+}
+
+void CArtifactFittingSet::setNewArtSlot(ArtifactPosition slot, CArtifactInstance * art, bool locked)
+{
+	ArtSlotInfo & asi = retrieveNewArtSlot(slot);
+	asi.artifact = art;
+	asi.locked = locked;
+}
+
+void CArtifactFittingSet::putArtifact(ArtifactPosition pos, CArtifactInstance * art)
+{
+	if(art && art->canBeDisassembled() && (pos < ArtifactPosition::AFTER_LAST))
+	{
+		for(auto & part : dynamic_cast<CCombinedArtifactInstance*>(art)->constituentsInfo)
+		{
+			// For the ArtFittingSet is no needed to do figureMainConstituent, just lock slots
+			this->setNewArtSlot(part.art->firstAvailableSlot(this), part.art, true);
+		}
+	}
+	else
+	{
+		this->setNewArtSlot(pos, art, false);
+	}
+}
+
+ArtBearer::ArtBearer CArtifactFittingSet::bearerType() const
+{
+	return this->Bearer;
+}
+
+DLL_LINKAGE ArtifactPosition ArtifactUtils::getArtifactDstPosition(	const CArtifactInstance * artifact,
+									const CArtifactSet * target, 
+									ArtBearer::ArtBearer bearer)
+{
+	for(auto slot : artifact->artType->possibleSlots.at(bearer))
+	{
+		auto existingArtifact = target->getArt(slot);
+		auto existingArtInfo = target->getSlot(slot);
+
+		if(!existingArtifact
+			&& (!existingArtInfo || !existingArtInfo->locked)
+			&& artifact->canBePutAt(target, slot))
+		{
+			return slot;
+		}
+	}
+	return ArtifactPosition(GameConstants::BACKPACK_START);
+}
+
+DLL_LINKAGE std::vector<ArtifactPosition> ArtifactUtils::unmovablePositions()
+{
+	return { ArtifactPosition::SPELLBOOK, ArtifactPosition::MACH4 };
+}
+
+DLL_LINKAGE bool ArtifactUtils::isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot)
+{
+	return slot.second.artifact
+		&& !slot.second.locked
+		&& !vstd::contains(unmovablePositions(), slot.first);
+}
+
+DLL_LINKAGE bool ArtifactUtils::checkSpellbookIsNeeded(const CGHeroInstance * heroPtr, ArtifactID artID, ArtifactPosition slot)
+{
+	// TODO what'll happen if Titan's thunder is equipped by pickin git up or the start of game?
+	// Titan's Thunder creates new spellbook on equip
+	if(artID == ArtifactID::TITANS_THUNDER && slot == ArtifactPosition::RIGHT_HAND)
+	{
+		if(heroPtr)
+		{
+			if(heroPtr && !heroPtr->hasSpellbook())
+				return true;
+		}
+	}
+	return false;
+}
+
 VCMI_LIB_NAMESPACE_END

lib/CArtHandler.h

@@ -303,8 +303,9 @@ struct DLL_LINKAGE ArtSlotInfo
 	ui8 locked; //if locked, then artifact points to the combined artifact
 
 	ArtSlotInfo() : locked(false) {}
+	const CArtifactInstance * getArt() const;
 
-	template <typename Handler> void serialize(Handler &h, const int version)
+	template <typename Handler> void serialize(Handler & h, const int version)
 	{
 		h & artifact;
 		h & locked;
@@ -363,4 +364,28 @@ private:
 	void serializeJsonSlot(JsonSerializeFormat & handler, const ArtifactPosition & slot, CMap * map);//normal slots
 };
 
+// Used to try on artifacts before the claimed changes have been applied
+class DLL_LINKAGE CArtifactFittingSet : public CArtifactSet
+{
+public:
+	CArtifactFittingSet(ArtBearer::ArtBearer Bearer);
+	void setNewArtSlot(ArtifactPosition slot, CArtifactInstance * art, bool locked);
+	void putArtifact(ArtifactPosition pos, CArtifactInstance * art) override;
+	ArtBearer::ArtBearer bearerType() const override;
+
+protected:
+	ArtBearer::ArtBearer Bearer;
+};
+
+namespace ArtifactUtils
+{
+	// Calculates where an artifact gets placed when it gets transferred from one hero to another.
+	DLL_LINKAGE ArtifactPosition getArtifactDstPosition(	const CArtifactInstance * artifact, 
+								const CArtifactSet * target,
+								ArtBearer::ArtBearer bearer);
+	DLL_LINKAGE std::vector<ArtifactPosition> unmovablePositions(); // TODO: Make this constexpr when the toolset is upgraded
+	DLL_LINKAGE bool isArtRemovable(const std::pair<ArtifactPosition, ArtSlotInfo> & slot);
+	DLL_LINKAGE bool checkSpellbookIsNeeded(const CGHeroInstance * heroPtr, ArtifactID artID, ArtifactPosition slot);
+}
+
 VCMI_LIB_NAMESPACE_END

lib/CBonusTypeHandler.cpp

@@ -94,7 +94,9 @@ std::string MacroString::build(const GetValue & getValue) const
 CBonusType::CBonusType()
 {
 	hidden = true;
-	icon = nameTemplate = descriptionTemplate = "";
+	icon.clear();
+	nameTemplate.clear();
+	descriptionTemplate.clear();
 }
 
 CBonusType::~CBonusType()

lib/CCreatureSet.cpp

@@ -1052,7 +1052,7 @@ void CStackBasicDescriptor::serializeJson(JsonSerializeFormat & handler)
 	{
 		std::string typeName("");
 		handler.serializeString("type", typeName);
-		if(typeName != "")
+		if(!typeName.empty())
 			setType(VLC->creh->getCreature("core", typeName));
 	}
 }

lib/CGameState.cpp

@@ -532,7 +532,7 @@ std::pair<Obj,int> CGameState::pickObject (CGObjectInstance *obj)
 			if (auto info = dynamic_cast<CCreGenAsCastleInfo*>(dwl->info))
 			{
 				faction = getRandomGenerator().nextInt((int)VLC->townh->size() - 1);
-				if(info->asCastle && info->instanceId != "")
+				if(info->asCastle && !info->instanceId.empty())
 				{
 					auto iter = map->instanceNames.find(info->instanceId);
 

lib/CModHandler.cpp

@@ -226,7 +226,7 @@ std::vector<CIdentifierStorage::ObjectData> CIdentifierStorage::getPossibleIdent
 
 		//for map format support core mod has access to any mod
 		//TODO: better solution for access from map?
-		if(request.localScope == "core" || request.localScope == "")
+		if(request.localScope == "core" || request.localScope.empty())
 		{
 			allowedScopes.insert(request.remoteScope);
 		}
@@ -1116,13 +1116,13 @@ void CModHandler::parseIdentifier(const std::string & fullIdentifier, std::strin
 	else
 	{
 		type = p.second;
-		identifier = "";
+		identifier.clear();
 	}
 }
 
 std::string CModHandler::makeFullIdentifier(const std::string & scope, const std::string & type, const std::string & identifier)
 {
-	if(type == "")
+	if(type.empty())
 		logGlobal->error("Full identifier (%s %s) requires type name", scope, identifier);
 
 	std::string actualScope = scope;
@@ -1137,13 +1137,13 @@ std::string CModHandler::makeFullIdentifier(const std::string & scope, const std
 		actualName = scopeAndName.second;
 	}
 
-	if(actualScope == "")
+	if(actualScope.empty())
 	{
-		return actualName == "" ? type : type + "." + actualName;
+		return actualName.empty() ? type : type + "." + actualName;
 	}
 	else
 	{
-		return actualName == "" ? actualScope+ ":" + type : actualScope + ":" + type + "." + actualName;
+		return actualName.empty() ? actualScope+ ":" + type : actualScope + ":" + type + "." + actualName;
 	}
 }
 

lib/CPathfinder.cpp

@@ -1252,7 +1252,7 @@ int CPathfinderHelper::getMovementCost(
 	if(src.x != dst.x && src.y != dst.y) //it's diagonal move
 	{
 		int old = ret;
-		ret = static_cast < int>(ret * 1.414213);
+		ret = static_cast<int>(ret * M_SQRT2);
 		//diagonal move costs too much but normal move is possible - allow diagonal move for remaining move points
 		if(ret > remainingMovePoints && remainingMovePoints >= old)
 		{

lib/CTownHandler.cpp

@@ -822,11 +822,11 @@ void CTownHandler::loadClientData(CTown &town, const JsonNode & source)
 	info.buildingsIcons = source["buildingsIcons"].String();
 
 	//left for back compatibility - will be removed later
-	if (source["guildBackground"].String() != "")
+	if(!source["guildBackground"].String().empty())
 		info.guildBackground = source["guildBackground"].String();
 	else
 		info.guildBackground = "TPMAGE.bmp";
-	if (source["tavernVideo"].String() != "")
+	if(!source["tavernVideo"].String().empty())
 	    info.tavernVideo = source["tavernVideo"].String();
 	else
 		info.tavernVideo = "TAVERN.BIK";
@@ -963,7 +963,6 @@ TerrainId CTownHandler::getDefaultTerrainForAlignment(EAlignment::EAlignment ali
 CFaction * CTownHandler::loadFromJson(const std::string & scope, const JsonNode & source, const std::string & identifier, size_t index)
 {
 	auto faction = new CFaction();
-	faction->index = index;
 
 	faction->index = static_cast<TFaction>(index);
 	faction->name = source["name"].String();

lib/HeroBonus.cpp

@@ -1016,7 +1016,7 @@ TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, co
 
 		// If a bonus system request comes with a caching string then look up in the map if there are any
 		// pre-calculated bonus results. Limiters can't be cached so they have to be calculated.
-		if (cachingStr != "")
+		if(!cachingStr.empty())
 		{
 			auto it = cachedRequests.find(cachingStr);
 			if(it != cachedRequests.end())
@@ -1032,7 +1032,7 @@ TConstBonusListPtr CBonusSystemNode::getAllBonuses(const CSelector &selector, co
 		cachedBonuses.getBonuses(*ret, selector, limit);
 
 		// Save the results in the cache
-		if(cachingStr != "")
+		if(!cachingStr.empty())
 			cachedRequests[cachingStr] = ret;
 
 		return ret;
@@ -1708,9 +1708,9 @@ JsonNode Bonus::toJsonNode() const
 		root["val"].Integer() = val;
 	if(valType != ADDITIVE_VALUE)
 		root["valueType"].String() = vstd::findKey(bonusValueMap, valType);
-	if(stacking != "")
+	if(!stacking.empty())
 		root["stacking"].String() = stacking;
-	if(description != "")
+	if(!description.empty())
 		root["description"].String() = description;
 	if(effectRange != NO_LIMIT)
 		root["effectRange"].String() = vstd::findKey(bonusLimitEffect, effectRange);

lib/IGameEventsReceiver.h

@@ -91,6 +91,7 @@ public:
 	virtual void artifactAssembled(const ArtifactLocation &al){};
 	virtual void artifactDisassembled(const ArtifactLocation &al){};
 	virtual void artifactMoved(const ArtifactLocation &src, const ArtifactLocation &dst){};
+	virtual void artifactPossibleAssembling(const ArtifactLocation & dst) {};
 
 	virtual void heroVisit(const CGHeroInstance *visitor, const CGObjectInstance *visitedObj, bool start){};
 	virtual void heroCreated(const CGHeroInstance*){};

lib/NetPacks.h

@@ -994,18 +994,64 @@ struct EraseArtifact : CArtifactOperationPack
 
 struct MoveArtifact : CArtifactOperationPack
 {
+	MoveArtifact() {}
+	MoveArtifact(ArtifactLocation * src, ArtifactLocation * dst) 
+		: src(*src), dst(*dst) {}
 	ArtifactLocation src, dst;
 
 	void applyCl(CClient *cl);
 	DLL_LINKAGE void applyGs(CGameState *gs);
 
-	template <typename Handler> void serialize(Handler &h, const int version)
+	template <typename Handler> void serialize(Handler & h, const int version)
 	{
 		h & src;
 		h & dst;
 	}
 };
 
+struct BulkMoveArtifacts : CArtifactOperationPack
+{
+	struct LinkedSlots
+	{
+		ArtifactPosition srcPos;
+		ArtifactPosition dstPos;
+
+		LinkedSlots() {}
+		LinkedSlots(ArtifactPosition srcPos, ArtifactPosition dstPos)
+			: srcPos(srcPos), dstPos(dstPos) {}
+		template <typename Handler> void serialize(Handler & h, const int version)
+		{
+			h & srcPos;
+			h & dstPos;
+		}
+	};
+
+	TArtHolder srcArtHolder;
+	TArtHolder dstArtHolder;
+
+	BulkMoveArtifacts()
+		: swap(false) {}
+	BulkMoveArtifacts(TArtHolder srcArtHolder, TArtHolder dstArtHolder, bool swap)
+		: srcArtHolder(srcArtHolder), dstArtHolder(dstArtHolder), swap(swap) {}
+
+	void applyCl(CClient * cl);
+	DLL_LINKAGE void applyGs(CGameState * gs);
+
+	std::vector<LinkedSlots> artsPack0;
+	std::vector<LinkedSlots> artsPack1;
+	bool swap;
+	CArtifactSet * getSrcHolderArtSet();
+	CArtifactSet * getDstHolderArtSet();
+	template <typename Handler> void serialize(Handler & h, const int version)
+	{
+		h & artsPack0;
+		h & artsPack1;
+		h & srcArtHolder;
+		h & dstArtHolder;
+		h & swap;
+	}
+};
+
 struct AssembledArtifact : CArtifactOperationPack
 {
 	ArtifactLocation al; //where assembly will be put
@@ -2217,6 +2263,27 @@ struct ExchangeArtifacts : public CPackForServer
 	}
 };
 
+struct BulkExchangeArtifacts : public CPackForServer
+{
+	ObjectInstanceID srcHero;
+	ObjectInstanceID dstHero;
+	bool swap;
+
+	BulkExchangeArtifacts() 
+		: swap(false) {}
+	BulkExchangeArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap)
+		: srcHero(srcHero), dstHero(dstHero), swap(swap) {}
+
+	bool applyGh(CGameHandler * gh);
+	template <typename Handler> void serialize(Handler & h, const int version)
+	{
+		h & static_cast<CPackForServer&>(*this);
+		h & srcHero;
+		h & dstHero;
+		h & swap;
+	}
+};
+
 struct AssembleArtifacts : public CPackForServer
 {
 	AssembleArtifacts():assemble(false){};

lib/NetPacksLib.cpp

@@ -845,18 +845,11 @@ DLL_LINKAGE CBonusSystemNode *ArtifactLocation::getHolderNode()
 
 DLL_LINKAGE const CArtifactInstance *ArtifactLocation::getArt() const
 {
-	const ArtSlotInfo *s = getSlot();
-	if(s && s->artifact)
-	{
-		if(!s->locked)
-			return s->artifact;
-		else
-		{
-			logNetwork->warn("ArtifactLocation::getArt: This location is locked!");
-			return nullptr;
-		}
-	}
-	return nullptr;
+	auto s = getSlot();
+	if(s)
+		return s->getArt();
+	else
+		return nullptr;
 }
 
 DLL_LINKAGE const CArtifactSet * ArtifactLocation::getHolderArtSet() const
@@ -1093,24 +1086,82 @@ DLL_LINKAGE void EraseArtifact::applyGs(CGameState *gs)
 	al.removeArtifact();
 }
 
-DLL_LINKAGE void MoveArtifact::applyGs(CGameState *gs)
+DLL_LINKAGE void MoveArtifact::applyGs(CGameState * gs)
 {
-	CArtifactInstance *a = src.getArt();
+	CArtifactInstance * art = src.getArt();
 	if(dst.slot < GameConstants::BACKPACK_START)
 		assert(!dst.getArt());
 
-	a->move(src, dst);
+	art->move(src, dst);
+}
+
+DLL_LINKAGE void BulkMoveArtifacts::applyGs(CGameState * gs)
+{
+	enum class EBulkArtsOp
+	{
+		BULK_MOVE,
+		BULK_REMOVE,
+		BULK_PUT
+	};
 
-	//TODO what'll happen if Titan's thunder is equipped by pickin git up or the start of game?
-	if (a->artType->id == ArtifactID::TITANS_THUNDER && dst.slot == ArtifactPosition::RIGHT_HAND) //Titan's Thunder creates new spellbook on equip
+	auto bulkArtsOperation = [this](std::vector<LinkedSlots> & artsPack, 
+		CArtifactSet * artSet, EBulkArtsOp operation) -> void
 	{
-		auto hPtr = boost::get<ConstTransitivePtr<CGHeroInstance> >(&dst.artHolder);
-		if(hPtr)
+		int numBackpackArtifactsMoved = 0;
+		for(auto & slot : artsPack)
 		{
-			CGHeroInstance *h = *hPtr;
-			if(h && !h->hasSpellbook())
-				gs->giveHeroArtifact(h, ArtifactID::SPELLBOOK);
+			// When an object gets removed from the backpack, the backpack shrinks
+			// so all the following indices will be affected. Thus, we need to update
+			// the subsequent artifact slots to account for that
+			auto srcPos = slot.srcPos;
+			if((srcPos >= GameConstants::BACKPACK_START) && (operation != EBulkArtsOp::BULK_PUT))
+			{
+				srcPos = ArtifactPosition(srcPos.num - numBackpackArtifactsMoved);
+			}
+			auto slotInfo = artSet->getSlot(srcPos);
+			assert(slotInfo);
+			auto art = const_cast<CArtifactInstance*>(slotInfo->getArt());
+			assert(art);
+			switch(operation)
+			{
+			case EBulkArtsOp::BULK_MOVE:
+				const_cast<CArtifactInstance*>(art)->move(
+					ArtifactLocation(srcArtHolder, srcPos), ArtifactLocation(dstArtHolder, slot.dstPos));
+				break;
+			case EBulkArtsOp::BULK_REMOVE:
+				art->removeFrom(ArtifactLocation(dstArtHolder, srcPos));
+				break;
+			case EBulkArtsOp::BULK_PUT:
+				art->putAt(ArtifactLocation(srcArtHolder, slot.dstPos));
+				break;
+			default:
+				break;
+			}
+
+			if(srcPos >= GameConstants::BACKPACK_START)
+			{
+				numBackpackArtifactsMoved++;
+			}
 		}
+	};
+	
+	if(swap)
+	{
+		// Swap
+		auto leftSet = getSrcHolderArtSet();
+		auto rightSet = getDstHolderArtSet();
+		CArtifactFittingSet artFittingSet(leftSet->bearerType());
+
+		artFittingSet.artifactsWorn = rightSet->artifactsWorn;
+		artFittingSet.artifactsInBackpack = rightSet->artifactsInBackpack;
+
+		bulkArtsOperation(artsPack1, rightSet, EBulkArtsOp::BULK_REMOVE);
+		bulkArtsOperation(artsPack0, leftSet, EBulkArtsOp::BULK_MOVE);
+		bulkArtsOperation(artsPack1, &artFittingSet, EBulkArtsOp::BULK_PUT);
+	}
+	else
+	{
+		bulkArtsOperation(artsPack0, getSrcHolderArtSet(), EBulkArtsOp::BULK_MOVE);
 	}
 }
 
@@ -1710,4 +1761,24 @@ DLL_LINKAGE void EntitiesChanged::applyGs(CGameState * gs)
 		gs->updateEntity(change.metatype, change.entityIndex, change.data);
 }
 
+const CArtifactInstance * ArtSlotInfo::getArt() const
+{
+	if(locked)
+	{
+		logNetwork->warn("ArtifactLocation::getArt: This location is locked!");
+		return nullptr;
+	}
+	return artifact;
+}
+
+CArtifactSet * BulkMoveArtifacts::getSrcHolderArtSet()
+{
+	return boost::apply_visitor(GetBase<CArtifactSet>(), srcArtHolder);
+}
+
+CArtifactSet * BulkMoveArtifacts::getDstHolderArtSet()
+{
+	return boost::apply_visitor(GetBase<CArtifactSet>(), dstArtHolder);
+}
+
 VCMI_LIB_NAMESPACE_END

lib/battle/CBattleInfoCallback.cpp

@@ -55,7 +55,7 @@ static void retrieveTurretDamageRange(const CGTownInstance * town, const battle:
 	const int baseDamage = 15;
 
 	outMinDmg = multiplier * (baseDamage + town->getTownLevel() * 3);
-	outMaxDmg = multiplier * (baseDamage + town->getTownLevel() * 3);
+	outMaxDmg = outMinDmg;
 }
 
 static BattleHex lineToWallHex(int line) //returns hex with wall in given line (y coordinate)

lib/mapObjects/CGHeroInstance.cpp

@@ -195,6 +195,23 @@ bool CGHeroInstance::canLearnSkill() const
 	return secSkills.size() < GameConstants::SKILL_PER_HERO;
 }
 
+bool CGHeroInstance::canLearnSkill(SecondarySkill which) const
+{
+	if ( !canLearnSkill())
+		return false;
+
+	if (!cb->isAllowed(2, which))
+		return false;
+
+	if (getSecSkillLevel(which) > 0)
+		return false;
+
+	if (type->heroClass->secSkillProbability[which] == 0)
+		return false;
+
+	return true;
+}
+
 int CGHeroInstance::maxMovePoints(bool onLand) const
 {
 	TurnInfo ti(this);
@@ -1117,7 +1134,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 	std::vector<SecondarySkill> obligatorySkills; //hero is offered magic school or wisdom if possible
 	if (!skillsInfo.wisdomCounter)
 	{
-		if (cb->isAllowed(2, SecondarySkill::WISDOM) && !getSecSkillLevel(SecondarySkill::WISDOM))
+		if (canLearnSkill(SecondarySkill::WISDOM))
 			obligatorySkills.push_back(SecondarySkill::WISDOM);
 	}
 	if (!skillsInfo.magicSchoolCounter)
@@ -1131,7 +1148,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 
 		for (auto skill : ss)
 		{
-			if (cb->isAllowed(2, skill) && !getSecSkillLevel(skill)) //only schools hero doesn't know yet
+			if (canLearnSkill(skill)) //only schools hero doesn't know yet
 			{
 				obligatorySkills.push_back(skill);
 				break; //only one
@@ -1143,7 +1160,7 @@ std::vector<SecondarySkill> CGHeroInstance::getLevelUpProposedSecondarySkills()
 	//picking sec. skills for choice
 	std::set<SecondarySkill> basicAndAdv, expert, none;
 	for(int i = 0; i < VLC->skillh->size(); i++)
-		if (cb->isAllowed(2,i))
+		if (canLearnSkill(SecondarySkill(i)))
 			none.insert(SecondarySkill(i));
 
 	for(auto & elem : secSkills)

lib/mapObjects/CGHeroInstance.h

@@ -187,6 +187,7 @@ public:
 
 	/// Returns true if hero has free secondary skill slot.
 	bool canLearnSkill() const;
+	bool canLearnSkill(SecondarySkill which) const;
 
 	void setPrimarySkill(PrimarySkill::PrimarySkill primarySkill, si64 value, ui8 abs);
 	void setSecSkillLevel(SecondarySkill which, int val, bool abs);// abs == 0 - changes by value; 1 - sets to value

lib/mapObjects/CGTownInstance.cpp

@@ -46,7 +46,7 @@ void CCreGenAsCastleInfo::serializeJson(JsonSerializeFormat & handler)
 
 	if(!handler.saving)
 	{
-		asCastle = (instanceId != "");
+		asCastle = !instanceId.empty();
 		allowedFactions.clear();
 	}
 

lib/mapObjects/CObjectClassesHandler.cpp

@@ -572,7 +572,7 @@ void AObjectTypeHandler::addTemplate(JsonNode config)
 	auto tmpl = new ObjectTemplate;
 	tmpl->id = Obj(type);
 	tmpl->subid = subtype;
-	tmpl->stringID = ""; // TODO?
+	tmpl->stringID.clear(); // TODO?
 	tmpl->readJson(config);
 	templates.emplace_back(tmpl);
 }

lib/mapObjects/CQuest.cpp

@@ -938,7 +938,7 @@ void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 		case MANA_POINTS:
 		case MORALE_BONUS:
 		case LUCK_BONUS:
-			identifier = "";
+			identifier.clear();
 			break;
 		case RESOURCES:
 			identifier = GameConstants::RESOURCE_NAMES[rID];
@@ -976,7 +976,7 @@ void CGSeerHut::serializeJsonOptions(JsonSerializeFormat & handler)
 
 		const JsonNode & rewardsJson = handler.getCurrent();
 
-		fullIdentifier = "";
+		fullIdentifier.clear();
 
 		if(rewardsJson.Struct().empty())
 			return;

lib/mapObjects/MiscObjects.cpp

@@ -1802,7 +1802,7 @@ void CGScholar::serializeJsonOptions(JsonSerializeFormat & handler)
 		//TODO: unify
 		const JsonNode & json = handler.getCurrent();
 		bonusType = RANDOM;
-		if(json["rewardPrimSkill"].String() != "")
+		if(!json["rewardPrimSkill"].String().empty())
 		{
 			auto raw = VLC->modh->identifiers.getIdentifier("core", "primSkill", json["rewardPrimSkill"].String());
 			if(raw)
@@ -1811,7 +1811,7 @@ void CGScholar::serializeJsonOptions(JsonSerializeFormat & handler)
 				bonusID = raw.get();
 			}
 		}
-		else if(json["rewardSkill"].String() != "")
+		else if(!json["rewardSkill"].String().empty())
 		{
 			auto raw = VLC->modh->identifiers.getIdentifier("core", "skill", json["rewardSkill"].String());
 			if(raw)
@@ -1820,7 +1820,7 @@ void CGScholar::serializeJsonOptions(JsonSerializeFormat & handler)
 				bonusID = raw.get();
 			}
 		}
-		else if(json["rewardSpell"].String() != "")
+		else if(!json["rewardSpell"].String().empty())
 		{
 			auto raw = VLC->modh->identifiers.getIdentifier("core", "spell", json["rewardSpell"].String());
 			if(raw)

lib/mapping/CMap.cpp

@@ -203,7 +203,7 @@ void CMapHeader::setupEvents()
 	standardVictory.effect.type = EventEffect::VICTORY;
 	standardVictory.effect.toOtherMessage = VLC->generaltexth->allTexts[5];
 	standardVictory.identifier = "standardVictory";
-	standardVictory.description = ""; // TODO: display in quest window
+	standardVictory.description.clear(); // TODO: display in quest window
 	standardVictory.onFulfill = VLC->generaltexth->allTexts[659];
 	standardVictory.trigger = EventExpression(victoryCondition);
 
@@ -212,7 +212,7 @@ void CMapHeader::setupEvents()
 	standardDefeat.effect.type = EventEffect::DEFEAT;
 	standardDefeat.effect.toOtherMessage = VLC->generaltexth->allTexts[8];
 	standardDefeat.identifier = "standardDefeat";
-	standardDefeat.description = ""; // TODO: display in quest window
+	standardDefeat.description.clear(); // TODO: display in quest window
 	standardDefeat.onFulfill = VLC->generaltexth->allTexts[7];
 	standardDefeat.trigger = EventExpression(defeatCondition);
 
@@ -651,7 +651,7 @@ void CMap::addNewObject(CGObjectInstance * obj)
 	if(obj->id != ObjectInstanceID((si32)objects.size()))
 		throw std::runtime_error("Invalid object instance id");
 
-	if(obj->instanceName == "")
+	if(obj->instanceName.empty())
 		throw std::runtime_error("Object instance name missing");
 
 	if (vstd::contains(instanceNames, obj->instanceName))

lib/mapping/MapFormatH3M.cpp

@@ -313,7 +313,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 	standardVictory.effect.type = EventEffect::VICTORY;
 	standardVictory.effect.toOtherMessage = VLC->generaltexth->allTexts[5];
 	standardVictory.identifier = "standardVictory";
-	standardVictory.description = ""; // TODO: display in quest window
+	standardVictory.description.clear(); // TODO: display in quest window
 	standardVictory.onFulfill = VLC->generaltexth->allTexts[659];
 	standardVictory.trigger = EventExpression(victoryCondition);
 
@@ -321,7 +321,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 	standardDefeat.effect.type = EventEffect::DEFEAT;
 	standardDefeat.effect.toOtherMessage = VLC->generaltexth->allTexts[8];
 	standardDefeat.identifier = "standardDefeat";
-	standardDefeat.description = ""; // TODO: display in quest window
+	standardDefeat.description.clear(); // TODO: display in quest window
 	standardDefeat.onFulfill = VLC->generaltexth->allTexts[7];
 	standardDefeat.trigger = EventExpression(defeatCondition);
 
@@ -338,7 +338,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		TriggeredEvent specialVictory;
 		specialVictory.effect.type = EventEffect::VICTORY;
 		specialVictory.identifier = "specialVictory";
-		specialVictory.description = ""; // TODO: display in quest window
+		specialVictory.description.clear(); // TODO: display in quest window
 
 		mapHeader->victoryIconIndex = ui16(vicCondition);
 		mapHeader->victoryMessage = VLC->generaltexth->victoryConditions[size_t(vicCondition) + 1];
@@ -526,7 +526,7 @@ void CMapLoaderH3M::readVictoryLossConditions()
 		specialDefeat.effect.type = EventEffect::DEFEAT;
 		specialDefeat.effect.toOtherMessage = VLC->generaltexth->allTexts[5];
 		specialDefeat.identifier = "specialDefeat";
-		specialDefeat.description = ""; // TODO: display in quest window
+		specialDefeat.description.clear(); // TODO: display in quest window
 
 		mapHeader->defeatIconIndex = ui16(lossCond);
 		mapHeader->defeatMessage = VLC->generaltexth->lossCondtions[size_t(lossCond) + 1];

lib/mapping/MapFormatJson.cpp

@@ -134,7 +134,7 @@ namespace TriggeredEventsDetail
 
 	static EMetaclass decodeMetaclass(const std::string & source)
 	{
-		if(source == "")
+		if(source.empty())
 			return EMetaclass::INVALID;
 		auto rawId = vstd::find_pos(NMetaclass::names, source);
 
@@ -286,7 +286,7 @@ namespace TriggeredEventsDetail
 				if(event.value > 0)
 					data["value"].Integer() = event.value;
 
-				if(event.objectInstanceName != "")
+				if(!event.objectInstanceName.empty())
 					data["object"].String() = event.objectInstanceName;
 			}
 			break;

lib/registerTypes/RegisterTypes.h

@@ -320,6 +320,7 @@ void registerTypesClientPacks2(Serializer &s)
 	s.template registerType<CArtifactOperationPack, MoveArtifact>();
 	s.template registerType<CArtifactOperationPack, AssembledArtifact>();
 	s.template registerType<CArtifactOperationPack, DisassembledArtifact>();
+	s.template registerType<CArtifactOperationPack, BulkMoveArtifacts>();
 
 	s.template registerType<CPackForClient, SaveGameClient>();
 	s.template registerType<CPackForClient, PlayerMessageClient>();
@@ -359,6 +360,7 @@ void registerTypesServerPacks(Serializer &s)
 	s.template registerType<CPackForServer, BulkMergeStacks>();
 	s.template registerType<CPackForServer, BulkSmartSplitStack>();
 	s.template registerType<CPackForServer, BulkMoveArmy>();
+	s.template registerType<CPackForServer, BulkExchangeArtifacts>();
 }
 
 template<typename Serializer>

lib/serializer/JsonDeserializer.cpp

@@ -38,7 +38,7 @@ void JsonDeserializer::serializeInternal(const std::string & fieldName, si32 & v
 
 	value = defaultValue ? defaultValue.get() : 0;
 
-	if(identifier != "")
+	if(!identifier.empty())
 	{
 		si32 rawId = decoder(identifier);
 

lib/serializer/JsonSerializer.cpp

@@ -108,7 +108,7 @@ void JsonSerializer::serializeLIC(const std::string & fieldName, LICSet & value)
 
 void JsonSerializer::serializeString(const std::string & fieldName, std::string & value)
 {
-	if(value != "")
+	if(!value.empty())
 		currentObject->operator[](fieldName).String() = value;
 }
 

mapeditor/objectbrowser.cpp

@@ -51,7 +51,7 @@ bool ObjectBrowser::filterAcceptsRow(int source_row, const QModelIndex & source_
 	auto factory = VLC->objtypeh->getHandlerFor(objId, objSubId);
 	auto templ = factory->getTemplates()[templateId];
 
-	result = result & templ->canBePlacedAt(terrain);
+	result = result && templ->canBePlacedAt(terrain);
 
 	//if we are here, just text filter will be applied
 	return result;

server/CGameHandler.cpp

@@ -3928,10 +3928,101 @@ bool CGameHandler::moveArtifact(const ArtifactLocation &al1, const ArtifactLocat
 		moveArtifact(dst, ArtifactLocation(dst.artHolder, ArtifactPosition(
 			(si32)dst.getHolderArtSet()->artifactsInBackpack.size() + GameConstants::BACKPACK_START)));
 	}
+	auto hero = boost::get<ConstTransitivePtr<CGHeroInstance>>(dst.artHolder);
+	if(ArtifactUtils::checkSpellbookIsNeeded(hero, srcArtifact->artType->id, dst.slot))
+		giveHeroNewArtifact(hero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
+
+	MoveArtifact ma(&src, &dst);
+	sendAndApply(&ma);
+	return true;
+}
+
+bool CGameHandler::bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap)
+{
+	// Make sure exchange is even possible between the two heroes.
+	if(!isAllowedExchange(srcHero, dstHero))
+		COMPLAIN_RET("That heroes cannot make any exchange!");
+
+	auto psrcHero = getHero(srcHero);
+	auto pdstHero = getHero(dstHero);
+	if((!psrcHero) || (!pdstHero))
+		COMPLAIN_RET("bulkMoveArtifacts: wrong hero's ID");
+
+	BulkMoveArtifacts ma(static_cast<ConstTransitivePtr<CGHeroInstance>>(psrcHero),
+		static_cast<ConstTransitivePtr<CGHeroInstance>>(pdstHero), swap);
+	auto & slotsSrcDst = ma.artsPack0;
+	auto & slotsDstSrc = ma.artsPack1;
+
+	if(swap)
+	{
+		auto moveArtsWorn = [this](const CGHeroInstance * srcHero, const CGHeroInstance * dstHero,
+			std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
+		{
+			for(auto & artifact : srcHero->artifactsWorn)
+			{
+				if(artifact.second.locked)
+					continue;
+				if(!ArtifactUtils::isArtRemovable(artifact))
+					continue;
+				slots.push_back(BulkMoveArtifacts::LinkedSlots(artifact.first, artifact.first));
+
+				auto art = artifact.second.getArt();
+				assert(art);
+				if(ArtifactUtils::checkSpellbookIsNeeded(dstHero, art->artType->id, artifact.first))
+					giveHeroNewArtifact(dstHero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
+			}
+		};
+		auto moveArtsInBackpack = [](const CGHeroInstance * pHero,
+			std::vector<BulkMoveArtifacts::LinkedSlots> & slots) -> void
+		{
+			for(auto & slotInfo : pHero->artifactsInBackpack)
+			{
+				auto slot = pHero->getArtPos(slotInfo.artifact);
+				slots.push_back(BulkMoveArtifacts::LinkedSlots(slot, slot));
+			}
+		};
+		// Move over artifacts that are worn srcHero -> dstHero
+		moveArtsWorn(psrcHero, pdstHero, slotsSrcDst);
+		// Move over artifacts that are worn dstHero -> srcHero
+		moveArtsWorn(pdstHero, psrcHero, slotsDstSrc);
+		// Move over artifacts that are in backpack srcHero -> dstHero
+		moveArtsInBackpack(psrcHero, slotsSrcDst);
+		// Move over artifacts that are in backpack dstHero -> srcHero
+		moveArtsInBackpack(pdstHero, slotsDstSrc);
+	}
+	else
+	{
+		// Temporary fitting set for artifacts. Used to select available slots before sending data.
+		CArtifactFittingSet artFittingSet(pdstHero->bearerType());
+		artFittingSet.artifactsInBackpack = pdstHero->artifactsInBackpack;
+		artFittingSet.artifactsWorn = pdstHero->artifactsWorn;
+
+		auto moveArtifact = [this, &artFittingSet, &slotsSrcDst](const CArtifactInstance * artifact,
+			ArtifactPosition srcSlot, const CGHeroInstance * pdstHero) -> void
+		{
+			assert(artifact);
+			auto dstSlot = ArtifactUtils::getArtifactDstPosition(artifact, &artFittingSet, pdstHero->bearerType());
+			artFittingSet.putArtifact(dstSlot, static_cast<ConstTransitivePtr<CArtifactInstance>>(artifact));
+			slotsSrcDst.push_back(BulkMoveArtifacts::LinkedSlots(srcSlot, dstSlot));
+
+			if(ArtifactUtils::checkSpellbookIsNeeded(pdstHero, artifact->artType->id, dstSlot))
+				giveHeroNewArtifact(pdstHero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
+		};
 
-	MoveArtifact ma;
-	ma.src = src;
-	ma.dst = dst;
+		// Move over artifacts that are worn
+		for(auto & artInfo : psrcHero->artifactsWorn)
+		{
+			if(ArtifactUtils::isArtRemovable(artInfo))
+			{
+				moveArtifact(psrcHero->getArt(artInfo.first), artInfo.first, pdstHero);
+			}
+		}
+		// Move over artifacts that are in backpack
+		for(auto & slotInfo : psrcHero->artifactsInBackpack)
+		{
+			moveArtifact(psrcHero->getArt(psrcHero->getArtPos(slotInfo.artifact)), psrcHero->getArtPos(slotInfo.artifact), pdstHero);
+		}
+	}
 	sendAndApply(&ma);
 	return true;
 }
@@ -3959,6 +4050,9 @@ bool CGameHandler::assembleArtifacts (ObjectInstanceID heroID, ArtifactPosition
 			COMPLAIN_RET("assembleArtifacts: Artifact being attempted to assemble is not a combined artifacts!");
 		if (!vstd::contains(destArtifact->assemblyPossibilities(hero), combinedArt))
 			COMPLAIN_RET("assembleArtifacts: It's impossible to assemble requested artifact!");
+		
+		if(ArtifactUtils::checkSpellbookIsNeeded(hero, assembleTo, artifactSlot))
+			giveHeroNewArtifact(hero, VLC->arth->objects[ArtifactID::SPELLBOOK], ArtifactPosition::SPELLBOOK);
 
 		AssembledArtifact aa;
 		aa.al = ArtifactLocation(hero, artifactSlot);
@@ -4095,7 +4189,7 @@ bool CGameHandler::buySecSkill(const IMarket *m, const CGHeroInstance *h, Second
 	if (!h->canLearnSkill())
 		COMPLAIN_RET("Hero can't learn any more skills");
 
-	if (h->type->heroClass->secSkillProbability.at(skill)==0)//can't learn this skill (like necromancy for most of non-necros)
+	if (!h->canLearnSkill(skill))
 		COMPLAIN_RET("The hero can't learn this skill!");
 
 	if (!vstd::contains(m->availableItemsIds(EMarketMode::RESOURCE_SKILL), skill))

server/CGameHandler.h

@@ -181,7 +181,8 @@ public:
 	void giveHeroArtifact(const CGHeroInstance *h, const CArtifactInstance *a, ArtifactPosition pos) override;
 	void putArtifact(const ArtifactLocation &al, const CArtifactInstance *a) override;
 	void removeArtifact(const ArtifactLocation &al) override;
-	bool moveArtifact(const ArtifactLocation &al1, const ArtifactLocation &al2) override;
+	bool moveArtifact(const ArtifactLocation & al1, const ArtifactLocation & al2) override;
+	bool bulkMoveArtifacts(ObjectInstanceID srcHero, ObjectInstanceID dstHero, bool swap);
 	void synchronizeArtifactHandlerLists();
 
 	void showCompInfo(ShowInInfobox * comp) override;

server/CQuery.cpp

@@ -370,6 +370,9 @@ bool CGarrisonDialogQuery::blocksPack(const CPack * pack) const
 	}
 	if(auto dismiss = dynamic_ptr_cast<DisbandCreature>(pack))
 		return !vstd::contains(ourIds, dismiss->id);
+	
+	if(auto arts = dynamic_ptr_cast<BulkExchangeArtifacts>(pack))
+		return !vstd::contains(ourIds, arts->srcHero) || !vstd::contains(ourIds, arts->dstHero);
 
 	if(auto dismiss = dynamic_ptr_cast<AssembleArtifacts>(pack))
 		return !vstd::contains(ourIds, dismiss->heroID);

server/NetPacksServer.cpp

@@ -180,6 +180,13 @@ bool ExchangeArtifacts::applyGh(CGameHandler * gh)
 	return gh->moveArtifact(src, dst);
 }
 
+bool BulkExchangeArtifacts::applyGh(CGameHandler * gh)
+{
+	const CGHeroInstance * pSrcHero = gh->getHero(srcHero);
+	throwOnWrongPlayer(gh, pSrcHero->getOwner());
+	return gh->bulkMoveArtifacts(srcHero, dstHero, swap);
+}
+
 bool AssembleArtifacts::applyGh(CGameHandler * gh)
 {
 	throwOnWrongOwner(gh, heroID);