went through the rest of cppcheck warnings, mostly harmless

AI/BattleAI/BattleAI.cpp

@@ -121,10 +121,10 @@ BattleAction CBattleAI::activeStack( const CStack * stack )
 				if(auto woundHp = stack->MaxHealth() - stack->firstHPleft)
 					woundHpToStack[woundHp] = stack;
 
-			if(woundHpToStack.size())
+			if(woundHpToStack.empty())
-				return BattleAction::makeHeal(stack, woundHpToStack.rbegin()->second); //last element of the woundHpToStack is the most wounded stack
-			else
 				return BattleAction::makeDefend(stack);
+			else
+				return BattleAction::makeHeal(stack, woundHpToStack.rbegin()->second); //last element of the woundHpToStack is the most wounded stack
 		}
 
 		if(cb->battleCanCastSpell())

AI/VCAI/AIUtility.cpp

@@ -80,8 +80,6 @@ ui64 evaluateDanger(crint3 tile)
 
 	//TODO mozna odwiedzic blockvis nie ruszajac straznika
 	return std::max(objectDanger, guardDanger);
-
-	return 0;
 }
 
 ui64 evaluateDanger(crint3 tile, const CGHeroInstance *visitor)

AI/VCAI/Fuzzy.cpp

@@ -102,7 +102,7 @@ void FuzzyHelper::initBank()
 			bankBlock.addRule(new fl::MamdaniRule("if BankInput is SET then BankDanger is Bank" + boost::lexical_cast<std::string>(i), engine));
 		}
 	}
-	catch (fl::FuzzyException fe)
+	catch (fl::FuzzyException & fe)
 	{
         logAi->errorStream() << "initBank " << fe.name() << ": " << fe.message();
 	}
@@ -178,11 +178,11 @@ void FuzzyHelper::initTacticalAdvantage()
 
 		engine.addRuleBlock (&tacticalAdvantage);
 	}
-	catch(fl::ParsingException pe)
+	catch(fl::ParsingException & pe)
 	{
         logAi->errorStream() << "initTacticalAdvantage " << pe.name() << ": " << pe.message();
 	}
-	catch (fl::FuzzyException fe)
+	catch (fl::FuzzyException & fe)
 	{
         logAi->errorStream() << "initTacticalAdvantage " << fe.name() << ": " << fe.message();
 	}
@@ -199,10 +199,9 @@ ui64 FuzzyHelper::estimateBankDanger (int ID)
 			case 4:
 				try
 				{
-					int bankVal;
 					for (int i = 0; i < 4; ++i)
 					{
-						bankVal = evaluateBankConfig (VLC->objh->banksInfo[ID][i]);
+						int bankVal = evaluateBankConfig (VLC->objh->banksInfo[ID][i]);
 						bankDanger->term("Bank" + boost::lexical_cast<std::string>(i))->setMinimum(bankVal * 0.5f);
 						bankDanger->term("Bank" + boost::lexical_cast<std::string>(i))->setMaximum(bankVal * 1.5f);
 					}
@@ -213,7 +212,7 @@ ui64 FuzzyHelper::estimateBankDanger (int ID)
 					engine.process (BANK_DANGER);
 					val = bankDanger->output().defuzzify(); //some expected value of this bank
 				}
-				catch (fl::FuzzyException fe)
+				catch (fl::FuzzyException & fe)
 				{
                     logAi->errorStream() << fe.name() << ": " << fe.message();
 				}
@@ -225,7 +224,7 @@ ui64 FuzzyHelper::estimateBankDanger (int ID)
                 logAi->warnStream() << ("Uhnandled bank config!");
 		}
 	}
-	catch (fl::FuzzyException fe)
+	catch (fl::FuzzyException & fe)
 	{
         logAi->errorStream() << "estimateBankDanger " << fe.name() << ": " << fe.message();
 	}
@@ -268,7 +267,7 @@ float FuzzyHelper::getTacticalAdvantage (const CArmedInstance *we, const CArmedI
 		engine.process (TACTICAL_ADVANTAGE);
 		output = threat->output().defuzzify();
 	}
-	catch (fl::FuzzyException fe)
+	catch (fl::FuzzyException & fe)
 	{
         logAi->errorStream() << "getTacticalAdvantage " << fe.name() << ": " << fe.message();
 	}

AI/VCAI/VCAI.cpp

@@ -77,10 +77,10 @@ struct ObjInfo
 	int3 pos;
 	std::string name;
 	ObjInfo(){}
-	ObjInfo(const CGObjectInstance *obj)
+	ObjInfo(const CGObjectInstance *obj):
+		pos(obj->pos),
+		name(obj->getHoverText())
 	{
-		pos = obj->pos;
-		name = obj->getHoverText();
 	}
 };
 
@@ -1397,7 +1397,7 @@ bool VCAI::isAccessibleForHero(const int3 & pos, HeroPtr h, bool includeAllies /
 	{ //don't visit tile occupied by allied hero
 		for (auto obj : cb->getVisitableObjs(pos))
 		{
-			if (obj->ID == Obj::HERO && obj->tempOwner == h->tempOwner && obj != h)
+			if (obj->ID == Obj::HERO && obj->tempOwner == h->tempOwner && obj != h.get())
 				return false;
 		}
 	}
@@ -2488,11 +2488,9 @@ bool isWeeklyRevisitable (const CGObjectInstance * obj)
 		case Obj::MAGIC_WELL:
 		case Obj::HILL_FORT:
 			return true;
-			break;
 		case Obj::BORDER_GATE:
 		case Obj::BORDERGUARD:
 			return (dynamic_cast <const CGKeys *>(obj))->wasMyColorVisited (ai->playerID); //FIXME: they could be revisited sooner than in a week
-			break;
 	}
 	return false;
 }
@@ -2580,10 +2578,8 @@ bool shouldVisit(HeroPtr h, const CGObjectInstance * obj)
 			break;
 		case Obj::MAGIC_WELL:
 			return h->mana < h->manaLimit();
-			break;
 		case Obj::PRISON:
 			return ai->myCb->getHeroesInfo().size() < GameConstants::MAX_HEROES_PER_PLAYER;
-			break;
 
 		case Obj::BOAT:
 			return false;

CMakeLists.txt

@@ -94,7 +94,7 @@ endif()
 
 if(CMAKE_COMPILER_IS_GNUCXX OR NOT WIN32) #so far all *nix compilers support such parameters
 	if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
-		set(CLANG_SPECIFIC_FLAGS "-Wno-mismatched-tags")
+                set(CLANG_SPECIFIC_FLAGS "-Wno-mismatched-tags")
 	endif()
 	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++0x -Wall -Wextra -Wpointer-arith -Wno-switch -Wno-sign-compare -Wno-unused-parameter -Wno-overloaded-virtual ${CLANG_SPECIFIC_FLAGS}")
 endif()
@@ -115,7 +115,7 @@ else()
 	include(GNUInstallDirs)
 
 	if (NOT BIN_DIR)
-			set(BIN_DIR "bin" CACHE STRING "Where to install binaries")
+                set(BIN_DIR "bin" CACHE STRING "Where to install binaries")
 	endif()
 	if (NOT LIB_DIR)
 		set(LIB_DIR "${CMAKE_INSTALL_LIBDIR}/vcmi" CACHE STRING "Where to install main library")

client/AdventureMapClasses.h

@@ -59,7 +59,7 @@ protected:
 
 	/**
 	 * @brief CList - protected constructor
-	 * @param Size - maximal amount of visible at once items
+	 * @param size - maximal amount of visible at once items
 	 * @param position - cordinates
 	 * @param btnUp - path to image to use as top button
 	 * @param btnDown - path to image to use as bottom button
@@ -129,7 +129,7 @@ class CHeroList	: public CList
 public:
 	/**
 	 * @brief CHeroList
-	 * @param Size, position, btnUp, btnDown @see CList::CList
+	 * @param size, position, btnUp, btnDown @see CList::CList
 	 */
 	CHeroList(int size, Point position, std::string btnUp, std::string btnDown);
 
@@ -163,7 +163,7 @@ class CTownList	: public CList
 public:
 	/**
 	 * @brief CTownList
-	 * @param Size, position, btnUp, btnDown @see CList::CList
+	 * @param size, position, btnUp, btnDown @see CList::CList
 	 */
 	CTownList(int size, Point position, std::string btnUp, std::string btnDown);
 
@@ -312,4 +312,4 @@ public:
 
 	/// for 3 seconds shows amount of town halls and players status
 	void showGameStatus();
-};
+};

client/CAdvmapInterface.cpp

@@ -1137,7 +1137,6 @@ void CAdvMapInt::tileLClicked(const int3 &mapPos)
 	if(!LOCPLINT->cb->isVisible(mapPos) || !LOCPLINT->makingTurn)
 		return;
 
-	std::vector < const CGObjectInstance * > bobjs = LOCPLINT->cb->getBlockingObjs(mapPos);//blocking objects at tile
 	const TerrainTile *tile = LOCPLINT->cb->getTile(mapPos);
 
 	const CGObjectInstance *topBlocking = getBlockingObject(mapPos);

client/CBitmapHandler.cpp

@@ -126,7 +126,10 @@ SDL_Surface * BitmapHandler::loadBitmapFromDir(std::string path, std::string fna
 			}
 		}
 		else
-            logGlobal->errorStream()<<"Failed to open "<<fname<<" as H3 PCX!";
+		{
+			logGlobal->errorStream()<<"Failed to open "<<fname<<" as H3 PCX!";
+			return nullptr;
+		}
 	}
 	else
 	{ //loading via SDL_Image
@@ -147,6 +150,7 @@ SDL_Surface * BitmapHandler::loadBitmapFromDir(std::string path, std::string fna
 		{
             logGlobal->errorStream()<<"Failed to open "<<fname<<" via SDL_Image";
 			logGlobal->errorStream()<<"Reason: " << IMG_GetError();
+			return nullptr;
 		}
 	}
 

client/CHeroWindow.cpp

@@ -201,12 +201,11 @@ void CHeroWindow::update(const CGHeroInstance * hero, bool redrawNeeded /*= fals
 	portraitImage->setFrame(curHero->portrait);
 
 	{
-		CAdventureMapButton * split = nullptr;
 		OBJ_CONSTRUCTION_CAPTURING_ALL;
 		if(!garr)
 		{
 			garr = new CGarrisonInt(15, 485, 8, Point(), background->bg, Point(15,485), curHero);
-			split = new CAdventureMapButton(CGI->generaltexth->allTexts[256], CGI->generaltexth->heroscrn[32],
+			auto split = new CAdventureMapButton(CGI->generaltexth->allTexts[256], CGI->generaltexth->heroscrn[32],
 					boost::bind(&CGarrisonInt::splitClick,garr), 539, 519, "hsbtns9.def", false, nullptr, false); //deleted by garrison destructor
 			boost::algorithm::replace_first(split->hoverTexts[0],"%s",CGI->generaltexth->allTexts[43]);
 

client/CPlayerInterface.cpp

@@ -657,13 +657,13 @@ void CPlayerInterface::battleStacksHealedRes(const std::vector<std::pair<ui32, u
 				textOff += 1;
 			}
 			CCS->soundh->playSound(soundBase::DRAINLIF);
-		}
 
-		//print info about life drain
+			//print info about life drain
-		char textBuf[1000];
+			char textBuf[1000];
-		sprintf(textBuf, CGI->generaltexth->allTexts[361 + textOff].c_str(), attacker->getCreature()->nameSing.c_str(),
+			sprintf(textBuf, CGI->generaltexth->allTexts[361 + textOff].c_str(), attacker->getCreature()->nameSing.c_str(),
-			healedStacks[0].second, defender->getCreature()->namePl.c_str());
+				healedStacks[0].second, defender->getCreature()->namePl.c_str());
-		battleInt->console->addText(textBuf);
+			battleInt->console->addText(textBuf);
+		}
 	}
 	if (tentHeal)
 	{

client/CPreGame.cpp

@@ -671,17 +671,7 @@ CSelectionScreen::CSelectionScreen(CMenuScreen::EState Type, CMenuScreen::EMulti
 
 	start->assignedKeys.insert(SDLK_RETURN);
 
-	std::string backName;
+	back = new CAdventureMapButton("", CGI->generaltexth->zelp[105].second, boost::bind(&CGuiHandler::popIntTotally, &GH, this), 581, 535, "SCNRBACK.DEF", SDLK_ESCAPE);
-	if(Type == CMenuScreen::campaignList)
-	{
-		backName = "SCNRBACK.DEF";
-	}
-	else
-	{
-		backName = "SCNRBACK.DEF";
-	}
-
-	back = new CAdventureMapButton("", CGI->generaltexth->zelp[105].second, boost::bind(&CGuiHandler::popIntTotally, &GH, this), 581, 535, backName, SDLK_ESCAPE);
 
 	if(network)
 	{
@@ -1196,7 +1186,6 @@ SelectionTab::SelectionTab(CMenuScreen::EState Type, const std::function<void(CM
 	}
 	else
 	{
-		std::vector<CCampaignHeader> cpm;
 		switch(tabType)
 		{
 		case CMenuScreen::newGame:

client/Client.cpp

@@ -136,20 +136,17 @@ void CClient::run()
 	setThreadName("CClient::run");
 	try
 	{
-		CPack *pack = nullptr;
 		while(!terminate)
 		{
-			pack = serv->retreivePack(); //get the package from the server
+			CPack *pack = serv->retreivePack(); //get the package from the server
 			
 			if (terminate) 
 			{
-				delete pack;
+				vstd::clear_pointer(pack);
-				pack = nullptr;
 				break;
 			}
 
 			handlePack(pack);
-			pack = nullptr;
 		}
 	} 
 	//catch only asio exceptions

client/GUIClasses.h

@@ -821,7 +821,7 @@ public:
 class CInGameConsole : public CIntObject
 {
 private:
-	std::list< std::pair< std::string, int > > texts; //<text to show, time of add>
+	std::list< std::pair< std::string, int > > texts; //list<text to show, time of add>
 	boost::mutex texts_mx;		// protects texts
 	std::vector< std::string > previouslyEntered; //previously entered texts, for up/down arrows to work
 	int prevEntDisp; //displayed entry from previouslyEntered - if none it's -1

client/battle/CBattleInterface.cpp

@@ -8,7 +8,7 @@
 #include "../CBitmapHandler.h"
 #include "../../lib/CObjectHandler.h"
 #include "../../lib/CHeroHandler.h"
-#include "../CDefHandler.h"
+# include "../CDefHandler.h"
 #include "../../lib/CSpellHandler.h"
 #include "../CMusicHandler.h"
 #include "../CMessage.h"
@@ -1338,8 +1338,6 @@ void CBattleInterface::spellCast( const BattleSpellCast * sc )
 	bool customSpell = false;
 	if(sc->affectedCres.size() == 1)
 	{
-		bool plural = false; //add singular / plural form of creature text if this is true
-		int textID = 0;
 		std::string text = CGI->generaltexth->allTexts[195];
 		if(sc->castedByHero)
 		{
@@ -1349,6 +1347,8 @@ void CBattleInterface::spellCast( const BattleSpellCast * sc )
 		}
 		else
 		{
+			bool plural = false; //add singular / plural form of creature text if this is true
+			int textID = 0;
 			switch(sc->id)
 			{
 				case SpellID::STONE_GAZE:
@@ -1544,6 +1544,7 @@ void CBattleInterface::castThisSpell(int spellID)
 
 	//choosing possible tragets
 	const CGHeroInstance * castingHero = (attackingHeroInstance->tempOwner == curInt->playerID) ? attackingHeroInstance : defendingHeroInstance;
+	assert(castingHero); // code below assumes non-null hero
 	sp = CGI->spellh->spells[spellID];
 	spellSelMode = ANY_LOCATION;
 	if(sp->getTargetType() == CSpell::CREATURE)
@@ -1552,7 +1553,7 @@ void CBattleInterface::castThisSpell(int spellID)
 	}
 	if(sp->getTargetType() == CSpell::CREATURE_EXPERT_MASSIVE)
 	{
-		if(castingHero && castingHero->getSpellSchoolLevel(sp) < 3)
+		if(castingHero->getSpellSchoolLevel(sp) < 3)
 			spellSelMode = selectionTypeByPositiveness(*sp);
 		else
 			spellSelMode = NO_LOCATION;
@@ -1960,10 +1961,10 @@ void CBattleInterface::startAction(const BattleAction* action)
 	if(action->actionType == Battle::WALK
 		|| (action->actionType == Battle::WALK_AND_ATTACK && action->destinationTile != stack->position))
 	{
+		assert(stack);
 		moveStarted = true;
 		if(creAnims[action->stackNumber]->framesInGroup(CCreatureAnim::MOVE_START))
 		{
-			const CStack * stack = curInt->cb->battleGetStackByID(action->stackNumber);
 			pendingAnims.push_back(std::make_pair(new CMovementStartAnimation(this, stack), false));
 		}
 	}

client/gui/CGuiHandler.cpp

@@ -168,7 +168,7 @@ void CGuiHandler::handleEvents()
 	{
 		SDL_Event ev;
 		boost::unique_lock<boost::mutex> lock(eventsM);
-		if(!events.size())
+		if(events.empty())
 		{
 			return;
 		}

client/gui/CIntObjectClasses.cpp

@@ -1267,13 +1267,12 @@ void CTextContainer::blitLine(SDL_Surface *to, Rect destRect, std::string what)
 	}
 
 	size_t begin = 0;
-	size_t end;
 	std::string delimeters = "{}";
 	size_t currDelimeter = 0;
 
 	do
 	{
-		end = what.find_first_of(delimeters[currDelimeter % 2], begin);
+		size_t end = what.find_first_of(delimeters[currDelimeter % 2], begin);
 		if (begin != end)
 		{
 			std::string toPrint = what.substr(begin, end-1);

client/gui/SDL_Extensions.cpp

@@ -175,13 +175,13 @@ static void prepareOutRect(SDL_Rect *src, SDL_Rect *dst, const SDL_Rect & clip_r
 template<int bpp>
 void CSDL_Ext::blitWithRotateClip(SDL_Surface *src,SDL_Rect * srcRect, SDL_Surface * dst, SDL_Rect * dstRect, ui8 rotation)//srcRect is not used, works with 8bpp sources and 24bpp dests
 {
-	static void (*blitWithRotate[])(const SDL_Surface *, const SDL_Rect *, SDL_Surface *, const SDL_Rect *) = {blitWithRotate1<bpp>, blitWithRotate2<bpp>, blitWithRotate3<bpp>};
 	if(!rotation)
 	{
 		CSDL_Ext::blitSurface(src, srcRect, dst, dstRect);
 	}
 	else
 	{
+		static void (*blitWithRotate[])(const SDL_Surface *, const SDL_Rect *, SDL_Surface *, const SDL_Rect *) = {blitWithRotate1<bpp>, blitWithRotate2<bpp>, blitWithRotate3<bpp>};
 		prepareOutRect(srcRect, dstRect, dst->clip_rect);
 		blitWithRotate[rotation-1](src, srcRect, dst, dstRect);
 	}
@@ -196,13 +196,13 @@ void CSDL_Ext::blitWithRotateClipVal( SDL_Surface *src,SDL_Rect srcRect, SDL_Sur
 template<int bpp>
 void CSDL_Ext::blitWithRotateClipWithAlpha(SDL_Surface *src,SDL_Rect * srcRect, SDL_Surface * dst, SDL_Rect * dstRect, ui8 rotation)//srcRect is not used, works with 8bpp sources and 24bpp dests
 {
-	static void (*blitWithRotate[])(const SDL_Surface *, const SDL_Rect *, SDL_Surface *, const SDL_Rect *) = {blitWithRotate1WithAlpha<bpp>, blitWithRotate2WithAlpha<bpp>, blitWithRotate3WithAlpha<bpp>};
 	if(!rotation)
 	{
 		blit8bppAlphaTo24bpp(src, srcRect, dst, dstRect);
 	}
 	else
 	{
+		static void (*blitWithRotate[])(const SDL_Surface *, const SDL_Rect *, SDL_Surface *, const SDL_Rect *) = {blitWithRotate1WithAlpha<bpp>, blitWithRotate2WithAlpha<bpp>, blitWithRotate3WithAlpha<bpp>};
 		prepareOutRect(srcRect, dstRect, dst->clip_rect);
 		blitWithRotate[rotation-1](src, srcRect, dst, dstRect);
 	}

client/mapHandler.cpp

@@ -882,8 +882,7 @@ bool CMapHandler::printObject(const CGObjectInstance *obj)
 				auto i = curt.objects.begin();
 				for(; i != curt.objects.end(); i++)
 				{
-					OCM_HLP cmp;
+					if(ocmptwo(toAdd, *i))
-					if(cmp(toAdd, *i))
 					{
 						curt.objects.insert(i, toAdd);
 						i = curt.objects.begin(); //to validate and avoid adding it second time

config/factions/inferno.json

@@ -143,7 +143,7 @@
 			"horde" : [ 0, 2 ],
 			"mageGuild" : 5,
 			"primaryResource" : "mercury",
-			"warMachine" : "ballista",
+			"warMachine" : "ammoCart",
 			"moatDamage" : 90,
 
 			"buildings" :

lib/BattleState.cpp

@@ -724,7 +724,7 @@ const CGHeroInstance * BattleInfo::getHero( PlayerColor player ) const
 	return nullptr;
 }
 
-std::vector<ui32> BattleInfo::calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2, const std::vector<const CStack*> affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode, int usedSpellPower, int spellLevel) const
+std::vector<ui32> BattleInfo::calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2, const std::vector<const CStack*> & affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode, int usedSpellPower, int spellLevel) const
 {
 	std::vector<ui32> ret;
 	for(auto & affectedCreature : affectedCreatures)

lib/BattleState.h

@@ -144,7 +144,7 @@ struct DLL_LINKAGE BattleInfo : public CBonusSystemNode, public CBattleInfoCallb
 
 	const CGHeroInstance * getHero(PlayerColor player) const; //returns fighting hero that belongs to given player
 
-	std::vector<ui32> calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2, const std::vector<const CStack*> affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode, int usedSpellPower, int spellLevel) const;
+	std::vector<ui32> calculateResistedStacks(const CSpell * sp, const CGHeroInstance * caster, const CGHeroInstance * hero2, const std::vector<const CStack*> & affectedCreatures, PlayerColor casterSideOwner, ECastingMode::ECastingMode mode, int usedSpellPower, int spellLevel) const;
 
 	const CStack * battleGetStack(BattleHex pos, bool onlyAlive); //returns stack at given tile
 	const CGHeroInstance * battleGetOwner(const CStack * stack) const; //returns hero that owns given stack; nullptr if none

lib/CArtHandler.cpp

@@ -711,7 +711,7 @@ ArtifactPosition CArtifactInstance::firstBackpackSlot(const CArtifactSet *h) con
 	return ArtifactPosition::PRE_FIRST;
 }
 
-bool CArtifactInstance::canBePutAt(const ArtifactLocation al, bool assumeDestRemoved /*= false*/) const
+bool CArtifactInstance::canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved /*= false*/) const
 {
 	return canBePutAt(al.getHolderArtSet(), al.slot, assumeDestRemoved);
 }

lib/CArtHandler.h

@@ -122,7 +122,7 @@ public:
 	SpellID getGivenSpellID() const; //to be used with scrolls (and similar arts), -1 if none
 
 	virtual bool canBePutAt(const CArtifactSet *artSet, ArtifactPosition slot, bool assumeDestRemoved = false) const;
-	bool canBePutAt(const ArtifactLocation al, bool assumeDestRemoved = false) const;  //forwards to the above one
+	bool canBePutAt(const ArtifactLocation & al, bool assumeDestRemoved = false) const;  //forwards to the above one
 	virtual bool canBeDisassembled() const;
 	virtual void putAt(ArtifactLocation al);
 	virtual void removeFrom(ArtifactLocation al);

lib/CBattleCallback.cpp

@@ -1151,7 +1151,7 @@ AccessibilityInfo CBattleInfoCallback::getAccesibility(const std::vector<BattleH
 	return ret;
 }
 
-ReachabilityInfo CBattleInfoCallback::makeBFS(const AccessibilityInfo &accessibility, const ReachabilityInfo::Parameters params) const
+ReachabilityInfo CBattleInfoCallback::makeBFS(const AccessibilityInfo &accessibility, const ReachabilityInfo::Parameters &params) const
 {
 	ReachabilityInfo ret;
 	ret.accessibility = accessibility;
@@ -1304,7 +1304,7 @@ ReachabilityInfo CBattleInfoCallback::getReachability(const ReachabilityInfo::Pa
 		return makeBFS(getAccesibility(params.knownAccessible), params);
 }
 
-ReachabilityInfo CBattleInfoCallback::getFlyingReachability(const ReachabilityInfo::Parameters params) const
+ReachabilityInfo CBattleInfoCallback::getFlyingReachability(const ReachabilityInfo::Parameters &params) const
 {
 	ReachabilityInfo ret;
 	ret.accessibility = getAccesibility(params.knownAccessible);
@@ -1684,7 +1684,8 @@ ESpellCastProblem::ESpellCastProblem CBattleInfoCallback::battleCanCastThisSpell
 	int spellIDs[] = {	SpellID::SUMMON_FIRE_ELEMENTAL, SpellID::SUMMON_EARTH_ELEMENTAL,
 						SpellID::SUMMON_WATER_ELEMENTAL, SpellID::SUMMON_AIR_ELEMENTAL };
 	//(fire, earth, water, air) elementals
-	int creIDs[] = {114, 113, 115, 112};
+	int creIDs[] = {CreatureID::FIRE_ELEMENTAL,  CreatureID::EARTH_ELEMENTAL,
+					CreatureID::WATER_ELEMENTAL, CreatureID::AIR_ELEMENTAL};
 
 	int arpos = vstd::find_pos(spellIDs, spell->id);
 	if(arpos < ARRAY_COUNT(spellIDs))

lib/CBattleCallback.h

@@ -302,8 +302,8 @@ public:
 	AccessibilityInfo getAccesibility(const std::vector<BattleHex> &accessibleHexes) const; //given hexes will be marked as accessible
 	std::pair<const CStack *, BattleHex> getNearestStack(const CStack * closest, boost::logic::tribool attackerOwned) const;
 protected:
-	ReachabilityInfo getFlyingReachability(const ReachabilityInfo::Parameters params) const;
+	ReachabilityInfo getFlyingReachability(const ReachabilityInfo::Parameters &params) const;
-	ReachabilityInfo makeBFS(const AccessibilityInfo &accessibility, const ReachabilityInfo::Parameters params) const;
+	ReachabilityInfo makeBFS(const AccessibilityInfo &accessibility, const ReachabilityInfo::Parameters &params) const;
 	ReachabilityInfo makeBFS(const CStack *stack) const; //uses default parameters -> stack position and owner's perspective
 	std::set<BattleHex> getStoppers(BattlePerspective::BattlePerspective whichSidePerspective) const; //get hexes with stopping obstacles (quicksands)
 

lib/CConsoleHandler.cpp

@@ -45,7 +45,7 @@ DLL_LINKAGE CConsoleHandler * console = nullptr;
 	#define CONSOLE_TEAL FOREGROUND_GREEN | FOREGROUND_BLUE | FOREGROUND_INTENSITY
 #endif
 
-TColor defColor;
+static TColor defColor;
 
 #ifdef _WIN32
 

lib/CGameInterface.h

@@ -14,7 +14,7 @@
  *
  */
 
-using namespace boost::logic;
+using boost::logic::tribool;
 class CCallback;
 class CBattleCallback;
 class ICallback;

lib/CGameState.cpp

@@ -719,29 +719,22 @@ int CGameState::getDate(Date::EDateType mode) const
 	{
 	case Date::DAY:
 		return day;
-		break;
 	case Date::DAY_OF_WEEK: //day of week
 		temp = (day)%7; // 1 - Monday, 7 - Sunday
-		if (temp)
+		return temp ? temp : 7;
-			return temp;
-		else return 7;
-		break;
 	case Date::WEEK:  //current week
 		temp = ((day-1)/7)+1;
 		if (!(temp%4))
 			return 4;
 		else
 			return (temp%4);
-		break;
 	case Date::MONTH: //current month
 		return ((day-1)/28)+1;
-		break;
 	case Date::DAY_OF_MONTH: //day of month
 		temp = (day)%28;
 		if (temp)
 			return temp;
 		else return 28;
-		break;
 	}
 	return 0;
 }
@@ -2152,7 +2145,6 @@ int CGameState::victoryCheck( PlayerColor player ) const
 				}
 			}
 			return 1;
-			break;
 		case EVictoryConditionType::TAKEMINES:
 			for(auto & elem : map->objects)
 			{
@@ -2166,7 +2158,6 @@ int CGameState::victoryCheck( PlayerColor player ) const
 				}
 			}
 			return 1;
-			break;
 		case EVictoryConditionType::TRANSPORTITEM:
 			{
 				const CGTownInstance *t = static_cast<const CGTownInstance *>(map->victoryCondition.obj);

lib/CObjectHandler.cpp

@@ -2311,6 +2311,7 @@ void CGTownInstance::initObj()
 			break;
 		case 5:
 			bonusingBuildings.push_back (new COPWBonus(BuildingID::MANA_VORTEX, this));
+			//fallthrough
 		case 2: case 3: case 6:
 			bonusingBuildings.push_back (new CTownBonus(BuildingID::SPECIAL_4, this));
 			break;
@@ -3524,12 +3525,11 @@ void CGCreature::fight( const CGHeroInstance *h ) const
 	{
 		stacksCount = 3;
 	}
-	int stackSize;
 	SlotID sourceSlot = stacks.begin()->first;
 	SlotID destSlot;
 	for (int stacksLeft = stacksCount; stacksLeft > 1; --stacksLeft)
 	{
-		stackSize = stacks.begin()->second->count / stacksLeft;
+		int stackSize = stacks.begin()->second->count / stacksLeft;
 		if (stackSize)
 		{
 			if ((destSlot = getFreeSlot()).validSlot())
@@ -6066,7 +6066,6 @@ void CGOnceVisitable::onHeroVisit( const CGHeroInstance * h ) const
 			cb->showBlockingDialog(&bd);
 			return;
 		}
-		break;
 	default:
         logGlobal->errorStream() << "Error: Unknown object (" << ID <<") treated as CGOnceVisitable!";
 		return;
@@ -7197,7 +7196,6 @@ void CArmedInstance::randomizeArmy(int type)
 			int level = (randID-VLC->creh->creatures.size()) / 2 -1;
 			bool upgrade = !(randID % 2);
 			elem.second->setType(VLC->townh->factions[type]->town->creatures[level][upgrade]);
-			randID = -1;
 		}
 
 		assert(elem.second->armyObj == this);
@@ -7431,8 +7429,6 @@ std::vector<int> IMarket::availableItemsIds(EMarketMode::EMarketMode mode) const
 	case EMarketMode::CREATURE_RESOURCE:
 		for (int i = 0; i < 7; i++)
 			ret.push_back(i);
-	default:
-		break;
 	}
 	return ret;
 }

lib/CObjectHandler.h

@@ -80,6 +80,7 @@ public:
 	bool isCustomFirst, isCustomNext, isCustomComplete;
 
 	CQuest(){missionType = MISSION_NONE;}; //default constructor
+	virtual ~CQuest(){};
 
 	virtual bool checkQuest (const CGHeroInstance * h) const; //determines whether the quest is complete or not
 	virtual void getVisitText (MetaString &text, std::vector<Component> &components, bool isCustom, bool FirstVisit, const CGHeroInstance * h = nullptr) const;
@@ -841,7 +842,8 @@ class DLL_LINKAGE IQuestObject
 public:
 	CQuest * quest;
 
-	IQuestObject(){quest = new CQuest;};
+	IQuestObject(): quest(new CQuest()){};
+	virtual ~IQuestObject() {};
 	virtual void getVisitText (MetaString &text, std::vector<Component> &components, bool isCustom, bool FirstVisit, const CGHeroInstance * h = nullptr) const;
 	virtual bool checkQuest (const CGHeroInstance * h) const;
 

lib/CThreadHelper.cpp

@@ -31,9 +31,9 @@ void CThreadHelper::run()
 }
 void CThreadHelper::processTasks()
 {
-	int pom;
 	while(true)
 	{
+		int pom;
 		{
 			boost::unique_lock<boost::mutex> lock(rtinm);
 			if((pom = currentTask) >= amount)

lib/JsonNode.cpp

@@ -90,12 +90,12 @@ bool JsonNode::operator == (const JsonNode &other) const
 	{
 		switch(type)
 		{
-			break; case DATA_NULL:   return true;
+			case DATA_NULL:   return true;
-			break; case DATA_BOOL:   return Bool() == other.Bool();
+			case DATA_BOOL:   return Bool() == other.Bool();
-			break; case DATA_FLOAT:  return Float() == other.Float();
+			case DATA_FLOAT:  return Float() == other.Float();
-			break; case DATA_STRING: return String() == other.String();
+			case DATA_STRING: return String() == other.String();
-			break; case DATA_VECTOR: return Vector() == other.Vector();
+			case DATA_VECTOR: return Vector() == other.Vector();
-			break; case DATA_STRUCT: return Struct() == other.Struct();
+			case DATA_STRUCT: return Struct() == other.Struct();
 		}
 	}
 	return false;
@@ -521,7 +521,7 @@ Bonus * JsonUtils::parseBonus (const JsonNode &ability)
 
 //returns first Key with value equal to given one
 template<class Key, class Val>
-Key reverseMapFirst(const Val & val, const std::map<Key, Val> map)
+Key reverseMapFirst(const Val & val, const std::map<Key, Val> & map)
 {
 	for(auto it : map)
 	{

lib/NetPacks.h

@@ -68,12 +68,12 @@ struct CPackForServer : public CPack
 	PlayerColor player;
 	CConnection *c;
 	CGameState* GS(CGameHandler *gh);
-	CPackForServer()
+	CPackForServer():
+		player(PlayerColor::NEUTRAL),
+		c(nullptr)
 	{
 		type = 2;
-		c = nullptr;
+	}
-		player = PlayerColor::NEUTRAL;
-	};
 
 	bool applyGh(CGameHandler *gh);//called after applying to gs
 };
@@ -177,10 +177,10 @@ struct StackLocation
 
 	StackLocation()
 	{}
-	StackLocation(const CArmedInstance *Army, SlotID Slot)
+	StackLocation(const CArmedInstance *Army, SlotID Slot):
+		army(const_cast<CArmedInstance*>(Army)), //we are allowed here to const cast -> change will go through one of our packages... do not abuse!
+		slot(Slot)
 	{
-		army = const_cast<CArmedInstance*>(Army); //we are allowed here to const cast -> change will go through one of our packages... do not abuse!
-		slot = Slot;
 	}
 
 	DLL_LINKAGE const CStackInstance *getStack();

lib/filesystem/Filesystem.h

@@ -79,7 +79,7 @@ public:
 	/**
 	 * Ctor. Can be used to create indentifier for resource loading using one parameter
 	 *
-	 * @param name The resource name including extension.
+	 * @param fullName The resource name including extension.
 	 */
 	explicit ResourceID(std::string fullName);
 
@@ -166,8 +166,8 @@ public:
 	 * Will load all filesystem data from Json data at this path (config/filesystem.json)
 	 * @param prefix - prefix for all paths in filesystem config
 	 */
-	static void loadMainFileSystem(const std::string & fsConfigURI);
 	static void loadModFileSystem(const std::string &prefix, const JsonNode & fsConfig);
+	static void loadMainFileSystem(const std::string & fsConfigURI);
 	static void loadDirectory(const std::string &prefix, const std::string & mountPoint, const JsonNode & config);
 	static void loadZipArchive(const std::string &prefix, const std::string & mountPoint, const JsonNode & config);
 	static void loadArchive(const std::string &prefix, const std::string & mountPoint, const JsonNode & config, EResType::Type archiveType);

lib/mapping/MapFormatH3M.cpp

@@ -1535,12 +1535,11 @@ void CMapLoaderH3M::readCreatureSet(CCreatureSet * out, int number)
 
 CGObjectInstance * CMapLoaderH3M::readHero(ObjectInstanceID idToBeGiven)
 {
-	auto  nhi = new CGHeroInstance();
+	auto nhi = new CGHeroInstance();
 
-	int identifier = 0;
 	if(map->version > EMapFormat::ROE)
 	{
-		identifier = reader.readUInt32();
+		unsigned int identifier = reader.readUInt32();
 		map->questIdentifierToId[identifier] = idToBeGiven;
 	}
 

scripting/erm/ERMInterpreter.cpp

@@ -889,7 +889,6 @@ struct IFPerformer : StandardReceiverVisitor<TUnusedType>
 
 	void operator()(TNormalBodyOption const& trig) const override
 	{
-		std::string message; //to be shown
 		switch(trig.optionCode)
 		{
 		case 'M': //Show the message (Text) or contents of z$ variable on the screen immediately.
@@ -1380,9 +1379,9 @@ struct ERMExpDispatch : boost::static_visitor<>
 		}
 		else if(trig.name == "HE")
 		{
-			const CGHeroInstance * hero = nullptr;
 			if(trig.identifier.is_initialized())
 			{
+				const CGHeroInstance * hero = nullptr;
 				ERM::Tidentifier tid = trig.identifier.get();
 				switch(tid.size())
 				{

server/CGameHandler.cpp

@@ -2401,7 +2401,6 @@ bool CGameHandler::buildStructure( ObjectInstanceID tid, BuildingID requestedID,
 
 		case CBuilding::BUILD_SPECIAL:
 			COMPLAIN_RET("This building can not be constructed!");
-			break;
 
 		case CBuilding::BUILD_GRAIL  :
 			if(requestedBuilding->mode == CBuilding::BUILD_GRAIL) //needs grail
@@ -3357,7 +3356,6 @@ bool CGameHandler::makeBattleAction( BattleAction &ba )
 			}
 			break;
 		}
-		break;
 	case Battle::WALK_AND_ATTACK: //walk or attack
 		{
 			StartAction start_action(ba);
@@ -5346,7 +5344,6 @@ void CGameHandler::attackCasting(const BattleAttack & bat, Bonus::BonusType atta
 				}
 			}
 			bool castMe = false;
-			int meleeRanged;
 			if(oneOfAttacked == nullptr) //all attacked creatures have been killed
 				return;
 			int spellLevel = 0;
@@ -5354,7 +5351,7 @@ void CGameHandler::attackCasting(const BattleAttack & bat, Bonus::BonusType atta
 			for(const Bonus *sf : *spellsByType)
 			{
 				vstd::amax(spellLevel, sf->additionalInfo % 1000); //pick highest level
-				meleeRanged = sf->additionalInfo / 1000;
+				int meleeRanged = sf->additionalInfo / 1000;
 				if (meleeRanged == 0 || (meleeRanged == 1 && bat.shot()) || (meleeRanged == 2 && !bat.shot()))
 					castMe = true;
 			}
@@ -5648,7 +5645,6 @@ bool CGameHandler::castSpell(const CGHeroInstance *h, SpellID spellID, const int
 	case SpellID::VIEW_AIR:
 	default:
 		COMPLAIN_RET("This spell is not implemented yet!");
-		break;
 	}
 
 	SetMana sm;

server/CQuery.cpp

@@ -268,7 +268,7 @@ CGarrisonDialogQuery::CGarrisonDialogQuery(const CArmedInstance *up, const CArme
 
 bool CGarrisonDialogQuery::blocksPack(const CPack *pack) const 
 {
-	std::set<ObjectInstanceID> ourIds, idsAttempted;
+	std::set<ObjectInstanceID> ourIds;
 	ourIds.insert(this->exchangingArmies[0]->id);
 	ourIds.insert(this->exchangingArmies[1]->id);
 

server/CVCMIServer.cpp

@@ -429,11 +429,9 @@ void CVCMIServer::start()
 		case 0:
 			firstConnection->close();
 			exit(0);
-			break;
 		case 1:
 			firstConnection->close();
 			return;
-			break;
 		case 2:
 			newGame();
 			break;