Further Download related cleanups

* Moved the memory buffer and mutex handling into the Download class to
  simplify the code updating the UI in ServerDialog and UpdaterWindow.

* Replaced the "DownloadUpdate" callback function with simply polling
  Download::getState, since in the end polling was happening anyway.

This changes also fixes handling of the Enter key while downloading
updates, which no longer cancels the update process. Also, when pressing
Escape while things are being downloaded, the first press cancels and
only the second press goes back to login.

Introduced a ThreadSafe template class, which wraps any type and makes
it only accessible by calling lock(). This ensures the data is never
accessed without locking the relevant mutex.

4 files changed, 266 insertions, 457 deletions

diff --git a/src/gui/serverdialog.cpp b/src/gui/serverdialog.cpp
index 90469662..4c54f2ce 100644
--- a/src/gui/serverdialog.cpp
+++ b/src/gui/serverdialog.cpp
@@ -43,7 +43,6 @@
 
 #include "utils/gettext.h"
 #include "utils/stringutils.h"
-#include "utils/xml.h"
 
 #include <guichan/font.hpp>
 
@@ -59,13 +58,11 @@ ServersListModel::ServersListModel(ServerInfos *servers, ServerDialog *parent):
 
 int ServersListModel::getNumberOfElements()
 {
-    MutexLocker lock(mParent->getMutex());
     return mServers->size();
 }
 
 std::string ServersListModel::getElementAt(int elementIndex)
 {
-    MutexLocker lock(mParent->getMutex());
     const ServerInfo &server = mServers->at(elementIndex);
     std::string myServer;
     myServer += server.hostname;
@@ -162,7 +159,6 @@ ServerDialog::ServerDialog(ServerInfo *serverInfo, const std::string &dir):
     loadCustomServers();
 
     mServersListModel = new ServersListModel(&mServers, this);
-
     mServersList = new ServersListBox(mServersListModel);
 
     auto *usedScroll = new ScrollArea(mServersList);
@@ -225,14 +221,6 @@ ServerDialog::ServerDialog(ServerInfo *serverInfo, const std::string &dir):
 
 ServerDialog::~ServerDialog()
 {
-    if (mDownload)
-    {
-        mDownload->cancel();
-
-        // Make sure thread is gone before deleting the ServersListModel
-        mDownload.reset();
-    }
-
     delete mServersListModel;
 }
 
@@ -345,7 +333,6 @@ void ServerDialog::valueChanged(const gcn::SelectionEvent &)
     // Update the server and post fields according to the new selection
     const ServerInfo &myServer = mServersListModel->getServer(index);
     mDescription->setCaption(myServer.description);
-
     mDeleteButton->setEnabled(myServer.save);
     mModifyButton->setEnabled(myServer.save);
 }
@@ -362,36 +349,42 @@ void ServerDialog::mouseClicked(gcn::MouseEvent &mouseEvent)
 
 void ServerDialog::logic()
 {
-    {
-        MutexLocker lock(&mMutex);
-        if (mDownloadStatus == DOWNLOADING_COMPLETE)
-        {
-            mDownloadStatus = DOWNLOADING_OVER;
+    Window::logic();
 
-            mDescription->setCaption(mServers[0].description);
-            mDownloadText->setCaption(std::string());
-        }
-        else if (mDownloadStatus == DOWNLOADING_IN_PROGRESS)
-        {
-            mDownloadText->setCaption(strprintf(_("Downloading server list..."
-                                                 "%2.0f%%"),
-                                      mDownloadProgress * 100));
-        }
-        else if (mDownloadStatus == DOWNLOADING_IDLE)
-        {
-            mDownloadText->setCaption(_("Waiting for server..."));
-        }
-        else if (mDownloadStatus == DOWNLOADING_PREPARING)
+    if (mDownloadDone)
+        return;
+
+    auto state = mDownload->getState();
+
+    switch (state.status) {
+    case DownloadStatus::IN_PROGRESS:
+        mDownloadText->setCaption(strprintf(_("Downloading server list..."
+                                              "%2.0f%%"),
+                                            state.progress * 100));
+        break;
+
+    case DownloadStatus::CANCELED:
+    case DownloadStatus::ERROR:
+        mDownloadDone = true;
+        logger->log("Error retrieving server list: %s", mDownload->getError());
+        mDownloadText->setCaption(_("Error retrieving server list!"));
+        break;
+
+    case DownloadStatus::COMPLETE:
+        mDownloadDone = true;
+        loadServers();
+
+        if (mServers.empty())
         {
-            mDownloadText->setCaption(_("Preparing download"));
+            mDownloadText->setCaption(_("No servers found!"));
         }
-        else if (mDownloadStatus == DOWNLOADING_ERROR)
+        else
         {
-            mDownloadText->setCaption(_("Error retreiving server list!"));
+            mDownloadText->setCaption(std::string());
+            mDescription->setCaption(mServers[0].description);
         }
+        break;
     }
-
-    Window::logic();
 }
 
 void ServerDialog::downloadServerList()
@@ -406,8 +399,7 @@ void ServerDialog::downloadServerList()
     if (listFile.empty())
         listFile = "https://www.manasource.org/serverlist.xml";
 
-    mDownload = std::make_unique<Net::Download>(this, listFile,
-                                                &ServerDialog::downloadUpdate);
+    mDownload = std::make_unique<Net::Download>(listFile);
     mDownload->setFile(mDir + "/serverlist.xml");
     mDownload->start();
 }
@@ -433,89 +425,92 @@ void ServerDialog::loadServers()
 
     for (auto serverNode : rootNode.children())
     {
-        if (serverNode.name() != "server")
-            continue;
+        if (serverNode.name() == "server")
+            loadServer(serverNode);
+    }
+}
 
-        ServerInfo server;
+void ServerDialog::loadServer(XML::Node serverNode)
+{
+    ServerInfo server;
 
-        std::string type = serverNode.getProperty("type", "unknown");
+    std::string type = serverNode.getProperty("type", "unknown");
 
-        server.type = ServerInfo::parseType(type);
+    server.type = ServerInfo::parseType(type);
 
-        // Ignore unknown server types
-        if (server.type == ServerType::UNKNOWN
+    // Ignore unknown server types
+    if (server.type == ServerType::UNKNOWN
 #ifndef MANASERV_SUPPORT
-            || server.type == ServerType::MANASERV
+        || server.type == ServerType::MANASERV
 #endif
         )
-        {
-            logger->log("Ignoring server entry with unknown type: %s",
-                        type.c_str());
-            continue;
-        }
+    {
+        logger->log("Ignoring server entry with unknown type: %s",
+                    type.c_str());
+        return;
+    }
 
-        server.name = serverNode.getProperty("name", std::string());
+    server.name = serverNode.getProperty("name", std::string());
 
-        std::string version = serverNode.getProperty("minimumVersion",
-                                               std::string());
+    std::string version = serverNode.getProperty("minimumVersion",
+                                                 std::string());
 
-        bool meetsMinimumVersion = compareStrI(version, PACKAGE_VERSION) <= 0;
+    bool meetsMinimumVersion = compareStrI(version, PACKAGE_VERSION) <= 0;
 
-        // For display in the list
-        if (meetsMinimumVersion)
-            version.clear();
-        else if (version.empty())
-            version = _("requires a newer version");
-        else
-            version = strprintf(_("requires v%s"), version.c_str());
+    // For display in the list
+    if (meetsMinimumVersion)
+        version.clear();
+    else if (version.empty())
+        version = _("requires a newer version");
+    else
+        version = strprintf(_("requires v%s"), version.c_str());
 
-        for (auto subNode : serverNode.children())
+    for (auto subNode : serverNode.children())
+    {
+        if (subNode.name() == "connection")
         {
-            if (subNode.name() == "connection")
-            {
-                server.hostname = subNode.getProperty("hostname", std::string());
-                server.port = subNode.getProperty("port", 0);
-                if (server.port == 0)
-                {
-                    // If no port is given, use the default for the given type
-                    server.port = ServerInfo::defaultPortForServerType(server.type);
-                }
-            }
-            else if (subNode.name() == "description")
-            {
-                server.description = subNode.textContent();
-            }
-            else if (subNode.name() == "persistentIp")
+            server.hostname = subNode.getProperty("hostname", std::string());
+            server.port = subNode.getProperty("port", 0);
+            if (server.port == 0)
             {
-                const auto text = subNode.textContent();
-                server.persistentIp = text == "1" || text == "true";
+                // If no port is given, use the default for the given type
+                server.port = ServerInfo::defaultPortForServerType(server.type);
             }
         }
+        else if (subNode.name() == "description")
+        {
+            server.description = subNode.textContent();
+        }
+        else if (subNode.name() == "persistentIp")
+        {
+            const auto text = subNode.textContent();
+            server.persistentIp = text == "1" || text == "true";
+        }
+    }
 
-        server.version.first = gui->getFont()->getWidth(version);
-        server.version.second = version;
+    server.version.first = gui->getFont()->getWidth(version);
+    server.version.second = version;
 
-        // Add the server to the local list if it's not already present
-        bool found = false;
-        int i = 0;
-        for (auto &s : mServers)
+    // Add the server to the local list if it's not already present
+    bool found = false;
+    int i = 0;
+    for (auto &s : mServers)
+    {
+        if (s == server)
         {
-            if (s == server)
-            {
-                // Use the name listed in the server list
-                s.name = server.name;
-                s.version = server.version;
-                s.description = server.description;
-                mServersListModel->setVersionString(i, version);
-                found = true;
-                break;
-            }
-            ++i;
+            // Use the name listed in the server list
+            s.name = server.name;
+            s.version = server.version;
+            s.description = server.description;
+            mServersListModel->setVersionString(i, version);
+            found = true;
+            break;
         }
-
-        if (!found)
-            mServers.push_back(server);
+        ++i;
     }
+
+    if (!found)
+        mServers.push_back(server);
 }
 
 void ServerDialog::loadCustomServers()
@@ -558,38 +553,6 @@ void ServerDialog::saveCustomServers(const ServerInfo &currentServer, int index)
     // Restore the correct description
     if (index < 0)
         index = 0;
-    mDescription->setCaption(mServers[index].description);
-}
-
-int ServerDialog::downloadUpdate(void *ptr, DownloadStatus status,
-                                 size_t dltotal, size_t dlnow)
-{
-    if (status == DOWNLOAD_STATUS_CANCELLED)
-        return -1;
-
-    auto *sd = reinterpret_cast<ServerDialog*>(ptr);
-    MutexLocker lock(&sd->mMutex);
-
-    if (status == DOWNLOAD_STATUS_COMPLETE)
-    {
-        sd->loadServers();
-        sd->mDownloadStatus = DOWNLOADING_COMPLETE;
-    }
-    else if (status < 0)
-    {
-        logger->log("Error retreiving server list: %s",
-                    sd->mDownload->getError());
-        sd->mDownloadStatus = DOWNLOADING_ERROR;
-    }
-    else
-    {
-        float progress = 0.0f;
-        if (dltotal > 0)
-            progress = static_cast<float>(dlnow) / dltotal;
-
-        sd->mDownloadStatus = DOWNLOADING_IN_PROGRESS;
-        sd->mDownloadProgress = progress;
-    }
-
-    return 0;
+    if (static_cast<size_t>(index) < mServers.size())
+        mDescription->setCaption(mServers[index].description);
 }
diff --git a/src/gui/serverdialog.h b/src/gui/serverdialog.h
index bf2a9512..e0d74006 100644
--- a/src/gui/serverdialog.h
+++ b/src/gui/serverdialog.h
@@ -25,8 +25,7 @@
 
 #include "net/download.h"
 #include "net/serverinfo.h"
-
-#include "utils/mutex.h"
+#include "utils/xml.h"
 
 #include <guichan/actionlistener.hpp>
 #include <guichan/keylistener.hpp>
@@ -90,7 +89,6 @@ class ServerDialog : public Window,
 {
     public:
         ServerDialog(ServerInfo *serverInfo, const std::string &dir);
-
         ~ServerDialog() override;
 
         /**
@@ -110,10 +108,8 @@ class ServerDialog : public Window,
         void logic() override;
 
     protected:
-        friend class ServersListModel;
-        Mutex *getMutex() { return &mMutex; }
-
         friend class CustomServerDialog;
+
         /**
          * Saves the new server entry in the custom server list.
          * Removes the given entry when the serverInfo is empty.
@@ -128,12 +124,10 @@ class ServerDialog : public Window,
          */
         void downloadServerList();
         void loadServers();
+        void loadServer(XML::Node serverNode);
 
         void loadCustomServers();
 
-        static int downloadUpdate(void *ptr, DownloadStatus status,
-                                  size_t dltotal, size_t dlnow);
-
         Label  *mDescription;
         Button *mQuitButton;
         Button *mConnectButton;
@@ -146,25 +140,9 @@ class ServerDialog : public Window,
 
         const std::string &mDir;
 
-        enum ServerDialogDownloadStatus
-        {
-            DOWNLOADING_ERROR,
-            DOWNLOADING_PREPARING,
-            DOWNLOADING_IDLE,
-            DOWNLOADING_IN_PROGRESS,
-            DOWNLOADING_COMPLETE,
-            DOWNLOADING_OVER
-        };
-
-        /** Status of the current download. */
-        ServerDialogDownloadStatus mDownloadStatus = DOWNLOADING_PREPARING;
-
         std::unique_ptr<Net::Download> mDownload;
+        bool mDownloadDone = false;
         Label *mDownloadText;
-
-        Mutex mMutex;
-        float mDownloadProgress = 0.0f;
-
         ServerInfos mServers;
         ServerInfo *mServerInfo;
 };
diff --git a/src/gui/updaterwindow.cpp b/src/gui/updaterwindow.cpp
index 852d3ade..34169195 100644
--- a/src/gui/updaterwindow.cpp
+++ b/src/gui/updaterwindow.cpp
@@ -123,7 +123,6 @@ UpdaterWindow::UpdaterWindow(const std::string &updateHost,
     Window(_("Updating...")),
     mUpdateHost(updateHost),
     mUpdatesDir(updatesDir),
-    mCurrentFile("news.txt"),
     mLoadUpdates(applyUpdates),
     mLinkHandler(std::make_unique<ItemLinkHandler>(this))
 {
@@ -161,41 +160,22 @@ UpdaterWindow::UpdaterWindow(const std::string &updateHost,
     setVisible(true);
     mCancelButton->requestFocus();
 
-    // Try to download the updates list
-    download();
+    startDownload("news.txt", true);
 }
 
 UpdaterWindow::~UpdaterWindow()
 {
     if (mLoadUpdates)
         loadUpdates();
-
-    if (mDownload)
-    {
-        mDownload->cancel();
-
-        // Make sure thread is gone before freeing the memory buffer
-        mDownload.reset();
-    }
-
-    free(mMemoryBuffer);
-}
-
-void UpdaterWindow::setProgress(float progress)
-{
-    // Do delayed progress bar update, since Guichan isn't thread-safe
-    MutexLocker lock(&mDownloadMutex);
-    mDownloadProgress = progress;
 }
 
 void UpdaterWindow::setLabel(const std::string &str)
 {
-    // Do delayed label text update, since Guichan isn't thread-safe
-    MutexLocker lock(&mDownloadMutex);
-    mNewLabelCaption = str;
+    mLabel->setCaption(str);
+    mLabel->adjustSize();
 }
 
-void UpdaterWindow::enable()
+void UpdaterWindow::enablePlay()
 {
     mCancelButton->setEnabled(false);
     mPlayButton->setEnabled(true);
@@ -205,20 +185,9 @@ void UpdaterWindow::enable()
 void UpdaterWindow::action(const gcn::ActionEvent &event)
 {
     if (event.getId() == "cancel")
-    {
-        // Register the user cancel
-        mUserCancel = true;
-        // Skip the updating process
-        if (mDownloadStatus != UPDATE_COMPLETE)
-        {
-            mDownload->cancel();
-            mDownloadStatus = UPDATE_ERROR;
-        }
-    }
+        cancel();
     else if (event.getId() == "play")
-    {
-        Client::setState(STATE_LOAD_DATA);
-    }
+        play();
 }
 
 void UpdaterWindow::keyPressed(gcn::KeyEvent &keyEvent)
@@ -227,114 +196,58 @@ void UpdaterWindow::keyPressed(gcn::KeyEvent &keyEvent)
 
     if (key.getValue() == Key::ESCAPE)
     {
-        action(gcn::ActionEvent(nullptr, mCancelButton->getActionEventId()));
-        Client::setState(STATE_WORLD_SELECT);
+        if (!cancel())
+        {
+            mLoadUpdates = false;
+            Client::setState(STATE_WORLD_SELECT);
+        }
     }
     else if (key.getValue() == Key::ENTER)
     {
-        if (mDownloadStatus == UPDATE_COMPLETE ||
-                mDownloadStatus == UPDATE_ERROR)
-        {
-            action(gcn::ActionEvent(nullptr, mPlayButton->getActionEventId()));
-        }
-        else
-        {
-            action(gcn::ActionEvent(nullptr, mCancelButton->getActionEventId()));
-        }
+        play();
     }
 }
 
-void UpdaterWindow::loadNews()
+bool UpdaterWindow::cancel()
 {
-    if (!mMemoryBuffer)
+    // Skip the updating process
+    if (mDialogState != DialogState::DONE)
     {
-        logger->log("Couldn't load news");
-        return;
+        mDownload->cancel();
+        return true;
     }
-
-    mBrowserBox->clearRows();
-    mBrowserBox->addRows(std::string_view(mMemoryBuffer, mDownloadedBytes));
-
-    // Free the memory buffer now that we don't need it anymore
-    free(mMemoryBuffer);
-    mMemoryBuffer = nullptr;
-
-    mScrollArea->setVerticalScrollAmount(0);
+    return false;
 }
 
-int UpdaterWindow::updateProgress(void *ptr, DownloadStatus status,
-                                  size_t dltotal, size_t dlnow)
+void UpdaterWindow::play()
 {
-    auto *uw = reinterpret_cast<UpdaterWindow *>(ptr);
-
-    if (status == DOWNLOAD_STATUS_COMPLETE)
-    {
-        uw->mDownloadComplete = true;
-    }
-    else if (status == DOWNLOAD_STATUS_ERROR ||
-             status == DOWNLOAD_STATUS_CANCELLED)
-    {
-        uw->mDownloadStatus = UPDATE_ERROR;
-    }
-
-    float progress = 0.0f;
-    if (dltotal > 0)
-        progress = static_cast<float>(dlnow) / dltotal;
-
-    uw->setLabel(
-            uw->mCurrentFile + " (" + toString((int) (progress * 100)) + "%)");
-    uw->setProgress(progress);
-
-    if (Client::getState() != STATE_UPDATE)
-    {
-        // If the action was canceled return an error code to stop the mThread
-        return -1;
-    }
-
-    return 0;
+    if (mPlayButton->isEnabled())
+        Client::setState(STATE_LOAD_DATA);
 }
 
-size_t UpdaterWindow::memoryWrite(char *ptr, size_t size, size_t nmemb, void *stream)
+void UpdaterWindow::loadNews()
 {
-    auto *uw = reinterpret_cast<UpdaterWindow *>(stream);
-    const size_t totalMem = size * nmemb;
-    uw->mMemoryBuffer = (char*) realloc(uw->mMemoryBuffer,
-                                        uw->mDownloadedBytes + totalMem);
-    if (uw->mMemoryBuffer)
-    {
-        memcpy(uw->mMemoryBuffer + uw->mDownloadedBytes, ptr, totalMem);
-        uw->mDownloadedBytes += totalMem;
-    }
+    mBrowserBox->clearRows();
+    mBrowserBox->addRows(mDownload->getBuffer());
 
-    return totalMem;
+    mScrollArea->setVerticalScrollAmount(0);
 }
 
-void UpdaterWindow::download()
+void UpdaterWindow::startDownload(const std::string &fileName,
+                                  bool storeInMemory,
+                                  std::optional<unsigned long> adler32)
 {
-    mDownload = std::make_unique<Net::Download>(this,
-                                                mUpdateHost + "/" + mCurrentFile,
-                                                &UpdaterWindow::updateProgress);
+    mDownload = std::make_unique<Net::Download>(mUpdateHost + "/" + fileName);
+    mCurrentFile = fileName;
 
-    if (mStoreInMemory)
-    {
-        mDownload->setWriteFunction(UpdaterWindow::memoryWrite);
-    }
+    if (storeInMemory)
+        mDownload->setUseBuffer();
     else
-    {
-        std::optional<unsigned long> adler32;
-        if (mDownloadStatus == UPDATE_RESOURCES)
-            adler32 = mCurrentChecksum;
-
-        mDownload->setFile(mUpdatesDir + "/" + mCurrentFile, adler32);
-    }
+        mDownload->setFile(mUpdatesDir + "/" + fileName, adler32);
 
-    if (mDownloadStatus != UPDATE_RESOURCES)
+    if (mDialogState != DialogState::DOWNLOAD_RESOURCES)
         mDownload->noCache();
 
-    setLabel(mCurrentFile + " (0%)");
-    mDownloadComplete = false;
-
-    // TODO: check return
     mDownload->start();
 }
 
@@ -359,127 +272,132 @@ void UpdaterWindow::loadUpdates()
 
 void UpdaterWindow::logic()
 {
-    const std::string xmlUpdateFile = "resources.xml";
-    const std::string txtUpdateFile = "resources2.txt";
+    Window::logic();
 
-    // Update Scroll logic
-    mScrollArea->logic();
+    if (mDialogState == DialogState::DONE)
+        return;
 
-    // Synchronize label caption when necessary
-    {
-        MutexLocker lock(&mDownloadMutex);
+    const auto state = mDownload->getState();
+    float progress = 0.0f;
 
-        if (mLabel->getCaption() != mNewLabelCaption)
-        {
-            mLabel->setCaption(mNewLabelCaption);
-            mLabel->adjustSize();
-        }
+    switch (state.status) {
+    case DownloadStatus::IN_PROGRESS: {
+        setLabel(mCurrentFile + " (" + toString((int) (state.progress * 100)) + "%)");
+        progress = state.progress;
+        break;
+    }
+
+    case DownloadStatus::CANCELED:
+        mDialogState = DialogState::DONE;
+
+        enablePlay();
+        setLabel(_("Download canceled"));
+        break;
+
+    case DownloadStatus::ERROR: {
+        mDialogState = DialogState::DONE;
 
-        mProgressBar->setProgress(mDownloadProgress);
+        std::string error = "##1";
+        error += mDownload->getError();
+        error += "\n\n##1";
+        error += _("The update process is incomplete. "
+                   "It is strongly recommended that you try again later.");
+        mBrowserBox->addRows(error);
+
+        int maxScroll = mScrollArea->getVerticalMaxScroll();
+        mScrollArea->setVerticalScrollAmount(maxScroll);
+
+        enablePlay();
+        setLabel(_("Error while downloading"));
+        break;
+    }
+
+    case DownloadStatus::COMPLETE:
+        downloadCompleted();
+        break;
     }
 
-    switch (mDownloadStatus)
+    mProgressBar->setProgress(progress);
+}
+
+void UpdaterWindow::downloadCompleted()
+{
+    switch (mDialogState)
     {
-        case UPDATE_ERROR: {
-            std::string error = "##1";
-            error += mDownload->getError();
-            error += "\n\n";
-            error += _("The update process is incomplete. "
-                       "It is strongly recommended that you try again later.");
-            mBrowserBox->addRows(error);
-
-            mScrollArea->setVerticalScrollAmount(
-                    mScrollArea->getVerticalMaxScroll());
-            mDownloadStatus = UPDATE_COMPLETE;
-            break;
-        }
-        case UPDATE_NEWS:
-            if (mDownloadComplete)
-            {
-                // Parse current memory buffer as news and dispose of the data
-                loadNews();
+    case DialogState::DOWNLOAD_NEWS:
+        loadNews();
 
-                mCurrentFile = xmlUpdateFile;
-                mStoreInMemory = false;
-                mDownloadStatus = UPDATE_LIST;
-                download(); // download() changes mDownloadComplete to false
-            }
-            break;
-        case UPDATE_LIST:
-            if (mDownloadComplete)
+        mDialogState = DialogState::DOWNLOAD_LIST;
+        startDownload(xmlUpdateFile, false);
+        break;
+
+    case DialogState::DOWNLOAD_LIST:
+        if (mCurrentFile == xmlUpdateFile)
+        {
+            mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile);
+            if (mUpdateFiles.empty())
             {
-                if (mCurrentFile == xmlUpdateFile)
-                {
-                    mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile);
-                    if (mUpdateFiles.empty())
-                    {
-                        logger->log("Warning this server does not have a %s"
-                                    " file falling back to %s",
-                                    xmlUpdateFile.c_str(), txtUpdateFile.c_str());
-
-                        // If the resources.xml file fails, fall back onto a older version
-                        mCurrentFile = txtUpdateFile;
-                        mStoreInMemory = false;
-                        mDownloadStatus = UPDATE_LIST;
-                        download();
-                        break;
-                    }
-                }
-                else if (mCurrentFile == txtUpdateFile)
-                {
-                    mUpdateFiles = loadTxtFile(mUpdatesDir + "/" + txtUpdateFile);
-                }
-                mStoreInMemory = false;
-                mDownloadStatus = UPDATE_RESOURCES;
+                logger->log("Warning this server does not have a %s"
+                            " file falling back to %s",
+                            xmlUpdateFile, txtUpdateFile);
+
+                // If the resources.xml file fails, fall back onto a older version
+                mDialogState = DialogState::DOWNLOAD_LIST;
+                startDownload(txtUpdateFile, false);
+                break;
             }
-            break;
-        case UPDATE_RESOURCES:
-            if (mDownloadComplete)
+        }
+        else if (mCurrentFile == txtUpdateFile)
+        {
+            mUpdateFiles = loadTxtFile(mUpdatesDir + "/" + txtUpdateFile);
+        }
+
+        mDialogState = DialogState::DOWNLOAD_RESOURCES;
+        break;
+
+    case DialogState::DOWNLOAD_RESOURCES:
+        if (mUpdateIndex < mUpdateFiles.size())
+        {
+            const UpdateFile &thisFile = mUpdateFiles[mUpdateIndex];
+            if (!thisFile.required)
             {
-                if (mUpdateIndex < mUpdateFiles.size())
+                if (!(thisFile.type == "music" && config.downloadMusic))
                 {
-                    const UpdateFile &thisFile = mUpdateFiles[mUpdateIndex];
-                    if (!thisFile.required)
-                    {
-                        if (!(thisFile.type == "music" && config.downloadMusic))
-                        {
-                            mUpdateIndex++;
-                            break;
-                        }
-                    }
-                    mCurrentFile = thisFile.name;
-                    std::stringstream ss(thisFile.hash);
-                    ss >> std::hex >> mCurrentChecksum;
-
-                    std::string filename = mUpdatesDir + "/" + mCurrentFile;
-                    FILE *file = fopen(filename.c_str(), "r+b");
-
-                    if (!file || Net::Download::fadler32(file) != mCurrentChecksum)
-                    {
-                        if (file)
-                            fclose(file);
-                        download();
-                    }
-                    else
-                    {
-                        fclose(file);
-                        logger->log("%s already here", mCurrentFile.c_str());
-                    }
                     mUpdateIndex++;
-                }
-                else
-                {
-                    // Download of updates completed
-                    mDownloadStatus = UPDATE_COMPLETE;
+                    break;
                 }
             }
-            break;
-        case UPDATE_COMPLETE:
-            enable();
+
+            unsigned long checksum;
+            std::stringstream ss(thisFile.hash);
+            ss >> std::hex >> checksum;
+
+            std::string filename = mUpdatesDir + "/" + thisFile.name;
+            FILE *file = fopen(filename.c_str(), "r+b");
+
+            if (!file || Net::Download::fadler32(file) != checksum)
+            {
+                if (file)
+                    fclose(file);
+                startDownload(thisFile.name, false, checksum);
+            }
+            else
+            {
+                fclose(file);
+                logger->log("%s already here", thisFile.name.c_str());
+            }
+            mUpdateIndex++;
+        }
+        else
+        {
+            // Download of updates completed
+            mDialogState = DialogState::DONE;
+            enablePlay();
             setLabel(_("Completed"));
-            mDownloadStatus = UPDATE_IDLE;
-            break;
-        case UPDATE_IDLE:
-            break;
+        }
+        break;
+
+    case DialogState::DONE:
+        break;
     }
 }
diff --git a/src/gui/updaterwindow.h b/src/gui/updaterwindow.h
index 5c749e18..249da067 100644
--- a/src/gui/updaterwindow.h
+++ b/src/gui/updaterwindow.h
@@ -25,8 +25,6 @@
 
 #include "net/download.h"
 
-#include "utils/mutex.h"
-
 #include <guichan/actionlistener.hpp>
 #include <guichan/keylistener.hpp>
 
@@ -73,27 +71,6 @@ class UpdaterWindow : public Window, public gcn::ActionListener,
 
     ~UpdaterWindow() override;
 
-    /**
-     * Set's progress bar status
-     */
-    void setProgress(float progress);
-
-    /**
-     * Set's label above progress
-     */
-    void setLabel(const std::string &);
-
-    /**
-     * Enables play button
-     */
-    void enable();
-
-    /**
-     * Loads and display news. Assumes the news file contents have been loaded
-     * into the memory buffer.
-     */
-    void loadNews();
-
     void action(const gcn::ActionEvent &event) override;
 
     void keyPressed(gcn::KeyEvent &keyEvent) override;
@@ -101,38 +78,38 @@ class UpdaterWindow : public Window, public gcn::ActionListener,
     void logic() override;
 
 private:
-    void download();
+    bool cancel();
+    void play();
 
-    /**
-     * Loads the updates this window has gotten into the resource manager
-     */
-    void loadUpdates();
+    void setLabel(const std::string &);
+    void enablePlay();
 
+    void startDownload(const std::string &fileName,
+                       bool storeInMemory,
+                       std::optional<unsigned long> adler32 = {});
+    void downloadCompleted();
 
     /**
-     * A download callback for progress updates.
+     * Loads and display news. Assumes the news file contents have been loaded
+     * into the memory buffer.
      */
-    static int updateProgress(void *ptr, DownloadStatus status,
-                              size_t dltotal, size_t dlnow);
+    void loadNews();
 
     /**
-     * A libcurl callback for writing to memory.
+     * Loads the updates this window has gotten into the resource manager
      */
-    static size_t memoryWrite(char *ptr, size_t size, size_t nmemb,
-                              void *stream);
+    void loadUpdates();
 
-    enum UpdateDownloadStatus
+    enum class DialogState
     {
-        UPDATE_ERROR,
-        UPDATE_IDLE,
-        UPDATE_LIST,
-        UPDATE_COMPLETE,
-        UPDATE_NEWS,
-        UPDATE_RESOURCES
+        DOWNLOAD_NEWS,
+        DOWNLOAD_LIST,
+        DOWNLOAD_RESOURCES,
+        DONE,
     };
 
     /** Status of the current download. */
-    UpdateDownloadStatus mDownloadStatus = UPDATE_NEWS;
+    DialogState mDialogState = DialogState::DOWNLOAD_NEWS;
 
     /** Host where we get the updated files. */
     std::string mUpdateHost;
@@ -143,33 +120,6 @@ private:
     /** The file currently downloading. */
     std::string mCurrentFile;
 
-    /** The new label caption to be set in the logic method. */
-    std::string mNewLabelCaption;
-
-    /** The new progress value to be set in the logic method. */
-    float mDownloadProgress = 0.0f;
-
-    /** The mutex used to guard access to mNewLabelCaption and mDownloadProgress. */
-    Mutex mDownloadMutex;
-
-    /** The Adler32 checksum of the file currently downloading. */
-    unsigned long mCurrentChecksum = 0;
-
-    /** A flag to indicate whether to use a memory buffer or a regular file. */
-    bool mStoreInMemory = true;
-
-    /** Flag that show if current download is complete. */
-    bool mDownloadComplete = true;
-
-    /** Flag that show if the user has canceled the update. */
-    bool mUserCancel = false;
-
-    /** Byte count currently downloaded in mMemoryBuffer. */
-    size_t mDownloadedBytes = 0;
-
-    /** Buffer for files downloaded to memory. */
-    char *mMemoryBuffer = nullptr;
-
     /** Download handle. */
     std::unique_ptr<Net::Download> mDownload;