Merge pull request #4443 from IvanSavenko/fix_sonar

Fix miscellaneous issues discovered by Sonar

AI/BattleAI/StackWithBonuses.h

@@ -24,7 +24,7 @@ class HypotheticBattle;
 class RNGStub final : public vstd::RNG
 {
 public:
-	virtual int nextInt() override
+	int nextInt() override
 	{
 		return 0;
 	}

AI/Nullkiller/Behaviors/CaptureObjectsBehavior.cpp

@@ -212,7 +212,7 @@ void CaptureObjectsBehavior::decomposeObjects(
 				vstd::concatenate(tasksLocal, getVisitGoals(paths, nullkiller, objToVisit, specificObjects));
 			}
 
-			std::lock_guard<std::mutex> lock(sync); // FIXME: consider using tbb::parallel_reduce instead to avoid mutex overhead
+			std::lock_guard lock(sync); // FIXME: consider using tbb::parallel_reduce instead to avoid mutex overhead
 			vstd::concatenate(result, tasksLocal);
 		});
 }

AI/Nullkiller/Engine/PriorityEvaluator.cpp

@@ -141,12 +141,10 @@ int32_t getResourcesGoldReward(const TResources & res)
 {
 	int32_t result = 0;
 
-	for(EGameResID r = EGameResID(0); r < EGameResID::COUNT; r.advance(1))
+	for(auto r : GameResID::ALL_RESOURCES())
 	{
 		if(res[r] > 0)
-		{
 			result += r == EGameResID::GOLD ? res[r] : res[r] * 100;
-		}
 	}
 
 	return result;
@@ -350,7 +348,7 @@ uint64_t RewardEvaluator::getArmyReward(
 			{
 				for(auto artID : info.reward.artifacts)
 				{
-					const CArtifact * art = dynamic_cast<const CArtifact *>(VLC->artifacts()->getById(artID));
+					const auto * art = dynamic_cast<const CArtifact *>(VLC->artifacts()->getById(artID));
 
 					rewardValue += evaluateArtifactArmyValue(art);
 				}
@@ -358,7 +356,7 @@ uint64_t RewardEvaluator::getArmyReward(
 
 			if(!info.reward.creatures.empty())
 			{
-				for(auto stackInfo : info.reward.creatures)
+				for(const auto & stackInfo : info.reward.creatures)
 				{
 					rewardValue += stackInfo.getType()->getAIValue() * stackInfo.getCount();
 				}

AI/Nullkiller/Pathfinding/ObjectGraphCalculator.cpp

@@ -321,7 +321,7 @@ void ObjectGraphCalculator::addObjectActor(const CGObjectInstance * obj)
 
 void ObjectGraphCalculator::addJunctionActor(const int3 & visitablePos, bool isVirtualBoat)
 {
-	std::lock_guard<std::mutex> lock(syncLock);
+	std::lock_guard lock(syncLock);
 
 	auto internalCb = temporaryActorHeroes.front()->cb;
 	auto objectActor = temporaryActorHeroes.emplace_back(std::make_unique<CGHeroInstance>(internalCb)).get();

client/adventureMap/AdventureMapShortcuts.cpp

@@ -530,7 +530,6 @@ bool AdventureMapShortcuts::optionCanVisitObject()
 	auto * hero = LOCPLINT->localState->getCurrentHero();
 	auto objects = LOCPLINT->cb->getVisitableObjs(hero->visitablePos());
 
-	//assert(vstd::contains(objects,hero));
 	return objects.size() > 1; // there is object other than our hero
 }
 

client/battle/BattleInterfaceClasses.cpp

@@ -433,7 +433,7 @@ QuickSpellPanel::QuickSpellPanel(BattleInterface & owner)
 	create();
 }
 
-std::vector<std::tuple<SpellID, bool>> QuickSpellPanel::getSpells()
+std::vector<std::tuple<SpellID, bool>> QuickSpellPanel::getSpells() const
 {
 	std::vector<SpellID> spellIds;
 	std::vector<bool> spellIdsFromSetting;

client/battle/BattleInterfaceClasses.h

@@ -169,7 +169,7 @@ public:
 
 	void create();
 
-	std::vector<std::tuple<SpellID, bool>> getSpells();
+	std::vector<std::tuple<SpellID, bool>> getSpells() const;
 
 	void show(Canvas & to) override;
 	void inputModeChanged(InputMode modi) override;

client/battle/BattleOverlayLogVisualizer.cpp

@@ -27,7 +27,7 @@ BattleOverlayLogVisualizer::BattleOverlayLogVisualizer(
 {
 }
 
-void BattleOverlayLogVisualizer::drawText(BattleHex hex, int lineNumber, std::string text)
+void BattleOverlayLogVisualizer::drawText(BattleHex hex, int lineNumber, const std::string & text)
 {
 	Point offset = owner.fieldController->hexPositionLocal(hex).topLeft() + Point(20, 20);
 	int h = graphics->fonts[EFonts::FONT_TINY]->getLineHeight();

client/battle/BattleOverlayLogVisualizer.h

@@ -24,5 +24,5 @@ private:
 public:
 	BattleOverlayLogVisualizer(BattleRenderer::RendererRef & target, BattleInterface & owner);
 
-	void drawText(BattleHex hex, int lineNumber, std::string text) override;
+	void drawText(BattleHex hex, int lineNumber, const std::string & text) override;
 };

client/gui/CIntObject.h

@@ -212,7 +212,7 @@ class EmptyStatusBar : public IStatusBar
 	virtual void setEnteredText(const std::string & text){};
 };
 
-class ObjectConstruction
+class ObjectConstruction : boost::noncopyable
 {
 public:
 	ObjectConstruction(CIntObject *obj);

client/lobby/OptionsTab.cpp

@@ -494,7 +494,7 @@ void OptionsTab::SelectionWindow::reopen()
 	if(type == SelType::HERO && SEL->getStartInfo()->playerInfos.find(color)->second.castle == FactionID::RANDOM)
 		close();
 	else{
-		auto window = std::shared_ptr<SelectionWindow>(new SelectionWindow(color, type, slider ? slider->getValue() : 0));
+		auto window = std::make_shared<SelectionWindow>(color, type, slider ? slider->getValue() : 0);
 		close();
 		if(CSH->isMyColor(color) || CSH->isHost())
 			GH.windows().pushWindow(window);

client/mainmenu/CMainMenu.cpp

@@ -475,7 +475,7 @@ void CMultiMode::joinTCP()
 	GH.windows().createAndPushWindow<CMultiPlayers>(getPlayersNames(), savedScreenType, false, ELoadMode::MULTI);
 }
 
-const std::vector<std::string> CMultiMode::getPlayersNames()
+std::vector<std::string> CMultiMode::getPlayersNames()
 {
 	std::vector<std::string> playerNames;
 

client/mainmenu/CMainMenu.h

@@ -97,7 +97,7 @@ public:
 	void joinTCP();
 
 	/// Get all configured player names. The first name would always be present and initialized to its default value.
-	const std::vector<std::string> getPlayersNames();
+	std::vector<std::string> getPlayersNames();
 
 	void onNameChange(std::string newText);
 };

client/mapView/MapOverlayLogVisualizer.cpp

@@ -54,8 +54,8 @@ void MapOverlayLogVisualizer::drawLine(int3 start, int3 end)
 void MapOverlayLogVisualizer::drawText(
 	int3 tile,
 	int lineNumber,
-	std::string text,
-	std::optional<ColorRGBA> background)
+	const std::string & text,
+	const std::optional<ColorRGBA> & background)
 {
 	const Point offset = Point(6, 6);
 

client/mapView/MapOverlayLogVisualizer.h

@@ -29,5 +29,5 @@ private:
 public:
 	MapOverlayLogVisualizer(Canvas & target, std::shared_ptr<MapViewModel> model);
 	void drawLine(int3 start, int3 end) override;
-	void drawText(int3 tile, int lineNumber, std::string text, std::optional<ColorRGBA> color) override;
+	void drawText(int3 tile, int lineNumber, const std::string & text, const std::optional<ColorRGBA> & color) override;
 };

client/mapView/MapRendererContext.cpp

@@ -291,7 +291,7 @@ ColorRGBA MapRendererAdventureContext::overlayTextColor(const int3 & coordinates
 	if (!tile.visitable)
 		return {};
 
-	auto * object = tile.visitableObjects.back();
+	const auto * object = tile.visitableObjects.back();
 
 	if (object->getOwner() == LOCPLINT->playerID)
 		return { 0, 192, 0};

client/widgets/Images.cpp

@@ -188,17 +188,6 @@ CAnimImage::CAnimImage(const AnimationPath & name, size_t Frame, size_t Group, i
 	init();
 }
 
-//CAnimImage::CAnimImage(std::shared_ptr<CAnimation> Anim, size_t Frame, size_t Group, int x, int y, ui8 Flags):
-//	anim(Anim),
-//	frame(Frame),
-//	group(Group),
-//	flags(Flags)
-//{
-//	pos.x += x;
-//	pos.y += y;
-//	init();
-//}
-
 CAnimImage::CAnimImage(const AnimationPath & name, size_t Frame, Rect targetPos, size_t Group, ui8 Flags):
 	anim(GH.renderHandler().loadAnimation(name, EImageBlitMode::COLORKEY)),
 	frame(Frame),

client/widgets/Images.h

@@ -114,7 +114,6 @@ public:
 	bool visible;
 
 	CAnimImage(const AnimationPath & name, size_t Frame, size_t Group=0, int x=0, int y=0, ui8 Flags=0);
-//	CAnimImage(std::shared_ptr<CAnimation> Anim, size_t Frame, size_t Group=0, int x=0, int y=0, ui8 Flags=0);
 	CAnimImage(const AnimationPath & name, size_t Frame, Rect targetPos, size_t Group=0, ui8 Flags=0);
 	~CAnimImage();
 

client/windows/CMapOverview.cpp

@@ -41,7 +41,7 @@
 #include "../../lib/texts/CGeneralTextHandler.h"
 #include "../../lib/texts/TextOperations.h"
 
-CMapOverview::CMapOverview(std::string mapName, std::string fileName, std::string date, std::string author, std::string version, ResourcePath resource, ESelectionScreen tabType)
+CMapOverview::CMapOverview(const std::string & mapName, const std::string & fileName, const std::string & date, const std::string & author, const std::string & version, const ResourcePath & resource, ESelectionScreen tabType)
 	: CWindowObject(BORDERED | RCLICK_POPUP), resource(resource), mapName(mapName), fileName(fileName), date(date), author(author), version(version), tabType(tabType)
 {
 

client/windows/CMapOverview.h

@@ -57,5 +57,5 @@ public:
 	const std::string version;
 	const ESelectionScreen tabType;
 
-	CMapOverview(std::string mapName, std::string fileName, std::string date, std::string author, std::string version, ResourcePath resource, ESelectionScreen tabType);
+	CMapOverview(const std::string & mapName, const std::string & fileName, const std::string & date, const std::string & author, const std::string & version, const ResourcePath & resource, ESelectionScreen tabType);
 };

client/windows/CPuzzleWindow.cpp

@@ -54,7 +54,7 @@ CPuzzleWindow::CPuzzleWindow(const int3 & GrailPos, double discoveredRatio)
 	{
 		const SPuzzleInfo & info = elem;
 
-		auto piece = std::make_shared<CPicture>(info.filename, info.x, info.y);
+		auto piece = std::make_shared<CPicture>(info.filename, info.position.x, info.position.y);
 
 		//piece that will slowly disappear
 		if(info.whenUncovered <= GameConstants::PUZZLE_MAP_PIECES * discoveredRatio)

client/windows/CSpellWindow.cpp

@@ -98,7 +98,7 @@ public:
 	}
 };
 
-CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells, std::function<void(SpellID)> onSpellSelect):
+CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells, const std::function<void(SpellID)> & onSpellSelect):
 	CWindowObject(PLAYER_COLORED | (settings["gameTweaks"]["enableLargeSpellbook"].Bool() ? BORDERED : 0)),
 	battleSpellsOnly(openOnBattleSpells),
 	selectedTab(4),

client/windows/CSpellWindow.h

@@ -119,7 +119,7 @@ class CSpellWindow : public CWindowObject
 	std::function<void(SpellID)> onSpellSelect; //external processing of selected spell
 
 public:
-	CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells = true, std::function<void(SpellID)> onSpellSelect = nullptr);
+	CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells = true, const std::function<void(SpellID)> & onSpellSelect = nullptr);
 	~CSpellWindow();
 
 	void fexitb();

lib/CArtHandler.cpp

@@ -760,7 +760,7 @@ const CArtifactInstance * CArtifactSet::getArtByInstanceId(const ArtifactInstanc
 	return nullptr;
 }
 
-const ArtifactPosition CArtifactSet::getArtPos(const CArtifactInstance * artInst) const
+ArtifactPosition CArtifactSet::getArtPos(const CArtifactInstance * artInst) const
 {
 	if(artInst)
 	{

lib/CArtHandler.h

@@ -207,7 +207,7 @@ public:
 	/// Looks for equipped artifact with given ID and returns its slot ID or -1 if none
 	/// (if more than one such artifact lower ID is returned)
 	ArtifactPosition getArtPos(const ArtifactID & aid, bool onlyWorn = true, bool allowLocked = true) const;
-	const ArtifactPosition getArtPos(const CArtifactInstance * art) const;
+	ArtifactPosition getArtPos(const CArtifactInstance * art) const;
 	std::vector<ArtifactPosition> getAllArtPositions(const ArtifactID & aid, bool onlyWorn, bool allowLocked, bool getAll) const;
 	std::vector<ArtifactPosition> getBackpackArtPositions(const ArtifactID & aid) const;
 	const CArtifactInstance * getArtByInstanceId(const ArtifactInstanceID & artInstId) const;

lib/bonuses/BonusEnum.h

@@ -222,7 +222,7 @@ namespace BonusDuration  //when bonus is automatically removed
 {
 	// We use uint16_t directly because std::bitset<11> eats whole 8 byte word.
 	using Type = uint16_t;
-	constexpr static size_t Size = 11;
+	constexpr size_t Size = 11;
 
 	enum BonusDuration : Type {
 		PERMANENT = 1 << 0,

lib/campaign/CampaignHandler.cpp

@@ -604,15 +604,15 @@ std::vector< std::vector<ui8> > CampaignHandler::getFile(std::unique_ptr<CInputS
 	if (magic == zipHeaderMagic) // ZIP archive - assume VCMP format
 	{
 		CInputStream * buffer(file.get());
-		std::shared_ptr<CIOApi> ioApi(new CProxyROIOApi(buffer));
+		auto ioApi = std::make_shared<CProxyROIOApi>(buffer);
 		CZipLoader loader("", "_", ioApi);
 
 		// load header
-		JsonPath resource = JsonPath::builtin(VCMP_HEADER_FILE_NAME);
-		if(!loader.existsResource(resource))
-			throw std::runtime_error(resource.getName() + " not found in " + filename);
-		auto data = loader.load(resource)->readAll();
-		ret.push_back(std::vector<ui8>(data.first.get(), data.first.get() + data.second));
+		JsonPath jsonPath = JsonPath::builtin(VCMP_HEADER_FILE_NAME);
+		if(!loader.existsResource(jsonPath))
+			throw std::runtime_error(jsonPath.getName() + " not found in " + filename);
+		auto data = loader.load(jsonPath)->readAll();
+		ret.emplace_back(data.first.get(), data.first.get() + data.second);
 
 		if(headerOnly)
 			return ret;
@@ -621,11 +621,11 @@ std::vector< std::vector<ui8> > CampaignHandler::getFile(std::unique_ptr<CInputS
 		JsonNode header(reinterpret_cast<const std::byte*>(data.first.get()), data.second, VCMP_HEADER_FILE_NAME);
 		for(auto scenario : header["scenarios"].Vector())
 		{
-			ResourcePath resource = ResourcePath(scenario["map"].String(), EResType::MAP);
-			if(!loader.existsResource(resource))
-				throw std::runtime_error(resource.getName() + " not found in " + filename);
-			auto data = loader.load(resource)->readAll();
-			ret.push_back(std::vector<ui8>(data.first.get(), data.first.get() + data.second));
+			ResourcePath mapPath(scenario["map"].String(), EResType::MAP);
+			if(!loader.existsResource(mapPath))
+				throw std::runtime_error(mapPath.getName() + " not found in " + filename);
+			auto data = loader.load(mapPath)->readAll();
+			ret.emplace_back(data.first.get(), data.first.get() + data.second);
 		}
 
 		return ret;

lib/entities/building/CBuilding.cpp

@@ -33,7 +33,7 @@ const std::map<std::string, CBuilding::ETowerHeight> CBuilding::TOWER_TYPES =
 		{ "skyship", CBuilding::HEIGHT_SKYSHIP }
 };
 
-const BuildingTypeUniqueID CBuilding::getUniqueTypeID() const
+BuildingTypeUniqueID CBuilding::getUniqueTypeID() const
 {
 	return BuildingTypeUniqueID(town->faction->getId(), bid);
 }

lib/entities/building/CBuilding.h

@@ -66,7 +66,7 @@ public:
 
 	CBuilding() : town(nullptr), mode(BUILD_NORMAL) {};
 
-	const BuildingTypeUniqueID getUniqueTypeID() const;
+	BuildingTypeUniqueID getUniqueTypeID() const;
 
 	std::string getJsonKey() const;
 

lib/entities/faction/CFaction.h

@@ -11,6 +11,7 @@
 
 #include <vcmi/Faction.h>
 
+#include "../../Point.h"
 #include "../../constants/EntityIdentifiers.h"
 #include "../../constants/Enumerations.h"
 #include "../../filesystem/ResourcePath.h"
@@ -21,8 +22,8 @@ class CTown;
 
 struct DLL_LINKAGE SPuzzleInfo
 {
+	Point position;
 	ui16 number; //type of puzzle
-	si16 x, y; //position
 	ui16 whenUncovered; //determines the sequence of discovering (the lesser it is the sooner puzzle will be discovered)
 	ImagePath filename; //file with graphic of this puzzle
 };

lib/entities/faction/CTownHandler.cpp

@@ -733,8 +733,8 @@ void CTownHandler::loadPuzzle(CFaction &faction, const JsonNode &source) const
 		size_t index = faction.puzzleMap.size();
 		SPuzzleInfo spi;
 
-		spi.x = static_cast<si16>(piece["x"].Float());
-		spi.y = static_cast<si16>(piece["y"].Float());
+		spi.position.x = static_cast<si16>(piece["x"].Float());
+		spi.position.y = static_cast<si16>(piece["y"].Float());
 		spi.whenUncovered = static_cast<ui16>(piece["index"].Float());
 		spi.number = static_cast<ui16>(index);
 

lib/gameState/GameStatistics.cpp

@@ -41,7 +41,7 @@ StatisticDataSetEntry StatisticDataSet::createEntry(const PlayerState * ps, cons
 	scenarioHighScores.isCampaign = false;
 
 	data.map = gs->map->name.toString();
-	data.timestamp = std::time(0);
+	data.timestamp = std::time(nullptr);
 	data.day = gs->getDate(Date::DAY);
 	data.player = ps->color;
 	data.team = ps->team;
@@ -129,7 +129,7 @@ std::string StatisticDataSet::toCsv()
 		ss << GameConstants::PLAYER_COLOR_NAMES[entry.player] << ";";
 		ss << entry.team.getNum() << ";";
 		ss << entry.isHuman << ";";
-		ss << (int)entry.status << ";";
+		ss << static_cast<int>(entry.status) << ";";
 		ss << entry.numberHeroes << ";";
 		ss << entry.numberTowns <<  ";";
 		ss << entry.numberArtifacts << ";";
@@ -170,8 +170,6 @@ std::vector<const CGMine *> Statistic::getMines(const CGameState * gs, const Pla
 {
 	std::vector<const CGMine *> tmp;
 
-	/// FIXME: Dirty dirty hack
-	/// Stats helper need some access to gamestate.
 	std::vector<const CGObjectInstance *> ownedObjects;
 	for(const CGObjectInstance * obj : gs->map->objects)
 	{
@@ -202,7 +200,7 @@ int Statistic::getNumberOfArts(const PlayerState * ps)
 	int ret = 0;
 	for(auto h : ps->heroes)
 	{
-		ret += (int)h->artifactsInBackpack.size() + (int)h->artifactsWorn.size();
+		ret += h->artifactsInBackpack.size() + h->artifactsWorn.size();
 	}
 	return ret;
 }
@@ -297,9 +295,7 @@ std::vector<std::vector<PlayerColor>> Statistic::getRank(std::vector<std::pair<P
 
 	//put first element
 	std::vector< std::vector<PlayerColor> > ret;
-	std::vector<PlayerColor> tmp;
-	tmp.push_back( stats[0].first );
-	ret.push_back( tmp );
+	ret.push_back( { stats[0].first } );
 
 	//the rest of elements
 	for(int g=1; g<stats.size(); ++g)
@@ -311,9 +307,7 @@ std::vector<std::vector<PlayerColor>> Statistic::getRank(std::vector<std::pair<P
 		else
 		{
 			//create next occupied rank
-			std::vector<PlayerColor> tmp;
-			tmp.push_back(stats[g].first);
-			ret.push_back(tmp);
+			ret.push_back( { stats[g].first });
 		}
 	}
 
@@ -332,7 +326,7 @@ float Statistic::getObeliskVisitedRatio(const CGameState * gs, const TeamID & t)
 {
 	if(!gs->map->obeliskCount)
 		return 0;
-	return (float)getObeliskVisited(gs, t) / (float)gs->map->obeliskCount;
+	return static_cast<float>(getObeliskVisited(gs, t)) / gs->map->obeliskCount;
 }
 
 std::map<EGameResID, int> Statistic::getNumMines(const CGameState * gs, const PlayerState * ps)

lib/gameState/HighScore.cpp

@@ -52,8 +52,8 @@ HighScoreCalculation::Result HighScoreCalculation::calculate()
 {
 	Result firstResult;
 	Result summary;
-	const std::array<double, 5> difficultyMultipliers{0.8, 1.0, 1.3, 1.6, 2.0}; 
-	for(auto & param : parameters)
+	const std::array<double, 5> difficultyMultipliers{0.8, 1.0, 1.3, 1.6, 2.0};
+	for(const auto & param : parameters)
 	{
 		double tmp = 200 - (param.day + 10) / (param.townAmount + 5) + (param.allEnemiesDefeated ? 25 : 0) + (param.hasGrail ? 25 : 0);
 		firstResult = Result{static_cast<int>(tmp), static_cast<int>(tmp * difficultyMultipliers.at(param.difficulty)), param.day, param.usedCheat};

lib/logging/VisualLogger.cpp

@@ -15,9 +15,9 @@ VCMI_LIB_NAMESPACE_BEGIN
 DLL_LINKAGE VisualLogger * logVisual = new VisualLogger();
 
 
-void VisualLogger::updateWithLock(std::string channel, std::function<void(IVisualLogBuilder & logBuilder)> func)
+void VisualLogger::updateWithLock(const std::string & channel, const std::function<void(IVisualLogBuilder & logBuilder)> & func)
 {
-	std::lock_guard<std::mutex> lock(mutex);
+	std::lock_guard lock(mutex);
 
 	mapLines[channel].clear();
 	mapTexts[channel].clear();
@@ -30,21 +30,21 @@ void VisualLogger::updateWithLock(std::string channel, std::function<void(IVisua
 
 void VisualLogger::visualize(IMapOverlayLogVisualizer & visulizer)
 {
-	std::lock_guard<std::mutex> lock(mutex);
+	std::lock_guard lock(mutex);
 
-	for(auto line : mapLines[keyToShow])
+	for(const auto & line : mapLines[keyToShow])
 	{
 		visulizer.drawLine(line.start, line.end);
 	}
 
 	std::map<int3, std::vector<Text<int3>>> textMap;
 
-	for(auto line : mapTexts[keyToShow])
+	for(const auto & line : mapTexts[keyToShow])
 	{
 		textMap[line.tile].push_back(line);
 	}
 
-	for(auto & pair : textMap)
+	for(const auto & pair : textMap)
 	{
 		for(int i = 0; i < pair.second.size(); i++)
 		{
@@ -55,7 +55,7 @@ void VisualLogger::visualize(IMapOverlayLogVisualizer & visulizer)
 
 void VisualLogger::visualize(IBattleOverlayLogVisualizer & visulizer)
 {
-	std::lock_guard<std::mutex> lock(mutex);
+	std::lock_guard lock(mutex);
 	std::map<BattleHex, std::vector<std::string>> textMap;
 
 	for(auto line : battleTexts[keyToShow])
@@ -72,12 +72,12 @@ void VisualLogger::visualize(IBattleOverlayLogVisualizer & visulizer)
 	}
 }
 
-void VisualLogger::setKey(std::string key)
+void VisualLogger::setKey(const std::string & key)
 {
 	keyToShow = key;
 }
 
-void IVisualLogBuilder::addText(int3 tile, std::string text, PlayerColor background)
+void IVisualLogBuilder::addText(int3 tile, const std::string & text, PlayerColor background)
 {
 	std::optional<ColorRGBA> rgbColor;
 

lib/logging/VisualLogger.h

@@ -20,23 +20,23 @@ class IMapOverlayLogVisualizer
 {
 public:
 	virtual void drawLine(int3 start, int3 end) = 0;
-	virtual void drawText(int3 tile, int lineNumber, std::string text, std::optional<ColorRGBA> background) = 0;
+	virtual void drawText(int3 tile, int lineNumber, const std::string & text, const std::optional<ColorRGBA> & color) = 0;
 };
 
 class IBattleOverlayLogVisualizer
 {
 public:
-	virtual void drawText(BattleHex tile, int lineNumber, std::string text) = 0;
+	virtual void drawText(BattleHex tile, int lineNumber, const std::string & text) = 0;
 };
 
 class DLL_LINKAGE IVisualLogBuilder
 {
 public:
 	virtual void addLine(int3 start, int3 end) = 0;
-	virtual void addText(int3 tile, std::string text, std::optional<ColorRGBA> color = {}) = 0;
-	virtual void addText(BattleHex tile, std::string text) = 0;
+	virtual void addText(int3 tile, const std::string & text, const std::optional<ColorRGBA> & color = {}) = 0;
+	virtual void addText(BattleHex tile, const std::string & text) = 0;
 
-	void addText(int3 tile, std::string text, PlayerColor background);
+	void addText(int3 tile, const std::string & text, PlayerColor background);
 };
 
 /// The logger is used to show screen overlay
@@ -89,12 +89,12 @@ private:
 			mapLines.emplace_back(start, end);
 		}
 
-		void addText(BattleHex tile, std::string text) override
+		void addText(BattleHex tile, const std::string & text) override
 		{
 			battleTexts.emplace_back(tile, text, std::optional<ColorRGBA>());
 		}
 
-		void addText(int3 tile, std::string text, std::optional<ColorRGBA> background) override
+		void addText(int3 tile, const std::string & text, const std::optional<ColorRGBA> & background) override
 		{
 			mapTexts.emplace_back(tile, text, background);
 		}
@@ -109,10 +109,10 @@ private:
 
 public:
 
-	void updateWithLock(std::string channel, std::function<void(IVisualLogBuilder & logBuilder)> func);
+	void updateWithLock(const std::string & channel, const std::function<void(IVisualLogBuilder & logBuilder)> & func);
 	void visualize(IMapOverlayLogVisualizer & visulizer);
 	void visualize(IBattleOverlayLogVisualizer & visulizer);
-	void setKey(std::string key);
+	void setKey(const std::string & key);
 };
 
 extern DLL_LINKAGE VisualLogger * logVisual;

lib/mapping/CMapEditManager.cpp

@@ -124,26 +124,26 @@ CMap * CMapEditManager::getMap()
 	return map;
 }
 
-void CMapEditManager::clearTerrain(vstd::RNG * gen)
+void CMapEditManager::clearTerrain(vstd::RNG * customGen)
 {
-	execute(std::make_unique<CClearTerrainOperation>(map, gen ? gen : this->gen.get()));
+	execute(std::make_unique<CClearTerrainOperation>(map, customGen ? customGen : gen.get()));
 }
 
-void CMapEditManager::drawTerrain(TerrainId terType, int decorationsPercentage, vstd::RNG * gen)
+void CMapEditManager::drawTerrain(TerrainId terType, int decorationsPercentage, vstd::RNG * customGen)
 {
-	execute(std::make_unique<CDrawTerrainOperation>(map, terrainSel, terType, decorationsPercentage, gen ? gen : this->gen.get()));
+	execute(std::make_unique<CDrawTerrainOperation>(map, terrainSel, terType, decorationsPercentage, customGen ? customGen : gen.get()));
 	terrainSel.clearSelection();
 }
 
-void CMapEditManager::drawRoad(RoadId roadType, vstd::RNG* gen)
+void CMapEditManager::drawRoad(RoadId roadType, vstd::RNG* customGen)
 {
-	execute(std::make_unique<CDrawRoadsOperation>(map, terrainSel, roadType, gen ? gen : this->gen.get()));
+	execute(std::make_unique<CDrawRoadsOperation>(map, terrainSel, roadType, customGen ? customGen : gen.get()));
 	terrainSel.clearSelection();
 }
 
-void CMapEditManager::drawRiver(RiverId riverType, vstd::RNG* gen)
+void CMapEditManager::drawRiver(RiverId riverType, vstd::RNG* customGen)
 {
-	execute(std::make_unique<CDrawRiversOperation>(map, terrainSel, riverType, gen ? gen : this->gen.get()));
+	execute(std::make_unique<CDrawRiversOperation>(map, terrainSel, riverType, customGen ? customGen : gen.get()));
 	terrainSel.clearSelection();
 }
 

lib/network/NetworkConnection.cpp

@@ -136,7 +136,7 @@ void NetworkConnection::setAsyncWritesEnabled(bool on)
 
 void NetworkConnection::sendPacket(const std::vector<std::byte> & message)
 {
-	std::lock_guard<std::mutex> lock(writeMutex);
+	std::lock_guard lock(writeMutex);
 	std::vector<std::byte> headerVector(sizeof(uint32_t));
 	uint32_t messageSize = message.size();
 	std::memcpy(headerVector.data(), &messageSize, sizeof(uint32_t));
@@ -177,7 +177,7 @@ void NetworkConnection::doSendData()
 
 void NetworkConnection::onDataSent(const boost::system::error_code & ec)
 {
-	std::lock_guard<std::mutex> lock(writeMutex);
+	std::lock_guard lock(writeMutex);
 	dataToSend.pop_front();
 	if (ec)
 	{

lib/networkPacks/NetPacksLib.cpp

@@ -1496,7 +1496,7 @@ void NewObject::applyGs(CGameState *gs)
 	gs->map->addBlockVisTiles(newObject);
 	gs->map->calculateGuardingGreaturePositions();
 
-	logGlobal->debug("Added object id=%d; address=%x; name=%s", newObject->id, (intptr_t)newObject, newObject->getObjectName());
+	logGlobal->debug("Added object id=%d; name=%s", newObject->id, newObject->getObjectName());
 }
 
 void NewArtifact::applyGs(CGameState *gs)

lib/serializer/BinaryDeserializer.h

@@ -58,7 +58,7 @@ class BinaryDeserializer : public CLoaderBase
 		if(reader->smartVectorMembersSerialization)
 		{
 			if(const auto *info = reader->getVectorizedTypeInfo<CArmedInstance, ObjectInstanceID>())
-				armyPtr = static_cast<CArmedInstance *>(reader->getVectorItemFromId<CArmedInstance, ObjectInstanceID>(*info, armyID));
+				armyPtr = reader->getVectorItemFromId<CArmedInstance, ObjectInstanceID>(*info, armyID);
 		}
 
 		if(slot != SlotID::COMMANDER_SLOT_PLACEHOLDER)

lib/texts/CGeneralTextHandler.h

@@ -58,7 +58,9 @@ public:
 	LegacyTextContainer turnDurations; //turn durations for pregame (1 Minute ... Unlimited)
 
 	//towns
-	LegacyTextContainer tcommands, hcommands, fcommands; //texts for town screen, town hall screen and fort screen
+	LegacyTextContainer tcommands; //texts for town screen,
+	LegacyTextContainer hcommands; // town hall screen
+	LegacyTextContainer fcommands; // fort screen
 	LegacyTextContainer tavernInfo;
 	LegacyTextContainer tavernRumors;
 

lib/texts/CLegacyConfigParser.cpp

@@ -48,7 +48,7 @@ std::string CLegacyConfigParser::extractQuotedPart()
 	assert(*curr == '\"');
 
 	curr++; // skip quote
-	char * begin = curr;
+	const char * begin = curr;
 
 	while (curr != end && *curr != '\"' && *curr != '\t')
 		curr++;
@@ -73,7 +73,7 @@ std::string CLegacyConfigParser::extractQuotedString()
 		//extract normal part
 		else if(curr < end && *curr != '\t' && *curr != '\r')
 		{
-			char * begin = curr;
+			const char * begin = curr;
 
 			while (curr < end && *curr != '\t' && *curr != '\r' && *curr != '\"')//find end of string or next quoted part start
 				curr++;
@@ -90,7 +90,7 @@ std::string CLegacyConfigParser::extractQuotedString()
 
 std::string CLegacyConfigParser::extractNormalString()
 {
-	char * begin = curr;
+	const char * begin = curr;
 
 	while (curr < end && *curr != '\t' && *curr != '\r')//find end of string
 		curr++;
@@ -140,7 +140,7 @@ float CLegacyConfigParser::readNumber()
 
 bool CLegacyConfigParser::isNextEntryEmpty() const
 {
-	char * nextSymbol = curr;
+	const char * nextSymbol = curr;
 	while (nextSymbol < end && *nextSymbol == ' ')
 		nextSymbol++; //find next meaningful symbol
 

lib/texts/CLegacyConfigParser.h

@@ -13,18 +13,14 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-//class CInputStream;
-//class JsonNode;
-//class JsonSerializeFormat;
-
 /// Parser for any text files from H3
 class DLL_LINKAGE CLegacyConfigParser
 {
 	std::string fileEncoding;
 
 	std::unique_ptr<char[]> data;
-	char * curr;
-	char * end;
+	const char * curr;
+	const char * end;
 
 	/// extracts part of quoted string.
 	std::string extractQuotedPart();
@@ -49,7 +45,7 @@ public:
 		std::vector<numeric> ret;
 		ret.reserve(size);
 		while (size--)
-			ret.push_back((numeric)readNumber());
+			ret.push_back(readNumber());
 		return ret;
 	}
 

lib/texts/TextLocalizationContainer.h

@@ -99,7 +99,7 @@ public:
 	template <typename Handler>
 	void serialize(Handler & h)
 	{
-		std::lock_guard<std::recursive_mutex> globalLock(globalTextMutex);
+		std::lock_guard globalLock(globalTextMutex);
 
 		if (h.version >= Handler::Version::SIMPLE_TEXT_CONTAINER_SERIALIZATION)
 		{
@@ -123,7 +123,7 @@ public:
 
 			if(h.saving)
 			{
-				for(auto s : stringsLocalizations)
+				for(auto & s : stringsLocalizations)
 				{
 					key = s.first;
 					h & key;

server/CGameHandler.cpp

@@ -671,7 +671,7 @@ void CGameHandler::onPlayerTurnEnded(PlayerColor which)
 
 void CGameHandler::addStatistics()
 {
-	for (auto & elem : gs->players)
+	for (const auto & elem : gs->players)
 	{
 		if (elem.first == PlayerColor::NEUTRAL || !elem.first.isValidPlayer())
 			continue;
@@ -707,7 +707,7 @@ void CGameHandler::onNewTurn()
 		}
 	}
 
-	for (auto & player : gs->players)
+	for (const auto & player : gs->players)
 	{
 		if (player.second.status != EPlayerStatus::INGAME)
 			continue;