Fix some of the new warnings from sonarcloud

AI/BattleAI/AttackPossibility.cpp

@@ -58,7 +58,7 @@ void DamageCache::buildObstacleDamageCache(std::shared_ptr<HypotheticBattle> hb,
 			return u->alive() && !u->isTurret() && u->getPosition().isValid();
 		});
 
-		std::shared_ptr<HypotheticBattle> inner = std::make_shared<HypotheticBattle>(hb->env, hb);
+		auto inner = std::make_shared<HypotheticBattle>(hb->env, hb);
 
 		for(auto stack : stacks)
 		{

AI/Nullkiller/AIGateway.cpp

@@ -649,12 +649,12 @@ void AIGateway::showBlockingDialog(const std::string & text, const std::vector<C
 				auto danger = nullkiller->dangerEvaluator->evaluateDanger(target, hero.get());
 				auto ratio = static_cast<float>(danger) / hero->getTotalStrength();
 
-				answer = 1;
+				answer = true;
 				
 				if(topObj->id != goalObjectID && nullkiller->dangerEvaluator->evaluateDanger(topObj) > 0)
 				{
 					// no if we do not aim to visit this object
-					answer = 0;
+					answer = false;
 				}
 				
 				logAi->trace("Query hook: %s(%s) by %s danger ratio %f", target.toString(), topObj->getObjectName(), hero.name(), ratio);

AI/Nullkiller/Pathfinding/AIPathfinderConfig.cpp

@@ -13,7 +13,7 @@
 #include "Rules/AIMovementAfterDestinationRule.h"
 #include "Rules/AIMovementToDestinationRule.h"
 #include "Rules/AIPreviousNodeRule.h"
-#include "../Engine//Nullkiller.h"
+#include "../Engine/Nullkiller.h"
 
 #include "../../../lib/pathfinder/CPathfinder.h"
 

AI/Nullkiller/Pathfinding/Actors.cpp

@@ -182,7 +182,7 @@ ExchangeResult HeroActor::tryExchangeNoLock(const ChainActor * specialActor, con
 		return &actor == specialActor;
 	});
 
-	result.actor = &(dynamic_cast<HeroActor *>(result.actor)->specialActors[index]);
+	result.actor = &(dynamic_cast<HeroActor *>(result.actor)->specialActors.at(index));
 
 	return result;
 }

AI/Nullkiller/Pathfinding/Actors.h

@@ -113,7 +113,7 @@ public:
 	static const int SPECIAL_ACTORS_COUNT = 7;
 
 private:
-	ChainActor specialActors[SPECIAL_ACTORS_COUNT];
+	std::array<ChainActor, SPECIAL_ACTORS_COUNT> specialActors;
 	std::unique_ptr<HeroExchangeMap> exchangeMap;
 
 	void setupSpecialActors();

client/Client.h

@@ -158,7 +158,7 @@ public:
 	friend class CBattleCallback; //handling players actions
 
 	void changeSpells(const CGHeroInstance * hero, bool give, const std::set<SpellID> & spells) override {};
-	void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> spells, bool accepted) override {};
+	void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> & spells, bool accepted) override {};
 	bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) override {return false;};
 	void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) override {};
 	void setOwner(const CGObjectInstance * obj, PlayerColor owner) override {};

client/PlayerLocalState.cpp

@@ -24,7 +24,7 @@ PlayerLocalState::PlayerLocalState(CPlayerInterface & owner)
 {
 }
 
-const PlayerSpellbookSetting & PlayerLocalState::getSpellbookSettings()
+const PlayerSpellbookSetting & PlayerLocalState::getSpellbookSettings() const
 {
 	return spellbookSettings;
 }

client/PlayerLocalState.h

@@ -55,7 +55,7 @@ public:
 	void setHeroAsleep(const CGHeroInstance * hero);
 	void setHeroAwaken(const CGHeroInstance * hero);
 
-	const PlayerSpellbookSetting & getSpellbookSettings();
+	const PlayerSpellbookSetting & getSpellbookSettings() const;
 	void setSpellbookSettings(const PlayerSpellbookSetting & newSettings);
 
 	const std::vector<const CGTownInstance *> & getOwnedTowns();

client/media/CVideoHandler.cpp

@@ -545,7 +545,7 @@ std::pair<std::unique_ptr<ui8 []>, si64> CAudioInstance::extractAudio(const Vide
 			frameSamplesBuffer.resize(std::max(frameSamplesBuffer.size(), bytesToRead));
 			uint8_t * frameSamplesPtr = frameSamplesBuffer.data();
 
-			int result = swr_convert(swr_ctx, &frameSamplesPtr, frame->nb_samples, (const uint8_t **)frame->data, frame->nb_samples);
+			int result = swr_convert(swr_ctx, &frameSamplesPtr, frame->nb_samples, const_cast<const uint8_t **>(frame->data), frame->nb_samples);
 
 			if (result < 0)
 				throwFFmpegError(result);

client/render/AssetGenerator.cpp

@@ -158,7 +158,7 @@ void AssetGenerator::createPlayerColoredBackground(const PlayerColor & player)
 	assert(player.isValidPlayer());
 	if (!player.isValidPlayer())
 	{
-		logGlobal->error("Unable to colorize to invalid player color %d!", static_cast<int>(player.getNum()));
+		logGlobal->error("Unable to colorize to invalid player color %d!", player.getNum());
 		return;
 	}
 

client/renderSDL/CTrueTypeFont.cpp

@@ -64,9 +64,9 @@ int CTrueTypeFont::getFontStyle(const JsonNode &config) const
 CTrueTypeFont::CTrueTypeFont(const JsonNode & fontConfig):
 	data(loadData(fontConfig)),
 	font(loadFont(fontConfig), TTF_CloseFont),
-	dropShadow(!fontConfig["noShadow"].Bool()),
+	blended(true),
 	outline(fontConfig["outline"].Bool()),
-	blended(true)
+	dropShadow(!fontConfig["noShadow"].Bool())
 {
 	assert(font);
 
@@ -95,14 +95,14 @@ size_t CTrueTypeFont::getLineHeightScaled() const
 	return TTF_FontHeight(font.get());
 }
 
-size_t CTrueTypeFont::getGlyphWidthScaled(const char *data) const
+size_t CTrueTypeFont::getGlyphWidthScaled(const char *text) const
 {
-	return getStringWidthScaled(std::string(data, TextOperations::getUnicodeCharacterSize(*data)));
+	return getStringWidthScaled(std::string(text, TextOperations::getUnicodeCharacterSize(*text)));
 }
 
-bool CTrueTypeFont::canRepresentCharacter(const char * data) const
+bool CTrueTypeFont::canRepresentCharacter(const char * text) const
 {
-	uint32_t codepoint = TextOperations::getUnicodeCodepoint(data, TextOperations::getUnicodeCharacterSize(*data));
+	uint32_t codepoint = TextOperations::getUnicodeCodepoint(text, TextOperations::getUnicodeCharacterSize(*text));
 #if SDL_TTF_VERSION_ATLEAST(2, 0, 18)
 	return TTF_GlyphIsProvided32(font.get(), codepoint);
 #elif SDL_TTF_VERSION_ATLEAST(2, 0, 12)
@@ -114,10 +114,10 @@ bool CTrueTypeFont::canRepresentCharacter(const char * data) const
 #endif
 }
 
-size_t CTrueTypeFont::getStringWidthScaled(const std::string & data) const
+size_t CTrueTypeFont::getStringWidthScaled(const std::string & text) const
 {
 	int width;
-	TTF_SizeUTF8(font.get(), data.c_str(), &width, nullptr);
+	TTF_SizeUTF8(font.get(), text.c_str(), &width, nullptr);
 	return width;
 }
 

client/widgets/TextControls.cpp

@@ -309,7 +309,7 @@ void CMultiLineLabel::splitText(const std::string & Txt, bool redrawAfter)
 	lines.clear();
 
 	const auto & fontPtr = GH.renderHandler().loadFont(font);
-	int lineHeight = static_cast<int>(fontPtr->getLineHeight());
+	int lineHeight = fontPtr->getLineHeight();
 
 	lines = CMessage::breakText(Txt, pos.w, font);
 
@@ -330,16 +330,16 @@ Rect CMultiLineLabel::getTextLocation()
 		return pos;
 
 	const auto & fontPtr = GH.renderHandler().loadFont(font);
-	Point textSize(pos.w, fontPtr->getLineHeight() * (int)lines.size());
-	Point textOffset(pos.w - textSize.x, pos.h - textSize.y);
+	Point textSizeComputed(pos.w, fontPtr->getLineHeight() * lines.size()); //FIXME: how is this different from textSize member?
+	Point textOffset(pos.w - textSizeComputed.x, pos.h - textSizeComputed.y);
 
 	switch(alignment)
 	{
-	case ETextAlignment::TOPLEFT:     return Rect(pos.topLeft(), textSize);
-	case ETextAlignment::TOPCENTER:   return Rect(pos.topLeft(), textSize);
-	case ETextAlignment::CENTER:      return Rect(pos.topLeft() + textOffset / 2, textSize);
-	case ETextAlignment::CENTERRIGHT: return Rect(pos.topLeft() + Point(textOffset.x, textOffset.y / 2), textSize);
-	case ETextAlignment::BOTTOMRIGHT: return Rect(pos.topLeft() + textOffset, textSize);
+	case ETextAlignment::TOPLEFT:     return Rect(pos.topLeft(), textSizeComputed);
+	case ETextAlignment::TOPCENTER:   return Rect(pos.topLeft(), textSizeComputed);
+	case ETextAlignment::CENTER:      return Rect(pos.topLeft() + textOffset / 2, textSizeComputed);
+	case ETextAlignment::CENTERRIGHT: return Rect(pos.topLeft() + Point(textOffset.x, textOffset.y / 2), textSizeComputed);
+	case ETextAlignment::BOTTOMRIGHT: return Rect(pos.topLeft() + textOffset, textSizeComputed);
 	}
 	assert(0);
 	return Rect();

client/windows/CCastleInterface.cpp

@@ -2007,10 +2007,10 @@ void CMageGuildScreen::updateSpells(ObjectInstanceID tID)
 
 	const CGTownInstance * town = LOCPLINT->cb->getTown(townId);
 
-	for(size_t i=0; i<town->getTown()->mageLevel; i++)
+	for(uint32_t i=0; i<town->getTown()->mageLevel; i++)
 	{
-		size_t spellCount = town->spellsAtLevel((int)i+1,false); //spell at level with -1 hmmm?
-		for(size_t j=0; j<spellCount; j++)
+		uint32_t spellCount = town->spellsAtLevel(i+1,false); //spell at level with -1 hmmm?
+		for(uint32_t j=0; j<spellCount; j++)
 		{
 			if(i<town->mageGuildLevel() && town->spells[i].size()>j)
 				spells.push_back(std::make_shared<Scroll>(positions[i][j], town->spells[i][j].toSpell(), townId));
@@ -2063,30 +2063,26 @@ void CMageGuildScreen::Scroll::clickPressed(const Point & cursorPosition)
 			resComps.push_back(std::make_shared<CComponent>(ComponentType::RESOURCE, i->resType, i->resVal, CComponent::ESize::medium));
 		}
 
-		auto showSpellResearchDialog = [this, resComps, town, cost, newSpell](){
-			std::vector<std::pair<AnimationPath, CFunctionList<void()>>> pom;
-			for(int i = 0; i < 3; i++)
-				pom.emplace_back(AnimationPath::builtin("settingsWindow/button80"), nullptr);
-
-			auto text = CGI->generaltexth->translate(LOCPLINT->cb->getResourceAmount().canAfford(cost) ? "vcmi.spellResearch.pay" : "vcmi.spellResearch.canNotAfford");
-			boost::replace_first(text, "%SPELL1", spell->id.toSpell()->getNameTranslated());
-			boost::replace_first(text, "%SPELL2", newSpell.toSpell()->getNameTranslated());
-			auto temp = std::make_shared<CInfoWindow>(text, LOCPLINT->playerID, resComps, pom);
-
-			temp->buttons[0]->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("spellResearch/accept")));
-			temp->buttons[0]->addCallback([this, town](){ LOCPLINT->cb->spellResearch(town, spell->id, true); });
-			temp->buttons[0]->addPopupCallback([](){ CRClickPopup::createAndPush(CGI->generaltexth->translate("vcmi.spellResearch.research")); });
-			temp->buttons[0]->setEnabled(LOCPLINT->cb->getResourceAmount().canAfford(cost));
-			temp->buttons[1]->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("spellResearch/reroll")));
-			temp->buttons[1]->addCallback([this, town](){ LOCPLINT->cb->spellResearch(town, spell->id, false); });
-			temp->buttons[1]->addPopupCallback([](){ CRClickPopup::createAndPush(CGI->generaltexth->translate("vcmi.spellResearch.skip")); });
-			temp->buttons[2]->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("spellResearch/close")));
-			temp->buttons[2]->addPopupCallback([](){ CRClickPopup::createAndPush(CGI->generaltexth->translate("vcmi.spellResearch.abort")); });
-
-			GH.windows().pushWindow(temp);
-		};
-
-		showSpellResearchDialog();
+		std::vector<std::pair<AnimationPath, CFunctionList<void()>>> pom;
+		for(int i = 0; i < 3; i++)
+			pom.emplace_back(AnimationPath::builtin("settingsWindow/button80"), nullptr);
+
+		auto text = CGI->generaltexth->translate(LOCPLINT->cb->getResourceAmount().canAfford(cost) ? "vcmi.spellResearch.pay" : "vcmi.spellResearch.canNotAfford");
+		boost::replace_first(text, "%SPELL1", spell->id.toSpell()->getNameTranslated());
+		boost::replace_first(text, "%SPELL2", newSpell.toSpell()->getNameTranslated());
+		auto temp = std::make_shared<CInfoWindow>(text, LOCPLINT->playerID, resComps, pom);
+
+		temp->buttons[0]->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("spellResearch/accept")));
+		temp->buttons[0]->addCallback([this, town](){ LOCPLINT->cb->spellResearch(town, spell->id, true); });
+		temp->buttons[0]->addPopupCallback([](){ CRClickPopup::createAndPush(CGI->generaltexth->translate("vcmi.spellResearch.research")); });
+		temp->buttons[0]->setEnabled(LOCPLINT->cb->getResourceAmount().canAfford(cost));
+		temp->buttons[1]->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("spellResearch/reroll")));
+		temp->buttons[1]->addCallback([this, town](){ LOCPLINT->cb->spellResearch(town, spell->id, false); });
+		temp->buttons[1]->addPopupCallback([](){ CRClickPopup::createAndPush(CGI->generaltexth->translate("vcmi.spellResearch.skip")); });
+		temp->buttons[2]->setOverlay(std::make_shared<CPicture>(ImagePath::builtin("spellResearch/close")));
+		temp->buttons[2]->addPopupCallback([](){ CRClickPopup::createAndPush(CGI->generaltexth->translate("vcmi.spellResearch.abort")); });
+
+		GH.windows().pushWindow(temp);
 	}
 	else
 		LOCPLINT->showInfoDialog(spell->getDescriptionTranslated(0), std::make_shared<CComponent>(ComponentType::SPELL, spell->id));

client/windows/GUIClasses.cpp

@@ -1611,7 +1611,7 @@ void CObjectListWindow::keyPressed(EShortcut key)
 	changeSelection(sel);
 }
 
-VideoWindow::VideoWindow(VideoPath video, ImagePath rim, bool showBackground, float scaleFactor, std::function<void(bool skipped)> closeCb)
+VideoWindow::VideoWindow(const VideoPath & video, const ImagePath & rim, bool showBackground, float scaleFactor, const std::function<void(bool skipped)> & closeCb)
 	: CWindowObject(BORDERED | SHADOW_DISABLED | NEEDS_ANIMATED_BACKGROUND), closeCb(closeCb)
 {
 	OBJECT_CONSTRUCTION;

client/windows/GUIClasses.h

@@ -513,7 +513,7 @@ class VideoWindow : public CWindowObject
 
 	void exit(bool skipped);
 public:
-	VideoWindow(VideoPath video, ImagePath rim, bool showBackground, float scaleFactor, std::function<void(bool)> closeCb);
+	VideoWindow(const VideoPath & video, const ImagePath & rim, bool showBackground, float scaleFactor, const std::function<void(bool)> & closeCb);
 
 	void clickPressed(const Point & cursorPosition) override;
 	void keyPressed(EShortcut key) override;

client/windows/InfoWindows.cpp

@@ -187,11 +187,6 @@ bool CRClickPopup::isPopupWindow() const
 	return true;
 }
 
-void CRClickPopup::close()
-{
-	WindowBase::close();
-}
-
 void CRClickPopup::createAndPush(const std::string & txt, const CInfoWindow::TCompsInfo & comps)
 {
 	PlayerColor player = LOCPLINT ? LOCPLINT->playerID : PlayerColor(1); //if no player, then use blue

client/windows/InfoWindows.h

@@ -64,7 +64,6 @@ public:
 class CRClickPopup : public WindowBase
 {
 public:
-	void close() override;
 	bool isPopupWindow() const override;
 
 	static std::shared_ptr<WindowBase> createCustomInfoWindow(Point position, const CGObjectInstance * specific);

lib/CPlayerState.cpp

@@ -22,9 +22,9 @@ VCMI_LIB_NAMESPACE_BEGIN
 
 PlayerState::PlayerState()
 	: color(-1)
-	, playerLocalSettings(std::make_unique<JsonNode>())
 	, human(false)
 	, cheated(false)
+	, playerLocalSettings(std::make_unique<JsonNode>())
 	, enteredWinningCheatCode(false)
 	, enteredLosingCheatCode(false)
 	, status(EPlayerStatus::INGAME)

lib/IGameCallback.h

@@ -94,7 +94,7 @@ public:
 	virtual void showInfoDialog(InfoWindow * iw) = 0;
 
 	virtual void changeSpells(const CGHeroInstance * hero, bool give, const std::set<SpellID> &spells)=0;
-	virtual void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> spells, bool accepted)=0;
+	virtual void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> & spells, bool accepted)=0;
 	virtual bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) = 0;
 	virtual void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) = 0;
 	virtual void setOwner(const CGObjectInstance * objid, PlayerColor owner)=0;

lib/json/JsonValidator.cpp

@@ -74,7 +74,7 @@ static int getLevenshteinDistance(const std::string & s, const std::string & t)
 
 /// Searches for keys similar to 'target' in 'candidates' map
 /// Returns closest match or empty string if no suitable candidates are found
-static std::string findClosestMatch(JsonMap candidates, std::string target)
+static std::string findClosestMatch(const JsonMap & candidates, const std::string & target)
 {
 	// Maximum distance at which we can consider strings to be similar
 	// If strings have more different symbols than this number then it is not a typo, but a completely different word

lib/modding/ContentTypeHandler.cpp

@@ -197,8 +197,8 @@ void ContentTypeHandler::afterLoadFinalization()
 			std::set<std::string> conflictingMods;
 			std::set<std::string> resolvedConflicts;
 
-			for (auto const & conflictModData : conflictModData.Struct())
-				conflictingMods.insert(conflictModData.first);
+			for (auto const & conflictModEntry: conflictModData.Struct())
+				conflictingMods.insert(conflictModEntry.first);
 
 			for (auto const & modID : conflictingMods)
 				resolvedConflicts.merge(VLC->modh->getModDependencies(modID));

lib/texts/TextLocalizationContainer.cpp

@@ -152,11 +152,9 @@ void TextLocalizationContainer::exportAllTexts(std::map<std::string, std::map<st
 		std::string textToWrite;
 		std::string modName = entry.second.baseStringModContext;
 
-		if (entry.second.baseStringModContext == entry.second.identifierModContext)
-		{
-			if (modName.find('.') != std::string::npos)
-				modName = modName.substr(0, modName.find('.'));
-		}
+		if (entry.second.baseStringModContext == entry.second.identifierModContext && modName.find('.') != std::string::npos)
+			modName = modName.substr(0, modName.find('.'));
+
 		boost::range::replace(modName, '.', '_');
 
 		textToWrite = entry.second.translatedText;

server/CGameHandler.cpp

@@ -1253,7 +1253,7 @@ void CGameHandler::changeSpells(const CGHeroInstance * hero, bool give, const st
 	sendAndApply(cs);
 }
 
-void CGameHandler::setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> spells, bool accepted)
+void CGameHandler::setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> & spells, bool accepted)
 {
 	SetResearchedSpells cs;
 	cs.tid = town->id;

server/CGameHandler.h

@@ -107,7 +107,7 @@ public:
 	//from IGameCallback
 	//do sth
 	void changeSpells(const CGHeroInstance * hero, bool give, const std::set<SpellID> &spells) override;
-	void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> spells, bool accepted) override;
+	void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> & spells, bool accepted) override;
 	bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) override;
 	void setOwner(const CGObjectInstance * obj, PlayerColor owner) override;
 	void giveExperience(const CGHeroInstance * hero, TExpType val) override;

server/CVCMIServer.cpp

@@ -299,17 +299,12 @@ void CVCMIServer::onDisconnected(const std::shared_ptr<INetworkConnection> & con
 	std::shared_ptr<CConnection> c = findConnection(connection);
 
 	// player may have already disconnected via clientDisconnected call
-	if (c)
+	if (c && gh && getState() == EServerState::GAMEPLAY)
 	{
-		//clientDisconnected(c);
-
-		if(gh && getState() == EServerState::GAMEPLAY)
-		{
-			LobbyClientDisconnected lcd;
-			lcd.c = c;
-			lcd.clientId = c->connectionID;
-			handleReceivedPack(lcd);
-		}
+		LobbyClientDisconnected lcd;
+		lcd.c = c;
+		lcd.clientId = c->connectionID;
+		handleReceivedPack(lcd);
 	}
 }
 

server/queries/VisitQueries.cpp

@@ -50,7 +50,6 @@ void MapObjectVisitQuery::onRemoval(PlayerColor color)
 {
 	gh->objectVisitEnded(visitingHero, players.front());
 
-	//TODO or should it be destructor?
 	//Can object visit affect 2 players and what would be desired behavior?
 	if(removeObjectAfterVisit)
 		gh->removeObject(visitedObject, color);
@@ -78,7 +77,7 @@ void TownBuildingVisitQuery::onAdded(PlayerColor color)
 	while (!visitedBuilding.empty() && owner->topQuery(color).get() == this)
 	{
 		visitingHero = visitedBuilding.back().hero;
-		auto * building = visitedTown->rewardableBuildings.at(visitedBuilding.back().building);
+		const auto * building = visitedTown->rewardableBuildings.at(visitedBuilding.back().building);
 		building->onHeroVisit(visitingHero);
 		visitedBuilding.pop_back();
 	}

test/mock/mock_IGameCallback.h

@@ -44,7 +44,7 @@ public:
 	void showInfoDialog(InfoWindow * iw) override {}
 
 	void changeSpells(const CGHeroInstance * hero, bool give, const std::set<SpellID> &spells) override {}
-	void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> spells, bool accepted) override {}
+	void setResearchedSpells(const CGTownInstance * town, int level, const std::vector<SpellID> & spells, bool accepted) override {}
 	bool removeObject(const CGObjectInstance * obj, const PlayerColor & initiator) override {return false;}
 	void createBoat(const int3 & visitablePosition, BoatId type, PlayerColor initiator) override {}
 	void setOwner(const CGObjectInstance * objid, PlayerColor owner) override {}