Fix issues detected by Sonar

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -904,6 +904,7 @@ ExchangeCandidate HeroChainCalculationTask::calculateExchange(
 	candidate.setCost(carrierParentNode->getCost() + otherParentNode->getCost() / 1000.0);
 	candidate.moveRemains = carrierParentNode->moveRemains;
 	candidate.danger = carrierParentNode->danger;
+	candidate.version = carrierParentNode->version;
 
 	if(carrierParentNode->turns < otherParentNode->turns)
 	{

client/Client.h

@@ -80,8 +80,7 @@ public:
 	void waitWhileContains(const T & item)
 	{
 		TLock lock(mx);
-		while(vstd::contains(items, item))
-			cond.wait(lock);
+		cond.wait(lock, [this, &item](){ return !vstd::contains(items, item);});
 
 		if (isTerminating)
 			throw TerminationRequestedException();

client/renderSDL/ScreenHandler.cpp

@@ -34,7 +34,6 @@
 // TODO: should be made into a private members of ScreenHandler
 SDL_Renderer * mainRenderer = nullptr;
 
-static const std::string NAME = GameConstants::VCMI_VERSION; //application name
 static constexpr Point heroes3Resolution = Point(800, 600);
 
 std::tuple<int, int> ScreenHandler::getSupportedScalingRange() const
@@ -436,7 +435,7 @@ SDL_Window * ScreenHandler::createWindowImpl(Point dimensions, int flags, bool c
 	int displayIndex = getPreferredDisplayIndex();
 	int positionFlags = center ? SDL_WINDOWPOS_CENTERED_DISPLAY(displayIndex) : SDL_WINDOWPOS_UNDEFINED_DISPLAY(displayIndex);
 
-	return SDL_CreateWindow(NAME.c_str(), positionFlags, positionFlags, dimensions.x, dimensions.y, flags);
+	return SDL_CreateWindow(GameConstants::VCMI_VERSION.c_str(), positionFlags, positionFlags, dimensions.x, dimensions.y, flags);
 }
 
 SDL_Window * ScreenHandler::createWindow()

clientapp/EntryPoint.cpp

@@ -293,8 +293,6 @@ int main(int argc, char * argv[])
 	testFile("DATA/PLAYERS.PAL", "Heroes III data files (Data/H3Bitmap.lod) are incomplete or corruped!\n Please reinstall them.");
 	testFile("SPRITES/DEFAULT.DEF", "Heroes III data files (Data/H3Sprite.lod) are incomplete or corruped!\n Please reinstall them.");
 
-	srand ( (unsigned int)time(nullptr) );
-
 	if(!settings["session"]["headless"].Bool())
 		ENGINE = std::make_unique<GameEngine>();
 

lib/CCreatureHandler.cpp

@@ -348,6 +348,14 @@ std::string CCreature::nodeName() const
 	return "\"" + getNamePluralTextID() + "\"";
 }
 
+int CCreature::getRandomAmount(vstd::RNG & ranGen) const
+{
+	if(ammMax > ammMin)
+		return ammMin + (ranGen.nextInt(ammMin, ammMax));
+	else
+		return ammMax;
+}
+
 void CCreature::updateFrom(const JsonNode & data)
 {
 	JsonUpdater handler(nullptr, data);

lib/CCreatureHandler.h

@@ -169,15 +169,7 @@ public:
 	void addBonus(int val, BonusType type, BonusSubtypeID subtype);
 	std::string nodeName() const override;
 
-	template<typename RanGen>
-	int getRandomAmount(RanGen ranGen) const
-	{
-		if(ammMax == ammMin)
-			return ammMax;
-		else
-			return ammMin + (ranGen() % (ammMax - ammMin));
-	}
-
+	int getRandomAmount(vstd::RNG & ranGen) const;
 	void updateFrom(const JsonNode & data);
 	void serializeJson(JsonSerializeFormat & handler);
 

lib/CCreatureSet.cpp

@@ -967,7 +967,7 @@ ui64 CStackInstance::getMarketValue() const
 	return getType()->getFullRecruitCost().marketValue() * getCount();
 }
 
-ArtBearer::ArtBearer CStackInstance::bearerType() const
+ArtBearer CStackInstance::bearerType() const
 {
 	return ArtBearer::CREATURE;
 }
@@ -1086,7 +1086,7 @@ void CCommanderInstance::levelUp ()
 	}
 }
 
-ArtBearer::ArtBearer CCommanderInstance::bearerType() const
+ArtBearer CCommanderInstance::bearerType() const
 {
 	return ArtBearer::COMMANDER;
 }

lib/CCreatureSet.h

@@ -161,7 +161,7 @@ public:
 	bool valid(bool allowUnrandomized) const;
 	ArtPlacementMap putArtifact(const ArtifactPosition & pos, const CArtifactInstance * art) override;//from CArtifactSet
 	void removeArtifact(const ArtifactPosition & pos) override;
-	ArtBearer::ArtBearer bearerType() const override; //from CArtifactSet
+	ArtBearer bearerType() const override; //from CArtifactSet
 	std::string nodeName() const override; //from CBonusSystemnode
 	PlayerColor getOwner() const override;
 
@@ -192,7 +192,7 @@ public:
 	ui64 getPower() const override {return 0;};
 	int getExpRank() const override;
 	int getLevel() const override;
-	ArtBearer::ArtBearer bearerType() const override; //from CArtifactSet
+	ArtBearer bearerType() const override; //from CArtifactSet
 
 	template <typename Handler> void serialize(Handler &h)
 	{

lib/constants/StringConstants.h

@@ -19,7 +19,7 @@ VCMI_LIB_NAMESPACE_BEGIN
 namespace GameConstants
 {
 	const std::string RESOURCE_NAMES [RESOURCE_QUANTITY] = {
-	    "wood", "mercury", "ore", "sulfur", "crystal", "gems", "gold", "mithril"
+		"wood", "mercury", "ore", "sulfur", "crystal", "gems", "gold", "mithril"
 	};
 
 	const std::string PLAYER_COLOR_NAMES [PlayerColor::PLAYER_LIMIT_I] = {
@@ -50,7 +50,7 @@ namespace NSecondarySkill
 
 	const std::vector<std::string> levels =
 	{
-	    "none", "basic", "advanced", "expert"
+		"none", "basic", "advanced", "expert"
 	};
 }
 
@@ -83,7 +83,7 @@ namespace NFaction
 
 namespace NArtifactPosition
 {
-	const std::string namesHero [19] =
+	inline constexpr std::array namesHero =
 	{
 		"head", "shoulders", "neck", "rightHand", "leftHand", "torso", //5
 		"rightRing", "leftRing", "feet", //8
@@ -92,12 +92,12 @@ namespace NArtifactPosition
 		"spellbook", "misc5" //18
 	};
 
-	const std::string namesCreature[1] =
+	inline constexpr std::array namesCreature =
 	{
 		"creature1"
 	};
 
-	const std::string namesCommander[6] =
+	inline constexpr std::array namesCommander =
 	{
 		"commander1", "commander2", "commander3", "commander4", "commander5", "commander6",
 	};
@@ -110,10 +110,10 @@ namespace NMetaclass
 {
     const std::string names [16] =
     {
-		"",
-		"artifact", "creature", "faction", "experience", "hero",
-		"heroClass", "luck", "mana", "morale", "movement",
-		"object", "primarySkill", "secondarySkill", "spell", "resource"
+        "",
+        "artifact", "creature", "faction", "experience", "hero",
+        "heroClass", "luck", "mana", "morale", "movement",
+        "object", "primarySkill", "secondarySkill", "spell", "resource"
     };
 }
 

lib/entities/artifact/ArtBearer.h

@@ -17,14 +17,11 @@ ART_BEARER(HERO)\
 	ART_BEARER(COMMANDER)\
 	ART_BEARER(ALTAR)
 
-namespace ArtBearer
+enum class ArtBearer
 {
-	enum ArtBearer
-	{
 #define ART_BEARER(x) x,
 		ART_BEARER_LIST
 #undef ART_BEARER
-	};
-}
+};
 
 VCMI_LIB_NAMESPACE_END

lib/entities/artifact/CArtHandler.cpp

@@ -189,7 +189,7 @@ std::shared_ptr<CArtifact> CArtHandler::loadFromJson(const std::string & scope,
 	const JsonNode & warMachine = node["warMachine"];
 	if(!warMachine.isNull())
 	{
-		LIBRARY->identifiers()->requestIdentifier("creature", warMachine, [=](si32 id)
+		LIBRARY->identifiers()->requestIdentifier("creature", warMachine, [art](si32 id)
 		{
 			art->warMachine = CreatureID(id);
 
@@ -198,7 +198,7 @@ std::shared_ptr<CArtifact> CArtHandler::loadFromJson(const std::string & scope,
 		});
 	}
 
-	LIBRARY->identifiers()->requestIdentifier(scope, "object", "artifact", [=](si32 index)
+	LIBRARY->identifiers()->requestIdentifier(scope, "object", "artifact", [scope, art](si32 index)
 	{
 		JsonNode conf;
 		conf.setModScope(scope);
@@ -292,9 +292,9 @@ void CArtHandler::loadSlots(CArtifact * art, const JsonNode & node) const
 	}
 }
 
-EArtifactClass::Type CArtHandler::stringToClass(const std::string & className)
+EArtifactClass CArtHandler::stringToClass(const std::string & className)
 {
-	static const std::map<std::string, EArtifactClass::Type> artifactClassMap =
+	static const std::map<std::string, EArtifactClass> artifactClassMap =
 	{
 		{"TREASURE", EArtifactClass::ART_TREASURE},
 		{"MINOR", EArtifactClass::ART_MINOR},
@@ -319,7 +319,7 @@ void CArtHandler::loadClass(CArtifact * art, const JsonNode & node) const
 void CArtHandler::loadType(CArtifact * art, const JsonNode & node) const
 {
 #define ART_BEARER(x) { #x, ArtBearer::x },
-	static const std::map<std::string, int> artifactBearerMap = { ART_BEARER_LIST };
+	static const std::map<std::string, ArtBearer> artifactBearerMap = { ART_BEARER_LIST };
 #undef ART_BEARER
 
 	for (const JsonNode & b : node["type"].Vector())
@@ -327,7 +327,7 @@ void CArtHandler::loadType(CArtifact * art, const JsonNode & node) const
 		auto it = artifactBearerMap.find (b.String());
 		if (it != artifactBearerMap.end())
 		{
-			int bearerType = it->second;
+			ArtBearer bearerType = it->second;
 			switch (bearerType)
 			{
 				case ArtBearer::HERO://TODO: allow arts having several possible bearers
@@ -388,7 +388,6 @@ void CArtHandler::makeItCommanderArt(CArtifact * a, bool onlyCommander)
 bool CArtHandler::legalArtifact(const ArtifactID & id) const
 {
 	auto art = id.toArtifact();
-	//assert ( (!art->constituents) || art->constituents->size() ); //artifacts is not combined or has some components
 
 	if(art->isCombined())
 		return false; //no combo artifacts spawning

lib/entities/artifact/CArtHandler.h

@@ -23,7 +23,7 @@ class DLL_LINKAGE CArtHandler : public CHandlerBase<ArtifactID, Artifact, CArtif
 public:
 	void addBonuses(CArtifact * art, const JsonNode & bonusList);
 
-	static EArtifactClass::Type stringToClass(const std::string & className); //TODO: rework EartClass to make this a constructor
+	static EArtifactClass stringToClass(const std::string & className); //TODO: rework EartClass to make this a constructor
 
 	bool legalArtifact(const ArtifactID & id) const;
 	static void makeItCreatureArt(CArtifact * a, bool onlyCreature = true);

lib/entities/artifact/CArtifact.cpp

@@ -229,9 +229,9 @@ bool CArtifact::canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot, b
 
 	if(slot == ArtifactPosition::FIRST_AVAILABLE)
 	{
-		for(const auto & slot : possibleSlots.at(artSet->bearerType()))
+		for(const auto & possibleSlot : possibleSlots.at(artSet->bearerType()))
 		{
-			if(artCanBePutAt(artSet, slot, assumeDestRemoved))
+			if(artCanBePutAt(artSet, possibleSlot, assumeDestRemoved))
 				return true;
 		}
 		return artCanBePutAt(artSet, ArtifactPosition::BACKPACK_START, assumeDestRemoved);
@@ -295,7 +295,7 @@ void CArtifact::addNewBonus(const std::shared_ptr<Bonus>& b)
 	CBonusSystemNode::addNewBonus(b);
 }
 
-const std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition>> & CArtifact::getPossibleSlots() const
+const std::map<ArtBearer, std::vector<ArtifactPosition>> & CArtifact::getPossibleSlots() const
 {
 	return possibleSlots;
 }
@@ -305,11 +305,11 @@ void CArtifact::updateFrom(const JsonNode& data)
 	//TODO:CArtifact::updateFrom
 }
 
-void CArtifact::setImage(int32_t iconIndex, std::string image, std::string large)
+void CArtifact::setImage(int32_t newIconIndex, const std::string & newImage, const std::string & newLargeImage)
 {
-	this->iconIndex = iconIndex;
-	this->image = image;
-	this->large = large;
+	iconIndex = newIconIndex;
+	image = newImage;
+	large = newLargeImage;
 }
 
 

lib/entities/artifact/CArtifact.h

@@ -77,10 +77,10 @@ class DLL_LINKAGE CArtifact final : public Artifact, public CBonusSystemNode, pu
 	uint32_t price;
 	CreatureID warMachine;
 	// Bearer Type => ids of slots where artifact can be placed
-	std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition>> possibleSlots;
+	std::map<ArtBearer, std::vector<ArtifactPosition>> possibleSlots;
 
 public:
-	EArtifactClass::Type aClass = EArtifactClass::ART_SPECIAL;
+	EArtifactClass aClass = EArtifactClass::ART_SPECIAL;
 	bool onlyOnWaterMap;
 
 	int32_t getIndex() const override;
@@ -107,12 +107,12 @@ public:
 	int getArtClassSerial() const; //0 - treasure, 1 - minor, 2 - major, 3 - relic, 4 - spell scroll, 5 - other
 	std::string nodeName() const override;
 	void addNewBonus(const std::shared_ptr<Bonus> & b) override;
-	const std::map<ArtBearer::ArtBearer, std::vector<ArtifactPosition>> & getPossibleSlots() const;
+	const std::map<ArtBearer, std::vector<ArtifactPosition>> & getPossibleSlots() const;
 
 	virtual bool canBePutAt(const CArtifactSet * artSet, ArtifactPosition slot = ArtifactPosition::FIRST_AVAILABLE, bool assumeDestRemoved = false) const;
 	void updateFrom(const JsonNode & data);
 	// Is used for testing purposes only
-	void setImage(int32_t iconIndex, std::string image, std::string large);
+	void setImage(int32_t iconIndex, const std::string & image, const std::string & large);
 
 	CArtifact();
 	~CArtifact();

lib/entities/artifact/CArtifactFittingSet.cpp

@@ -13,7 +13,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-CArtifactFittingSet::CArtifactFittingSet(IGameCallback *cb, ArtBearer::ArtBearer bearer)
+CArtifactFittingSet::CArtifactFittingSet(IGameCallback *cb, ArtBearer bearer)
 	: CArtifactSet(cb)
 	, GameCallbackHolder(cb)
 	, bearer(bearer)
@@ -28,7 +28,7 @@ CArtifactFittingSet::CArtifactFittingSet(const CArtifactSet & artSet)
 	artifactsTransitionPos = artSet.artifactsTransitionPos;
 }
 
-ArtBearer::ArtBearer CArtifactFittingSet::bearerType() const
+ArtBearer CArtifactFittingSet::bearerType() const
 {
 	return this->bearer;
 }

lib/entities/artifact/CArtifactFittingSet.h

@@ -24,12 +24,12 @@ class DLL_LINKAGE CArtifactFittingSet : public CArtifactSet, public GameCallback
 	}
 
 public:
-	CArtifactFittingSet(IGameCallback * cb, ArtBearer::ArtBearer Bearer);
+	CArtifactFittingSet(IGameCallback * cb, ArtBearer Bearer);
 	explicit CArtifactFittingSet(const CArtifactSet & artSet);
-	ArtBearer::ArtBearer bearerType() const override;
+	ArtBearer bearerType() const override;
 
 protected:
-	ArtBearer::ArtBearer bearer;
+	ArtBearer bearer;
 };
 
 VCMI_LIB_NAMESPACE_END

lib/entities/artifact/CArtifactInstance.cpp

@@ -20,8 +20,8 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 CCombinedArtifactInstance::PartInfo::PartInfo(const CArtifactInstance * artifact, ArtifactPosition slot)
-	: artifactID(artifact->getId())
-	, artifactPtr(artifact)
+	: artifactPtr(artifact)
+	, artifactID(artifact->getId())
 	, slot(slot)
 {
 }

lib/entities/artifact/CArtifactSet.cpp

@@ -376,12 +376,12 @@ void CArtifactSet::serializeJsonSlot(JsonSerializeFormat & handler, const Artifa
 		if(info != nullptr && !info->locked)
 		{
 			artifactID = info->getArt()->getTypeId();
-			handler.serializeId(NArtifactPosition::namesHero[slot.num], artifactID, ArtifactID::NONE);
+			handler.serializeId(NArtifactPosition::namesHero.at(slot.num), artifactID, ArtifactID::NONE);
 		}
 	}
 	else
 	{
-		handler.serializeId(NArtifactPosition::namesHero[slot.num], artifactID, ArtifactID::NONE);
+		handler.serializeId(NArtifactPosition::namesHero.at(slot.num), artifactID, ArtifactID::NONE);
 
 		if(artifactID != ArtifactID::NONE)
 		{

lib/entities/artifact/CArtifactSet.h

@@ -41,7 +41,7 @@ public:
 	bool isPositionFree(const ArtifactPosition & pos, bool onlyLockCheck = false) const;
 
 	virtual IGameCallback * getCallback() const = 0;
-	virtual ArtBearer::ArtBearer bearerType() const = 0;
+	virtual ArtBearer bearerType() const = 0;
 	virtual ArtPlacementMap putArtifact(const ArtifactPosition & slot, const CArtifactInstance * art);
 	virtual void removeArtifact(const ArtifactPosition & slot);
 	CArtifactSet(IGameCallback * cb);

lib/entities/artifact/EArtifactClass.h

@@ -11,16 +11,13 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-namespace EArtifactClass
+enum class EArtifactClass
 {
-	enum Type
-	{
-		ART_SPECIAL = 1,
-		ART_TREASURE = 2,
-		ART_MINOR = 4,
-		ART_MAJOR = 8,
-		ART_RELIC = 16
-	};
-}
+	ART_SPECIAL = 1,
+	ART_TREASURE = 2,
+	ART_MINOR = 4,
+	ART_MAJOR = 8,
+	ART_RELIC = 16
+};
 
 VCMI_LIB_NAMESPACE_END

lib/gameState/CGameState.cpp

@@ -1664,7 +1664,7 @@ vstd::RNG & CGameState::getRandomGenerator()
 	return cb->getRandomGenerator();
 }
 
-ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, int flags, std::function<bool(ArtifactID)> accepts)
+ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, std::optional<EArtifactClass> type, std::function<bool(ArtifactID)> accepts)
 {
 	std::set<ArtifactID> potentialPicks;
 
@@ -1678,16 +1678,7 @@ ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, int flags, std::func
 
 		assert(artifact->aClass != EArtifactClass::ART_SPECIAL); // should be filtered out when allowedArtifacts is initialized
 
-		if ((flags & EArtifactClass::ART_TREASURE) == 0 && artifact->aClass == EArtifactClass::ART_TREASURE)
-			continue;
-
-		if ((flags & EArtifactClass::ART_MINOR) == 0 && artifact->aClass == EArtifactClass::ART_MINOR)
-			continue;
-
-		if ((flags & EArtifactClass::ART_MAJOR) == 0 && artifact->aClass == EArtifactClass::ART_MAJOR)
-			continue;
-
-		if ((flags & EArtifactClass::ART_RELIC) == 0 && artifact->aClass == EArtifactClass::ART_RELIC)
+		if (type.has_value() && *type != artifact->aClass)
 			continue;
 
 		if (!accepts(artifact->getId()))
@@ -1730,12 +1721,12 @@ ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, std::set<ArtifactID>
 
 ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, std::function<bool(ArtifactID)> accepts)
 {
-	return pickRandomArtifact(rand, 0xff, std::move(accepts));
+	return pickRandomArtifact(rand, std::nullopt, std::move(accepts));
 }
 
-ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, int flags)
+ArtifactID CGameState::pickRandomArtifact(vstd::RNG & rand, std::optional<EArtifactClass> type)
 {
-	return pickRandomArtifact(rand, flags, [](const ArtifactID &) { return true; });
+	return pickRandomArtifact(rand, type, [](const ArtifactID &) { return true; });
 }
 
 CArtifactInstance * CGameState::createScroll(const SpellID & spellId)

lib/gameState/CGameState.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "../bonuses/CBonusSystemNode.h"
+#include "../entities/artifact/EArtifactClass.h"
 #include "../IGameCallback.h"
 #include "../GameCallbackHolder.h"
 #include "../LoadProgress.h"
@@ -111,9 +112,9 @@ public:
 	std::vector<const CGObjectInstance*> guardingCreatures (int3 pos) const;
 
 	/// Gets a artifact ID randomly and removes the selected artifact from this handler.
-	ArtifactID pickRandomArtifact(vstd::RNG & rand, int flags);
+	ArtifactID pickRandomArtifact(vstd::RNG & rand, std::optional<EArtifactClass> type);
 	ArtifactID pickRandomArtifact(vstd::RNG & rand, std::function<bool(ArtifactID)> accepts);
-	ArtifactID pickRandomArtifact(vstd::RNG & rand, int flags, std::function<bool(ArtifactID)> accepts);
+	ArtifactID pickRandomArtifact(vstd::RNG & rand, std::optional<EArtifactClass> type, std::function<bool(ArtifactID)> accepts);
 	ArtifactID pickRandomArtifact(vstd::RNG & rand, std::set<ArtifactID> filtered);
 
 	/// Creates instance of spell scroll artifact with provided spell

lib/gameState/InfoAboutArmy.cpp

@@ -25,7 +25,7 @@ ArmyDescriptor::ArmyDescriptor(const CArmedInstance *army, bool detailed)
 	for(const auto & elem : army->Slots())
 	{
 		if(detailed)
-			(*this)[elem.first] = *elem.second;
+			(*this)[elem.first] = CStackBasicDescriptor(elem.second->getCreature(), elem.second->getCount());
 		else
 			(*this)[elem.first] = CStackBasicDescriptor(elem.second->getCreature(), (int)elem.second->getQuantityID());
 	}

lib/json/JsonRandom.cpp

@@ -153,7 +153,7 @@ JsonRandomizationException::JsonRandomizationException(const std::string & messa
 	{
 		assert(value.isStruct());
 
-		std::set<EArtifactClass::Type> allowedClasses;
+		std::set<EArtifactClass> allowedClasses;
 		std::set<ArtifactPosition> allowedPositions;
 		ui32 minValue = 0;
 		ui32 maxValue = std::numeric_limits<ui32>::max();

lib/mapObjects/CGHeroInstance.cpp

@@ -1424,7 +1424,7 @@ EDiggingStatus CGHeroInstance::diggingStatus() const
 	return cb->getTileDigStatus(visitablePos());
 }
 
-ArtBearer::ArtBearer CGHeroInstance::bearerType() const
+ArtBearer CGHeroInstance::bearerType() const
 {
 	return ArtBearer::HERO;
 }

lib/mapObjects/CGHeroInstance.h

@@ -287,7 +287,7 @@ public:
 	PlayerColor getOwner() const override;
 
 	///ArtBearer
-	ArtBearer::ArtBearer bearerType() const override;
+	ArtBearer bearerType() const override;
 
 	///IBonusBearer
 	CBonusSystemNode & whereShouldBeAttached(CGameState & gs) override;

lib/mapObjects/IMarket.h

@@ -31,7 +31,7 @@ public:
 		{}
 
 		IGameCallback * getCallback() const override {return cb;};
-		ArtBearer::ArtBearer bearerType() const override {return ArtBearer::ALTAR;};
+		ArtBearer bearerType() const override {return ArtBearer::ALTAR;};
 	};
 
 	virtual ObjectInstanceID getObjInstanceID() const = 0;	// The market is always an object on the map

lib/mapObjects/MiscObjects.cpp

@@ -631,7 +631,7 @@ void CGArtifact::pickRandomObject(vstd::RNG & rand)
 	switch(ID.toEnum())
 	{
 		case MapObjectID::RANDOM_ART:
-			subID = cb->gameState().pickRandomArtifact(rand, EArtifactClass::ART_TREASURE | EArtifactClass::ART_MINOR | EArtifactClass::ART_MAJOR | EArtifactClass::ART_RELIC);
+			subID = cb->gameState().pickRandomArtifact(rand, std::nullopt);
 			break;
 		case MapObjectID::RANDOM_TREASURE_ART:
 			subID = cb->gameState().pickRandomArtifact(rand, EArtifactClass::ART_TREASURE);

lib/mapping/CMap.cpp

@@ -954,7 +954,7 @@ void CMap::parseUidCounter()
 			const int current_index = std::stoi(index_part);
 			max_index = std::max(max_index, current_index);
 		}
-		catch (const std::invalid_argument& e) {
+		catch (const std::invalid_argument&) {
 			logGlobal->error("Instance name %s contains non-numeric index part: %s", key, index_part);
 		}
 		catch (const std::out_of_range&) {

lib/networkPacks/PacksForServer.h

@@ -657,7 +657,6 @@ struct DLL_LINKAGE QueryReply : public CPackForServer
 	{
 	}
 	QueryID qid;
-	PlayerColor player;
 	std::optional<int32_t> reply;
 
 	void visitTyped(ICPackVisitor & visitor) override;
@@ -666,7 +665,6 @@ struct DLL_LINKAGE QueryReply : public CPackForServer
 	{
 		h & static_cast<CPackForServer &>(*this);
 		h & qid;
-		h & player;
 		h & reply;
 	}
 };

mapeditor/mapsettings/loseconditions.cpp

@@ -103,8 +103,9 @@ void LoseConditions::initialize(MapController & c)
 							assert(loseValueWidget);
 							loseValueWidget->setText(QString::number(json["value"].Integer()));
 							break;
+						}
 
-						case EventCondition::IS_HUMAN:
+						case EventCondition::IS_HUMAN: {
 							break; //ignore because always applicable for defeat conditions
 						}
 

server/CGameHandler.cpp

@@ -1163,7 +1163,7 @@ void CGameHandler::takeCreatures(ObjectInstanceID objid, const std::vector<CStac
 	if (remainerForTaking.empty())
 		return;
 
-	const CArmedInstance* army = static_cast<const CArmedInstance*>(getObj(objid));
+	const auto * army = dynamic_cast<const CArmedInstance*>(getObj(objid));
 
 	for (const CStackBasicDescriptor &stackToTake : remainerForTaking)
 	{
@@ -2844,7 +2844,7 @@ bool CGameHandler::manageBackpackArtifacts(const PlayerColor & player, const Obj
 	{
 		makeSortBackpackRequest([](const ArtSlotInfo & inf) -> int32_t
 			{
-				return inf.getArt()->getType()->aClass;
+									return static_cast<int32_t>(inf.getArt()->getType()->aClass);
 			});
 	}
 	else
@@ -4058,7 +4058,7 @@ void CGameHandler::spawnWanderingMonsters(CreatureID creatureID)
 		tile = tiles.begin();
 		logGlobal->trace("\tSpawning monster at %s", tile->toString());
 		{
-			auto count = cre->getRandomAmount(std::rand);
+			auto count = cre->getRandomAmount(getRandomGenerator());
 
 			createWanderingMonster(*tile, creatureID);
 			auto monsterId = getTopObj(*tile)->id;
@@ -4115,7 +4115,7 @@ void CGameHandler::removeAfterVisit(const ObjectInstanceID & id)
 	}
 
 	//If we haven't returned so far, there is no query and no visit, call was wrong
-	assert("This function needs to be called during the object visit!");
+	throw std::runtime_error("This function needs to be called during the object visit!");
 }
 
 void CGameHandler::changeFogOfWar(int3 center, ui32 radius, PlayerColor player, ETileVisibility mode)

server/CVCMIServer.cpp

@@ -606,7 +606,7 @@ void CVCMIServer::updateAndPropagateLobbyState()
 	}
 
 	LobbyUpdateState lus;
-	lus.state = *this;
+	lus.state = *static_cast<LobbyState*>(this);
 	announcePack(lus);
 }
 

server/NetPacksLobbyServer.cpp

@@ -382,7 +382,7 @@ void ApplyOnServerNetPackVisitor::visitLobbyPvPAction(LobbyPvPAction & pack)
 	switch(pack.action) {
 		case LobbyPvPAction::COIN:
 			txt.appendTextID("vcmi.lobby.pvp.coin.hover");
-			txt.appendRawString(" - " + std::to_string(std::rand()%2));
+			txt.appendRawString(" - " + std::to_string(srv.gh->getRandomGenerator().nextInt(1)));
 			srv.announceTxt(txt);
 			break;
 		case LobbyPvPAction::RANDOM_TOWN:

serverapp/EntryPoint.cpp

@@ -83,7 +83,6 @@ int main(int argc, const char * argv[])
 	logConfigurator.configure();
 
 	LIBRARY->initializeLibrary();
-	std::srand(static_cast<uint32_t>(time(nullptr)));
 
 	{
 		bool connectToLobby = opts.count("lobby");

test/scripting/ScriptFixture.cpp

@@ -40,11 +40,11 @@ void ScriptFixture::loadScript(const JsonNode & scriptConfig)
 	EXPECT_CALL(*pool, getContext(Eq(subject.get()))).WillRepeatedly(Return(context));
 }
 
-void ScriptFixture::loadScript(ModulePtr module, const std::string & scriptSource)
+void ScriptFixture::loadScript(ModulePtr modulePtr, const std::string & scriptSource)
 {
 	subject = std::make_shared<ScriptImpl>(LIBRARY->scriptHandler.get());
 
-	subject->host = module;
+	subject->host = modulePtr;
 	subject->sourceText = scriptSource;
 	subject->identifier = "test";
 	subject->compile(&loggerMock);
@@ -54,11 +54,11 @@ void ScriptFixture::loadScript(ModulePtr module, const std::string & scriptSourc
 	EXPECT_CALL(*pool, getContext(Eq(subject.get()))).WillRepeatedly(Return(context));
 }
 
-void ScriptFixture::loadScript(ModulePtr module, const std::vector<std::string> & scriptSource)
+void ScriptFixture::loadScript(ModulePtr modulePtr, const std::vector<std::string> & scriptSource)
 {
 	std::string source = boost::algorithm::join(scriptSource, "\n");
 
-	loadScript(module, source);
+	loadScript(modulePtr, source);
 }
 
 void ScriptFixture::setUp()
@@ -83,9 +83,9 @@ JsonNode ScriptFixture::runServer(const JsonNode & scriptState)
 	return context->saveState();
 }
 
-JsonNode ScriptFixture::runScript(ModulePtr module, const std::string & scriptSource, const JsonNode & scriptState)
+JsonNode ScriptFixture::runScript(ModulePtr modulePtr, const std::string & scriptSource, const JsonNode & scriptState)
 {
-	loadScript(module, scriptSource);
+	loadScript(modulePtr, scriptSource);
 	return runClientServer(scriptState);
 }
 

test/scripting/ScriptFixture.h

@@ -65,13 +65,13 @@ public:
 
 	void loadScriptFromFile(const std::string & path);
 	void loadScript(const JsonNode & scriptConfig);
-	void loadScript(ModulePtr module, const std::string & scriptSource);
-	void loadScript(ModulePtr module, const std::vector<std::string> & scriptSource);
+	void loadScript(ModulePtr modulePtr, const std::string & scriptSource);
+	void loadScript(ModulePtr modulePtr, const std::vector<std::string> & scriptSource);
 
 	JsonNode runClientServer(const JsonNode & scriptState = JsonNode());
 	JsonNode runServer(const JsonNode & scriptState = JsonNode());
 
-	JsonNode runScript(ModulePtr module, const std::string & scriptSource, const JsonNode & scriptState = JsonNode());
+	JsonNode runScript(ModulePtr modulePtr, const std::string & scriptSource, const JsonNode & scriptState = JsonNode());
 
 protected:
 	void setUp();