Made ConsoleMan not use forward slash as one of its trigger keys

Cleaned up comments in PresetMan::LoadAllDataModules
Cleaned up commented assert in DataModule and added issue link to a todo
Cleaned up comments and commented code in Entity.cpp, used Limit when setting RandomWeight, changed tabbing in init list, fixed newline in for loop, fixed returning false instead of int
Changed spacing for defines in Entity.h, reorganized ClassInfo members so they match the init list organization, fixed some method comments, added TODO
Removed unused Writer(string, bool) constructor

Author: garethyr

Managers/ConsoleMan.cpp

@@ -319,7 +319,7 @@ void ConsoleMan::Update()
 {
 	bool inputConsumed = false;
 
-	if ((g_UInputMan.FlagCtrlState() && g_UInputMan.KeyPressed(KEY_TILDE)) || (g_UInputMan.FlagCtrlState() && g_UInputMan.KeyPressed(KEY_SLASH)))
+	if (g_UInputMan.FlagCtrlState() && g_UInputMan.KeyPressed(KEY_TILDE))
 	{
 		inputConsumed = true;
 		if (IsForceVisible())
@@ -388,7 +388,7 @@ void ConsoleMan::Update()
     }
 
     // Toggle enabling with traditional tilde key
-    if (!inputConsumed && (g_UInputMan.KeyPressed(KEY_TILDE) || g_UInputMan.KeyPressed(KEY_SLASH)))
+    if (!inputConsumed && g_UInputMan.KeyPressed(KEY_TILDE))
     {
         if (IsEnabled())
         {
@@ -422,8 +422,7 @@ void ConsoleMan::Update()
     m_pGUIController->Update();
 
     // Remove any junk input that will have been entered by the opening/closing of the console
-    if (g_UInputMan.KeyPressed(KEY_TILDE) || g_UInputMan.KeyPressed(KEY_SLASH) ||
-        g_UInputMan.KeyHeld(KEY_TILDE) || g_UInputMan.KeyHeld(KEY_SLASH))
+    if (g_UInputMan.KeyPressed(KEY_TILDE) ||g_UInputMan.KeyHeld(KEY_TILDE))
     {
         // Restore any text being worked on in the input, as the box keeps getting junk added to it
         m_pInputTextBox->SetText(m_LastInputString);

Managers/PresetMan.cpp

@@ -206,11 +206,8 @@ bool PresetMan::LoadAllDataModules() {
 				moduleID = GetModuleID(moduleInfo.name);
 				// Make sure we don't load properties of already loaded official modules
 				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.
-					}
+					// NOTE: LoadDataModule can return false (especially since it may try to load already loaded modules, which is okay) and shouldn't cause stop, so we can ignore its return value here.
+                    LoadDataModule(string(moduleInfo.name), false, &LoadingGUI::LoadingSplashProgressReport);
 				}
 			}
 		}

System/DataModule.cpp

@@ -321,7 +321,6 @@ namespace RTE {
 		std::map<std::string, std::list<std::pair<std::string, Entity *>>>::iterator classItr = m_TypeMap.find((type.empty() || type == "All") ? "Entity" : type);
 
 		if (classItr != m_TypeMap.end()) {
-			//RTEAssert(!(type.empty() || type == "All") && !classItr->second.empty(), "DataModule has class entry without instances in its map!?");
 			RTEAssert(!classItr->second.empty(), "DataModule has class entry without instances in its map!?");
 			for (std::list<std::pair<std::string, Entity *>>::iterator instItr = classItr->second.begin(); instItr != classItr->second.end(); ++instItr) {
 				if (instItr->second->IsInGroup(group)) {
@@ -382,6 +381,7 @@ 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
+	// See https://github.com/cortex-command-community/Cortex-Command-Community-Project-Source/issues/87
 	Entity * DataModule::GetEntityIfExactType(const std::string &exactType, const std::string &instanceName) {
 		if (exactType.empty() || instanceName == "None" || instanceName.empty()) {
 			return 0;

System/Entity.cpp

@@ -24,8 +24,7 @@ namespace RTE {
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	int Entity::Create() {
-		// Special "All" group that includes.. all
-		m_Groups.push_back("All");
+		m_Groups.push_back("All"); // Special "All" group that includes.. all
 		return 0;
 	}
 
@@ -53,15 +52,8 @@ namespace RTE {
 
 			if (pPreset) {
 				pPreset->Clone(this);
-				// Couldn't find the preset to copy from? Then just read it in as an original
 			} else {
-				/*	Just read as an original preset instead of failing
-					char error[256];
-					sprintf_s(error, sizeof(error), "Referring to an instance ('%s') to copy from that hasn't been defined!", refName.c_str());
-					reader.ReportError(error);
-					return -1;
-				*/
-				// Do report this error to the console though!
+				// If we couldn't find the preset to copy from, read it as an original but report the problem in the console
 				g_ConsoleMan.PrintString("ERROR: Couldn't find the preset '" + refName + "' accessed in " + reader.GetCurrentFilePath() + " at line " + reader.GetCurrentFileLineString());
 
 				// Preset name might have "[ModuleName]/" preceding it, detect it here and select proper module!
@@ -86,8 +78,7 @@ namespace RTE {
 			reader >> m_PresetDescription;
 		} else if (propName == "RandomWeight") {
 			reader >> m_RandomWeight;
-			if (m_RandomWeight < 0) { m_RandomWeight = 0; }
-			if (m_RandomWeight > 100) { m_RandomWeight = 100; }
+			m_RandomWeight = Limit(m_RandomWeight, 100, 0);
 		} else if (propName == "AddToGroup") {
 			std::string newGroup;
 			reader >> newGroup;
@@ -167,8 +158,7 @@ namespace RTE {
 		if (m_DefinedInModule == whichModule) {
 			return false;
 		}
-		// This now a unique snowflake
-		m_IsOriginalPreset = true;
+		m_IsOriginalPreset = true; // This now a unique snowflake
 		m_DefinedInModule = whichModule;
 		return true;
 	}
@@ -205,9 +195,8 @@ namespace RTE {
 	Reader & operator>>(Reader &reader, Entity &operand) {
 		// Get this before reading Entity, since if it's the last one in its datafile, the stream will show the parent file instead
 		std::string objectFilePath = reader.GetCurrentFilePath();
-		// Read the Entity from the file
+		// Read the Entity from the file and try to add it to PresetMan
 		operand.Create(reader);
-		// Try to add it to the PresetMan
 		g_PresetMan.AddEntityPreset(&operand, reader.GetReadModuleID(), reader.GetPresetOverwriting(), objectFilePath);
 
 		return reader;
@@ -219,9 +208,8 @@ namespace RTE {
 		if (operand) {
 			// Get this before reading Entity, since if it's the last one in its datafile, the stream will show the parent file instead
 			std::string objectFilePath = reader.GetCurrentFilePath();
-			// Read the Entity from the file
+			// Read the Entity from the file and try to add it to PresetMan
 			operand->Create(reader);
-			// Try to add it to the PresetMan
 			g_PresetMan.AddEntityPreset(operand, reader.GetReadModuleID(), reader.GetPresetOverwriting(), objectFilePath);
 		} else {
 			reader.ReportError("Tried to read an .ini file into a null Entity pointer!");
@@ -238,11 +226,11 @@ namespace RTE {
 		m_fpDeallocate(fpDeallocFunc),
 		m_fpNewInstance(fpNewFunc),
 		m_NextClass(m_sClassHead) {
-		m_sClassHead = this;
+			m_sClassHead = this;
 
-		m_AllocatedPool.clear();
-		m_PoolAllocBlockCount = allocBlockCount > 0 ? allocBlockCount : 10;
-	}
+			m_AllocatedPool.clear();
+			m_PoolAllocBlockCount = allocBlockCount > 0 ? allocBlockCount : 10;
+		}
 
 /////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
@@ -284,8 +272,9 @@ namespace RTE {
 
 		// If concrete class, fill up the pool with pre-allocated memory blocks the size of the type
 		if (m_fpAllocate && fillAmount > 0) {
-			// As many as we're asked to make
-			for (int i = 0; i < fillAmount; ++i) { m_AllocatedPool.push_back(m_fpAllocate()); }
+			for (int i = 0; i < fillAmount; ++i) {
+				m_AllocatedPool.push_back(m_fpAllocate());
+			}
 		}
 	}
 
@@ -313,7 +302,7 @@ namespace RTE {
 
 	int Entity::ClassInfo::ReturnPoolMemory(void *pReturnedMemory) {
 		if (!pReturnedMemory) {
-			return false;
+			return 0;
 		}
 		m_AllocatedPool.push_back(pReturnedMemory);
 

System/Entity.h

@@ -11,41 +11,41 @@ namespace RTE {
 
 #pragma region Global Macro Definitions
 	#define ABSTRACTCLASSINFO(TYPE, PARENT)	\
-    Entity::ClassInfo TYPE::m_sClass(#TYPE, &PARENT::m_sClass);
+		Entity::ClassInfo TYPE::m_sClass(#TYPE, &PARENT::m_sClass);
 
 	#define CONCRETECLASSINFO(TYPE, PARENT, BLOCKCOUNT) \
-    Entity::ClassInfo TYPE::m_sClass(#TYPE, &PARENT::m_sClass, TYPE::Allocate, TYPE::Deallocate, TYPE::NewInstance, BLOCKCOUNT);
+		Entity::ClassInfo TYPE::m_sClass(#TYPE, &PARENT::m_sClass, TYPE::Allocate, TYPE::Deallocate, TYPE::NewInstance, BLOCKCOUNT);
 
 	#define CONCRETESUBCLASSINFO(TYPE, SUPER, PARENT, BLOCKCOUNT) \
-    Entity::ClassInfo SUPER::TYPE::m_sClass(#TYPE, &PARENT::m_sClass, SUPER::TYPE::Allocate, SUPER::TYPE::Deallocate, SUPER::TYPE::NewInstance, BLOCKCOUNT);
+		Entity::ClassInfo SUPER::TYPE::m_sClass(#TYPE, &PARENT::m_sClass, SUPER::TYPE::Allocate, SUPER::TYPE::Deallocate, SUPER::TYPE::NewInstance, BLOCKCOUNT);
 
 	/// <summary>
 	/// Convenience macro to cut down on duplicate ClassInfo methods in classes that extend Entity.
 	/// </summary>
 	#define CLASSINFOGETTERS \
-	const Entity::ClassInfo & GetClass() const { return m_sClass; } \
-	const std::string & GetClassName() const { return m_sClass.GetName(); }
+		const Entity::ClassInfo & GetClass() const { return m_sClass; } \
+		const std::string & GetClassName() const { return m_sClass.GetName(); }
 
 	/// <summary>
 	/// Static method used in conjunction with ClassInfo to allocate an Entity. 
 	/// This function is passed into the constructor of this Entity's static ClassInfo's constructor, so that it can instantiate MovableObjects.
 	/// </summary>
 	/// <returns>A pointer to the newly dynamically allocated Entity. Ownership is transferred as well.</returns>
-	#define ENTITYALLOCATION(TYPE)																	\
-	static void * operator new (size_t size) { return TYPE::m_sClass.GetPoolMemory(); }				\
-	static void operator delete (void *pInstance) { TYPE::m_sClass.ReturnPoolMemory(pInstance); }	\
-	static void * operator new (size_t size, void *p) throw() { return p; }							\
-	static void operator delete (void *, void *) throw() {  }										\
-	static void * Allocate() { return malloc(sizeof(TYPE)); }										\
-	static void Deallocate(void *pInstance) { free(pInstance); }									\
-	static Entity * NewInstance() { return new TYPE; }												\
-	virtual Entity * Clone(Entity *pCloneTo = 0) const {											\
-		TYPE *pEnt = pCloneTo ? dynamic_cast<TYPE *>(pCloneTo) : new TYPE();						\
-		RTEAssert(pEnt, "Tried to clone to an incompatible instance!");								\
-		if (pCloneTo) { pEnt->Destroy(); }															\
-		pEnt->Create(*this);																		\
-		return pEnt;																				\
-	}
+	#define ENTITYALLOCATION(TYPE)																		\
+		static void * operator new (size_t size) { return TYPE::m_sClass.GetPoolMemory(); }				\
+		static void operator delete (void *pInstance) { TYPE::m_sClass.ReturnPoolMemory(pInstance); }	\
+		static void * operator new (size_t size, void *p) throw() { return p; }							\
+		static void operator delete (void *, void *) throw() {  }										\
+		static void * Allocate() { return malloc(sizeof(TYPE)); }										\
+		static void Deallocate(void *pInstance) { free(pInstance); }									\
+		static Entity * NewInstance() { return new TYPE; }												\
+		virtual Entity * Clone(Entity *pCloneTo = 0) const {											\
+			TYPE *pEnt = pCloneTo ? dynamic_cast<TYPE *>(pCloneTo) : new TYPE();						\
+			RTEAssert(pEnt, "Tried to clone to an incompatible instance!");								\
+			if (pCloneTo) { pEnt->Destroy(); }															\
+			pEnt->Create(*this);																		\
+			return pEnt;																				\
+		}
 #pragma endregion
 
 	/// <summary>
@@ -178,17 +178,17 @@ namespace RTE {
 			const std::string m_Name; //!< A string with the friendly - formatted name of this ClassInfo.
 			const ClassInfo *m_pParentInfo; //!< A pointer to the parent ClassInfo.
 
+			MemoryAllocate m_fpAllocate; //!< Raw memory allocation for the size of the type this ClassInfo describes.
+			MemoryDeallocate m_fpDeallocate; //!< Raw memory deallocation for the size of the type this ClassInfo describes.
+			// TODO: figure out why this doesn't want to work when defined as std::function.
+			Entity *(*m_fpNewInstance)(); //!< Returns an actual new instance of the type that this describes.
+
 			ClassInfo *m_NextClass; //!< Next ClassInfo after this one on aforementioned unordered linked list.
 
 			std::vector<void *> m_AllocatedPool; //!< Pool of pre-allocated objects of the type described by this ClassInfo.
 			int m_PoolAllocBlockCount; //!< The number of instances to fill up the pool of this type with each time it runs dry.
 			int m_InstancesInUse; //!< The number of allocated instances passed out from the pool.
 
-			MemoryAllocate m_fpAllocate; //!< Raw memory allocation for the size of the type this ClassInfo describes.
-			MemoryDeallocate m_fpDeallocate; //!< Raw memory deallocation for the size of the type this ClassInfo describes.
-
-			// TODO: figure out why this doesn't want to work when defined as std::function.
-			Entity *(*m_fpNewInstance)(); //!< Returns an actual new instance of the type that this describes.
 
 			// Forbidding copying
 			ClassInfo(const ClassInfo &reference) {}
@@ -220,7 +220,7 @@ namespace RTE {
 		/// </summary>
 		/// <param name="reader">A Reader that the Serializable will create itself from.</param>
 		/// <param name="checkType">Whether there is a class name in the stream to check against to make sure the correct type is being read from the stream.</param>
-		/// <param name="doCreate"></param>
+		/// <param name="doCreate">Whether to do any additional initialization of the object after reading in all the properties from the Reader. This is done by calling Create().</param>
 		/// <returns>An error return value signaling success or any particular failure. Anything below 0 is an error signal.</returns>
 		virtual int Create(Reader &reader, bool checkType = true, bool doCreate = true) { return Serializable::Create(reader, checkType, doCreate); }
 
@@ -312,7 +312,7 @@ namespace RTE {
 		const std::string & GetDescription() const { return m_PresetDescription; }
 
 		/// <summary>
-		/// Sets the plain text description of this Entity's data Preset. Should only be a couple of sentences.
+		/// Sets the plain text description of this Entity's data Preset. Shouldn't be more than a couple of sentences.
 		/// </summary>
 		/// <param name="newDesc">A string reference with the preset description.</param>
 		void SetDescription(const std::string &newDesc) { m_PresetDescription = newDesc; }
@@ -324,7 +324,7 @@ namespace RTE {
 		std::string GetModuleAndPresetName() const;
 
 		/// <summary>
-		/// Indicated whether this Entity was explicitly given a new instance name upon creation, or if it was just copied/inherited implicitly.
+		/// Indicates whether this Entity was explicitly given a new instance name upon creation, or if it was just copied/inherited implicitly.
 		/// </summary>
 		/// <returns>Whether this Entity was given a new Preset Name upon creation.</returns>
 		bool IsOriginalPreset() const { return m_IsOriginalPreset; }
@@ -430,6 +430,7 @@ namespace RTE {
 		bool m_IsOriginalPreset; //!< Whether this is to be added to the PresetMan as an original preset instance.  
 		int m_DefinedInModule; //!< The DataModule ID that this was successfully added to at some point. -1 if not added to anything yet.
 
+		//TODO Consider replacing this with an unordered_set. See https://github.com/cortex-command-community/Cortex-Command-Community-Project-Source/issues/88
 		std::list<std::string> m_Groups; //!< List of all tags associated with this. The groups are used to categorize and organize Entities.  
 		std::string m_LastGroupSearch; //!< Last group search string, for more efficient response on multiple tries for the same group name.  
 		bool m_LastGroupResult; //!< Last group search result, for more efficient response on multiple tries for the same group name.

System/Writer.h

@@ -24,7 +24,6 @@ namespace RTE {
 		/// <param name="filename">Path to the file to open for writing.</param>
 		/// <param name="append">Whether to append to the file if it exists, or to overwrite it.</param>
 		Writer(const char *filename, bool append = false) { Clear(); Create(filename, append); }
-		Writer(std::string filename, bool append = false) { Clear(); Create(filename.c_str(), append); }
 
 		/// <summary>
 		/// Makes the Writer object ready for use.