Merge pull request #244 from Architector4/patch-some-code-cleanups

cppcheck lints - change a lot of method params to use const ref, etc etc

Author: Causeless

Source/Activities/AssemblyEditor.cpp

@@ -377,7 +377,7 @@ void AssemblyEditor::Draw(BITMAP* pTargetBitmap, const Vector& targetPos) {
 	EditorActivity::Draw(pTargetBitmap, targetPos);
 }
 
-BunkerAssembly* AssemblyEditor::BuildAssembly(std::string saveAsName) {
+BunkerAssembly* AssemblyEditor::BuildAssembly(const std::string& saveAsName) {
 	// Create new bunker assembly to save
 	BunkerAssembly* pBA = new BunkerAssembly();
 	pBA->Create(m_pEditorGUI->GetCurrentAssemblyScheme());

Source/Activities/AssemblyEditor.h

@@ -107,7 +107,7 @@ namespace RTE {
 		/// it's pointer. Owhership IS transfered.
 		/// @param saveAsName New assembly name.
 		/// @return Built BunkerAssembly
-		BunkerAssembly* BuildAssembly(std::string saveAsName);
+		BunkerAssembly* BuildAssembly(const std::string& saveAsName);
 
 		/// Saves the current BunkerAssembly to an appropriate ini file, and asks user if they want to overwrite first if a BunkerAssembly of this name exists.
 		/// @param saveAsName The name of the new BunkerAssembly to be saved.

Source/Activities/GAScripted.h

@@ -49,8 +49,8 @@ namespace RTE {
 		/// @return An error return value signaling sucess or any particular failure.
 		/// Anything below 0 is an error signal.
 		int Create(std::string scriptPath, std::string scriptClassName) {
-			m_ScriptPath = scriptPath;
-			m_LuaClassName = scriptClassName;
+			m_ScriptPath = std::move(scriptPath);
+			m_LuaClassName = std::move(scriptClassName);
 			return Create();
 		};
 

Source/Activities/GATutorial.cpp

@@ -35,10 +35,11 @@ GATutorial::~GATutorial() {
 	Destroy(true);
 }
 
-GATutorial::TutStep::TutStep(std::string text, int stepDuration, std::string screensPath, int frameCount, int frameDuration) {
-	m_Text = text;
-	m_Duration = stepDuration;
-	m_FrameDuration = frameDuration;
+GATutorial::TutStep::TutStep(std::string text, int stepDuration, const std::string& screensPath, int frameCount, int frameDuration) :
+	m_Text(std::move(text)),
+	m_Duration(stepDuration),
+	m_FrameDuration(frameDuration)
+	{
 
 	if (!screensPath.empty()) {
 		ContentFile(screensPath.c_str()).GetAsAnimation(m_pScreens, frameCount);
@@ -677,7 +678,7 @@ void GATutorial::DrawGUI(BITMAP* pTargetBitmap, const Vector& targetPos, int whi
 			int timePhase = (int)m_AreaTimer.GetElapsedRealTimeMS() % 1200;
 			revealText = revealText + (timePhase > 900 ? "..." : (timePhase > 600 ? ".. " : (timePhase > 300 ? ".  " : "   ")));
 		}
-		g_FrameMan.GetSmallFont()->DrawAligned(&pBitmapInt, screenTextPos.m_X, screenTextPos.m_Y, revealText.c_str(), GUIFont::Centre);
+		g_FrameMan.GetSmallFont()->DrawAligned(&pBitmapInt, screenTextPos.m_X, screenTextPos.m_Y, revealText, GUIFont::Centre);
 	}
 }
 

Source/Activities/GATutorial.h

@@ -150,7 +150,7 @@ namespace RTE {
 			// The duration of one frame
 			int m_FrameDuration;
 
-			TutStep(std::string text, int stepDuration, std::string screensPath = "", int frameCount = 1, int frameDuration = 250);
+			TutStep(std::string text, int stepDuration, const std::string& screensPath = "", int frameCount = 1, int frameDuration = 250);
 		};
 
 		// Member variables

Source/Activities/GameActivity.cpp

@@ -282,7 +282,7 @@ void GameActivity::Destroy(bool notInherited) {
 	Clear();
 }
 
-void GameActivity::SetTeamTech(int team, std::string tech) {
+void GameActivity::SetTeamTech(int team, const std::string& tech) {
 	if (team >= Teams::TeamOne && team < Teams::MaxTeamCount) {
 		if (tech == "-All-" || tech == "-Random-")
 			m_TeamTech[team] = tech;
@@ -388,7 +388,7 @@ void GameActivity::SwitchToPrevActor(int player, int team, Actor* pSkip) {
 	}
 }
 
-void GameActivity::AddObjectivePoint(std::string description, Vector objPos, int whichTeam, ObjectiveArrowDir arrowDir) {
+void GameActivity::AddObjectivePoint(const std::string& description, Vector objPos, int whichTeam, ObjectiveArrowDir arrowDir) {
 	m_Objectives.push_back(ObjectivePoint(description, objPos, whichTeam, arrowDir));
 }
 
@@ -468,7 +468,7 @@ int GameActivity::SetOverridePurchaseList(const Loadout* pLoadout, int player) {
 	return finalListCost;
 }
 
-int GameActivity::SetOverridePurchaseList(std::string loadoutName, int player) {
+int GameActivity::SetOverridePurchaseList(const std::string& loadoutName, int player) {
 	// Find out the native module of this player
 	int nativeModule = 0;
 	MetaPlayer* pMetaPlayer = g_MetaMan.GetMetaPlayerOfInGamePlayer(player);
@@ -664,7 +664,7 @@ void GameActivity::SetupPlayers() {
 
 int GameActivity::Start() {
 	// Set the split screen config before the Scene (and it SceneLayers, specifially) are loaded
-	int humanCount = GetHumanCount();
+	uint8_t humanCount = GetHumanCount();
 	// Depending on the resolution aspect ratio, split first horizontally (if wide screen)
 	if (((float)g_WindowMan.GetResX() / (float)g_WindowMan.GetResY()) >= 1.6)
 		g_FrameMan.ResetSplitScreens(humanCount > 1, humanCount > 2);
@@ -2474,7 +2474,7 @@ void GameActivity::ObjectivePoint::Draw(BITMAP* pTargetBitmap, BITMAP* pArrowBit
 	}
 }
 
-std::string& GameActivity::GetNetworkPlayerName(int player) {
+const std::string& GameActivity::GetNetworkPlayerName(int player) {
 	if (player >= Players::PlayerOne && player < Players::MaxPlayerCount)
 		return m_NetworkPlayerNames[player];
 	else
@@ -2483,5 +2483,5 @@ std::string& GameActivity::GetNetworkPlayerName(int player) {
 
 void GameActivity::SetNetworkPlayerName(int player, std::string name) {
 	if (player >= Players::PlayerOne && player < Players::MaxPlayerCount)
-		m_NetworkPlayerNames[player] = name;
+		m_NetworkPlayerNames[player] = std::move(name);
 }

Source/Activities/GameActivity.h

@@ -238,7 +238,7 @@ namespace RTE {
 		/// @param objPos The very short description of what the objective is (three short words max)
 		/// @param whichTeam The absolute scene coordiante position of the objective. (default: Teams::TeamOne)
 		/// @param arrowDir The desired direction of the arrow when the point is on screen. (default: ARROWDOWN)
-		void AddObjectivePoint(std::string description, Vector objPos, int whichTeam = Teams::TeamOne, ObjectiveArrowDir arrowDir = ARROWDOWN);
+		void AddObjectivePoint(const std::string& description, Vector objPos, int whichTeam = Teams::TeamOne, ObjectiveArrowDir arrowDir = ARROWDOWN);
 
 		/// Sorts all objective points according to their positions on the Y axis.
 		void YSortObjectivePoints();
@@ -265,7 +265,7 @@ namespace RTE {
 		/// @param loadoutName The name of the Loadout preset to set the override purchase list to
 		/// represent.
 		/// @return The new total value of what's in the override purchase list.
-		int SetOverridePurchaseList(std::string loadoutName, int player);
+		int SetOverridePurchaseList(const std::string& loadoutName, int player);
 
 		/// Clears all items from a specific player's override purchase list.
 		/// @param m_PurchaseOverride[player].clear( Which player's override purchase list to clear.
@@ -352,7 +352,7 @@ namespace RTE {
 
 		/// Sets tech module name for specified team. Module must set must be loaded.
 		/// @param team Team to set module, module name, for example Dummy.rte
-		void SetTeamTech(int team, std::string tech);
+		void SetTeamTech(int team, const std::string& tech);
 
 		/// Indicates whether a specific team is assigned a CPU player in the current game.
 		/// @param team Which team index to check.
@@ -448,7 +448,7 @@ namespace RTE {
 		/// Returns network player name
 		/// @param player Player
 		/// @return Network player name
-		std::string& GetNetworkPlayerName(int player);
+		const std::string& GetNetworkPlayerName(int player);
 
 		/// Sets network player name
 		/// @param player Player number, player name
@@ -470,12 +470,13 @@ namespace RTE {
 				m_Team = Teams::NoTeam;
 				m_ArrowDir = ARROWDOWN;
 			}
-			ObjectivePoint(const std::string& desc, const Vector& pos, int team = -1, ObjectiveArrowDir arrowDir = ARROWDOWN) {
-				m_Description = desc;
-				m_ScenePos = pos;
-				m_Team = (Teams)team;
-				m_ArrowDir = arrowDir;
-			}
+
+			ObjectivePoint(std::string desc, const Vector& pos, int team = -1, ObjectiveArrowDir arrowDir = ARROWDOWN) :
+				m_Description(std::move(desc)),
+				m_ScenePos(pos),
+				m_Team((Teams)team),
+				m_ArrowDir(arrowDir)
+			{}
 
 			/// Simply draws this' arrow relative to a point on a bitmap.
 			/// @param pTargetBitmap A pointer to the BITMAP to draw on.

Source/Entities/AHuman.cpp

@@ -686,7 +686,7 @@ bool AHuman::EquipFirearm(bool doEquip) {
 	return false;
 }
 
-bool AHuman::EquipDeviceInGroup(std::string group, bool doEquip) {
+bool AHuman::EquipDeviceInGroup(const std::string& group, bool doEquip) {
 	if (!(m_pFGArm && m_pFGArm->IsAttached())) {
 		return false;
 	}
@@ -748,7 +748,7 @@ bool AHuman::EquipDeviceInGroup(std::string group, bool doEquip) {
 	return false;
 }
 
-bool AHuman::EquipLoadedFirearmInGroup(std::string group, std::string excludeGroup, bool doEquip) {
+bool AHuman::EquipLoadedFirearmInGroup(const std::string& group, const std::string& excludeGroup, bool doEquip) {
 	if (!(m_pFGArm && m_pFGArm->IsAttached())) {
 		return false;
 	}

Source/Entities/AHuman.h

@@ -243,7 +243,7 @@ namespace RTE {
 		/// @param doEquip Whether to actually equip any matching item found in the inventory, (default: true)
 		/// or just report that it's there or not.
 		/// @return Whether a firearm was successfully switched to, or already held.
-		bool EquipDeviceInGroup(std::string group, bool doEquip = true);
+		bool EquipDeviceInGroup(const std::string& group, bool doEquip = true);
 
 		/// Switches the currently held device (if any) to the first loaded HDFirearm
 		/// of the specified group in the inventory. If no such weapon is in the
@@ -253,7 +253,7 @@ namespace RTE {
 		/// @param doEquip Whether to actually equip any matching item found in the inventory, (default: true)
 		/// or just report that it's there or not.
 		/// @return Whether a firearm was successfully switched to, or already held.
-		bool EquipLoadedFirearmInGroup(std::string group, std::string exludeGroup, bool doEquip = true);
+		bool EquipLoadedFirearmInGroup(const std::string& group, const std::string& exludeGroup, bool doEquip = true);
 
 		/// Switches the equipped HeldDevice (if any) to the first found device with the specified preset name in the inventory.
 		/// If the equipped HeldDevice is of that module and preset name, nothing happens.

Source/Entities/Activity.cpp

@@ -467,8 +467,8 @@ void Activity::ClearPlayers(bool resetFunds) {
 	m_PlayerCount = m_TeamCount = 0;
 }
 
-int Activity::GetHumanCount() const {
-	int humans = 0;
+uint8_t Activity::GetHumanCount() const {
+	uint8_t humans = 0;
 	for (int player = Players::PlayerOne; player < Players::MaxPlayerCount; ++player) {
 		if (m_IsActive[player] && m_IsHuman[player]) {
 			humans++;