Review changes

Author: MaximDude

Main.cpp

@@ -133,6 +133,7 @@ int g_DifficultySetting = 4;
 int g_StationOffsetX;
 int g_StationOffsetY;
 
+LoadingGUI *g_LoadingGUI;
 MainMenuGUI *g_pMainMenuGUI = 0;
 ScenarioGUI *g_pScenarioGUI = 0;
 Controller *g_pMainMenuController = 0;
@@ -1957,7 +1958,7 @@ int main(int argc, char *argv[]) {
 		g_AudioMan.SetMusicVolume(0);
 	}
 
-	g_LoadingGUI.InitLoadingScreen();
+	g_LoadingGUI->InitLoadingScreen();
 	InitMainMenu();
 
 	if (g_LaunchIntoEditor) { 

Managers/ConsoleMan.cpp

@@ -241,7 +241,7 @@ void ConsoleMan::PrintString(std::string toPrint)
     // Add the input line to the console
     m_pConsoleText->SetText(m_pConsoleText->GetText() + "\n" + toPrint);
 	// Print the input line to the command-line
-	if (g_System.GetLogToCLI()) { g_System.PrintConsoleToCLI(toPrint); }
+	if (g_System.GetLogToCLI()) { g_System.PrintToCLI(toPrint); }
 }
 
 

Managers/PresetMan.cpp

@@ -208,15 +208,9 @@ bool PresetMan::LoadAllDataModules() {
 				if (strlen(moduleInfo.name) > 0 && (moduleID < 0 || moduleID >= GetOfficialModuleCount()) && string(moduleInfo.name) != "Metagames.rte" && string(moduleInfo.name) != "Scenes.rte") {
 					// Actually load the unofficial data module
 					if (!LoadDataModule(string(moduleInfo.name), false, &LoadingGUI::LoadingSplashProgressReport)) {
+						// LoadDataModule can return false (esp since it may try to load already loaded modules, and that's ok) and shouldn't cause stop
 						// TODO: Report error and skip loading module.
 					}
-				} else {
-					// LoadDataModule can return false (esp since it may try to load already loaded modules, and that's ok) and shouldn't cause stop
-					// TODO: Log this and continue gracefully instead
-					// char error[512];
-					// sprintf_s(error, sizeof(error), "Failed to load Data Module: %s\n\nMake sure it contains an Index.ini file that defines a \"DataModule\"!", moduleInfo.name);
-					// RTEAbort(error);
-					// return false;
 				}
 			}
 		}

Menus/LoadingGUI.cpp

@@ -20,26 +20,22 @@ extern volatile bool g_Quit;
 
 namespace RTE {
 
-	LoadingGUI g_LoadingGUI;
-
-	GUIControlManager *LoadingGUI::m_LoadingGUI = 0;
-
-	AllegroInput *LoadingGUI::m_GUIInput = 0;
-	AllegroScreen *LoadingGUI::m_GUIScreen = 0;
-	BITMAP *LoadingGUI::m_LoadingGUIBitmap = 0;
-	int LoadingGUI::m_LoadingGUIPosX = 0;
-	int LoadingGUI::m_LoadingGUIPosY = 0;
-
-	Writer *LoadingGUI::m_LoadingLogWriter = 0;
+	GUIControlManager *LoadingGUI::s_LoadingGUI = 0;
+	AllegroInput *LoadingGUI::s_GUIInput = 0;
+	AllegroScreen *LoadingGUI::s_GUIScreen = 0;
+	Writer *LoadingGUI::s_LoadingLogWriter = 0;
+	BITMAP *LoadingGUI::s_LoadingGUIBitmap = 0;
+	int LoadingGUI::s_LoadingGUIPosX = 0;
+	int LoadingGUI::s_LoadingGUIPosY = 0;
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	void LoadingGUI::InitLoadingScreen() {
 		g_FrameMan.LoadPalette("Base.rte/palette.bmp");
 
 		// Create the main GUI
-		m_GUIInput = new AllegroInput(-1);
-		m_GUIScreen = new AllegroScreen(g_FrameMan.GetBackBuffer32());
+		s_GUIInput = new AllegroInput(-1);
+		s_GUIScreen = new AllegroScreen(g_FrameMan.GetBackBuffer32());
 
 		// Loading splash screen
 		g_FrameMan.ClearBackBuffer32();
@@ -59,22 +55,22 @@ namespace RTE {
 		g_FrameMan.FlipFrameBuffers();
 
 		// Set up the loading GUI
-		if (!m_LoadingGUI) {
-			m_LoadingGUI = new GUIControlManager();
+		if (!s_LoadingGUI) {
+			s_LoadingGUI = new GUIControlManager();
 
 			// TODO: This should be using the 32bpp main menu skin, but isn't because it needs the config of the base for its listbox
 			// Can get away with this hack for now because the list box that the loading menu uses displays ok when drawn on a 32bpp buffer,
 			// when it's 8bpp internally, since it does not use any masked_blit calls to draw list boxes.
 			// Note also how the GUIScreen passed in here has been created with an 8bpp bitmap, since that is what determines what the GUI manager uses internally
-			if (!m_LoadingGUI->Create(m_GUIScreen, m_GUIInput, "Base.rte/GUIs/Skins/MainMenu", "LoadingSkin.ini")) {
+			if (!s_LoadingGUI->Create(s_GUIScreen, s_GUIInput, "Base.rte/GUIs/Skins/MainMenu", "LoadingSkin.ini")) {
 				RTEAbort("Failed to create GUI Control Manager and load it from Base.rte/GUIs/Skins/MainMenu/LoadingSkin.ini");
 			}
-			m_LoadingGUI->Load("Base.rte/GUIs/LoadingGUI.ini");
+			s_LoadingGUI->Load("Base.rte/GUIs/LoadingGUI.ini");
 		}
 
 		// Place and clear the sectionProgress box
-		dynamic_cast<GUICollectionBox *>(m_LoadingGUI->GetControl("root"))->SetSize(g_FrameMan.GetResX(), g_FrameMan.GetResY());
-		GUIListBox *pBox = dynamic_cast<GUIListBox *>(m_LoadingGUI->GetControl("ProgressBox"));
+		dynamic_cast<GUICollectionBox *>(s_LoadingGUI->GetControl("root"))->SetSize(g_FrameMan.GetResX(), g_FrameMan.GetResY());
+		GUIListBox *pBox = dynamic_cast<GUIListBox *>(s_LoadingGUI->GetControl("ProgressBox"));
 		// Make the box a bit bigger if there's room in higher, HD resolutions
 		if (g_FrameMan.GetResX() >= 960) {
 			// Make the loading progress box fill the right third of the screen
@@ -87,17 +83,17 @@ namespace RTE {
 		pBox->ClearList();
 
 		// New mechanism to speed up loading times as it turned out that a massive amount of time is spent to update GUI control.
-		if (!g_SettingsMan.DisableLoadingScreen() && !m_LoadingGUIBitmap) {
+		if (!g_SettingsMan.DisableLoadingScreen() && !s_LoadingGUIBitmap) {
 			pBox->SetVisible(false);
-			m_LoadingGUIBitmap = create_bitmap_ex(8, pBox->GetWidth(), pBox->GetHeight());
-			clear_to_color(m_LoadingGUIBitmap, 54);
-			rect(m_LoadingGUIBitmap, 0, 0, pBox->GetWidth() - 1, pBox->GetHeight() - 1, 33);
-			rect(m_LoadingGUIBitmap, 1, 1, pBox->GetWidth() - 2, pBox->GetHeight() - 2, 33);
-			m_LoadingGUIPosX = pBox->GetXPos();
-			m_LoadingGUIPosY = pBox->GetYPos();
+			s_LoadingGUIBitmap = create_bitmap_ex(8, pBox->GetWidth(), pBox->GetHeight());
+			clear_to_color(s_LoadingGUIBitmap, 54);
+			rect(s_LoadingGUIBitmap, 0, 0, pBox->GetWidth() - 1, pBox->GetHeight() - 1, 33);
+			rect(s_LoadingGUIBitmap, 1, 1, pBox->GetWidth() - 2, pBox->GetHeight() - 2, 33);
+			s_LoadingGUIPosX = pBox->GetXPos();
+			s_LoadingGUIPosY = pBox->GetYPos();
 		}
 		// Create the loading log writer
-		if (!m_LoadingLogWriter) { m_LoadingLogWriter = new Writer("LogLoading.txt"); }
+		if (!s_LoadingLogWriter) { s_LoadingLogWriter = new Writer("LogLoading.txt"); }
 
 		// Load all the data modules
 		LoadDataModules();
@@ -108,26 +104,26 @@ namespace RTE {
 	void LoadingGUI::LoadingSplashProgressReport(std::string reportString, bool newItem) {
 		if (g_System.GetLogToCLI()) { g_System.PrintLoadingToCLI(reportString, newItem); }
 
-		if (m_LoadingGUI) {
+		if (s_LoadingGUI) {
 			g_UInputMan.Update();
 			if (newItem) {
 				// Write out the last line to the log file before starting a new one
-				if (m_LoadingLogWriter->WriterOK()) { *m_LoadingLogWriter << reportString << "\n"; }
+				if (s_LoadingLogWriter->WriterOK()) { *s_LoadingLogWriter << reportString << "\n"; }
 				// Scroll bitmap upwards
-				if (m_LoadingGUIBitmap) { blit(m_LoadingGUIBitmap, m_LoadingGUIBitmap, 2, 12, 2, 2, m_LoadingGUIBitmap->w - 3, m_LoadingGUIBitmap->h - 12); }
+				if (s_LoadingGUIBitmap) { blit(s_LoadingGUIBitmap, s_LoadingGUIBitmap, 2, 12, 2, 2, s_LoadingGUIBitmap->w - 3, s_LoadingGUIBitmap->h - 12); }
 			}
-			if (m_LoadingGUIBitmap) {
-				AllegroBitmap bmp(m_LoadingGUIBitmap);
+			if (s_LoadingGUIBitmap) {
+				AllegroBitmap bmp(s_LoadingGUIBitmap);
 				// Clear current line
-				rectfill(m_LoadingGUIBitmap, 2, m_LoadingGUIBitmap->h - 12, m_LoadingGUIBitmap->w - 3, m_LoadingGUIBitmap->h - 3, 54);
+				rectfill(s_LoadingGUIBitmap, 2, s_LoadingGUIBitmap->h - 12, s_LoadingGUIBitmap->w - 3, s_LoadingGUIBitmap->h - 3, 54);
 				// Print new line
-				g_FrameMan.GetSmallFont()->DrawAligned(&bmp, 5, m_LoadingGUIBitmap->h - 12, reportString.c_str(), GUIFont::Left);
+				g_FrameMan.GetSmallFont()->DrawAligned(&bmp, 5, s_LoadingGUIBitmap->h - 12, reportString.c_str(), GUIFont::Left);
 				// DrawAligned - MaxWidth is useless here, so we're just drawing lines manually
-				vline(m_LoadingGUIBitmap, m_LoadingGUIBitmap->w - 2, m_LoadingGUIBitmap->h - 12, m_LoadingGUIBitmap->h - 2, 33);
-				vline(m_LoadingGUIBitmap, m_LoadingGUIBitmap->w - 1, m_LoadingGUIBitmap->h - 12, m_LoadingGUIBitmap->h - 2, 33);
+				vline(s_LoadingGUIBitmap, s_LoadingGUIBitmap->w - 2, s_LoadingGUIBitmap->h - 12, s_LoadingGUIBitmap->h - 2, 33);
+				vline(s_LoadingGUIBitmap, s_LoadingGUIBitmap->w - 1, s_LoadingGUIBitmap->h - 12, s_LoadingGUIBitmap->h - 2, 33);
 
 				// Draw onto current frame buffer
-				blit(m_LoadingGUIBitmap, g_FrameMan.GetBackBuffer32(), 0, 0, m_LoadingGUIPosX, m_LoadingGUIPosY, m_LoadingGUIBitmap->w, m_LoadingGUIBitmap->h);
+				blit(s_LoadingGUIBitmap, g_FrameMan.GetBackBuffer32(), 0, 0, s_LoadingGUIPosX, s_LoadingGUIPosY, s_LoadingGUIBitmap->w, s_LoadingGUIBitmap->h);
 
 				g_FrameMan.FlipFrameBuffers();
 			}

Menus/LoadingGUI.h

@@ -10,6 +10,9 @@ namespace RTE {
 	class GUIControlManager;
 	class Writer;
 
+	/// <summary>
+	/// Represents the loading screen GUI when starting the game.
+	/// </summary>
 	class LoadingGUI {
 
 	public:
@@ -40,20 +43,18 @@ namespace RTE {
 
 	protected:
 
-		static GUIControlManager *m_LoadingGUI; //!< Manager of the whole LoadingGUI.
-
-		static AllegroInput *m_GUIInput; //!< Input interface of this.
-		static AllegroScreen *m_GUIScreen; //!< Screen interface of this.
-		static BITMAP *m_LoadingGUIBitmap; //!< BITMAP that the progress report will be drawn into.
-		static int m_LoadingGUIPosX; //!< Position of the progress report box on X axis.
-		static int m_LoadingGUIPosY; //!< Position of the progress report box on Y axis.
-
-		static Writer *m_LoadingLogWriter; //!< The Writer that generates the loading log.
-
 		static const unsigned int s_MaxFileName = 256; //!< Maximum length of output file directory + name string.
 		static const unsigned int s_FileBufferSize = 8192; //!< Buffer to hold data read from the zip file.
 		static const unsigned int s_MaxUnzippedFileSize = 104857600; //!< Maximum size of single file being extracted from zip archive (100MiB).
+
+		static GUIControlManager *s_LoadingGUI; //!< Manager of the whole LoadingGUI.
+		static AllegroInput *s_GUIInput; //!< Input interface of this.
+		static AllegroScreen *s_GUIScreen; //!< Screen interface of this.
+		static Writer *s_LoadingLogWriter; //!< The Writer that generates the loading log.
+
+		static BITMAP *s_LoadingGUIBitmap; //!< BITMAP that the progress report will be drawn into.
+		static int s_LoadingGUIPosX; //!< Position of the progress report box on X axis.
+		static int s_LoadingGUIPosY; //!< Position of the progress report box on Y axis.
 	};
-	extern LoadingGUI g_LoadingGUI;
 }
 #endif

System/Box.h

@@ -90,11 +90,6 @@ namespace RTE {
 #pragma endregion
 
 #pragma region Destruction
-		/// <summary>
-		/// Destructor method used to clean up a Box object before deletion.
-		/// </summary>
-		virtual ~Box() { ; }
-
 		/// <summary>
 		/// Resets the entire Box object to the default settings or values.
 		/// </summary>

System/Color.h

@@ -27,7 +27,7 @@ namespace RTE {
 		Color() { Clear(); }
 
 		/// <summary>
-		/// Constructor method used to instantiate a Color object from RGB values
+		/// Constructor method used to instantiate a Color object from RGB values.
 		/// </summary>
 		/// <param name="R">Initial Red value of this color.</param>
 		/// <param name="G">Initial Green value of this color.</param>
@@ -63,11 +63,6 @@ namespace RTE {
 #pragma endregion
 
 #pragma region Destruction
-		/// <summary>
-		/// Destructor method used to clean up a Color object before deletion.
-		/// </summary>
-		virtual ~Color() { ; }
-
 		/// <summary>
 		/// Sets RGB of this Color to zero.
 		/// </summary>

System/DataModule.cpp

@@ -19,8 +19,9 @@ namespace RTE {
 		m_PresetList.clear();
 		m_EntityList.clear();
 		m_TypeMap.clear();
-		for (int i = 0; i < c_PaletteEntriesNumber; ++i) { m_MaterialMappings[i] = 0; }
-
+		for (int i = 0; i < c_PaletteEntriesNumber; ++i) { 
+			m_MaterialMappings[i] = 0; 
+		}
 		m_ScanFolderContents = false;
 		m_IgnoreMissingItems = false;
 		m_CrabToHumanSpawnRatio = 0;
@@ -51,14 +52,8 @@ namespace RTE {
 		if (std::experimental::filesystem::exists(indexPath.c_str()) && reader.Create(indexPath.c_str(), true, fpProgressCallback) >= 0) {
 			int result = Serializable::Create(reader);
 
-			// Report loading result
+			// Print an empty line to separate the end of a module from the beginning of the next one in the loading progress log.
 			if (fpProgressCallback) {
-				// TODO: more info here?
-				/*
-				char report[512];
-				sprintf_s(report, sizeof(report), "%s loading:", m_FileName.c_str());
-				fpProgressCallback(std::string(report), true);
-				*/
 				fpProgressCallback(std::string(" "), true);
 			}
 
@@ -152,7 +147,6 @@ namespace RTE {
 			// Check for required dependencies if we're not load properties
 			std::string requiredModule;
 			reader >> requiredModule;
-
 			if (!reader.GetSkipIncludes() && g_PresetMan.GetModuleID(requiredModule) == -1) {
 				reader.ReportError("\"" + m_FileName + "\" requires \"" + requiredModule + "\" in order to load!\n");
 			}
@@ -216,7 +210,7 @@ namespace RTE {
 			return 0;
 		}
 
-		map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator classItr = m_TypeMap.find(exactType);
+		std::map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator classItr = m_TypeMap.find(exactType);
 		if (classItr != m_TypeMap.end()) {
 			// Find an instance of that EXACT type and name; derived types are not matched
 			for (std::list<std::pair<std::string, Entity *>>::iterator instItr = (*classItr).second.begin(); instItr != (*classItr).second.end(); ++instItr) {
@@ -289,14 +283,15 @@ namespace RTE {
 		if (withType == "All" || withType.empty()) {
 			for (std::list<std::string>::const_iterator gItr = m_GroupRegister.begin(); gItr != m_GroupRegister.end(); ++gItr) {
 				groupList.push_back(*gItr);
-				//TODO it seems weird that foundAny isn't set to true here, given that the list gets filled. But I suppose no actual finding is done. Investigate this and see where it's called, maybe this should be changed
+				// TODO: it seems weird that foundAny isn't set to true here, given that the list gets filled. 
+				// But I suppose no actual finding is done. Investigate this and see where it's called, maybe this should be changed
 			}
 		} else {
-			map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator clsItr = m_TypeMap.find(withType);
-			if (clsItr != m_TypeMap.end()) {
+			std::map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator classItr = m_TypeMap.find(withType);
+			if (classItr != m_TypeMap.end()) {
 				const std::list<std::string> *pGroupList = 0;
 				// Go through all the entities of that type, adding the groups they belong to
-				for (std::list<std::pair<std::string, Entity *>>::iterator instItr = clsItr->second.begin(); instItr != clsItr->second.end(); ++instItr) {
+				for (std::list<std::pair<std::string, Entity *>>::iterator instItr = classItr->second.begin(); instItr != classItr->second.end(); ++instItr) {
 					pGroupList = instItr->second->GetGroupList();
 
 					for (std::list<std::string>::const_iterator gItr = pGroupList->begin(); gItr != pGroupList->end(); ++gItr) {
@@ -385,13 +380,14 @@ namespace RTE {
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-	//TODO This method is almost identical to GetEntityPreset, except it doesn't return a const Entity *. Investigate if the latter needs to return const (based on what's using it) and if not, get rid of this and replace its uses. At the very least, consider renaming this
+	// TODO: This method is almost identical to GetEntityPreset, except it doesn't return a const Entity *. 
+	// Investigate if the latter needs to return const (based on what's using it) and if not, get rid of this and replace its uses. At the very least, consider renaming this
 	Entity * DataModule::GetEntityIfExactType(const std::string &exactType, const std::string &instanceName) {
 		if (exactType.empty() || instanceName == "None" || instanceName.empty()) {
 			return 0;
 		}
 
-		map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator classItr = m_TypeMap.find(exactType);
+		std::map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator classItr = m_TypeMap.find(exactType);
 		if (classItr != m_TypeMap.end()) {
 			// Find an instance of that EXACT type and name; derived types are not matched
 			for (std::list<std::pair<std::string, Entity *>>::iterator instItr = (*classItr).second.begin(); instItr != (*classItr).second.end(); ++instItr) {
@@ -412,15 +408,15 @@ namespace RTE {
 
 		// Walk up the class hierarchy till we reach the top, adding an entry of the passed in entity into each typelist as we go along
 		for (const Entity::ClassInfo *pClass = &(pEntToAdd->GetClass()); pClass != 0; pClass = pClass->GetParent()) {
-			map<string, list<pair<string, Entity *> > >::iterator clsItr = m_TypeMap.find(pClass->GetName());
+			std::map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator classItr = m_TypeMap.find(pClass->GetName());
 
 			// No instances of this entity have been added yet so add a class category for it
-			if (clsItr == m_TypeMap.end()) {
-				clsItr = (m_TypeMap.insert(pair<string, list<pair<string, Entity *> > >(pClass->GetName(), list<pair<string, Entity *> >()))).first;
+			if (classItr == m_TypeMap.end()) {
+				classItr = (m_TypeMap.insert(std::pair<std::string, std::list<std::pair<std::string, Entity *>>>(pClass->GetName(), std::list<std::pair<std::string, Entity *>>()))).first;
 			}
 
-			// NOTE We're adding the entity to the class category list but not transfering ownership. Also, we're not checking for collisions as they're assumed to have been checked for already
-			(*clsItr).second.push_back(pair<string, Entity *>(pEntToAdd->GetPresetName(), pEntToAdd));
+			// NOTE We're adding the entity to the class category list but not transferring ownership. Also, we're not checking for collisions as they're assumed to have been checked for already
+			(*classItr).second.push_back(std::pair<std::string, Entity *>(pEntToAdd->GetPresetName(), pEntToAdd));
 		}
 		return true;
 	}