From cabcff881cf054d62dfeec23eff140063113440b Mon Sep 17 00:00:00 2001 From: dfighter1985 Date: Sun, 13 Jul 2014 21:38:14 +0200 Subject: [PATCH] Trying to load an invalid world editor file shouldn't crash the application, and an error messagebox should inform the user. --- .../world_editor/world_editor_misc.cpp | 95 +++++++++++-------- .../plugins/world_editor/world_editor_misc.h | 2 + .../world_editor/world_editor_window.cpp | 7 +- 3 files changed, 66 insertions(+), 38 deletions(-) diff --git a/code/studio/src/plugins/world_editor/world_editor_misc.cpp b/code/studio/src/plugins/world_editor/world_editor_misc.cpp index 7401f49fb..5d6b7f94e 100644 --- a/code/studio/src/plugins/world_editor/world_editor_misc.cpp +++ b/code/studio/src/plugins/world_editor/world_editor_misc.cpp @@ -34,35 +34,11 @@ namespace WorldEditor namespace Utils { -void syntaxError(const char *filename, xmlNodePtr xmlNode, const char *format, ...) +std::string lastError; + +std::string getLastError() { - char buffer[1024]; - - if (format) - { - va_list args; - va_start( args, format ); - sint ret = vsnprintf( buffer, 1024, format, args ); - va_end( args ); - } - else - { - strcpy(buffer, "Unknown error"); - } - - nlerror("(%s), node (%s), line (%s) :\n%s", filename, xmlNode->name, xmlNode->content, buffer); -} - -bool getPropertyString(std::string &result, const char *filename, xmlNodePtr xmlNode, const char *propName) -{ - // Call the CIXml version - if (!NLMISC::CIXml::getPropertyString(result, xmlNode, propName)) - { - // Output a formated error - syntaxError(filename, xmlNode, "Missing XML node property (%s)", propName); - return false; - } - return true; + return lastError; } uint32 getUniqueId() @@ -81,6 +57,8 @@ bool loadWorldEditFile(const std::string &fileName, WorldEditList &worldEditList { bool result = false; + lastError = ""; + // Load the document NLMISC::CIFile file; if (file.open(fileName)) @@ -110,20 +88,31 @@ bool loadWorldEditFile(const std::string &fileName, WorldEditList &worldEditList } if (version == -1) - syntaxError(fileName.c_str(), rootNode, "No version node"); + { + std::string error = "No version node in file " + fileName; + lastError = error; + nlinfo( "%s", error.c_str() ); + return false; + } else { // Old format, if (version <= 1) { - syntaxError(fileName.c_str(), rootNode, "Old version node"); + std::string error = "Old version node in file " + fileName; + lastError = error; + nlinfo( "%s", error.c_str() ); + return false; } else { // Read it if (version > WORLD_EDITOR_FILE_VERSION) { - syntaxError(fileName.c_str(), node, "Unknown file version"); + std::string error = "Unknown file version in file " + fileName; + lastError = error; + nlinfo( "%s", error.c_str() ); + return false; } else { @@ -153,11 +142,12 @@ bool loadWorldEditFile(const std::string &fileName, WorldEditList &worldEditList { // Get the type std::string type; - if (getPropertyString(type, fileName.c_str(), node, "TYPE")) + + if ( NLMISC::CIXml::getPropertyString(type, node, "TYPE")) { // Read the filename std::string filenameChild; - if (getPropertyString(filenameChild, fileName.c_str(), node, "FILENAME")) + if ( NLMISC::CIXml::getPropertyString(filenameChild, node, "FILENAME")) { // Is it a landscape ? if (type == "landscape") @@ -179,11 +169,34 @@ bool loadWorldEditFile(const std::string &fileName, WorldEditList &worldEditList } } else + if ( type == "primitive" ) { worldEditList.push_back(WorldEditItem(PrimitiveType, filenameChild)); } + else + if( type.empty() ) + { + std::string error = "Empty type node property in file " + fileName; + lastError = error; + nlinfo( "%s", error.c_str() ); + return false; + } } + else + { + std::string error = "Missing filename node property in file " + fileName; + lastError = error; + nlinfo( "%s", error.c_str() ); + return false; + } + } + else + { + std::string error = "Missing type node property in file " + fileName; + lastError = error; + nlinfo( "%s", error.c_str() ); + return false; } } while (node = NLMISC::CIXml::getNextChildNode(node, "DATABASE_ELEMENT")); @@ -197,18 +210,26 @@ bool loadWorldEditFile(const std::string &fileName, WorldEditList &worldEditList } else { - // Error - syntaxError(fileName.c_str(), rootNode, "Unknown file header : %s", rootNode->name); + std::string error = "Unknown file header in file " + fileName; + lastError = error; + nlinfo( "%s", error.c_str() ); + return false; } } } catch (NLMISC::Exception &e) { - nlerror("Error reading file %s : %s", fileName.c_str(), e.what()); + std::string error = "Error reading file " + fileName + " : " + std::string( e.what() ); + lastError = error; + nlinfo( "%s", error.c_str() ); } } else - nlerror("Can't open the file %s for reading.", fileName.c_str()); + { + std::string error = "Can't open the file " + fileName + " for reading."; + lastError = error; + nlinfo( "%s", error.c_str() ); + } return result; } diff --git a/code/studio/src/plugins/world_editor/world_editor_misc.h b/code/studio/src/plugins/world_editor/world_editor_misc.h index d29b2553b..36c7b1a6a 100644 --- a/code/studio/src/plugins/world_editor/world_editor_misc.h +++ b/code/studio/src/plugins/world_editor/world_editor_misc.h @@ -49,6 +49,8 @@ typedef std::vector WorldEditList; // Generate unique identificator uint32 getUniqueId(); +std::string getLastError(); + // Load *.worldedit file and return list primitives and landscapes. bool loadWorldEditFile(const std::string &fileName, WorldEditList &worldEditList); diff --git a/code/studio/src/plugins/world_editor/world_editor_window.cpp b/code/studio/src/plugins/world_editor/world_editor_window.cpp index afb667358..d4ae355f4 100644 --- a/code/studio/src/plugins/world_editor/world_editor_window.cpp +++ b/code/studio/src/plugins/world_editor/world_editor_window.cpp @@ -174,7 +174,12 @@ void WorldEditorWindow::loadWorldEditFile(const QString &fileName) Utils::WorldEditList worldEditList; if (!Utils::loadWorldEditFile(fileName.toUtf8().constData(), worldEditList)) { - // TODO: add the message box + std::string error = Utils::getLastError(); + + QMessageBox::critical( this, + tr( "Error opening world editor file" ), + tr( error.c_str() ) ); + return; }