Merge pull request #7496 from IvanSavenko/sonar

Fixes for issues reported by Sonar

.github/workflows/aab-from-build.yml

@@ -24,7 +24,9 @@ jobs:
         java-version: '11'
 
     - name: Download & unpack archive
-      run: curl -L '${{ inputs.build_dir_xz_url }}' | tar -xf - --xz
+      env:
+        BUILD_DIR_XZ_URL: ${{ inputs.build_dir_xz_url }}
+      run: curl -L "$BUILD_DIR_XZ_URL" | tar -xf - --xz
 
     - name: Build aab
       run: |

AI/Nullkiller2/Behaviors/RecruitHeroBehavior.cpp

@@ -139,7 +139,7 @@ void RecruitHeroBehavior::calculateFinalDecision(
 	const int treasureSourcesCount
 )
 {
-	if(!vstd::isAlmostZero(bestChoice.score))
+	if(bestChoice.hero != nullptr && !vstd::isAlmostZero(bestChoice.score))
 	{
 		if(ourHeroes.empty()
 		   || treasureSourcesCount > ourHeroes.size() * 5

AI/Nullkiller2/Behaviors/StartupBehavior.cpp

@@ -71,6 +71,9 @@ bool needToRecruitHero(const Nullkiller * aiNk, const CGTownInstance * startupTo
 
 	for(auto obj : aiNk->objectClusterizer->getNearbyObjects())
 	{
+		if(!obj)
+			continue;
+
 		auto armed = dynamic_cast<const CArmedInstance *>(obj);
 
 		if(armed && armed->getArmyStrength() > 0)

AI/Nullkiller2/Pathfinding/AINodeStorage.h

@@ -41,12 +41,12 @@ struct AIPathNode : public CGPathNode
 	const AIPathNode * chainOther = nullptr;
 	const ChainActor * actor = nullptr;
 
-	uint64_t danger;
-	uint64_t armyLoss;
-	uint32_t version;
+	uint64_t danger = 0;
+	uint64_t armyLoss = 0;
+	uint32_t version = 0;
 
-	int16_t manaCost;
-	DayFlags dayFlags;
+	int16_t manaCost = 0;
+	DayFlags dayFlags = DayFlags::NONE;
 
 	void addSpecialAction(std::shared_ptr<const SpecialAction> action);
 

client/NetPacksClient.cpp

@@ -460,6 +460,8 @@ void ApplyClientNetPackVisitor::visitRemoveBonus(RemoveBonus & pack)
 void ApplyFirstClientNetPackVisitor::visitRemoveObject(RemoveObject & pack)
 {
 	const CGObjectInstance *o = cl.gameInfo().getObj(pack.objectID);
+	if(!o)
+		return;
 	const auto * h = dynamic_cast<const CGHeroInstance*>(o);
 
 	GAME->map().onObjectFadeOut(o, pack.initiator);
@@ -474,7 +476,8 @@ void ApplyFirstClientNetPackVisitor::visitRemoveObject(RemoveObject & pack)
 	{
 		//below line contains little cheat for AI so it will be aware of deletion of enemy heroes that moved or got re-covered by FoW
 		//TODO: loose requirements as next AI related crashes appear, for example another pack.player collects object that got re-covered by FoW, unsure if AI code workarounds this
-		if(gs.isVisibleFor(o, i->first) || (!cl.gameInfo().getPlayerState(i->first)->human && o->ID == Obj::HERO && o->tempOwner != i->first))
+		const auto * playerState = cl.gameInfo().getPlayerState(i->first);
+		if(gs.isVisibleFor(o, i->first) || (playerState && !playerState->human && o->ID == Obj::HERO && o->tempOwner != i->first))
 		{
 			i->second->objectRemoved(o, pack.initiator);
 			if (h && h->inBoat())

client/lobby/CBonusSelection.cpp

@@ -317,7 +317,7 @@ void CBonusSelection::createBonusesIcons()
 				desc.replaceTextID(TextIdentifier("core", "skilllev", bonusValue.mastery - 1).get());
 				desc.replaceName(bonusValue.skill);
 				if (!skill->at(bonusValue.mastery).scenarioBonus.empty())
-					picName = skill->at(bonusValue.mastery).scenarioBonus.empty();
+					picName = skill->at(bonusValue.mastery).scenarioBonus;
 				else
 					picNumber = bonusValue.skill.getNum() * 3 + bonusValue.mastery - 1;
 				break;

client/media/CMusicHandler.cpp

@@ -210,12 +210,12 @@ MusicEntry::MusicEntry(CMusicHandler * owner, std::string setName, const AudioPa
 	: owner(owner)
 	, music(nullptr)
 	, setName(std::move(setName))
+	, currentName()
 	, startTime(static_cast<uint32_t>(-1))
 	, startPosition(0)
 	, loop(looped ? -1 : 1)
 	, fromStart(fromStart)
 	, playing(false)
-
 {
 	if(!musicURI.empty())
 		load(musicURI);

client/windows/CKingdomInterface.cpp

@@ -508,6 +508,9 @@ void CKingdomInterface::generateObjectsList(const std::vector<const CGObjectInst
 	std::map<int, OwnedObjectInfo> visibleObjects;
 	for(const CGObjectInstance * object : ownedObjects)
 	{
+		if(!object)
+			continue;
+
 		//Dwellings
 		if(auto * dwelling = dynamic_cast<const CGDwelling *>(object))
 		{

client/windows/CQuestLog.cpp

@@ -168,7 +168,7 @@ void CQuestLog::recreateLabelList()
 				toSeer.replaceRawString(seersHut->seerName);
 				text.replaceRawString(toSeer.toString());
 			}
-			else
+			else if(questObject)
 				text.replaceRawString(questObject->getObjectName()); //get name of the object
 		}
 		auto label = std::make_shared<CQuestLabel>(Rect(13, 195, 149,31), FONT_SMALL, ETextAlignment::TOPLEFT, Colors::WHITE, text.toString());

client/windows/CStackExperienceDetailsWindow.cpp

@@ -411,7 +411,7 @@ CStackWindow::StackExperienceDetailsWindow::StackExperienceDetailsWindow(const C
 			return PreferredRowPresentation{LIBRARY->generaltexth->translate("vcmi.stackExperience.table.maxDamage"), ImagePath::builtin("stackExperienceIconMaxDamage"), LIBRARY->generaltexth->translate("vcmi.stackExperience.desc.maxDamage"), std::nullopt};
 		if(key.type == BonusType::STACK_HEALTH)
 		{
-			auto override = [selector = makeStackExpSelector(key)](const CStackInstance & stackInst)
+			auto valueOverride = [selector = makeStackExpSelector(key)](const CStackInstance & stackInst)
 			{
 				int result = 0;
 				auto bonuses = stackInst.getBonuses(selector);
@@ -419,7 +419,7 @@ CStackWindow::StackExperienceDetailsWindow::StackExperienceDetailsWindow(const C
 					result += b->val;
 				return result;
 			};
-			return PreferredRowPresentation{LIBRARY->generaltexth->allTexts[388], ImagePath::builtin("stackExperienceIconHealth"), LIBRARY->generaltexth->translate("vcmi.stackExperience.desc.health"), override};
+			return PreferredRowPresentation{LIBRARY->generaltexth->allTexts[388], ImagePath::builtin("stackExperienceIconHealth"), LIBRARY->generaltexth->translate("vcmi.stackExperience.desc.health"), valueOverride};
 		}
 		if(key.type == BonusType::STACKS_SPEED)
 			return PreferredRowPresentation{LIBRARY->generaltexth->allTexts[193], ImagePath::builtin("stackExperienceIconSpeed"), LIBRARY->generaltexth->translate("vcmi.stackExperience.desc.speed"), std::nullopt};

client/windows/InfoWindows.cpp

@@ -270,6 +270,7 @@ void CRClickPopupInt::mouseDraggedPopup(const Point & cursorPosition, const Poin
 }
 
 template<typename... Args>
+	requires (sizeof...(Args) != 1 || (!std::is_base_of_v<AdventureMapPopup, std::remove_cvref_t<Args>> && ...))
 AdventureMapPopup::AdventureMapPopup(Args&&... args) :
 	CWindowObject(std::forward<Args>(args)...), dragDistance(Point(0, 0))
 {

client/windows/InfoWindows.h

@@ -101,6 +101,7 @@ class AdventureMapPopup : public CWindowObject
 
 public:
 	template<typename... Args>
+		requires (sizeof...(Args) != 1 || (!std::is_base_of_v<AdventureMapPopup, std::remove_cvref_t<Args>> && ...))
 	AdventureMapPopup(Args&&... args);
 	void mouseDraggedPopup(const Point & cursorPosition, const Point & lastUpdateDistance) override;
 };

clientapp/EntryPoint.cpp

@@ -12,6 +12,7 @@
 
 #include "StdInc.h"
 #include "../Global.h"
+#include <vstd/DateUtils.h>
 
 #include "../client/ClientCommandManager.h"
 #include "../client/CMT.h"
@@ -109,8 +110,9 @@ static void prog_version()
 static void prog_help(const po::options_description &opts)
 {
 	auto time = std::time(nullptr);
+	std::tm tm = vstd::safeLocalTime(time);
 	printf("%s - A Heroes of Might and Magic 3 clone\n", GameConstants::VCMI_PROJECT_NAME_VERSIONED);
-	printf("Copyright (C) 2007-%d VCMI dev team - see AUTHORS file\n", std::localtime(&time)->tm_year + 1900);
+	printf("Copyright (C) 2007-%d VCMI dev team - see AUTHORS file\n", tm.tm_year + 1900);
 	printf("This is free software; see the source for copying conditions. There is NO\n");
 	printf("warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n");
 	printf("\n");

docs/maintainers/scripts/download/FOOTER.html

@@ -8,6 +8,6 @@
 <a href="https://github.com/vcmi/vcmi">GitHub</a>
 </h4>
 <h5>
-<a href="https://m.do.co/c/b2d5cf698ed1"><img src="https://opensource.nyc3.cdn.digitaloceanspaces.com/attribution/assets/PoweredByDO/DO_Powered_by_Badge_black.svg" width="201px"></a>
+<a href="https://m.do.co/c/b2d5cf698ed1"><img src="https://opensource.nyc3.cdn.digitaloceanspaces.com/attribution/assets/PoweredByDO/DO_Powered_by_Badge_black.svg" width="201px" alt="Powered by DigitalOcean"></a>
 </h5>
 </body></html>

include/vstd/DateUtils.h

@@ -8,6 +8,17 @@ namespace vstd
 	DLL_LINKAGE std::string getFormattedDateTime(std::time_t dt, std::string format);
 	DLL_LINKAGE std::string getDateTimeISO8601Basic(std::time_t dt);
 
+	inline std::tm safeLocalTime(std::time_t dt)
+	{
+		std::tm tm{};
+#ifdef _WIN32
+		localtime_s(&tm, &dt);
+#else
+		localtime_r(&dt, &tm);
+#endif
+		return tm;
+	}
+
 }
 
 VCMI_LIB_NAMESPACE_END

ios/rpath_remove_symlinks.sh

@@ -10,7 +10,7 @@ for binary in "../$EXECUTABLE_NAME" $(find . -type f -iname '*.dylib'); do
 	echo "checking $binary"
 	# dyld_info sample output: @rpath/libogg.0.dylib
 	for lib in $(dyld_info -linked_dylibs "$binary" | awk -F / '/@rpath/ {print $2}'); do
-		if [ -L "$lib" ]; then
+		if [[ -L "$lib" ]]; then
 			echo "- symlink: $lib"
 			rpathSymlinks+=("$lib")
 		fi

launcher/firstLaunch/firstlaunch_moc.cpp

@@ -357,7 +357,7 @@ QString FirstLaunchView::getHeroesInstallDir()
 static QString defaultStartDirForOpen()
 {
 #if defined(VCMI_MOBILE)
-	const QStandardPaths::StandardLocation mobilePrefs[] = {
+	const std::array mobilePrefs = {
 		QStandardPaths::HomeLocation
 	};
 	for(auto location : mobilePrefs)

lib/CConsoleHandler.cpp

@@ -57,9 +57,9 @@ VCMI_LIB_NAMESPACE_BEGIN
 static void createMemoryDump(MINIDUMP_EXCEPTION_INFORMATION * meinfo)
 {
 	//create file where dump will be placed
-	wchar_t executablePath[MAX_PATH + 1];
-	GetModuleFileNameW(nullptr, executablePath, MAX_PATH);
-	const auto dumpName = boost::filesystem::path(executablePath).filename().wstring() + L"_crashinfo.dmp";
+	std::array<wchar_t, MAX_PATH + 1> executablePath{};
+	GetModuleFileNameW(nullptr, executablePath.data(), MAX_PATH);
+	const auto dumpName = boost::filesystem::path(executablePath.data()).filename().wstring() + L"_crashinfo.dmp";
 	const auto dumpPath = VCMIDirs::get().userLogsPath() / dumpName;
 	HANDLE dfile = CreateFileW(dumpPath.c_str(), GENERIC_READ|GENERIC_WRITE, FILE_SHARE_WRITE|FILE_SHARE_READ, 0, CREATE_ALWAYS, 0, 0);
 	logGlobal->error("Crash info will be put in %s", dumpPath.string());

lib/CSkillHandler.cpp

@@ -229,16 +229,16 @@ std::shared_ptr<CSkill> CSkillHandler::loadFromJson(const std::string & scope, c
 			skill->tags.push_back(tag.first);
 
 	if (json["onlyOnWaterMap"].Bool() && !vstd::contains(skill->tags, "onlyOnWaterMap"))
-		skill->tags.push_back("onlyOnWaterMap");
+		skill->tags.emplace_back("onlyOnWaterMap");
 
 	if (json["special"].Bool() && !vstd::contains(skill->tags, "special"))
-		skill->tags.push_back("special");
+		skill->tags.emplace_back("special");
 
 	if (json["obligatoryMajor"].Bool() && !vstd::contains(skill->tags, "wisdom"))
-		skill->tags.push_back("wisdom");
+		skill->tags.emplace_back("wisdom");
 
 	if (json["obligatoryMinor"].Bool() && !vstd::contains(skill->tags, "spellSchool"))
-		skill->tags.push_back("spellSchool");
+		skill->tags.emplace_back("spellSchool");
 
 	LIBRARY->generaltexth->registerString(scope, skill->getNameTextID(), json["name"]);
 

lib/ResourceSet.cpp

@@ -165,13 +165,19 @@ const ResourceSet::nziterator::ResEntry * ResourceSet::nziterator::operator->()
 
 void ResourceSet::nziterator::advance()
 {
-	do
+	const auto resourceCount = static_cast<int>(LIBRARY->resourceTypeHandler->getAllObjects().size());
+	while(true)
 	{
 		++cur.resType;
-	} while(static_cast<int>(cur.resType) < LIBRARY->resourceTypeHandler->getAllObjects().size() && !(cur.resVal=rs[cur.resType]));
-
-	if(static_cast<int>(cur.resType) >= LIBRARY->resourceTypeHandler->getAllObjects().size())
-		cur.resVal = -1;
+		if(static_cast<int>(cur.resType) >= resourceCount)
+		{
+			cur.resVal = -1;
+			return;
+		}
+		cur.resVal = rs[cur.resType];
+		if(cur.resVal)
+			return;
+	}
 }
 
 ResourceSet::nziterator::nziterator(const ResourceSet &RS)

lib/callback/Calendar.cpp

@@ -15,7 +15,7 @@
 VCMI_LIB_NAMESPACE_BEGIN
 
 Calendar::Calendar(const IGameSettings & settings, int day)
-	: settings(&settings), day(day)
+	: gameSettings(&settings), day(day)
 {
 }
 
@@ -53,7 +53,7 @@ int Calendar::getMonth() const
 
 int Calendar::getDaysInWeek() const
 {
-	return settings->getInteger(EGameSettings::GENERAL_DAYS_PER_WEEK);
+	return gameSettings->getInteger(EGameSettings::GENERAL_DAYS_PER_WEEK);
 }
 
 int Calendar::getDaysInMonth() const
@@ -63,12 +63,12 @@ int Calendar::getDaysInMonth() const
 
 int Calendar::getWeeksInMonth() const
 {
-	return settings->getInteger(EGameSettings::GENERAL_WEEKS_PER_MONTH);
+	return gameSettings->getInteger(EGameSettings::GENERAL_WEEKS_PER_MONTH);
 }
 
 Calendar Calendar::nextDay() const
 {
-	return Calendar(*settings, day + 1);
+	return Calendar(*gameSettings, day + 1);
 }
 
 VCMI_LIB_NAMESPACE_END

lib/callback/Calendar.h

@@ -18,7 +18,7 @@ class IGameSettings;
 /// to the IGameSettings that provides week/month length.
 class DLL_LINKAGE Calendar final
 {
-	const IGameSettings * settings;
+	const IGameSettings * gameSettings;
 	int day;
 
 public:

lib/filesystem/CZipSaver.cpp

@@ -11,6 +11,8 @@
 #include "StdInc.h"
 #include "CZipSaver.h"
 
+#include <vstd/DateUtils.h>
+
 VCMI_LIB_NAMESPACE_BEGIN
 
 ///CZipOutputStream
@@ -23,13 +25,13 @@ CZipOutputStream::CZipOutputStream(CZipSaver * owner_, zipFile archive, const st
 	std::time_t t = time(nullptr);
 	fileInfo.dosDate = 0;
 
-	struct tm * localTime = std::localtime(&t);
-	fileInfo.tmz_date.tm_hour = localTime->tm_hour;
-	fileInfo.tmz_date.tm_mday = localTime->tm_mday;
-	fileInfo.tmz_date.tm_min  = localTime->tm_min;
-	fileInfo.tmz_date.tm_mon  = localTime->tm_mon;
-	fileInfo.tmz_date.tm_sec  = localTime->tm_sec;
-	fileInfo.tmz_date.tm_year = localTime->tm_year;
+	std::tm localTime = vstd::safeLocalTime(t);
+	fileInfo.tmz_date.tm_hour = localTime.tm_hour;
+	fileInfo.tmz_date.tm_mday = localTime.tm_mday;
+	fileInfo.tmz_date.tm_min  = localTime.tm_min;
+	fileInfo.tmz_date.tm_mon  = localTime.tm_mon;
+	fileInfo.tmz_date.tm_sec  = localTime.tm_sec;
+	fileInfo.tmz_date.tm_year = localTime.tm_year;
 
 	fileInfo.external_fa = 0; //???
 	fileInfo.internal_fa = 0;

lib/networkPacks/PacksForServer.h

@@ -658,7 +658,6 @@ struct DLL_LINKAGE HireHero : public CPackForServer
 	HeroTypeID hid; //available hero serial
 	HeroTypeID nhid; //next hero
 	ObjectInstanceID tid; //town (tavern) id
-	PlayerColor player;
 
 	void visitTyped(ICPackVisitor & visitor) override;
 
@@ -668,7 +667,6 @@ struct DLL_LINKAGE HireHero : public CPackForServer
 		h & hid;
 		h & nhid;
 		h & tid;
-		h & player;
 	}
 };
 

lib/serializer/CSaveFile.cpp

@@ -16,7 +16,7 @@ CSaveFile::CSaveFile()
 	: serializer(this)
 {
 	saveData.reserve(128*1024);
-	static const char * SAVE_HEADER = "VCMI";
+	static constexpr const char * SAVE_HEADER = "VCMI";
 
 	write(reinterpret_cast<const std::byte*>(SAVE_HEADER), 4); //write magic identifier
 	serializer & ESerializationVersion::CURRENT; //write format version

lib/spells/CSpell.cpp

@@ -65,15 +65,15 @@ bool CSpell::adventureCast(SpellCastEnvironment * env, const AdventureSpellCastP
 	return adventureMechanics->adventureCast(env, parameters);
 }
 
-const CSpell::LevelInfo & CSpell::getLevelInfo(const int32_t level) const
+const CSpell::LevelInfo & CSpell::getLevelInfo(const int32_t schoolLevel) const
 {
-	if(level < 0 || level >= GameConstants::SPELL_SCHOOL_LEVELS)
+	if(schoolLevel < 0 || schoolLevel >= GameConstants::SPELL_SCHOOL_LEVELS)
 	{
-		logGlobal->error("CSpell::getLevelInfo: invalid school mastery level %d", level);
+		logGlobal->error("CSpell::getLevelInfo: invalid school mastery level %d", schoolLevel);
 		return levels.at(MasteryLevel::EXPERT);
 	}
 
-	return levels.at(level);
+	return levels.at(schoolLevel);
 }
 
 int64_t CSpell::calculateDamage(const spells::Caster * caster) const
@@ -132,8 +132,8 @@ SpellID CSpell::getId() const
 
 std::string CSpell::getNameTextID() const
 {
-	TextIdentifier id("spell", modScope, identifier, "name");
-	return id.get();
+	TextIdentifier textId("spell", modScope, identifier, "name");
+	return textId.get();
 }
 
 std::string CSpell::getNameTranslated() const
@@ -141,15 +141,15 @@ std::string CSpell::getNameTranslated() const
 	return LIBRARY->generaltexth->translate(getNameTextID());
 }
 
-std::string CSpell::getDescriptionTextID(int32_t level) const
+std::string CSpell::getDescriptionTextID(int32_t schoolLevel) const
 {
-	TextIdentifier textID("spell", modScope, identifier, "description", LEVEL_NAMES[level]);
+	TextIdentifier textID("spell", modScope, identifier, "description", LEVEL_NAMES[schoolLevel]);
 	return textID.get();
 }
 
-std::string CSpell::getDescriptionTranslated(int32_t level) const
+std::string CSpell::getDescriptionTranslated(int32_t schoolLevel) const
 {
-	return LIBRARY->generaltexth->translate(getDescriptionTextID(level));
+	return LIBRARY->generaltexth->translate(getDescriptionTextID(schoolLevel));
 }
 
 std::string CSpell::getAdventureEffectTextID(const std::string & effectType, const std::string & field) const
@@ -330,21 +330,21 @@ si32 CSpell::getProbability(const FactionID & factionId) const
 	return probabilities.at(factionId);
 }
 
-void CSpell::getEffects(std::vector<Bonus> & lst, const int level, const bool cumulative, const si32 duration, std::optional<si32 *> maxDuration) const
+void CSpell::getEffects(std::vector<Bonus> & lst, const int schoolLevel, const bool cumulative, const si32 duration, std::optional<si32 *> maxDuration) const
 {
-	if(level < 0 || level >= GameConstants::SPELL_SCHOOL_LEVELS)
+	if(schoolLevel < 0 || schoolLevel >= GameConstants::SPELL_SCHOOL_LEVELS)
 	{
-		logGlobal->error("invalid school level %d", level);
+		logGlobal->error("invalid school level %d", schoolLevel);
 		return;
 	}
 
-	const auto & levelObject = levels.at(level);
+	const auto & levelObject = levels.at(schoolLevel);
 
 	const auto & effectsJson = cumulative ? levelObject.cumulativeEffects : levelObject.effects;
 
 	if(effectsJson.Struct().empty())
 	{
-		logGlobal->error("This spell (%s) has no effects for level %d", getNameTranslated(), level);
+		logGlobal->error("This spell (%s) has no effects for level %d", getNameTranslated(), schoolLevel);
 		return;
 	}
 

lib/spells/ISpellMechanics.cpp

@@ -86,9 +86,9 @@ public:
 //to be used for spells configured with old format
 class FallbackMechanicsFactory : public CustomMechanicsFactory
 {
-	JsonNode usePowerAsVal(const JsonNode & effects, si32 power) const
+	JsonNode usePowerAsVal(const JsonNode & effectsNode, si32 power) const
 	{
-		JsonNode result = effects;
+		JsonNode result = effectsNode;
 		for(auto & [name, bonusNode] : result.Struct())
 			if(bonusNode["val"].isNull())
 				bonusNode["val"].Integer() = power;

lib/spells/effects/Effects.cpp

@@ -145,7 +145,7 @@ Effects::EffectsMap Effects::loadJson(const JsonNode & effectMap, const std::str
 		effect->spellIdentifier = spellIdentifier;
 		effect->init(std::move(data));
 
-		result.emplace(name, std::move(effect));
+		result.try_emplace(name, std::move(effect));
 	}
 
 	return result;

lib/texts/CLegacyConfigParser.cpp

@@ -128,7 +128,10 @@ float CLegacyConfigParser::readNumber()
 	std::istringstream stream(input);
 
 	if(input.find(',') != std::string::npos) // code to handle conversion with comma as decimal separator
-		stream.imbue(std::locale(std::locale(), new LocaleWithComma()));
+	{
+		static const std::locale commaLocale(std::locale(), new LocaleWithComma());
+		stream.imbue(commaLocale);
+	}
 
 	float result;
 	if ( !(stream >> result) )

lib/vstd/DateUtils.cpp

@@ -17,7 +17,7 @@ namespace vstd
 
 	DLL_LINKAGE std::string getFormattedDateTime(std::time_t dt, std::string format)
 	{
-		std::tm tm = *std::localtime(&dt);
+		std::tm tm = safeLocalTime(dt);
 		std::stringstream s;
 		s << std::put_time(&tm, format.c_str());
 		return s.str();

luascript/LuaModule.cpp

@@ -19,24 +19,8 @@
 #include "../lib/GameLibrary.h"
 #include "../lib/spells/effects/SpellEffectService.h"
 
-#ifdef __GNUC__
-#	define strcpy_s(a, b, c) strncpy(a, c, b)
-#endif
-
-static const char * const g_cszAiName = "Lua interpreter";
-
 VCMI_LIB_NAMESPACE_BEGIN
 
-extern "C" DLL_EXPORT void GetAiName(char * name)
-{
-	strcpy_s(name, strlen(g_cszAiName) + 1, g_cszAiName);
-}
-
-extern "C" DLL_EXPORT void GetNewModule(std::unique_ptr<scripting::Service> & out)
-{
-	out = std::make_unique<scripting::LuaModule>();
-}
-
 namespace scripting
 {
 

luascript/LuaModule.h

@@ -24,8 +24,7 @@ namespace scripting
 
 class LuaScriptInstance;
 
-/// Top-level Lua scripting service loaded as a DLL plugin by ScriptingHandler; owns script factories and creates script pools.
-/// Entry point exposed to the engine via GetNewModule() and GetAiName() C exports.
+/// Top-level Lua scripting service; owns script factories and creates script pools.
 class DLL_LINKAGE LuaModule final : public Service
 {
 public:

luascript/LuaWrapper.h

@@ -120,7 +120,7 @@ public:
 	void pushMetatable(lua_State * L) const final
 	{
 		static const auto KEY = api::Registry::get()->getTypeName<UDataType>();
-		static auto S_KEY = api::Registry::get()->getTypeName<CUDataType>();
+		static const auto S_KEY = api::Registry::get()->getTypeName<CUDataType>();
 
 		LuaStack S(L);
 

luascript/api/library/Bonus.cpp

@@ -70,7 +70,7 @@ si32        BonusProxy::getParametersAsNumber(const Bonus & b) { return b.parame
 
 std::vector<BonusDuration::BonusDuration> BonusProxy::getDuration(const Bonus & b)
 {
-	static constexpr BonusDuration::BonusDuration all[] = {
+	static constexpr std::array all = {
 		BonusDuration::PERMANENT,
 		BonusDuration::ONE_BATTLE,
 		BonusDuration::ONE_DAY,

mapeditor/inspector/abilitieswidget.cpp

@@ -28,7 +28,7 @@ const std::string AbilitiesWidget::V_CATEGORY="secondarySkill";
 const std::string AbilitiesWidget::V_NAME="gainedSkill";
 
 AbilitiesWidget::AbilitiesWidget(CRewardableObject & hut, MapController & controller, QWidget * parent)
-	: hut(hut), controller(controller), QDialog(parent), extractor(controller.getCallback()), ui(new Ui::AbilitiesWidget)
+	: QDialog(parent), ui(new Ui::AbilitiesWidget), hut(hut), extractor(controller.getCallback()), controller(controller)
 {
 	ui->setupUi(this);
 }
@@ -176,7 +176,7 @@ bool AbilitiesWidget::isSetToDefault()
 	return !ui->customize->isChecked();
 }
 
-AbilitiesDelegate::AbilitiesDelegate(MapController & controller, CRewardableObject & hut) : BaseInspectorItemDelegate(), controller(controller), hut(hut) {}
+AbilitiesDelegate::AbilitiesDelegate(MapController & controller, CRewardableObject & hut) : BaseInspectorItemDelegate(), hut(hut), controller(controller) {}
 
 QWidget * AbilitiesDelegate::createEditor(QWidget * parent, const QStyleOptionViewItem & option, const QModelIndex & index) const
 {

mapeditor/inspector/inspector.cpp

@@ -979,7 +979,7 @@ QTableWidgetItem * Inspector::addProperty(const std::set<PlayerColor> & value)
 {
 	QString tooltip = QObject::tr("Available for:\n");
 	QStringList colors;
-	if(value.size() > 0)
+	if(!value.empty())
 		for (const PlayerColor &color : value)
 			colors << QString::fromStdString(PlayerColor::encode(color));
 

mapeditor/inspector/scholarwidget.cpp

@@ -21,12 +21,8 @@
 #include "lib/modding/IdentifierStorage.h"
 #include "lib/spells/CSpellHandler.h"
 
-const std::string ScholarWidget::presetNotFoundWarning =
-	"<font color='red'>The scholar has %1 preset set to \"%2\", "
-	"but the value is unknown. Maybe it is a mod configuration problem?</font color='red'>";
-
 ScholarWidget::ScholarWidget(CRewardableObject & scholar, MapController & controller, QWidget * parent)
-	: scholar(scholar), controller(controller), QDialog(parent), extractor(controller.getCallback()), ui(new Ui::ScholarWidget)
+	: QDialog(parent), ui(new Ui::ScholarWidget), scholar(scholar), extractor(controller.getCallback()), controller(controller)
 {
 	ui->setupUi(this);
 	rewardsData = {
@@ -145,13 +141,13 @@ void ScholarWidget::changeComboBoxesAllowedState()
 
 void ScholarWidget::showInvalidPresetWarning(std::string type, std::string name)
 {
-	auto warning = tr(presetNotFoundWarning.c_str()).arg(type.c_str()).arg(name.c_str());
+	auto warning = tr(presetNotFoundWarning).arg(type.c_str()).arg(name.c_str());
 	ui->label->setText(warning);
 	adjustSize();
 }
 
 ScholarDelegate::ScholarDelegate(MapController & controller, CRewardableObject & scholar)
-	: BaseInspectorItemDelegate(), controller(controller), scholar(scholar)
+	: BaseInspectorItemDelegate(), scholar(scholar), controller(controller)
 {
 }
 

mapeditor/inspector/scholarwidget.h

@@ -44,7 +44,7 @@ private:
 	{
 		QRadioButton * radioButton;
 		QComboBox * comboBox;
-		std::string variables[2];
+		std::array<std::string, 2> variables;
 		std::string name;
 		JsonNode dice;
 	};
@@ -59,7 +59,9 @@ private:
 	MapController & controller;
 
 	std::vector<RewardData> rewardsData;
-	static const std::string presetNotFoundWarning;
+	static constexpr const char * presetNotFoundWarning =
+		"<font color='red'>The scholar has %1 preset set to \"%2\", "
+		"but the value is unknown. Maybe it is a mod configuration problem?</font color='red'>";
 };
 
 class ScholarDelegate : public BaseInspectorItemDelegate

mapeditor/validator.cpp

@@ -87,9 +87,9 @@ std::set<Validator::Issue> Validator::validate(const CMap * map)
 				continue;
 
 			if(o->isVisitable() && !map->isInTheMap(o->visitablePos()))
-				issues.insert({ tr("Object's %1 visitable position %2 is outside of the map bounds")
+				issues.emplace(tr("Object's %1 visitable position %2 is outside of the map bounds")
 					.arg(o->instanceName.c_str())
-					.arg(QString::fromStdString(o->visitablePos().toString())), false });
+					.arg(QString::fromStdString(o->visitablePos().toString())), false);
 
 			//owners for objects
 			if(o->getOwner() == PlayerColor::UNFLAGGABLE)
@@ -161,9 +161,8 @@ std::set<Validator::Issue> Validator::validate(const CMap * map)
 			{
 				if(!presetIsValid(map, o, "secondarySkill", "gainedSkill", map->allowedAbilities))
 				{
-					issues.insert({tr("A witch hut at x: %1 y: %2 on %3 layer holds an invalid reward")
-						.arg(o->pos.x).arg(o->pos.y).arg(o->pos.z), true}
-					);
+					issues.emplace(tr("A witch hut at x: %1 y: %2 on %3 layer holds an invalid reward")
+						.arg(o->pos.x).arg(o->pos.y).arg(o->pos.z), true);
 				}
 			}
 			if(o->ID == MapObjectID::SCHOLAR)
@@ -171,8 +170,8 @@ std::set<Validator::Issue> Validator::validate(const CMap * map)
 				if(!presetIsValid(map, o, "secondarySkill", "gainedSkill", map->allowedAbilities)
 				   || !presetIsValid(map, o, "spell", "gainedSpell", map->allowedSpells))
 				{
-					issues.insert({tr("A scholar at x: %1 y: %2 on %3 layer holds an invalid reward")
-						.arg(o->pos.x).arg(o->pos.y).arg(o->pos.z), true});
+					issues.emplace(tr("A scholar at x: %1 y: %2 on %3 layer holds an invalid reward")
+						.arg(o->pos.x).arg(o->pos.y).arg(o->pos.z), true);
 				}
 			}
 		}
@@ -217,11 +216,10 @@ std::set<Validator::Issue> Validator::validate(const CMap * map)
 					const QString placeholderName = placeholder->heroType.has_value() ?
 						QString::fromStdString(placeholder->heroType->toHeroType()->getNameTranslated()) :
 						Validator::tr("hero placeholder");
-					issues.insert({
+					issues.emplace(
 						Validator::tr("Triggered event '%1' uses %2 condition targeting %3 at %4. This setup is unusual and should be avoided; map will stay playable, but the condition remains unresolved unless placeholder replacement is supported.")
 							.arg(event.identifier.c_str(), conditionName, placeholderName, QString::fromStdString(condition.position.toString())),
-						false
-					});
+						false);
 				}
 
 				return condition;

server/CVCMIServer.cpp

@@ -469,7 +469,7 @@ bool CVCMIServer::passHost(GameConnectionID toConnectionId)
 	return false;
 }
 
-void CVCMIServer::clientConnected(std::shared_ptr<GameConnection> c, std::vector<std::string> & names, const std::string & uuid, EStartMode mode)
+void CVCMIServer::clientConnected(std::shared_ptr<GameConnection> c, const std::vector<std::string> & names, const std::string & uuid, EStartMode mode)
 {
 	assert(getState() == EServerState::LOBBY);
 

server/CVCMIServer.h

@@ -110,7 +110,7 @@ public:
 	void setPlayerConnectedId(PlayerSettings & pset, PlayerConnectionID player) const;
 	void updateStartInfoOnMapChange(std::shared_ptr<CMapInfo> mapInfo, std::shared_ptr<CMapGenOptions> mapGenOpt = {});
 
-	void clientConnected(std::shared_ptr<GameConnection> c, std::vector<std::string> & names, const std::string & uuid, EStartMode mode);
+	void clientConnected(std::shared_ptr<GameConnection> c, const std::vector<std::string> & names, const std::string & uuid, EStartMode mode);
 	void clientDisconnected(std::shared_ptr<GameConnection> c);
 
 	void announceMessage(const MetaString & txt);

server/NetPacksLobbyServer.cpp

@@ -467,7 +467,7 @@ void ApplyOnServerNetPackVisitor::visitLobbyDelete(LobbyDelete & pack)
 	}
 
 	LobbyUpdateState lus;
-	lus.state = srv;
+	lus.state = *static_cast<LobbyState*>(&srv);
 	lus.refreshList = true;
 	srv.announcePack(lus);
 }

server/processors/HeroPoolProcessor.cpp

@@ -153,11 +153,17 @@ bool HeroPoolProcessor::hireHero(const ObjectInstanceID & objectID, const HeroTy
 	const CGTownInstance * town = gameHandler->gameInfo().getTown(objectID);
 	const auto & heroesPool = gameHandler->gameState().heroesPool;
 
-	if (!mapObject && gameHandler->complain("Invalid map object!"))
+	if (!mapObject)
+	{
+		gameHandler->complain("Invalid map object!");
 		return false;
+	}
 
-	if (!playerState && gameHandler->complain("Invalid player!"))
+	if (!playerState)
+	{
+		gameHandler->complain("Invalid player!");
 		return false;
+	}
 
 	if (playerState->resources[EGameResID::GOLD] < GameConstants::HERO_GOLD_COST && gameHandler->complain("Not enough gold for buying hero!"))
 		return false;

server/processors/TurnOrderProcessor.cpp

@@ -224,9 +224,6 @@ bool TurnOrderProcessor::mustActBefore(PlayerColor left, PlayerColor right) cons
 	if (leftInfo->isHuman() && !rightInfo->isHuman())
 		return true;
 
-	if (!leftInfo->isHuman() && rightInfo->isHuman())
-		return false;
-
 	return false;
 }
 

server/queries/MapQueries.cpp

@@ -219,11 +219,11 @@ void CTeleportDialogQuery::notifyObjectAboutRemoval(const CGObjectInstance * vis
 		logGlobal->error("Invalid instance in teleport query");
 }
 
-CTeleportDialogQuery::CTeleportDialogQuery(CGameHandler * owner, const TeleportDialog & td) :
+CTeleportDialogQuery::CTeleportDialogQuery(CGameHandler * owner, const TeleportDialog & dialog) :
 	CDialogQuery(owner, TYPE)
 {
-	this->td = td;
-	addPlayer(gh->gameInfo().getHero(td.hero)->getOwner());
+	td = dialog;
+	addPlayer(gh->gameInfo().getHero(dialog.hero)->getOwner());
 }
 
 CHeroLevelUpDialogQuery::CHeroLevelUpDialogQuery(CGameHandler * owner, const HeroLevelUp & Hlu, const CGHeroInstance * Hero):

server/queries/MapQueries.h

@@ -98,7 +98,7 @@ public:
 
 	TeleportDialog td; //copy of pack... debug purposes
 
-	CTeleportDialogQuery(CGameHandler * owner, const TeleportDialog &td);
+	CTeleportDialogQuery(CGameHandler * owner, const TeleportDialog & dialog);
 
 	void notifyObjectAboutRemoval(const CGObjectInstance * visitedObject, const CGHeroInstance * visitingHero) const override;
 };

serverapp/EntryPoint.cpp

@@ -9,6 +9,8 @@
  */
 #include "StdInc.h"
 
+#include <vstd/DateUtils.h>
+
 #include "../server/CVCMIServer.h"
 
 #include "../lib/CConsoleHandler.h"
@@ -253,8 +255,9 @@ static void handleCommandOptions(int argc, const char * argv[], boost::program_o
 	if(options.count("help"))
 	{
 		auto time = std::time(nullptr);
+		std::tm tm = vstd::safeLocalTime(time);
 		printf("%s - A Heroes of Might and Magic 3 clone\n", GameConstants::VCMI_PROJECT_NAME_VERSIONED);
-		printf("Copyright (C) 2007-%d VCMI dev team - see AUTHORS file\n", std::localtime(&time)->tm_year + 1900);
+		printf("Copyright (C) 2007-%d VCMI dev team - see AUTHORS file\n", tm.tm_year + 1900);
 		printf("This is free software; see the source for copying conditions. There is NO\n");
 		printf("warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n");
 		printf("\n");

test/spells/effects/DemonSummonTest.cpp

@@ -101,7 +101,6 @@ public:
 	const int64_t corpseTotalHealth = 1000;
 	const int64_t effectValue = 400;
 	const BattleHex corpsePosition = BattleHex(5, 5);
-	// finalAmount = min(floor(1000/200)=5, 10, floor(400/200)=2) = 2
 	const int32_t expectedAmount = 2;
 
 	bool permanent;

test/spells/effects/ObstacleTest.cpp

@@ -209,7 +209,7 @@ TEST_F(ObstacleApplyTest, PlacesObstacleWithMultiHexShape)
 	JsonNode config;
 	JsonNode shape;
 	JsonNode firstShapeEntry;
-	firstShapeEntry.Vector().push_back(JsonNode());  // empty direction list ⇒ NONE
+	firstShapeEntry.Vector().emplace_back();  // empty direction list ⇒ NONE
 	JsonNode secondShapeEntry;
 	JsonNode trDir;
 	trDir.String() = "TR";
@@ -326,12 +326,12 @@ TEST_F(ObstacleApplyTest, NoServerCallWhenNoAvailableTiles)
 TEST_F(ObstacleApplyTest, UsesDefenderSideOptions)
 {
 	JsonNode config;
-	config["attacker"]["shape"].Vector().push_back(JsonNode());
-	config["attacker"]["shape"].Vector()[0].Vector().push_back(JsonNode());
+	config["attacker"]["shape"].Vector().emplace_back();
+	config["attacker"]["shape"].Vector()[0].Vector().emplace_back();
 	config["attacker"]["shape"].Vector()[0].Vector()[0].String() = "TL";
 
-	config["defender"]["shape"].Vector().push_back(JsonNode());
-	config["defender"]["shape"].Vector()[0].Vector().push_back(JsonNode());
+	config["defender"]["shape"].Vector().emplace_back();
+	config["defender"]["shape"].Vector()[0].Vector().emplace_back();
 	config["defender"]["shape"].Vector()[0].Vector()[0].String() = "TR";
 
 	setupEffect(config);