Simplify and print XML errors from TinyXML-2

All XML functions now check the return value of
tinyxml2::XMLDocument::Error() after each document gets loaded in to
TinyXML-2. If there's an error, then all functions return. This isn't
strictly necessary, but printing the error message that TinyXML-2 is the
bare minimum we could do to be useful.

Additionally, I've standardized the error messages of missing or
corrupted XML files.

Also, the way the game went about making the XML handles was... a bit
roundabout. There were two XML handles, one for the document and one for
the root element - although only one XML handle suffices. So I've
cleaned that up too.

I could've gone further and added error checking for a whole bunch of
things (e.g. missing elements, missing attributes), but this is good
enough.

Also, if unlock.vvv or settings.vvv don't exist yet, the game is
guaranteed to no-op instead of continuing with the function. Nothing bad
seems to happen if the function continues, but the return statements
should be there anyway to clearly indicate intent.

desktop_version/src/Game.cpp

@@ -100,33 +100,34 @@ static bool GetButtonFromString(const char *pText, SDL_GameControllerButton *but
 
 static const char* get_summary(
     const char* filename,
-    const char* save,
+    const char* savename,
     tinyxml2::XMLDocument& doc
 ) {
     tinyxml2::XMLHandle hDoc(&doc);
     tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
     bool success;
     const char* retval = "";
 
     success = FILESYSTEM_loadTiXml2Document(filename, doc);
     if (!success)
     {
-        vlog_info("%s Not Found", save);
+        vlog_info("%s not found", savename);
-        return "";
+        goto end;
     }
 
-    pElem = hDoc.FirstChildElement().ToElement();
+    if (doc.Error())
-
-    if (!pElem)
     {
-        vlog_error("%s Appears Corrupted: No XML Root", save);
+        vlog_error("Error parsing %s: %s", savename, doc.ErrorStr());
+        goto end;
     }
 
-    // save this for later
+    for (pElem = hDoc
-    hRoot = tinyxml2::XMLHandle(pElem);
+        .FirstChildElement()
-
+        .FirstChildElement("Data")
-    for (pElem = hRoot.FirstChildElement("Data").FirstChild().ToElement(); pElem; pElem = pElem->NextSiblingElement())
+        .FirstChildElement()
+        .ToElement();
+    pElem != NULL;
+    pElem = pElem->NextSiblingElement())
     {
         const char* pKey = pElem->Value();
         const char* pText = pElem->GetText();
@@ -142,6 +143,7 @@ static const char* get_summary(
         }
     }
 
+end:
     return retval;
 }
 
@@ -316,11 +318,11 @@ void Game::init(void)
     saveFilePath = FILESYSTEM_getUserSaveDirectory();
 
     tinyxml2::XMLDocument doc;
-    quicksummary = get_summary("saves/qsave.vvv", "Quick Save", doc);
+    quicksummary = get_summary("saves/qsave.vvv", "qsave.vvv", doc);
 
 
     tinyxml2::XMLDocument docTele;
-    telesummary = get_summary("saves/tsave.vvv", "Teleporter Save", doc);
+    telesummary = get_summary("saves/tsave.vvv", "tsave.vvv", doc);
 
     screenshake = flashlight = 0 ;
 
@@ -460,6 +462,8 @@ void Game::deletecustomlevelstats(void)
 void Game::loadcustomlevelstats(void)
 {
     tinyxml2::XMLDocument doc;
+    tinyxml2::XMLHandle hDoc(&doc);
+
     if (!FILESYSTEM_loadTiXml2Document("saves/levelstats.vvv", doc))
     {
         //No levelstats file exists; start new
@@ -468,29 +472,28 @@ void Game::loadcustomlevelstats(void)
         return;
     }
 
+    if (doc.Error())
+    {
+        vlog_error("Error parsing levelstats.vvv: %s", doc.ErrorStr());
+        return;
+    }
+
     // Old system
     std::vector<std::string> customlevelnames;
     std::vector<int> customlevelscores;
 
-    tinyxml2::XMLHandle hDoc(&doc);
     tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
+    tinyxml2::XMLElement* firstElement;
 
-    {
+    firstElement = hDoc
-        pElem=hDoc.FirstChildElement().ToElement();
+        .FirstChildElement()
-        // should always have a valid root but handle gracefully if it does
+        .FirstChildElement("Data")
-        if (!pElem)
+        .FirstChildElement()
-        {
+        .ToElement();
-            vlog_error("Error: Levelstats file corrupted");
-        }
-
-        // save this for later
-        hRoot=tinyxml2::XMLHandle(pElem);
-    }
 
     // First pass, look for the new system of storing stats
     // If they don't exist, then fall back to the old system
-    for (pElem = hRoot.FirstChildElement("Data").FirstChild().ToElement(); pElem; pElem = pElem->NextSiblingElement())
+    for (pElem = firstElement; pElem != NULL; pElem = pElem->NextSiblingElement())
     {
         const char* pKey = pElem->Value();
         const char* pText = pElem->GetText();
@@ -524,7 +527,7 @@ void Game::loadcustomlevelstats(void)
 
 
     // Since we're still here, we must be on the old system
-    for( pElem = hRoot.FirstChildElement( "Data" ).FirstChild().ToElement(); pElem; pElem=pElem->NextSiblingElement())
+    for (pElem = firstElement; pElem; pElem=pElem->NextSiblingElement())
     {
         const char* pKey = pElem->Value();
         const char* pText = pElem->GetText() ;
@@ -575,6 +578,11 @@ void Game::savecustomlevelstats(void)
     {
         vlog_info("No levelstats.vvv found. Creating new file");
     }
+    else if (doc.Error())
+    {
+        vlog_error("Error parsing existing levelstats.vvv: %s", doc.ErrorStr());
+        vlog_info("Creating new levelstats.vvv");
+    }
 
     xml::update_declaration(doc);
 
@@ -4000,34 +4008,31 @@ void Game::unlocknum( int t )
 void Game::loadstats(ScreenSettings* screen_settings)
 {
     tinyxml2::XMLDocument doc;
+    tinyxml2::XMLHandle hDoc(&doc);
+    tinyxml2::XMLElement* pElem;
+    tinyxml2::XMLElement* dataNode;
+
     if (!FILESYSTEM_loadTiXml2Document("saves/unlock.vvv", doc))
     {
         // Save unlock.vvv only. Maybe we have a settings.vvv laying around too,
         // and we don't want to overwrite that!
         savestats(screen_settings);
+        return;
     }
 
-    tinyxml2::XMLHandle hDoc(&doc);
+    if (doc.Error())
-    tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
-
-
     {
-        pElem=hDoc.FirstChildElement().ToElement();
+        vlog_error("Error parsing unlock.vvv: %s", doc.ErrorStr());
-        // should always have a valid root but handle gracefully if it does
+        return;
-        if (!pElem)
-        {
-
-        }
-        ;
-
-        // save this for later
-        hRoot=tinyxml2::XMLHandle(pElem);
     }
 
-    tinyxml2::XMLElement* dataNode = hRoot.FirstChildElement("Data").FirstChild().ToElement();
+    dataNode = hDoc
+        .FirstChildElement()
+        .FirstChildElement("Data")
+        .FirstChildElement()
+        .ToElement();
 
-    for( pElem = dataNode; pElem; pElem=pElem->NextSiblingElement())
+    for (pElem = dataNode; pElem != NULL; pElem=pElem->NextSiblingElement())
     {
         const char* pKey = pElem->Value();
         const char* pText = pElem->GetText() ;
@@ -4325,6 +4330,11 @@ bool Game::savestats(const ScreenSettings* screen_settings)
     {
         vlog_info("No unlock.vvv found. Creating new file");
     }
+    else if (doc.Error())
+    {
+        vlog_error("Error parsing existing unlock.vvv: %s", doc.ErrorStr());
+        vlog_info("Creating new unlock.vvv");
+    }
 
     xml::update_declaration(doc);
 
@@ -4545,29 +4555,26 @@ void Game::serializesettings(tinyxml2::XMLElement* dataNode, const ScreenSetting
 void Game::loadsettings(ScreenSettings* screen_settings)
 {
     tinyxml2::XMLDocument doc;
+    tinyxml2::XMLHandle hDoc(&doc);
+    tinyxml2::XMLElement* dataNode;
+
     if (!FILESYSTEM_loadTiXml2Document("saves/settings.vvv", doc))
     {
         savesettings(screen_settings);
         return;
     }
 
-    tinyxml2::XMLHandle hDoc(&doc);
+    if (doc.Error())
-    tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
-
     {
-        pElem = hDoc.FirstChildElement().ToElement();
+        vlog_error("Error parsing settings.vvv: %s", doc.ErrorStr());
-        // should always have a valid root but handle gracefully if it doesn't
+        return;
-        if (!pElem)
-        {
-        }
-        ;
-
-        // save this for later
-        hRoot = tinyxml2::XMLHandle(pElem);
     }
 
-    tinyxml2::XMLElement* dataNode = hRoot.FirstChildElement("Data").FirstChild().ToElement();
+    dataNode = hDoc
+        .FirstChildElement()
+        .FirstChildElement("Data")
+        .FirstChildElement()
+        .ToElement();
 
     deserializesettings(dataNode, screen_settings);
 }
@@ -4593,6 +4600,11 @@ bool Game::savesettings(const ScreenSettings* screen_settings)
     {
         vlog_info("No settings.vvv found. Creating new file");
     }
+    else if (doc.Error())
+    {
+        vlog_error("Error parsing existing settings.vvv: %s", doc.ErrorStr());
+        vlog_info("Creating new settings.vvv");
+    }
 
     xml::update_declaration(doc);
 
@@ -4827,32 +4839,30 @@ void Game::loadquick(void)
     tinyxml2::XMLDocument doc;
     if (!FILESYSTEM_loadTiXml2Document("saves/qsave.vvv", doc)) return;
 
-    readmaingamesave(doc);
+    readmaingamesave("qsave.vvv", doc);
 }
 
-void Game::readmaingamesave(tinyxml2::XMLDocument& doc)
+void Game::readmaingamesave(const char* savename, tinyxml2::XMLDocument& doc)
 {
     tinyxml2::XMLHandle hDoc(&doc);
     tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
-
 
+    if (doc.Error())
     {
-        pElem=hDoc.FirstChildElement().ToElement();
+        vlog_error("Error parsing %s: %s", savename, doc.ErrorStr());
-        // should always have a valid root but handle gracefully if it does
+        return;
-        if (!pElem)
-        {
-            vlog_error("Save Not Found");
-        }
-
-        // save this for later
-        hRoot=tinyxml2::XMLHandle(pElem);
     }
 
-    for( pElem = hRoot.FirstChildElement( "Data" ).FirstChild().ToElement(); pElem; pElem=pElem->NextSiblingElement())
+    for (pElem = hDoc
+        .FirstChildElement()
+        .FirstChildElement("Data")
+        .FirstChildElement()
+        .ToElement();
+    pElem != NULL;
+    pElem = pElem->NextSiblingElement())
     {
-        const char* pKey = pElem->Value();;
+        const char* pKey = pElem->Value();
-        const char* pText = pElem->GetText() ;
+        const char* pText = pElem->GetText();
         if(pText == NULL)
         {
             pText = "";
@@ -4997,7 +5007,13 @@ void Game::readmaingamesave(tinyxml2::XMLDocument& doc)
 
 void Game::customloadquick(std::string savfile)
 {
-    if (cliplaytest) {
+    tinyxml2::XMLDocument doc;
+    tinyxml2::XMLHandle hDoc(&doc);
+    tinyxml2::XMLElement* pElem;
+    std::string levelfile;
+
+    if (cliplaytest)
+    {
         savex = playx;
         savey = playy;
         saverx = playrx;
@@ -5007,28 +5023,26 @@ void Game::customloadquick(std::string savfile)
         return;
     }
 
-    std::string levelfile = savfile.substr(7);
+    levelfile = savfile.substr(7);
-    tinyxml2::XMLDocument doc;
+    if (!FILESYSTEM_loadTiXml2Document(("saves/"+levelfile+".vvv").c_str(), doc))
-    if (!FILESYSTEM_loadTiXml2Document(("saves/"+levelfile+".vvv").c_str(), doc)) return;
-
-    tinyxml2::XMLHandle hDoc(&doc);
-    tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
-
-
     {
-        pElem=hDoc.FirstChildElement().ToElement();
+        vlog_error("%s.vvv not found", levelfile.c_str());
-        // should always have a valid root but handle gracefully if it does
+        return;
-        if (!pElem)
-        {
-            vlog_error("Save Not Found");
-        }
-
-        // save this for later
-        hRoot=tinyxml2::XMLHandle(pElem);
     }
 
-    for( pElem = hRoot.FirstChildElement( "Data" ).FirstChild().ToElement(); pElem; pElem=pElem->NextSiblingElement())
+    if (doc.Error())
+    {
+        vlog_error("Error parsing %s.vvv: %s", levelfile.c_str(), doc.ErrorStr());
+        return;
+    }
+
+    for (pElem = hDoc
+        .FirstChildElement()
+        .FirstChildElement("Data")
+        .FirstChildElement()
+        .ToElement();
+    pElem != NULL;
+    pElem = pElem->NextSiblingElement())
     {
         const char* pKey = pElem->Value();
         const char* pText = pElem->GetText() ;
@@ -5199,29 +5213,34 @@ struct Summary
 };
 
 static void loadthissummary(
+    const char* filename,
     struct Summary* summary,
     tinyxml2::XMLDocument& doc
 ) {
     tinyxml2::XMLHandle hDoc(&doc);
     tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
-
 
+    if (doc.Error())
     {
-        pElem=hDoc.FirstChildElement().ToElement();
+        vlog_error("Error parsing %s: %s", filename, doc.ErrorStr());
-        // should always have a valid root but handle gracefully if it does
+        return;
-        if (!pElem)
-        {
-            vlog_error("Save Not Found");
-        }
-
-        // save this for later
-        hRoot=tinyxml2::XMLHandle(pElem);
     }
-    for( pElem = hRoot.FirstChildElement( "Data" ).FirstChild().ToElement(); pElem; pElem=pElem->NextSiblingElement())
+
+    for (pElem = hDoc
+        .FirstChildElement()
+        .FirstChildElement("Data")
+        .FirstChildElement()
+        .ToElement();
+    pElem != NULL;
+    pElem = pElem->NextSiblingElement())
     {
         const char* pKey = pElem->Value();
-        const char* pText = pElem->GetText() ;
+        const char* pText = pElem->GetText();
+
+        if (pText == NULL)
+        {
+            pText = "";
+        }
 
         if (pText == NULL)
         {
@@ -5279,7 +5298,7 @@ void Game::loadsummary(void)
         struct Summary summary;
         SDL_zero(summary);
 
-        loadthissummary(&summary, doc);
+        loadthissummary("tsave.vvv", &summary, doc);
 
         telesummary = summary.summary;
         tele_gametime = giventimestring(
@@ -5304,7 +5323,7 @@ void Game::loadsummary(void)
         struct Summary summary;
         SDL_zero(summary);
 
-        loadthissummary(&summary, doc);
+        loadthissummary("qsave.vvv", &summary, doc);
 
         quicksummary = summary.summary;
         quick_gametime = giventimestring(
@@ -5349,6 +5368,12 @@ bool Game::savetele(void)
     {
         vlog_info("No tsave.vvv found. Creating new file");
     }
+    else if (doc.Error())
+    {
+        vlog_error("Error parsing existing tsave.vvv: %s", doc.ErrorStr());
+        vlog_info("Creating new tsave.vvv");
+    }
+
     telesummary = writemaingamesave(doc);
 
     if(!FILESYSTEM_saveTiXml2Document("saves/tsave.vvv", doc))
@@ -5376,6 +5401,12 @@ bool Game::savequick(void)
     {
         vlog_info("No qsave.vvv found. Creating new file");
     }
+    else if (doc.Error())
+    {
+        vlog_error("Error parsing existing qsave.vvv: %s", doc.ErrorStr());
+        vlog_info("Creating new qsave.vvv");
+    }
+
     quicksummary = writemaingamesave(doc);
 
     if(!FILESYSTEM_saveTiXml2Document("saves/qsave.vvv", doc))
@@ -5513,6 +5544,11 @@ bool Game::customsavequick(std::string savfile)
     {
         vlog_info("No %s.vvv found. Creating new file", levelfile.c_str());
     }
+    else if (doc.Error())
+    {
+        vlog_error("Error parsing existing %s.vvv: %s", levelfile.c_str(), doc.ErrorStr());
+        vlog_info("Creating new %s.vvv", levelfile.c_str());
+    }
 
     xml::update_declaration(doc);
 
@@ -5648,7 +5684,7 @@ void Game::loadtele(void)
     tinyxml2::XMLDocument doc;
     if (!FILESYSTEM_loadTiXml2Document("saves/tsave.vvv", doc)) return;
 
-    readmaingamesave(doc);
+    readmaingamesave("tsave.vvv", doc);
 }
 
 std::string Game::unrescued(void)

desktop_version/src/Game.h

@@ -203,7 +203,7 @@ public:
 
     void loadsummary(void);
 
-    void readmaingamesave(tinyxml2::XMLDocument& doc);
+    void readmaingamesave(const char* savename, tinyxml2::XMLDocument& doc);
     std::string writemaingamesave(tinyxml2::XMLDocument& doc);
 
     void initteleportermode(void);

desktop_version/src/editor.cpp

@@ -1797,7 +1797,6 @@ bool editorclass::load(std::string& _path)
     tinyxml2::XMLDocument doc;
     tinyxml2::XMLHandle hDoc(&doc);
     tinyxml2::XMLElement* pElem;
-    tinyxml2::XMLHandle hRoot(NULL);
 
     reset();
 
@@ -1819,31 +1818,30 @@ bool editorclass::load(std::string& _path)
 
     if (!FILESYSTEM_loadTiXml2Document(_path.c_str(), doc))
     {
-        vlog_warn("No level %s to load :(", _path.c_str());
+        vlog_warn("%s not found", _path.c_str());
-        return false;
+        goto fail;
+    }
+
+    if (doc.Error())
+    {
+        vlog_error("Error parsing %s: %s", _path.c_str(), doc.ErrorStr());
+        goto fail;
     }
 
     loaded_filepath = _path;
 
     version = 0;
 
-    {
+    for (pElem = hDoc
-        pElem=hDoc.FirstChildElement().ToElement();
+        .FirstChildElement()
-        // should always have a valid root but handle gracefully if it does
+        .FirstChildElement("Data")
-        if (!pElem)
+        .FirstChildElement()
-        {
+        .ToElement();
-            vlog_error("No valid root! Corrupt level file?");
+    pElem != NULL;
-        }
+    pElem = pElem->NextSiblingElement())
-
-        pElem->QueryIntAttribute("version", &version);
-        // save this for later
-        hRoot=tinyxml2::XMLHandle(pElem);
-    }
-
-    for( pElem = hRoot.FirstChildElement( "Data" ).FirstChild().ToElement(); pElem; pElem=pElem->NextSiblingElement())
     {
         const char* pKey = pElem->Value();
-        const char* pText = pElem->GetText() ;
+        const char* pText = pElem->GetText();
         if(pText == NULL)
         {
             pText = "";