Reduced usage of global variables - removed or made const / constexpr

AI/BattleAI/AttackPossibility.h

@@ -10,7 +10,6 @@
 #pragma once
 #include "../../lib/battle/CUnitState.h"
 #include "../../CCallback.h"
-#include "common.h"
 #include "StackWithBonuses.h"
 
 #define BATTLE_TRACE_LEVEL 0

AI/BattleAI/BattleAI.cpp

@@ -49,7 +49,6 @@ CBattleAI::~CBattleAI()
 
 void CBattleAI::initBattleInterface(std::shared_ptr<Environment> ENV, std::shared_ptr<CBattleCallback> CB)
 {
-	setCbc(CB);
 	env = ENV;
 	cb = CB;
 	playerID = *CB->getPlayerID();
@@ -121,7 +120,6 @@ void CBattleAI::activeStack(const BattleID & battleID, const CStack * stack )
 	};
 
 	BattleAction result = BattleAction::makeDefend(stack);
-	setCbc(cb); //TODO: make solid sure that AIs always use their callbacks (need to take care of event handlers too)
 
 	auto start = std::chrono::high_resolution_clock::now();
 

AI/BattleAI/CMakeLists.txt

@@ -2,7 +2,6 @@ set(battleAI_SRCS
 		AttackPossibility.cpp
 		BattleAI.cpp
 		BattleEvaluator.cpp
-		common.cpp
 		EnemyInfo.cpp
 		PossibleSpellcast.cpp
 		PotentialTargets.cpp
@@ -17,7 +16,6 @@ set(battleAI_HEADERS
 		AttackPossibility.h
 		BattleAI.h
 		BattleEvaluator.h
-		common.h
 		EnemyInfo.h
 		PotentialTargets.h
 		PossibleSpellcast.h

AI/BattleAI/common.cpp

@@ -1,23 +0,0 @@
-/*
- * common.cpp, part of VCMI engine
- *
- * Authors: listed in file AUTHORS in main folder
- *
- * License: GNU General Public License v2.0 or later
- * Full text of license available in license.txt file, in main folder
- *
- */
-#include "StdInc.h"
-#include "common.h"
-
-std::shared_ptr<CBattleCallback> cbc;
-
-void setCbc(std::shared_ptr<CBattleCallback> cb)
-{
-	cbc = cb;
-}
-
-std::shared_ptr<CBattleCallback> getCbc()
-{
-	return cbc;
-}

AI/BattleAI/common.h

@@ -1,26 +0,0 @@
-/*
- * common.h, part of VCMI engine
- *
- * Authors: listed in file AUTHORS in main folder
- *
- * License: GNU General Public License v2.0 or later
- * Full text of license available in license.txt file, in main folder
- *
- */
-#pragma once
-
-class CBattleCallback;
-
-template<typename Key, typename Val, typename Val2>
-const Val getValOr(const std::map<Key, Val> &Map, const Key &key, const Val2 defaultValue)
-{
-	//returning references here won't work: defaultValue must be converted into Val, creating temporary
-	auto i = Map.find(key);
-	if(i != Map.end())
-		return i->second;
-	else
-		return defaultValue;
-}
-
-void setCbc(std::shared_ptr<CBattleCallback> cb);
-std::shared_ptr<CBattleCallback> getCbc();

AI/BattleAI/main.cpp

@@ -15,7 +15,7 @@
 #define strcpy_s(a, b, c) strncpy(a, c, b)
 #endif
 
-static const char *g_cszAiName = "Battle AI";
+static const char * const g_cszAiName = "Battle AI";
 
 extern "C" DLL_EXPORT int GetGlobalAiVersion()
 {

AI/EmptyAI/main.cpp

@@ -11,7 +11,6 @@
 
 #include "CEmptyAI.h"
 
-std::set<CGlobalAI*> ais;
 extern "C" DLL_EXPORT int GetGlobalAiVersion()
 {
 	return AI_INTERFACE_VER;

AI/Nullkiller/Analyzers/DangerHitMapAnalyzer.cpp

@@ -16,7 +16,7 @@
 namespace NKAI
 {
 
-HitMapInfo HitMapInfo::NoThreat;
+const HitMapInfo HitMapInfo::NoThreat;
 
 double HitMapInfo::value() const
 {
@@ -285,8 +285,6 @@ const HitMapNode & DangerHitMapAnalyzer::getTileThreat(const int3 & tile) const
 	return hitMap[tile.x][tile.y][tile.z];
 }
 
-const std::set<const CGObjectInstance *> empty = {};
-
 std::set<const CGObjectInstance *> DangerHitMapAnalyzer::getOneTurnAccessibleObjects(const CGHeroInstance * enemy) const
 {
 	std::set<const CGObjectInstance *> result;

AI/Nullkiller/Analyzers/DangerHitMapAnalyzer.h

@@ -18,7 +18,7 @@ struct AIPath;
 
 struct HitMapInfo
 {
-	static HitMapInfo NoThreat;
+	static const HitMapInfo NoThreat;
 
 	uint64_t danger;
 	uint8_t turn;

AI/Nullkiller/Analyzers/HeroManager.cpp

@@ -17,7 +17,7 @@
 namespace NKAI
 {
 
-SecondarySkillEvaluator HeroManager::wariorSkillsScores = SecondarySkillEvaluator(
+const SecondarySkillEvaluator HeroManager::wariorSkillsScores = SecondarySkillEvaluator(
 	{
 		std::make_shared<SecondarySkillScoreMap>(
 			std::map<SecondarySkill, float>
@@ -46,7 +46,7 @@ SecondarySkillEvaluator HeroManager::wariorSkillsScores = SecondarySkillEvaluato
 		std::make_shared<AtLeastOneMagicRule>()
 	});
 
-SecondarySkillEvaluator HeroManager::scountSkillsScores = SecondarySkillEvaluator(
+const SecondarySkillEvaluator HeroManager::scountSkillsScores = SecondarySkillEvaluator(
 	{
 		std::make_shared<SecondarySkillScoreMap>(
 			std::map<SecondarySkill, float>
@@ -332,7 +332,7 @@ void WisdomRule::evaluateScore(const CGHeroInstance * hero, SecondarySkill skill
 		score += 1.5;
 }
 
-std::vector<SecondarySkill> AtLeastOneMagicRule::magicSchools = {
+const std::vector<SecondarySkill> AtLeastOneMagicRule::magicSchools = {
 	SecondarySkill::AIR_MAGIC,
 	SecondarySkill::EARTH_MAGIC,
 	SecondarySkill::FIRE_MAGIC,

AI/Nullkiller/Analyzers/HeroManager.h

@@ -58,8 +58,8 @@ public:
 class DLL_EXPORT HeroManager : public IHeroManager
 {
 private:
-	static SecondarySkillEvaluator wariorSkillsScores;
-	static SecondarySkillEvaluator scountSkillsScores;
+	static const SecondarySkillEvaluator wariorSkillsScores;
+	static const SecondarySkillEvaluator scountSkillsScores;
 
 	CCallback * cb; //this is enough, but we downcast from CCallback
 	const Nullkiller * ai;
@@ -114,7 +114,7 @@ public:
 class AtLeastOneMagicRule : public ISecondarySkillRule
 {
 private:
-	static std::vector<SecondarySkill> magicSchools;
+	static const std::vector<SecondarySkill> magicSchools;
 
 public:
 	void evaluateScore(const CGHeroInstance * hero, SecondarySkill skill, float & score) const override;

AI/Nullkiller/Pathfinding/AINodeStorage.cpp

@@ -332,7 +332,7 @@ std::vector<CGPathNode *> AINodeStorage::calculateNeighbours(
 	return neighbours;
 }
 
-EPathfindingLayer phisycalLayers[2] = {EPathfindingLayer::LAND, EPathfindingLayer::SAIL};
+constexpr std::array phisycalLayers = {EPathfindingLayer::LAND, EPathfindingLayer::SAIL};
 
 bool AINodeStorage::increaseHeroChainTurnLimit()
 {

AI/Nullkiller/Pathfinding/Actors.cpp

@@ -18,7 +18,7 @@
 
 using namespace NKAI;
 
-CCreatureSet emptyArmy;
+const CCreatureSet emptyArmy;
 
 bool HeroExchangeArmy::needsLastStack() const
 {

AI/Nullkiller/main.cpp

@@ -14,7 +14,7 @@
 #define strcpy_s(a, b, c) strncpy(a, c, b)
 #endif
 
-static const char * g_cszAiName = "Nullkiller";
+static const char * const g_cszAiName = "Nullkiller";
 
 extern "C" DLL_EXPORT int GetGlobalAiVersion()
 {

AI/StupidAI/main.cpp

@@ -16,7 +16,7 @@
 #define strcpy_s(a, b, c) strncpy(a, c, b)
 #endif
 
-static const char *g_cszAiName = "Stupid AI 0.1";
+static const char * const g_cszAiName = "Stupid AI 0.1";
 
 extern "C" DLL_EXPORT int GetGlobalAiVersion()
 {

AI/VCAI/BuildingManager.cpp

@@ -148,7 +148,7 @@ BuildingID::DWELL_LVL_4_UP, BuildingID::DWELL_LVL_5_UP, BuildingID::DWELL_LVL_6_
 static const std::vector<BuildingID> unitGrowth = { BuildingID::HORDE_1, BuildingID::HORDE_1_UPGR, BuildingID::HORDE_2, BuildingID::HORDE_2_UPGR };
 static const std::vector<BuildingID> _spells = { BuildingID::MAGES_GUILD_1, BuildingID::MAGES_GUILD_2, BuildingID::MAGES_GUILD_3,
 BuildingID::MAGES_GUILD_4, BuildingID::MAGES_GUILD_5 };
-static const std::vector<BuildingID> extra = { BuildingID::MARKETPLACE, BuildingID::BLACKSMITH, BuildingID::RESOURCE_SILO, BuildingID::SPECIAL_1, BuildingID::SPECIAL_2, 
+static const std::vector<BuildingID> extra = { BuildingID::MARKETPLACE, BuildingID::BLACKSMITH, BuildingID::RESOURCE_SILO, BuildingID::SPECIAL_1, BuildingID::SPECIAL_2,
 BuildingID::SPECIAL_3, BuildingID::SPECIAL_4, BuildingID::SHIPYARD }; // all remaining buildings
 
 bool BuildingManager::getBuildingOptions(const CGTownInstance * t)

AI/VCAI/main.cpp

@@ -14,7 +14,7 @@
 #define strcpy_s(a, b, c) strncpy(a, c, b)
 #endif
 
-static const char * g_cszAiName = "VCAI";
+static const char * const g_cszAiName = "VCAI";
 
 extern "C" DLL_EXPORT int GetGlobalAiVersion()
 {

client/CMT.cpp

@@ -56,7 +56,6 @@ namespace po = boost::program_options;
 namespace po_style = boost::program_options::command_line_style;
 
 static std::atomic<bool> quitRequestedDuringOpeningPlayback = false;
-static po::variables_map vm;
 
 #ifndef VCMI_IOS
 void processCommand(const std::string &message);
@@ -118,6 +117,8 @@ int main(int argc, char * argv[])
 #endif
 	std::cout << "Starting... " << std::endl;
 	po::options_description opts("Allowed options");
+	po::variables_map vm;
+
 	opts.add_options()
 		("help,h", "display help and exit")
 		("version,v", "display version information and exit")

client/CMusicHandler.cpp

@@ -30,7 +30,7 @@
 #define VCMI_SOUND_FILE(y) #y,
 
 // sounds mapped to soundBase enum
-static std::string sounds[] = {
+static const std::string sounds[] = {
 	"", // invalid
 	"", // todo
 	VCMI_SOUND_LIST

client/windows/CCastleInterface.cpp

@@ -562,27 +562,6 @@ void HeroSlots::swapArmies()
 		LOCPLINT->cb->swapGarrisonHero(town);
 }
 
-class SORTHELP
-{
-public:
-	bool operator() (const CIntObject * a, const CIntObject * b)
-	{
-		auto b1 = dynamic_cast<const CBuildingRect *>(a);
-		auto b2 = dynamic_cast<const CBuildingRect *>(b);
-
-		if(!b1 && !b2)
-			return intptr_t(a) < intptr_t(b);
-		if(b1 && !b2)
-			return false;
-		if(!b1 && b2)
-			return true;
-
-		return (*b1)<(*b2);
-	}
-};
-
-SORTHELP buildSorter;
-
 CCastleBuildings::CCastleBuildings(const CGTownInstance* Town):
 	town(Town),
 	selectedBuilding(nullptr)
@@ -650,6 +629,21 @@ void CCastleBuildings::recreate()
 		buildings.push_back(std::make_shared<CBuildingRect>(this, town, toAdd));
 	}
 
+	auto const & buildSorter = [] (const CIntObject * a, const CIntObject * b)
+	{
+		auto b1 = dynamic_cast<const CBuildingRect *>(a);
+		auto b2 = dynamic_cast<const CBuildingRect *>(b);
+
+		if(!b1 && !b2)
+			return intptr_t(a) < intptr_t(b);
+		if(b1 && !b2)
+			return false;
+		if(!b1 && b2)
+			return true;
+
+		return (*b1)<(*b2);
+	};
+
 	boost::sort(children, buildSorter); //TODO: create building in blit order
 }
 

client/windows/CSpellWindow.cpp

@@ -94,7 +94,7 @@ public:
 
 		return A->getNameTranslated() < B->getNameTranslated();
 	}
-} spellsorter;
+};
 
 CSpellWindow::CSpellWindow(const CGHeroInstance * _myHero, CPlayerInterface * _myInt, bool openOnBattleSpells):
 	CWindowObject(PLAYER_COLORED | (settings["gameTweaks"]["enableLargeSpellbook"].Bool() ? BORDERED : 0)),
@@ -293,6 +293,8 @@ void CSpellWindow::processSpells()
 		if(!spell->isCreatureAbility() && myHero->canCastThisSpell(spell) && searchTextFound)
 			mySpells.push_back(spell);
 	}
+
+	SpellbookSpellSorter spellsorter;
 	std::sort(mySpells.begin(), mySpells.end(), spellsorter);
 
 	//initializing sizes of spellbook's parts

lib/rmg/Zone.cpp

@@ -19,7 +19,7 @@
 
 VCMI_LIB_NAMESPACE_BEGIN
 
-std::function<bool(const int3 &)> AREA_NO_FILTER = [](const int3 & t)
+const std::function<bool(const int3 &)> AREA_NO_FILTER = [](const int3 & t)
 {
 	return true;
 };

lib/rmg/Zone.h

@@ -30,7 +30,7 @@ class CMapGenerator;
 class Modificator;
 class CRandomGenerator;
 
-extern std::function<bool(const int3 &)> AREA_NO_FILTER;
+extern const std::function<bool(const int3 &)> AREA_NO_FILTER;
 
 typedef std::list<std::shared_ptr<Modificator>> TModificators;
 

scripting/lua/LuaScriptModule.cpp

@@ -17,7 +17,7 @@
 #define strcpy_s(a, b, c) strncpy(a, c, b)
 #endif
 
-static const char *g_cszAiName = "Lua interpreter";
+static const char * const g_cszAiName = "Lua interpreter";
 
 VCMI_LIB_NAMESPACE_BEGIN