[Scummvm-cvs-logs] scummvm master -> b929699ad20dc06a379fedb3704a28aec0f82347

fingolfin max at quendi.de
Mon Apr 18 18:23:33 CEST 2011 

This automated email contains information about 8 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
73f04118f3 COMMON: Rename Error to ErrorCode, introduce new Error class
bac8fa70fd COMMON: Cleanup error messages
7ccba1fced COMMON: Fix typo
7c13aa48cd COMMON: Tweak extra text handling in Common::Error
cd2fcaf4ca COMMON: Remove kInvalidPathError
1037ed2470 COMMON: Clarify error naming conventions
3a574199b0 COMMON: Cleanup names/handling of some error codes
b929699ad2 SCUMM: Make use of new Common::Error type


Commit: 73f04118f3d03e25fa1139cfea2b1330f0098bd4
    https://github.com/scummvm/scummvm/commit/73f04118f3d03e25fa1139cfea2b1330f0098bd4
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:02-07:00

Commit Message:
COMMON: Rename Error to ErrorCode, introduce new Error class

Changed paths:
    backends/saves/default/default-saves.cpp
    base/main.cpp
    common/error.cpp
    common/error.h
    engines/agi/agi.h
    engines/agos/agos.h
    engines/kyra/kyra_v1.h
    engines/kyra/lol.cpp
    engines/kyra/saveload.cpp
    engines/lure/lure.h
    engines/parallaction/parallaction.h
    engines/parallaction/saveload.cpp
    engines/scumm/scumm.h
    engines/sky/sky.h
    engines/sword1/sword1.h
    engines/sword25/sword25.cpp
    engines/testbed/savegame.cpp
    engines/tinsel/tinsel.cpp
    engines/touche/menu.cpp
    engines/tsage/core.cpp
    gui/error.cpp
    gui/error.h



diff --git a/backends/saves/default/default-saves.cpp b/backends/saves/default/default-saves.cpp
index 0e16f16..75e10cf 100644
--- a/backends/saves/default/default-saves.cpp
+++ b/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
diff --git a/base/main.cpp b/base/main.cpp
index 65aa0ab..f45067f 100644
--- a/base/main.cpp
+++ b/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) {
-
-		// 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));
+	if (!engine || err.getCode() != Common::kNoError) {
 
+		// 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)
-		return res;
+	res = Base::processSettings(command, settings);
+	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
diff --git a/common/error.cpp b/common/error.cpp
index 6d1e349..d0e301b 100644
--- a/common/error.cpp
+++ b/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
  */
-
-struct ErrorMessage {
-	Error error;
-	const char *errMsg;
-};
-
-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") },
+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");
+
+	case kReadPermissionDenied:
+		return _s("Read permission denied");
+	case kWritePermissionDenied:
+		return _s("Write permission denied");
 
 	// The following three overlap a bit with kInvalidPathError and each other. Which to keep?
-	{ kPathDoesNotExist, _s("Path not exists") },
-	{ kPathNotDirectory, _s("Path not a directory") },
-	{ kPathNotFile, _s("Path not a file") },
-
-	{ kCreatingFileFailed, _s("Cannot create file") },
-	{ kReadingFailed, _s("Reading failed") },
-	{ kWritingFailed, _s("Writing data failed") },
-
-	{ kUnknownError, _s("Unknown Error") }
-};
-
-const char *errorToString(Error error) {
-
-	for (int i = 0; i < ARRAYSIZE(_errMsgTable); i++) {
-		if (error == _errMsgTable[i].error) {
-			return _errMsgTable[i].errMsg;
-		}
+	case kPathDoesNotExist:
+		return _s("Path not exists");
+	case kPathNotDirectory:
+		return _s("Path not a directory");
+	case kPathNotFile:
+		return _s("Path not a file");
+
+	case kCreatingFileFailed:
+		return _s("Cannot create file");
+	case kReadingFailed:
+		return _s("Reading failed");
+	case kWritingFailed:
+		return _s("Writing data failed");
+
+	case kUnknownError:
+	case kPluginNotFound:
+	case kPluginNotSupportSaves:
+	case kNoSavesError:
+	case kArgumentNotProcessed:
+	default:
+		return _s("Unknown Error");
 	}
+}
 
-	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
diff --git a/common/error.h b/common/error.h
index 5834311..9d74cca 100644
--- a/common/error.h
+++ b/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.
- *
- * @param error error code to be converted
- * @return a pointer to string description of the error
+ * An Error instance pairs an error code with string description providing more
+ * details about the error. For every error code, a default description is
+ * provided, but it is possible to optionally augment that description with
+ * 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
 
diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 1bee78e..aca0b32 100644
--- a/engines/agi/agi.h
+++ b/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();
 	}
diff --git a/engines/agos/agos.h b/engines/agos/agos.h
index 1d9602f..735920e 100644
--- a/engines/agos/agos.h
+++ b/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();
 	}
diff --git a/engines/kyra/kyra_v1.h b/engines/kyra/kyra_v1.h
index cf51774..801d31d 100644
--- a/engines/kyra/kyra_v1.h
+++ b/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();
 	}
diff --git a/engines/kyra/lol.cpp b/engines/kyra/lol.cpp
index 5928c40..78c9ed2 100644
--- a/engines/kyra/lol.cpp
+++ b/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;
 		}
diff --git a/engines/kyra/saveload.cpp b/engines/kyra/saveload.cpp
index 44579c3..d14001a 100644
--- a/engines/kyra/saveload.cpp
+++ b/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;
diff --git a/engines/lure/lure.h b/engines/lure/lure.h
index 99e9e3d..52b785a 100644
--- a/engines/lure/lure.h
+++ b/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();
 	}
diff --git a/engines/parallaction/parallaction.h b/engines/parallaction/parallaction.h
index a8a57ed..c5b6b23 100644
--- a/engines/parallaction/parallaction.h
+++ b/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();
 	}
diff --git a/engines/parallaction/saveload.cpp b/engines/parallaction/saveload.cpp
index b8116d6..50a777f 100644
--- a/engines/parallaction/saveload.cpp
+++ b/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());
 		}
 	}
diff --git a/engines/scumm/scumm.h b/engines/scumm/scumm.h
index f3af84b..266a2c4 100644
--- a/engines/scumm/scumm.h
+++ b/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();
 	}
diff --git a/engines/sky/sky.h b/engines/sky/sky.h
index d8ced1e..378bba7 100644
--- a/engines/sky/sky.h
+++ b/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();
 	}
diff --git a/engines/sword1/sword1.h b/engines/sword1/sword1.h
index 592d2da..255299d 100644
--- a/engines/sword1/sword1.h
+++ b/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();
 	}
diff --git a/engines/sword25/sword25.cpp b/engines/sword25/sword25.cpp
index aac21f4..8740e44 100644
--- a/engines/sword25/sword25.cpp
+++ b/engines/sword25/sword25.cpp
@@ -73,10 +73,10 @@ Sword25Engine::~Sword25Engine() {
 
 Common::Error Sword25Engine::run() {
 	// Engine initialisation
-	Common::Error errorCode = appStart();
-	if (errorCode != Common::kNoError) {
+	Common::Error error = appStart();
+	if (error.getCode() != Common::kNoError) {
 		appEnd();
-		return errorCode;
+		return error;
 	}
 
 	// Run the game
diff --git a/engines/testbed/savegame.cpp b/engines/testbed/savegame.cpp
index b91d9fc..0ffd367 100644
--- a/engines/testbed/savegame.cpp
+++ b/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;
diff --git a/engines/tinsel/tinsel.cpp b/engines/tinsel/tinsel.cpp
index e1396f9..d78175d 100644
--- a/engines/tinsel/tinsel.cpp
+++ b/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;
 	}
 
diff --git a/engines/touche/menu.cpp b/engines/touche/menu.cpp
index 52967c2..eb10c61 100644
--- a/engines/touche/menu.cpp
+++ b/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;
 				}
 			}
diff --git a/engines/tsage/core.cpp b/engines/tsage/core.cpp
index b3b1693..2d0a69f 100644
--- a/engines/tsage/core.cpp
+++ b/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) {
diff --git a/gui/error.cpp b/gui/error.cpp
index 3332eb5..f6da795 100644
--- a/gui/error.cpp
+++ b/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();
 }
diff --git a/gui/error.h b/gui/error.h
index a55f555..f048a0c 100644
--- a/gui/error.h
+++ b/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.


Commit: bac8fa70fdd4c32d06288b3579ee30ba40e77ece
    https://github.com/scummvm/scummvm/commit/bac8fa70fdd4c32d06288b3579ee30ba40e77ece
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:03-07:00

Commit Message:
COMMON: Cleanup error messages

Changed paths:
    common/error.cpp



diff --git a/common/error.cpp b/common/error.cpp
index d0e301b..68702d4 100644
--- a/common/error.cpp
+++ b/common/error.cpp
@@ -41,13 +41,13 @@ static String errorToString(ErrorCode errorCode) {
 	case kNoError:
 		return _s("No error");
 	case kInvalidPathError:
-		return _s("Invalid Path");
+		return _s("Invalid path");
 	case kNoGameDataFoundError:
-		return _s("Game Data not found");
+		return _s("Game data not found");
 	case kUnsupportedGameidError:
-		return _s("Game Id not supported");
+		return _s("Game id not supported");
 	case kUnsupportedColorMode:
-		return _s("Unsupported Color Mode");
+		return _s("Unsupported color mode");
 
 	case kReadPermissionDenied:
 		return _s("Read permission denied");
@@ -56,7 +56,7 @@ static String errorToString(ErrorCode errorCode) {
 
 	// The following three overlap a bit with kInvalidPathError and each other. Which to keep?
 	case kPathDoesNotExist:
-		return _s("Path not exists");
+		return _s("Path does not exist");
 	case kPathNotDirectory:
 		return _s("Path not a directory");
 	case kPathNotFile:
@@ -65,7 +65,7 @@ static String errorToString(ErrorCode errorCode) {
 	case kCreatingFileFailed:
 		return _s("Cannot create file");
 	case kReadingFailed:
-		return _s("Reading failed");
+		return _s("Reading data failed");
 	case kWritingFailed:
 		return _s("Writing data failed");
 
@@ -75,7 +75,7 @@ static String errorToString(ErrorCode errorCode) {
 	case kNoSavesError:
 	case kArgumentNotProcessed:
 	default:
-		return _s("Unknown Error");
+		return _s("Unknown error");
 	}
 }
 


Commit: 7ccba1fced2f35a49b396ec58081e5bf084b6cab
    https://github.com/scummvm/scummvm/commit/7ccba1fced2f35a49b396ec58081e5bf084b6cab
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:03-07:00

Commit Message:
COMMON: Fix typo

Changed paths:
    common/error.h



diff --git a/common/error.h b/common/error.h
index 9d74cca..86d891b 100644
--- a/common/error.h
+++ b/common/error.h
@@ -66,7 +66,7 @@ enum ErrorCode {
 	kWritingFailed,				///< Failure to write data -- disk full?
 
 	// The following are used by --list-saves
-	kPluginNotFound,			///< Failed to find plugin to handle tager
+	kPluginNotFound,			///< Failed to find plugin to handle target
 	kPluginNotSupportSaves,		///< Failed if plugin does not support saves
 	kNoSavesError,				///< There are no saves to show
 


Commit: 7c13aa48cd3f103564ac1807f1152d94f1863979
    https://github.com/scummvm/scummvm/commit/7c13aa48cd3f103564ac1807f1152d94f1863979
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:03-07:00

Commit Message:
COMMON: Tweak extra text handling in Common::Error

Changed paths:
    common/error.cpp
    common/error.h



diff --git a/common/error.cpp b/common/error.cpp
index 68702d4..eac8ba1 100644
--- a/common/error.cpp
+++ b/common/error.cpp
@@ -84,7 +84,7 @@ Error::Error(ErrorCode code)
 }
 
 Error::Error(ErrorCode code, const String &desc)
-	: _code(code), _desc(errorToString(code) + ": " + desc) {
+	: _code(code), _desc(errorToString(code) + " (" + desc + ")") {
 }
 
 
diff --git a/common/error.h b/common/error.h
index 86d891b..dcbe670 100644
--- a/common/error.h
+++ b/common/error.h
@@ -93,8 +93,8 @@ public:
 
 	/**
 	 * 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 message. Specifically, the provided extra text is suitably
+	 * appended to the default message.
 	 */
 	Error(ErrorCode code, const String &extra);
 


Commit: cd2fcaf4ca338400ed8ee44566b9a767287efd69
    https://github.com/scummvm/scummvm/commit/cd2fcaf4ca338400ed8ee44566b9a767287efd69
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:03-07:00

Commit Message:
COMMON: Remove kInvalidPathError

Changed paths:
    base/main.cpp
    common/error.cpp
    common/error.h
    engines/scumm/detection.cpp



diff --git a/base/main.cpp b/base/main.cpp
index f45067f..4dfed0f 100644
--- a/base/main.cpp
+++ b/base/main.cpp
@@ -131,7 +131,7 @@ static Common::Error runGame(const EnginePlugin *plugin, OSystem &system, const
 
 	// Verify that the game path refers to an actual directory
 	if (!(dir.exists() && dir.isDirectory()))
-		err = Common::kInvalidPathError;
+		err = Common::kPathNotDirectory;
 
 	// Create the game engine
 	if (err.getCode() == Common::kNoError)
diff --git a/common/error.cpp b/common/error.cpp
index eac8ba1..86557ee 100644
--- a/common/error.cpp
+++ b/common/error.cpp
@@ -40,8 +40,6 @@ 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:
@@ -54,7 +52,6 @@ static String errorToString(ErrorCode errorCode) {
 	case kWritePermissionDenied:
 		return _s("Write permission denied");
 
-	// The following three overlap a bit with kInvalidPathError and each other. Which to keep?
 	case kPathDoesNotExist:
 		return _s("Path does not exist");
 	case kPathNotDirectory:
diff --git a/common/error.h b/common/error.h
index dcbe670..f69ada9 100644
--- a/common/error.h
+++ b/common/error.h
@@ -47,7 +47,6 @@ namespace Common {
  */
 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
 	kUnsupportedGameidError,	///< Engine initialization: Gameid not supported by this (Meta)Engine
 	kUnsupportedColorMode,		///< Engine initialization: Engine does not support backend's color mode
@@ -56,7 +55,6 @@ enum ErrorCode {
 	kReadPermissionDenied,		///< Unable to read data due to missing read permission
 	kWritePermissionDenied,		///< Unable to write data due to missing write permission
 
-	// The following three overlap a bit with kInvalidPathError and each other. Which to keep?
 	kPathDoesNotExist,			///< The specified path does not exist
 	kPathNotDirectory,			///< The specified path does not point to a directory
 	kPathNotFile,				///< The specified path does not point to a file
diff --git a/engines/scumm/detection.cpp b/engines/scumm/detection.cpp
index 87ec7b8..6db3ea7 100644
--- a/engines/scumm/detection.cpp
+++ b/engines/scumm/detection.cpp
@@ -994,7 +994,7 @@ Common::Error ScummMetaEngine::createInstance(OSystem *syst, Engine **engine) co
 	Common::FSList fslist;
 	Common::FSNode dir(ConfMan.get("path"));
 	if (!dir.isDirectory())
-		return Common::kInvalidPathError;
+		return Common::kPathNotDirectory;
 	if (!dir.getChildren(fslist, Common::FSNode::kListFilesOnly))
 		return Common::kNoGameDataFoundError;
 


Commit: 1037ed247042485df1ad312d9ddfbd17251ed907
    https://github.com/scummvm/scummvm/commit/1037ed247042485df1ad312d9ddfbd17251ed907
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:04-07:00

Commit Message:
COMMON: Clarify error naming conventions

Not all error codes comply with these yet.

Changed paths:
    common/error.h



diff --git a/common/error.h b/common/error.h
index f69ada9..5c19761 100644
--- a/common/error.h
+++ b/common/error.h
@@ -39,11 +39,10 @@ namespace Common {
 /**
  * Error codes which may be reported by plugins under various circumstances.
  *
- * @todo Clarify the names; add more codes, resp. verify all existing ones are acutally useful.
- *       Also, try to avoid overlap.
- * @todo Maybe introduce a naming convention? E.g. k-NOUN/ACTION-CONDITION-Error, so
- *       kPathInvalidError would be correct, but these would not be: kInvalidPath,
- *       kPathInvalid, kPathIsInvalid, kInvalidPathError
+ * @note Error names should follow the pattern k-NOUN/ACTION-CONDITION-Error.
+ *       So kPathInvalidError would be correct, but these would not be:
+ *       kInvalidPath, kPathInvalid, kPathIsInvalid, kInvalidPathError.
+ * @todo Adjust all error codes to comply with these conventions.
  */
 enum ErrorCode {
 	kNoError = 0,				///< No error occurred
@@ -69,6 +68,7 @@ enum ErrorCode {
 	kNoSavesError,				///< There are no saves to show
 
 	kArgumentNotProcessed,		///< Used in command line parsing
+
 	kUnknownError				///< Catch-all error, used if no other error code matches
 };
 


Commit: 3a574199b0b3e848d786d71a50a1107536507479
    https://github.com/scummvm/scummvm/commit/3a574199b0b3e848d786d71a50a1107536507479
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:04-07:00

Commit Message:
COMMON: Cleanup names/handling of some error codes

Changed paths:
    base/commandLine.cpp
    base/main.cpp
    common/error.cpp
    common/error.h



diff --git a/base/commandLine.cpp b/base/commandLine.cpp
index f920dd0..7c482d3 100644
--- a/base/commandLine.cpp
+++ b/base/commandLine.cpp
@@ -641,29 +641,30 @@ static Common::Error listSaves(const char *target) {
 	GameDescriptor game = EngineMan.findGame(gameid, &plugin);
 
 	if (!plugin) {
-		warning("Could not find any plugin to handle target '%s' (gameid '%s')", target, gameid.c_str());
-		return Common::kPluginNotFound;
+		return Common::Error(Common::kEnginePluginNotFound,
+						Common::String::format("target '%s', gameid '%s", target, gameid.c_str()));
 	}
 
 	if (!(*plugin)->hasFeature(MetaEngine::kSupportsListSaves)) {
 		// TODO: Include more info about the target (desc, engine name, ...) ???
-		printf("ScummVM does not support listing save states for target '%s' (gameid '%s') .\n", target, gameid.c_str());
-		result = Common::kPluginNotSupportSaves;
+		return Common::Error(Common::kEnginePluginNotSupportSaves,
+						Common::String::format("target '%s', gameid '%s", target, gameid.c_str()));
 	} else {
 		// Query the plugin for a list of savegames
 		SaveStateList saveList = (*plugin)->listSaves(target);
 
-		// TODO: Include more info about the target (desc, engine name, ...) ???
-		printf("Saves for target '%s' (gameid '%s'):\n", target, gameid.c_str());
-		printf("  Slot Description                                           \n"
-		       "  ---- ------------------------------------------------------\n");
-
-		if (saveList.size() == 0)
-			result = Common::kNoSavesError;
+		if (saveList.size() > 0) {
+			// TODO: Include more info about the target (desc, engine name, ...) ???
+			printf("Save states for target '%s' (gameid '%s'):\n", target, gameid.c_str());
+			printf("  Slot Description                                           \n"
+				   "  ---- ------------------------------------------------------\n");
 
-		for (SaveStateList::const_iterator x = saveList.begin(); x != saveList.end(); ++x) {
-			printf("  %-4s %s\n", x->save_slot().c_str(), x->description().c_str());
-			// TODO: Could also iterate over the full hashmap, printing all key-value pairs
+			for (SaveStateList::const_iterator x = saveList.begin(); x != saveList.end(); ++x) {
+				printf("  %-4s %s\n", x->save_slot().c_str(), x->description().c_str());
+				// TODO: Could also iterate over the full hashmap, printing all key-value pairs
+			}
+		} else {
+			printf("There are no save states for target '%s' (gameid '%s'):\n", target, gameid.c_str());
 		}
 	}
 
diff --git a/base/main.cpp b/base/main.cpp
index 4dfed0f..1335903 100644
--- a/base/main.cpp
+++ b/base/main.cpp
@@ -111,13 +111,12 @@ static const EnginePlugin *detectPlugin() {
 	if (plugin == 0) {
 		printf("failed\n");
 		warning("%s is an invalid gameid. Use the --list-games option to list supported gameid", gameid.c_str());
-		return 0;
 	} else {
 		printf("%s\n", plugin->getName());
-	}
 
-	// FIXME: Do we really need this one?
-	printf("  Starting '%s'\n", game.description().c_str());
+		// FIXME: Do we really need this one?
+		printf("  Starting '%s'\n", game.description().c_str());
+	}
 
 	return plugin;
 }
@@ -352,8 +351,10 @@ extern "C" int scummvm_main(int argc, const char * const argv[]) {
 
 	// TODO: deal with settings that require plugins to be loaded
 	res = Base::processSettings(command, settings);
-	if (res.getCode() != Common::kArgumentNotProcessed)
+	if (res.getCode() != Common::kArgumentNotProcessed) {
+		warning("%s", res.getDesc().c_str());
 		return res.getCode();
+	}
 
 	// Init the backend. Must take place after all config data (including
 	// the command line params) was read.
diff --git a/common/error.cpp b/common/error.cpp
index 86557ee..8fa58e2 100644
--- a/common/error.cpp
+++ b/common/error.cpp
@@ -66,11 +66,15 @@ static String errorToString(ErrorCode errorCode) {
 	case kWritingFailed:
 		return _s("Writing data failed");
 
-	case kUnknownError:
-	case kPluginNotFound:
-	case kPluginNotSupportSaves:
-	case kNoSavesError:
+	case kEnginePluginNotFound:
+		return _s("Could not find suitable engine plugin");
+	case kEnginePluginNotSupportSaves:
+		return _s("Engine plugin does not support save states");
+
 	case kArgumentNotProcessed:
+		return _s("Command line argument not processed");
+
+	case kUnknownError:
 	default:
 		return _s("Unknown error");
 	}
diff --git a/common/error.h b/common/error.h
index 5c19761..1ffbba7 100644
--- a/common/error.h
+++ b/common/error.h
@@ -63,9 +63,8 @@ enum ErrorCode {
 	kWritingFailed,				///< Failure to write data -- disk full?
 
 	// The following are used by --list-saves
-	kPluginNotFound,			///< Failed to find plugin to handle target
-	kPluginNotSupportSaves,		///< Failed if plugin does not support saves
-	kNoSavesError,				///< There are no saves to show
+	kEnginePluginNotFound,		///< Failed to find plugin to handle target
+	kEnginePluginNotSupportSaves,	///< Failed if plugin does not support listing save states
 
 	kArgumentNotProcessed,		///< Used in command line parsing
 


Commit: b929699ad20dc06a379fedb3704a28aec0f82347
    https://github.com/scummvm/scummvm/commit/b929699ad20dc06a379fedb3704a28aec0f82347
Author: Max Horn (max at quendi.de)
Date: 2011-04-18T09:22:04-07:00

Commit Message:
SCUMM: Make use of new Common::Error type

Changed paths:
    engines/scumm/scumm.cpp



diff --git a/engines/scumm/scumm.cpp b/engines/scumm/scumm.cpp
index 5aea36e..67c5356 100644
--- a/engines/scumm/scumm.cpp
+++ b/engines/scumm/scumm.cpp
@@ -1162,7 +1162,7 @@ Common::Error ScummEngine::init() {
 				warning("Starting game without the required 16bit color support.\nYou may experience color glitches");
 				initGraphics(screenWidth, screenHeight, (screenWidth > 320));
 			} else {
-				error("16bit color support is required for this game");
+				return Common::Error(Common::kUnsupportedColorMode, "16bit color support is required for this game");
 			}
 #endif
 		} else {

More information about the Scummvm-git-logs mailing list