From a24394a6764253e268a0967710094eb167e983d6 Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver.hamlet@gmail.com>
Date: Tue, 14 Jul 2015 10:41:20 +0100
Subject: [PATCH] Improve Masterlist loading semantics.

Masterlist::Load() shouldn't also update the masterlist when there's a
separate Masterlist::Update() function available, it's confusing.
---
 src/api/api.cpp            |  3 +--
 src/backend/masterlist.cpp | 18 +-----------------
 src/backend/masterlist.h   |  4 +---
 src/gui/handler.cpp        | 14 ++++++++++----
 4 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/src/api/api.cpp b/src/api/api.cpp
index 0fa1da23..e6ed9fdd 100644
--- a/src/api/api.cpp
+++ b/src/api/api.cpp
@@ -311,8 +311,7 @@ LOOT_API unsigned int loot_load_lists(loot_db db, const char * const masterlistP
 
     try {
         if (boost::filesystem::exists(masterlistPath)) {
-            // We don't want to update the masterlist too.
-            temp.MetadataList::Load(masterlistPath);
+            temp.Load(masterlistPath);
         }
         else {
             return c_error(loot_error_path_not_found, std::string("The given masterlist path does not exist: ") + masterlistPath);
diff --git a/src/backend/masterlist.cpp b/src/backend/masterlist.cpp
index 1afdee63..2455f83e 100644
--- a/src/backend/masterlist.cpp
+++ b/src/backend/masterlist.cpp
@@ -35,22 +35,6 @@ namespace fs = boost::filesystem;
 namespace lc = boost::locale;
 
 namespace loot {
-    bool Masterlist::Load(Game& game, const unsigned int language) {
-        try {
-            return Update(game);
-        }
-        catch (error& e) {
-            if (e.code() != error::ok) {
-                // Error wasn't a parsing error. Need to try parsing masterlist if it exists.
-                try {
-                    MetadataList::Load(game.MasterlistPath());
-                }
-                catch (...) {}
-            }
-            throw;
-        }
-    }
-
     std::string Masterlist::GetRevision(const boost::filesystem::path& path, bool shortID) {
         if (revision.empty() || (shortID && revision.length() == 40) || (!shortID && revision.length() < 40))
             GetGitInfo(path, shortID);
@@ -393,7 +377,7 @@ namespace loot {
             //Now try parsing the masterlist.
             BOOST_LOG_TRIVIAL(debug) << "Testing masterlist parsing.";
             try {
-                this->MetadataList::Load(path);
+                this->Load(path);
 
                 for (auto &plugin : plugins) {
                     plugin.ParseAllConditions();
diff --git a/src/backend/masterlist.h b/src/backend/masterlist.h
index ef281e27..12f79a50 100644
--- a/src/backend/masterlist.h
+++ b/src/backend/masterlist.h
@@ -37,8 +37,7 @@ namespace loot {
     class Masterlist : public MetadataList {
     public:
 
-        bool Load(Game& game, const unsigned int language);  //Handles update with load fallback.
-
+        bool Update(const Game& game);
         bool Update(const boost::filesystem::path& path,
                     const std::string& repoURL,
                     const std::string& repoBranch);
@@ -47,7 +46,6 @@ namespace loot {
         std::string GetDate(const boost::filesystem::path& path);
 
     private:
-        bool Update(const Game& game);
         void GetGitInfo(const boost::filesystem::path& path, bool shortID);
 
         std::string revision;
diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp
index a1ecde06..0df37ba4 100644
--- a/src/gui/handler.cpp
+++ b/src/gui/handler.cpp
@@ -652,7 +652,7 @@ namespace loot {
                     SendProgressUpdate(frame, loc::translate("Parsing masterlist..."));
                     BOOST_LOG_TRIVIAL(debug) << "Parsing masterlist.";
                     try {
-                        _lootState.CurrentGame().masterlist.MetadataList::Load(_lootState.CurrentGame().MasterlistPath());
+                        _lootState.CurrentGame().masterlist.Load(_lootState.CurrentGame().MasterlistPath());
                     }
                     catch (exception &e) {
                         _lootState.CurrentGame().masterlist.messages.push_back(Message(Message::error, (boost::format(loc::translate("An error occurred while parsing the masterlist: %1%")) % e.what()).str()));
@@ -815,7 +815,7 @@ namespace loot {
             bool wasChanged = true;
             try {
                 SendProgressUpdate(frame, loc::translate("Updating and parsing masterlist..."));
-                wasChanged = _lootState.CurrentGame().masterlist.Load(_lootState.CurrentGame(), language);
+                wasChanged = _lootState.CurrentGame().masterlist.Update(_lootState.CurrentGame());
             }
             catch (loot::error &e) {
                 if (e.code() == loot::error::ok) {
@@ -825,8 +825,14 @@ namespace loot {
                     _lootState.CurrentGame().masterlist.messages.push_back(Message(Message::error, e.what()));
                     wasChanged = true;
                 }
-                else
-                    throw;
+                else {
+                    // Error wasn't a parsing error. Need to try parsing masterlist if it exists.
+                    try {
+                        _lootState.CurrentGame().masterlist.Load(_lootState.CurrentGame().MasterlistPath());
+                    }
+                    catch (...) {}
+                }
+                throw;
             }
 
             // Now regenerate the JS-side masterlist data if the masterlist was changed.