COMMON: Rename Error to ErrorCode, introduce new Error class

backends/saves/default/default-saves.cpp

@@ -60,7 +60,7 @@ void DefaultSaveFileManager::checkPath(const Common::FSNode &dir) {
 Common::StringArray DefaultSaveFileManager::listSavefiles(const Common::String &pattern) {
 	Common::String savePathName = getSavePath();
 	checkPath(Common::FSNode(savePathName));
-	if (getError() != Common::kNoError)
+	if (getError().getCode() != Common::kNoError)
 		return Common::StringArray();
 
 	// recreate FSNode since checkPath may have changed/created the directory
@@ -84,7 +84,7 @@ Common::InSaveFile *DefaultSaveFileManager::openForLoading(const Common::String
 	// Ensure that the savepath is valid. If not, generate an appropriate error.
 	Common::String savePathName = getSavePath();
 	checkPath(Common::FSNode(savePathName));
-	if (getError() != Common::kNoError)
+	if (getError().getCode() != Common::kNoError)
 		return 0;
 
 	// recreate FSNode since checkPath may have changed/created the directory
@@ -104,7 +104,7 @@ Common::OutSaveFile *DefaultSaveFileManager::openForSaving(const Common::String
 	// Ensure that the savepath is valid. If not, generate an appropriate error.
 	Common::String savePathName = getSavePath();
 	checkPath(Common::FSNode(savePathName));
-	if (getError() != Common::kNoError)
+	if (getError().getCode() != Common::kNoError)
 		return 0;
 
 	// recreate FSNode since checkPath may have changed/created the directory
@@ -121,7 +121,7 @@ Common::OutSaveFile *DefaultSaveFileManager::openForSaving(const Common::String
 bool DefaultSaveFileManager::removeSavefile(const Common::String &filename) {
 	Common::String savePathName = getSavePath();
 	checkPath(Common::FSNode(savePathName));
-	if (getError() != Common::kNoError)
+	if (getError().getCode() != Common::kNoError)
 		return false;
 
 	// recreate FSNode since checkPath may have changed/created the directory

base/main.cpp

@@ -134,21 +134,17 @@ static Common::Error runGame(const EnginePlugin *plugin, OSystem &system, const
 		err = Common::kInvalidPathError;
 
 	// Create the game engine
-	if (err == Common::kNoError)
+	if (err.getCode() == Common::kNoError)
 		err = (*plugin)->createInstance(&system, &engine);
 
 	// Check for errors
-	if (!engine || err != Common::kNoError) {
+	if (!engine || err.getCode() != Common::kNoError) {
-
-		// TODO: An errorDialog for this and engine related errors is displayed already in the scummvm_main function
-		// Is a separate dialog here still required?
-
-		//GUI::displayErrorDialog("ScummVM could not find any game in the specified directory!");
-		const char *errMsg = _(Common::errorToString(err));
 
+		// Print a warning; note that scummvm_main will also
+		// display an error dialog, so we don't have to do this here.
 		warning("%s failed to instantiate engine: %s (target '%s', path '%s')",
 			plugin->getName(),
-			errMsg,
+			err.getDesc().c_str(),
 			ConfMan.getActiveDomainName().c_str(),
 			dir.getPath().c_str()
 			);
@@ -355,8 +351,9 @@ extern "C" int scummvm_main(int argc, const char * const argv[]) {
 	Common::Error res;
 
 	// TODO: deal with settings that require plugins to be loaded
-	if ((res = Base::processSettings(command, settings)) != Common::kArgumentNotProcessed)
+	res = Base::processSettings(command, settings);
-		return res;
+	if (res.getCode() != Common::kArgumentNotProcessed)
+		return res.getCode();
 
 	// Init the backend. Must take place after all config data (including
 	// the command line params) was read.
@@ -430,14 +427,14 @@ extern "C" int scummvm_main(int argc, const char * const argv[]) {
 		#endif	
 			
 			// Did an error occur ?
-			if (result != Common::kNoError) {
+			if (result.getCode() != Common::kNoError) {
 				// Shows an informative error dialog if starting the selected game failed.
 				GUI::displayErrorDialog(result, _("Error running game:"));
 			}
 
 			// Quit unless an error occurred, or Return to launcher was requested
 			#ifndef FORCE_RTL
-			if (result == 0 && !g_system->getEventManager()->shouldRTL())
+			if (result.getCode() == Common::kNoError && !g_system->getEventManager()->shouldRTL())
 				break;
 			#endif
 			// Reset RTL flag in case we want to load another engine

common/error.cpp

@@ -31,44 +31,61 @@
 namespace Common {
 
 /**
- * Error Table: Maps error codes to their default descriptions
+ * Maps an error code to equivalent string description.
+ *
+ * @param errorCode error code to be converted
+ * @return a pointer to string description of the error
  */
+static String errorToString(ErrorCode errorCode) {
+	switch (errorCode) {
+	case kNoError:
+		return _s("No error");
+	case kInvalidPathError:
+		return _s("Invalid Path");
+	case kNoGameDataFoundError:
+		return _s("Game Data not found");
+	case kUnsupportedGameidError:
+		return _s("Game Id not supported");
+	case kUnsupportedColorMode:
+		return _s("Unsupported Color Mode");
 
-struct ErrorMessage {
+	case kReadPermissionDenied:
-	Error error;
+		return _s("Read permission denied");
-	const char *errMsg;
+	case kWritePermissionDenied:
-};
+		return _s("Write permission denied");
-
-static const ErrorMessage _errMsgTable[] = {
-	{ kInvalidPathError, _s("Invalid Path") },
-	{ kNoGameDataFoundError, _s("Game Data not found") },
-	{ kUnsupportedGameidError, _s("Game Id not supported") },
-	{ kUnsupportedColorMode, _s("Unsupported Color Mode") },
-
-	{ kReadPermissionDenied, _s("Read permission denied") },
-	{ kWritePermissionDenied, _s("Write permission denied") },
 
 	// The following three overlap a bit with kInvalidPathError and each other. Which to keep?
-	{ kPathDoesNotExist, _s("Path not exists") },
+	case kPathDoesNotExist:
-	{ kPathNotDirectory, _s("Path not a directory") },
+		return _s("Path not exists");
-	{ kPathNotFile, _s("Path not a file") },
+	case kPathNotDirectory:
+		return _s("Path not a directory");
+	case kPathNotFile:
+		return _s("Path not a file");
 
-	{ kCreatingFileFailed, _s("Cannot create file") },
+	case kCreatingFileFailed:
-	{ kReadingFailed, _s("Reading failed") },
+		return _s("Cannot create file");
-	{ kWritingFailed, _s("Writing data failed") },
+	case kReadingFailed:
+		return _s("Reading failed");
+	case kWritingFailed:
+		return _s("Writing data failed");
 
-	{ kUnknownError, _s("Unknown Error") }
+	case kUnknownError:
-};
+	case kPluginNotFound:
-
+	case kPluginNotSupportSaves:
-const char *errorToString(Error error) {
+	case kNoSavesError:
-
+	case kArgumentNotProcessed:
-	for (int i = 0; i < ARRAYSIZE(_errMsgTable); i++) {
+	default:
-		if (error == _errMsgTable[i].error) {
+		return _s("Unknown Error");
-			return _errMsgTable[i].errMsg;
 	}
-	}
-
-	return _("Unknown Error");
 }
 
+Error::Error(ErrorCode code)
+	: _code(code), _desc(errorToString(code)) {
+}
+
+Error::Error(ErrorCode code, const String &desc)
+	: _code(code), _desc(errorToString(code) + ": " + desc) {
+}
+
+
 } // End of namespace Common

common/error.h

@@ -26,6 +26,8 @@
 #ifndef COMMON_ERROR_H
 #define COMMON_ERROR_H
 
+#include "common/str.h"
+
 namespace Common {
 
 /**
@@ -43,7 +45,7 @@ namespace Common {
  *       kPathInvalidError would be correct, but these would not be: kInvalidPath,
  *       kPathInvalid, kPathIsInvalid, kInvalidPathError
  */
-enum Error {
+enum ErrorCode {
 	kNoError = 0,				///< No error occurred
 	kInvalidPathError,			///< Engine initialization: Invalid game path was passed
 	kNoGameDataFoundError,		///< Engine initialization: No game data was found in the specified location
@@ -73,12 +75,39 @@ enum Error {
 };
 
 /**
- * Maps an error code to equivalent string description.
+ * An Error instance pairs an error code with string description providing more
- *
+ * details about the error. For every error code, a default description is
- * @param error error code to be converted
+ * provided, but it is possible to optionally augment that description with
- * @return a pointer to string description of the error
+ * extra information when creating a new Error instance.
  */
-const char *errorToString(Error error);
+class Error {
+protected:
+	ErrorCode _code;
+	String _desc;
+public:
+	/**
+	 * Construct a new Error with the specified error code and the default
+	 * error message.
+	 */
+	Error(ErrorCode code = kUnknownError);
+
+	/**
+	 * Construct a new Error with the specified error code and an augmented
+	 * error message. Specifically, the provided extra text is appended
+	 * to the default message, with ": " inserted in between.
+	 */
+	Error(ErrorCode code, const String &extra);
+
+	/**
+	 * Get the description of this error.
+	 */
+	const String &getDesc() const { return _desc; }
+
+	/**
+	 * Get the error code of this error.
+	 */
+	ErrorCode getCode() const { return _code; }
+};
 
 } // End of namespace Common
 

engines/agi/agi.h

@@ -727,7 +727,7 @@ protected:
 	virtual Common::Error run() {
 		Common::Error err;
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/agos/agos.h

@@ -186,7 +186,7 @@ class AGOSEngine : public Engine {
 	virtual Common::Error run() {
 		Common::Error err;
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/kyra/kyra_v1.h

@@ -247,7 +247,7 @@ protected:
 		Common::Error err;
 		registerDefaultSettings();
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/kyra/lol.cpp

@@ -591,7 +591,9 @@ Common::Error LoLEngine::go() {
 	if (action == 0) {
 		startupNew();
 	} else if (_gameToLoad != -1) {
-		if (loadGameState(_gameToLoad) != Common::kNoError)
+		// FIXME: Instead of throwing away the error returned by
+		// loadGameState, we should use it / augment it.
+		if (loadGameState(_gameToLoad).getCode() != Common::kNoError)
 			error("Couldn't load game slot %d on startup", _gameToLoad);
 		_gameToLoad = -1;
 	}
@@ -918,7 +920,9 @@ void LoLEngine::runLoop() {
 
 	while (!shouldQuit() && _runFlag) {
 		if (_gameToLoad != -1) {
-			if (loadGameState(_gameToLoad) != Common::kNoError)
+			// FIXME: Instead of throwing away the error returned by
+			// loadGameState, we should use it / augment it.
+			if (loadGameState(_gameToLoad).getCode() != Common::kNoError)
 				error("Couldn't load game slot %d", _gameToLoad);
 			_gameToLoad = -1;
 		}

engines/kyra/saveload.cpp

@@ -257,7 +257,9 @@ void KyraEngine_v1::checkAutosave() {
 }
 
 void KyraEngine_v1::loadGameStateCheck(int slot) {
-	if (loadGameState(slot) != Common::kNoError) {
+	// FIXME: Instead of throwing away the error returned by
+	// loadGameState, we should use it / augment it.
+	if (loadGameState(slot).getCode() != Common::kNoError) {
 		const char *filename = getSavegameFilename(slot);
 		Common::String errorMessage = "Could not load savegame: '";
 		errorMessage += filename;

engines/lure/lure.h

@@ -98,7 +98,7 @@ public:
 	virtual Common::Error run() {
 		Common::Error err;
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/parallaction/parallaction.h

@@ -268,7 +268,7 @@ public:
 	virtual Common::Error run() {
 		Common::Error err;
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/parallaction/saveload.cpp

@@ -313,7 +313,7 @@ void SaveLoad_ns::renameOldSavefiles() {
 		if (_saveFileMan->renameSavefile(oldName, newName)) {
 			success++;
 		} else {
-			warning("Error %i (%s) occurred while renaming %s to %s", _saveFileMan->getError(),
+			warning("Error %i (%s) occurred while renaming %s to %s", _saveFileMan->getError().getCode(),
 				_saveFileMan->getErrorDesc().c_str(), oldName.c_str(), newName.c_str());
 		}
 	}

engines/scumm/scumm.h

@@ -463,7 +463,7 @@ public:
 	virtual Common::Error run() {
 		Common::Error err;
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/sky/sky.h

@@ -106,7 +106,7 @@ protected:
 	virtual Common::Error run() {
 		Common::Error err;
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/sword1/sword1.h

@@ -100,7 +100,7 @@ protected:
 	virtual Common::Error run() {
 		Common::Error err;
 		err = init();
-		if (err != Common::kNoError)
+		if (err.getCode() != Common::kNoError)
 			return err;
 		return go();
 	}

engines/sword25/sword25.cpp

@@ -73,10 +73,10 @@ Sword25Engine::~Sword25Engine() {
 
 Common::Error Sword25Engine::run() {
 	// Engine initialisation
-	Common::Error errorCode = appStart();
+	Common::Error error = appStart();
-	if (errorCode != Common::kNoError) {
+	if (error.getCode() != Common::kNoError) {
 		appEnd();
-		return errorCode;
+		return error;
 	}
 
 	// Run the game

engines/testbed/savegame.cpp

@@ -138,9 +138,9 @@ TestExitStatus SaveGametests::testListingSavefile() {
 
 	Common::Error error = saveFileMan->getError();
 
-	if (error != Common::kNoError) {
+	if (error.getCode() != Common::kNoError) {
 		// Abort. Some Error in writing files
-		Testsuite::logDetailedPrintf("Error while creating savefiles: %s\n", Common::errorToString(error));
+		Testsuite::logDetailedPrintf("Error while creating savefiles: %s\n", error.getDesc().c_str());
 		return kTestFailed;
 	}
 
@@ -177,7 +177,7 @@ TestExitStatus SaveGametests::testErrorMessages() {
 	readAndVerifyData("tBedSomeNonExistentSaveFile.0", "File doesn't exists!");
 
 	Common::Error error = saveFileMan->getError();
-	if (error == Common::kNoError) {
+	if (error.getCode() == Common::kNoError) {
 		// blunder! how come?
 		Testsuite::logDetailedPrintf("SaveFileMan.getError() failed\n");
 		return kTestFailed;

engines/tinsel/tinsel.cpp

@@ -972,7 +972,7 @@ Common::Error TinselEngine::run() {
 	// errors when loading the save state.
 
 	if (ConfMan.hasKey("save_slot")) {
-		if (loadGameState(ConfMan.getInt("save_slot")) == Common::kNoError)
+		if (loadGameState(ConfMan.getInt("save_slot")).getCode() == Common::kNoError)
 			loadingFromGMM = true;
 	}
 

engines/touche/menu.cpp

@@ -331,14 +331,14 @@ void ToucheEngine::handleMenuAction(void *menu, int actionId) {
 		break;
 	case kActionPerformSaveLoad:
 		if (menuData->mode == kMenuLoadStateMode) {
-			if (loadGameState(_saveLoadCurrentSlot) == Common::kNoError) {
+			if (loadGameState(_saveLoadCurrentSlot).getCode() == Common::kNoError) {
 				menuData->quit = true;
 			}
 		} else if (menuData->mode == kMenuSaveStateMode) {
 			_system->setFeatureState(OSystem::kFeatureVirtualKeyboard, false);
 			const char *description = menuData->saveLoadDescriptionsTable[_saveLoadCurrentSlot];
 			if (strlen(description) > 0) {
-				if (saveGameState(_saveLoadCurrentSlot, description) == Common::kNoError) {
+				if (saveGameState(_saveLoadCurrentSlot, description).getCode() == Common::kNoError) {
 					menuData->quit = true;
 				}
 			}

engines/tsage/core.cpp

@@ -3547,7 +3547,10 @@ void SceneHandler::dispatch() {
 	if (_saveGameSlot != -1) {
 		int saveSlot = _saveGameSlot;
 		_saveGameSlot = -1;
-		if (_saver->save(saveSlot, _saveName) != Common::kNoError)
+		Common::Error err = _saver->save(saveSlot, _saveName);
+		// FIXME: Make use of the description string in err to enhance
+		// the error reported to the user.
+		if (err.getCode() != Common::kNoError)
 			GUIErrorMessage(SAVE_ERROR_MSG);
 	}
 	if (_loadGameSlot != -1) {

gui/error.cpp

@@ -36,10 +36,10 @@ void displayErrorDialog(const char *text) {
 	alert.runModal();
 }
 
-void displayErrorDialog(Common::Error error, const char *extraText) {
+void displayErrorDialog(const Common::Error &error, const char *extraText) {
 	Common::String errorText(extraText);
 	errorText += " ";
-	errorText += _(Common::errorToString(error));
+	errorText += _(error.getDesc());
 	GUI::MessageDialog alert(errorText);
 	alert.runModal();
 }

gui/error.h

@@ -36,7 +36,7 @@ namespace GUI {
  * @param error error code
  * @param extraText extra text to be displayed in addition to default string description(optional)
  */
-void displayErrorDialog(Common::Error error, const char *extraText = "");
+void displayErrorDialog(const Common::Error &error, const char *extraText = "");
 
 /**
  * Displays an error dialog for a given message.