Some cleanups in UpdaterWindow and BrowserBox

Doing some cleanups before working towards optimizing this code.

Removed needless additional wrapping code in BrowserBox::addRow, since
the text will be relayouted anyway.

Simplified layouting code a little. For example, there's no need to keep
track of the number of wrapped lines.

Use more optimal data structures, like an std::deque for the text rows
and a plain std::vector for the line parts. Both have less fragmentation
than an std::list.

4 files changed, 79 insertions, 170 deletions

diff --git a/src/gui/updaterwindow.cpp b/src/gui/updaterwindow.cpp
index bedaa9da..b26119e6 100644
--- a/src/gui/updaterwindow.cpp
+++ b/src/gui/updaterwindow.cpp
@@ -51,9 +51,9 @@ const std::string txtUpdateFile = "resources2.txt";
 /**
  * Load the given file into a vector of updateFiles.
  */
-std::vector<updateFile> loadXMLFile(const std::string &fileName)
+std::vector<UpdateFile> loadXMLFile(const std::string &fileName)
 {
-    std::vector<updateFile> files;
+    std::vector<UpdateFile> files;
     XML::Document doc(fileName, false);
     xmlNodePtr rootNode = doc.rootNode();
 
@@ -69,15 +69,12 @@ std::vector<updateFile> loadXMLFile(const std::string &fileName)
         if (!xmlStrEqual(fileNode->name, BAD_CAST "update"))
             continue;
 
-        updateFile file;
+        UpdateFile file;
         file.name = XML::getProperty(fileNode, "file", "");
         file.hash = XML::getProperty(fileNode, "hash", "");
         file.type = XML::getProperty(fileNode, "type", "data");
         file.desc = XML::getProperty(fileNode, "description", "");
-        if (XML::getProperty(fileNode, "required", "yes") == "yes")
-            file.required = true;
-        else
-            file.required = false;
+        file.required = XML::getProperty(fileNode, "required", "yes") == "yes";
 
         files.push_back(file);
     }
@@ -85,9 +82,9 @@ std::vector<updateFile> loadXMLFile(const std::string &fileName)
     return files;
 }
 
-std::vector<updateFile> loadTxtFile(const std::string &fileName)
+std::vector<UpdateFile> loadTxtFile(const std::string &fileName)
 {
-    std::vector<updateFile> files;
+    std::vector<UpdateFile> files;
     std::ifstream fileHandler;
     fileHandler.open(fileName.c_str(), std::ios::in);
 
@@ -99,7 +96,7 @@ std::vector<updateFile> loadTxtFile(const std::string &fileName)
             fileHandler.getline(name, 256, ' ');
             fileHandler.getline(hash, 50);
 
-            updateFile thisFile;
+            UpdateFile thisFile;
             thisFile.name = name;
             thisFile.hash = hash;
             thisFile.type = "data";
@@ -179,11 +176,11 @@ UpdaterWindow::~UpdaterWindow()
     free(mMemoryBuffer);
 }
 
-void UpdaterWindow::setProgress(float p)
+void UpdaterWindow::setProgress(float progress)
 {
     // Do delayed progress bar update, since Guichan isn't thread-safe
     MutexLocker lock(&mDownloadMutex);
-    mDownloadProgress = p;
+    mDownloadProgress = progress;
 }
 
 void UpdaterWindow::setLabel(const std::string &str)
@@ -359,10 +356,11 @@ void UpdaterWindow::loadUpdates()
 {
     ResourceManager *resman = ResourceManager::getInstance();
 
-    if (!mUpdateFiles.size())
-    {   // updates not downloaded
+    if (mUpdateFiles.empty())
+    {
+        // updates not downloaded
         mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile);
-        if (!mUpdateFiles.size())
+        if (mUpdateFiles.empty())
         {
             logger->log("Warning this server does not have a"
                         " %s file falling back to %s", xmlUpdateFile.c_str(),
@@ -371,10 +369,9 @@ void UpdaterWindow::loadUpdates()
         }
     }
 
-    for (mUpdateIndex = 0; mUpdateIndex < mUpdateFiles.size(); mUpdateIndex++)
+    for (const UpdateFile &file : mUpdateFiles)
     {
-        resman->addToSearchPath(mUpdatesDir + "/"
-                                + mUpdateFiles[mUpdateIndex].name, false);
+        resman->addToSearchPath(mUpdatesDir + "/" + file.name, false);
     }
 }
 
@@ -435,7 +432,7 @@ void UpdaterWindow::logic()
                 if (mCurrentFile == xmlUpdateFile)
                 {
                     mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile);
-                    if (mUpdateFiles.size() == 0)
+                    if (mUpdateFiles.empty())
                     {
                         logger->log("Warning this server does not have a %s"
                                     " file falling back to %s",
@@ -462,7 +459,7 @@ void UpdaterWindow::logic()
             {
                 if (mUpdateIndex < mUpdateFiles.size())
                 {
-                    updateFile thisFile = mUpdateFiles[mUpdateIndex];
+                    UpdateFile thisFile = mUpdateFiles[mUpdateIndex];
                     if (!thisFile.required)
                     {
                         // This statement checks to see if the file type is music, and if download-music is true
diff --git a/src/gui/updaterwindow.h b/src/gui/updaterwindow.h
index 6c69cd9b..2d1b059b 100644
--- a/src/gui/updaterwindow.h
+++ b/src/gui/updaterwindow.h
@@ -39,14 +39,13 @@ class Button;
 class ProgressBar;
 class ScrollArea;
 
-struct updateFile
+struct UpdateFile
 {
-    public:
-        std::string name;
-        std::string hash;
-        std::string type;
-        bool required;
-        std::string desc;
+    std::string name;
+    std::string hash;
+    std::string type;
+    bool required;
+    std::string desc;
 };
 
 /**
@@ -76,7 +75,7 @@ class UpdaterWindow : public Window, public gcn::ActionListener,
     /**
      * Set's progress bar status
      */
-    void setProgress(float p);
+    void setProgress(float progress);
 
     /**
      * Set's label above progress
@@ -176,7 +175,7 @@ private:
     Net::Download *mDownload = nullptr;
 
     /** List of files to download. */
-    std::vector<updateFile> mUpdateFiles;
+    std::vector<UpdateFile> mUpdateFiles;
 
     /** Index of the file to be downloaded. */
     unsigned int mUpdateIndex = 0;
diff --git a/src/gui/widgets/browserbox.cpp b/src/gui/widgets/browserbox.cpp
index bc424962..7b5d40bb 100644
--- a/src/gui/widgets/browserbox.cpp
+++ b/src/gui/widgets/browserbox.cpp
@@ -164,61 +164,8 @@ void BrowserBox::addRow(const std::string &row)
             setWidth(w);
     }
 
-    // TODO: Optimize! There's no point in wrapping all text rows again, just
-    // do the one that was just added (but take into account discarded rows?).
-    if (mMode == AUTO_WRAP)
-    {
-        unsigned int wrapCount = 0;
-        unsigned int nextChar;
-        const char *tilde = "~";
-        int tildeWidth = font->getWidth(tilde);
-        int x = 0;
-
-        for (auto row : mTextRows)
-        {
-            for (unsigned int j = 0; j < row.size(); ++j)
-            {
-                std::string character = row.substr(j, 1);
-                x += font->getWidth(character);
-                nextChar = j + 1;
-
-                // Wraping between words (at blank spaces)
-                if (nextChar < row.size() && row.at(nextChar) == ' ')
-                {
-                    int nextSpacePos = static_cast<int>(
-                        row.find(" ", (nextChar + 1)));
-                    if (nextSpacePos <= 0)
-                        nextSpacePos = static_cast<int>(row.size()) - 1;
-
-                    int nextWordWidth = font->getWidth(
-                            row.substr(nextChar,
-                                (nextSpacePos - nextChar)));
-
-                    if (x + nextWordWidth + 10 > getWidth())
-                    {
-                        x = 15; // Indent in new line
-                        ++wrapCount;
-                        ++j;
-                    }
-                }
-                // Wrapping looong lines (brutal force)
-                else if (x + 2 * tildeWidth > getWidth())
-                {
-                    x = 15; // Ident in new line
-                    ++wrapCount;
-                }
-            }
-        }
-
-        setHeight(lineHeight * (mTextRows.size() + wrapCount - 1)
-                  + fontHeight);
-    }
-    else
-    {
-        setHeight(lineHeight * (mTextRows.size() - 1) + fontHeight);
-    }
     mUpdateTime = 0;
-    updateHeight();
+    maybeRelayoutText();
 }
 
 void BrowserBox::clearRows()
@@ -228,7 +175,7 @@ void BrowserBox::clearRows()
     setWidth(0);
     setHeight(0);
     mSelectedLink = -1;
-    updateHeight();
+    maybeRelayoutText();
 }
 
 struct MouseOverLink
@@ -269,14 +216,14 @@ void BrowserBox::mouseMoved(gcn::MouseEvent &event)
 
 void BrowserBox::draw(gcn::Graphics *graphics)
 {
-    gcn::ClipRectangle cr = graphics->getCurrentClipArea();
+    const gcn::ClipRectangle &cr = graphics->getCurrentClipArea();
     mYStart = cr.y - cr.yOffset;
     int yEnd = mYStart + cr.height;
     if (mYStart < 0)
         mYStart = 0;
 
-    if (getWidth() != mWidth)
-        updateHeight();
+    if (getWidth() != mLastLayoutWidth)
+        maybeRelayoutText();
 
     if (mOpaque)
     {
@@ -308,11 +255,11 @@ void BrowserBox::draw(gcn::Graphics *graphics)
         }
     }
 
-    for (auto &part : mLineParts)
+    for (const auto &part : mLineParts)
     {
-        if (part.getY() + 50 < mYStart)
+        if (part.y + 50 < mYStart)
             continue;
-        if (part.getY() > yEnd)
+        if (part.y > yEnd)
             break;
 
         // Use the correct font
@@ -322,69 +269,64 @@ void BrowserBox::draw(gcn::Graphics *graphics)
         if (mShadows)
         {
             graphics->setColor(Theme::getThemeColor(Theme::SHADOW,
-                                                    part.getColor().a / 2));
+                                                    part.color.a / 2));
+
             if (mOutline)
-            {
-                graphics->drawText(part.getText(), part.getX() + 2,
-                                   part.getY() + 2);
-            }
+                graphics->drawText(part.text, part.x + 2, part.y + 2);
             else
-            {
-                graphics->drawText(part.getText(), part.getX() + 1,
-                                   part.getY() + 1);
-            }
+                graphics->drawText(part.text, part.x + 1, part.y + 1);
         }
 
         if (mOutline)
         {
             // Text outline
             graphics->setColor(Theme::getThemeColor(Theme::OUTLINE,
-                                                    part.getColor().a / 4));
-            graphics->drawText(part.getText(), part.getX() + 1, part.getY());
-            graphics->drawText(part.getText(), part.getX() - 1, part.getY());
-            graphics->drawText(part.getText(), part.getX(), part.getY() + 1);
-            graphics->drawText(part.getText(), part.getX(), part.getY() - 1);
+                                                    part.color.a / 4));
+            graphics->drawText(part.text, part.x + 1, part.y);
+            graphics->drawText(part.text, part.x - 1, part.y);
+            graphics->drawText(part.text, part.x, part.y + 1);
+            graphics->drawText(part.text, part.x, part.y - 1);
         }
 
         // the main text
-        graphics->setColor(part.getColor());
-        graphics->drawText(part.getText(), part.getX(), part.getY());
+        graphics->setColor(part.color);
+        graphics->drawText(part.text, part.x, part.y);
     }
-
-    return;
 }
 
-int BrowserBox::calcHeight()
+/**
+ * Relayouts all text rows.
+ */
+void BrowserBox::relayoutText()
 {
-    int x = 0, y = 0;
-    int wrappedLines = 0;
+    int y = 0;
     unsigned link = 0;
-    gcn::Font *font = getFont();
+    const gcn::Font *font = getFont();
 
     const int fontHeight = font->getHeight();
     const int minusWidth = font->getWidth("-");
     const int tildeWidth = font->getWidth("~");
 
     int lineHeight = fontHeight;
-    if (auto *ttf = dynamic_cast<TrueTypeFont*>(font))
-        lineHeight = ttf->getLineHeight();
+    if (auto *trueTypeFont = dynamic_cast<const TrueTypeFont*>(font))
+        lineHeight = trueTypeFont->getLineHeight();
 
     gcn::Color selColor = Theme::getThemeColor(Theme::TEXT);
     const gcn::Color &textColor = Theme::getThemeColor(Theme::TEXT);
 
     mLineParts.clear();
 
-    for (auto row : mTextRows)
+    for (const auto &row : mTextRows)
     {
         bool wrapped = false;
-        x = 0;
+        int x = 0;
 
         // Check for separator lines
         if (row.find("---", 0) == 0)
         {
             for (x = 0; x < getWidth(); x++)
             {
-                mLineParts.push_back(LinePart(x, y, selColor, "-"));
+                mLineParts.push_back(LinePart { x, y, selColor, "-" });
                 x += minusWidth - 2;
             }
 
@@ -438,7 +380,6 @@ int BrowserBox::calcHeight()
                     }
                     else
                     {
-
                         switch (c)
                         {
                             case '1': selColor = RED; break;
@@ -456,6 +397,7 @@ int BrowserBox::calcHeight()
                         }
                     }
 
+                    // Update the position of the links
                     if (c == '<' && link < mLinks.size())
                     {
                         const int size =
@@ -520,8 +462,8 @@ int BrowserBox::calcHeight()
                 if (forced)
                 {
                     x -= tildeWidth; // Remove the wrap-notifier accounting
-                    mLineParts.push_back(LinePart(getWidth() - tildeWidth,
-                                                  y, selColor, "~"));
+                    mLineParts.push_back(LinePart { getWidth() - tildeWidth,
+                                                    y, selColor, "~" });
                     end++; // Skip to the next character
                 }
                 else
@@ -530,10 +472,9 @@ int BrowserBox::calcHeight()
                 }
 
                 wrapped = true;
-                wrappedLines++;
             }
 
-            mLineParts.push_back(LinePart(x, y, selColor, part));
+            mLineParts.push_back(LinePart { x, y, selColor, part });
 
             const int partWidth = font->getWidth(part);
             if (mMode == AUTO_WRAP && partWidth == 0)
@@ -541,19 +482,20 @@ int BrowserBox::calcHeight()
 
             x += partWidth;
         }
+
         y += lineHeight;
     }
-    return (mTextRows.size() + wrappedLines - 1) * lineHeight + fontHeight;
+
+    mLastLayoutWidth = getWidth();
+    setHeight(y);
 }
 
-void BrowserBox::updateHeight()
+void BrowserBox::maybeRelayoutText()
 {
     if (mAlwaysUpdate || !mUpdateTime || std::abs(mUpdateTime - tick_time) > 10
         || mTextRows.size() < 3)
     {
-        mWidth = getWidth();
-        mHeight = calcHeight();
-        setHeight(mHeight);
+        relayoutText();
         mUpdateTime = tick_time;
     }
 }
diff --git a/src/gui/widgets/browserbox.h b/src/gui/widgets/browserbox.h
index 6d09f77b..83cdef7c 100644
--- a/src/gui/widgets/browserbox.h
+++ b/src/gui/widgets/browserbox.h
@@ -26,7 +26,7 @@
 #include <guichan/mouselistener.hpp>
 #include <guichan/widget.hpp>
 
-#include <list>
+#include <deque>
 #include <vector>
 
 class LinkHandler;
@@ -38,30 +38,12 @@ struct BrowserLink
     std::string caption;
 };
 
-class LinePart
+struct LinePart
 {
-    public:
-        LinePart(int x, int y, gcn::Color color, std::string text) :
-            mX(x), mY(y), mColor(color), mText(text)
-        {
-        }
-
-        int getX() const
-        { return mX; }
-
-        int getY() const
-        { return mY; }
-
-        const std::string &getText() const
-        { return mText; }
-
-        const gcn::Color &getColor() const
-        { return mColor; }
-
-    private:
-        int mX, mY;
-        gcn::Color mColor;
-        std::string mText;
+    int x;
+    int y;
+    gcn::Color color;
+    std::string text;
 };
 
 /**
@@ -132,14 +114,14 @@ class BrowserBox : public gcn::Widget,
          */
         void draw(gcn::Graphics *graphics) override;
 
-        void updateHeight();
+        void maybeRelayoutText();
 
         /**
          * BrowserBox modes.
          */
         enum
         {
-            AUTO_SIZE = 0,
+            AUTO_SIZE,
             AUTO_WRAP       /**< Maybe it needs a fix or to be redone. */
         };
 
@@ -176,27 +158,17 @@ class BrowserBox : public gcn::Widget,
             BACKGROUND = 2
         };
 
-        using TextRows = std::list<std::string>;
-
-        TextRows &getRows()
-        { return mTextRows; }
-
         void setAlwaysUpdate(bool n)
         { mAlwaysUpdate = n; }
 
     private:
-        int calcHeight();
-
-        using TextRowIterator = TextRows::iterator;
-        TextRows mTextRows;
+        void relayoutText();
+        int layoutTextRow(const std::string &row, int y);
 
-        using LinePartList = std::list<LinePart>;
-        using LinePartIterator = LinePartList::iterator;
-        LinePartList mLineParts;
+        std::deque<std::string> mTextRows;
 
-        using Links = std::vector<BrowserLink>;
-        using LinkIterator = Links::iterator;
-        Links mLinks;
+        std::vector<LinePart> mLineParts;
+        std::vector<BrowserLink> mLinks;
 
         LinkHandler *mLinkHandler = nullptr;
         unsigned int mMode;
@@ -207,8 +179,7 @@ class BrowserBox : public gcn::Widget,
         bool mUseLinksAndUserColors = true;
         int mSelectedLink = -1;
         unsigned int mMaxRows = 0;
-        int mHeight = 0;
-        int mWidth = 0;
+        int mLastLayoutWidth = 0;
         int mYStart = 0;
         int mUpdateTime = -1;
         bool mAlwaysUpdate = true;