5 files changed, 268 insertions, 98 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index a596c542..7681b482 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -3229,7 +3229,7 @@ void MainWindow::displayModInformation(
 
 	  //Open the tab first if we want to use the standard indexes of the tabs.
 	  if (tabID != ModInfoTabIDs::None) {
-		  dialog.setTab(tabID);
+		  dialog.selectTab(tabID);
 	  }
 
     dialog.restoreState(m_OrganizerCore.settings());
diff --git a/src/modinfodialog.cpp b/src/modinfodialog.cpp
index c8ffa35b..adbd22df 100644
--- a/src/modinfodialog.cpp
+++ b/src/modinfodialog.cpp
@@ -170,8 +170,11 @@ ModInfoDialog::ModInfoDialog(
       [&, id=tabInfo.tab->tabID()]{ switchToTab(id); });
   }
 
-  connect(ui->tabWidget, &QTabWidget::currentChanged, [&]{ onTabChanged(); });
+  connect(ui->tabWidget, &QTabWidget::currentChanged, [&]{ onTabSelectionChanged(); });
   connect(ui->tabWidget->tabBar(), &QTabBar::tabMoved, [&]{ onTabMoved(); });
+  connect(ui->close, &QPushButton::clicked, [&]{ onCloseButton(); });
+  connect(ui->previousMod, &QPushButton::clicked, [&]{ onPreviousMod(); });
+  connect(ui->nextMod, &QPushButton::clicked, [&]{ onNextMod(); });
 }
 
 ModInfoDialog::~ModInfoDialog() = default;
@@ -242,7 +245,7 @@ void ModInfoDialog::setMod(const QString& name)
   setMod(mod);
 }
 
-void ModInfoDialog::setTab(ModInfoTabIDs id)
+void ModInfoDialog::selectTab(ModInfoTabIDs id)
 {
   if (!isVisible()) {
     m_initialTab = id;
@@ -323,6 +326,10 @@ void ModInfoDialog::setTabsVisibility(bool firstTime)
     return;
   }
 
+  // something changed; save the current order (if necessary) because some tabs
+  // will be removed and others added
+  saveTabOrder(Settings::instance());
+
   // remember selection
   const int selIndex = ui->tabWidget->currentIndex();
   auto sel = ModInfoTabIDs::None;
@@ -337,6 +344,68 @@ void ModInfoDialog::setTabsVisibility(bool firstTime)
   reAddTabs(visibility, sel);
 }
 
+void ModInfoDialog::reAddTabs(
+  const std::vector<bool>& visibility, ModInfoTabIDs sel)
+{
+  Q_ASSERT(visibility.size() == m_tabs.size());
+
+  // ordered tab names from settings
+  const auto orderedNames = getOrderedTabNames();
+
+  bool canSort = true;
+
+  // gathering visible tabs
+  std::vector<TabInfo*> visibleTabs;
+  for (std::size_t i=0; i<m_tabs.size(); ++i) {
+    if (visibility[i]) {
+      visibleTabs.push_back(&m_tabs[i]);
+
+      if (canSort) {
+        const auto objectName = m_tabs[i].widget->objectName();
+        auto itor = std::find(orderedNames.begin(), orderedNames.end(), objectName);
+        if (itor == orderedNames.end()) {
+          qCritical() << "can't sort tabs, '" << objectName << "' not found";
+          canSort = false;
+        }
+      }
+    }
+  }
+
+  // sorting tabs
+  if (canSort) {
+    std::sort(visibleTabs.begin(), visibleTabs.end(), [&](auto&& a, auto&& b){
+      auto aItor = std::find(orderedNames.begin(), orderedNames.end(), a->widget->objectName());
+      auto bItor = std::find(orderedNames.begin(), orderedNames.end(), b->widget->objectName());
+
+      // this was checked above
+      Q_ASSERT(aItor != orderedNames.end() && bItor != orderedNames.end());
+
+      return (aItor < bItor);
+    });
+  }
+
+
+  // removing all tabs
+  ui->tabWidget->clear();
+
+  // reset real positions
+  for (auto& tabInfo : m_tabs) {
+    tabInfo.realPos = -1;
+  }
+
+  // add visible tabs
+  for (std::size_t i=0; i<visibleTabs.size(); ++i) {
+    auto& tabInfo = *visibleTabs[i];
+
+    tabInfo.realPos = static_cast<int>(i);
+    ui->tabWidget->addTab(tabInfo.widget, tabInfo.icon, tabInfo.caption);
+
+    if (tabInfo.tab->tabID() == sel) {
+      ui->tabWidget->setCurrentIndex(static_cast<int>(i));
+    }
+  }
+}
+
 void ModInfoDialog::updateTabs(bool becauseOriginChanged)
 {
   auto* origin = getOrigin();
@@ -440,10 +509,7 @@ MOShared::FilesOrigin* ModInfoDialog::getOrigin()
 
 void ModInfoDialog::saveState(Settings& s) const
 {
-  const auto tabState = saveTabState();
-  if (!tabState.isEmpty()) {
-    s.directInterface().setValue("mod_info_tab_order", tabState);
-  }
+  saveTabOrder(s);
 
   // remove 2.2.0 settings
   s.directInterface().remove("mod_info_tabs");
@@ -466,21 +532,24 @@ void ModInfoDialog::restoreState(const Settings& s)
   }
 }
 
-QString ModInfoDialog::saveTabState() const
+void ModInfoDialog::saveTabOrder(Settings& s) const
 {
   if (static_cast<int>(m_tabs.size()) != ui->tabWidget->count()) {
     // only save tab state when all tabs are visible
-    return {};
+    return;
   }
 
-  QString result;
-  QTextStream stream(&result);
+  QString names;
 
   for (int i=0; i<ui->tabWidget->count(); ++i) {
-    stream << ui->tabWidget->widget(i)->objectName() << " ";
+    if (!names.isEmpty()) {
+      names += " ";
+    }
+
+    names += ui->tabWidget->widget(i)->objectName();
   }
 
-  return result.trimmed();
+  s.directInterface().setValue("mod_info_tab_order", names);
 }
 
 std::vector<QString> ModInfoDialog::getOrderedTabNames() const
@@ -516,68 +585,6 @@ std::vector<QString> ModInfoDialog::getOrderedTabNames() const
   return v;
 }
 
-void ModInfoDialog::reAddTabs(
-  const std::vector<bool>& visibility, ModInfoTabIDs sel)
-{
-  Q_ASSERT(visibility.size() == m_tabs.size());
-
-  // ordered tab names from settings
-  const auto orderedNames = getOrderedTabNames();
-
-  bool canSort = true;
-
-  // gathering visible tabs
-  std::vector<TabInfo*> visibleTabs;
-  for (std::size_t i=0; i<m_tabs.size(); ++i) {
-    if (visibility[i]) {
-      visibleTabs.push_back(&m_tabs[i]);
-
-      if (canSort) {
-        const auto objectName = m_tabs[i].widget->objectName();
-        auto itor = std::find(orderedNames.begin(), orderedNames.end(), objectName);
-        if (itor == orderedNames.end()) {
-          qCritical() << "can't sort tabs, '" << objectName << "' not found";
-          canSort = false;
-        }
-      }
-    }
-  }
-
-  // sorting tabs
-  if (canSort) {
-    std::sort(visibleTabs.begin(), visibleTabs.end(), [&](auto&& a, auto&& b){
-      auto aItor = std::find(orderedNames.begin(), orderedNames.end(), a->widget->objectName());
-      auto bItor = std::find(orderedNames.begin(), orderedNames.end(), b->widget->objectName());
-
-      // this was checked above
-      Q_ASSERT(aItor != orderedNames.end() && bItor != orderedNames.end());
-
-      return (aItor < bItor);
-    });
-  }
-
-
-  // removing all tabs
-  ui->tabWidget->clear();
-
-  // reset real positions
-  for (auto& tabInfo : m_tabs) {
-    tabInfo.realPos = -1;
-  }
-
-  // add visible tabs
-  for (std::size_t i=0; i<visibleTabs.size(); ++i) {
-    auto& tabInfo = *visibleTabs[i];
-
-    tabInfo.realPos = static_cast<int>(i);
-    ui->tabWidget->addTab(tabInfo.widget, tabInfo.icon, tabInfo.caption);
-
-    if (tabInfo.tab->tabID() == sel) {
-      ui->tabWidget->setCurrentIndex(static_cast<int>(i));
-    }
-  }
-}
-
 void ModInfoDialog::onOriginModified(int originID)
 {
   emit originModified(originID);
@@ -600,7 +607,7 @@ void ModInfoDialog::closeEvent(QCloseEvent* e)
   }
 }
 
-void ModInfoDialog::on_closeButton_clicked()
+void ModInfoDialog::onCloseButton()
 {
   if (tryClose()) {
     close();
@@ -618,7 +625,7 @@ bool ModInfoDialog::tryClose()
   return true;
 }
 
-void ModInfoDialog::onTabChanged()
+void ModInfoDialog::onTabSelectionChanged()
 {
   if (m_arrangingTabs) {
     // this can be fired while re-arranging tabs, which happens before mods
@@ -660,7 +667,7 @@ void ModInfoDialog::onTabMoved()
   }
 }
 
-void ModInfoDialog::on_nextButton_clicked()
+void ModInfoDialog::onNextMod()
 {
   auto mod = m_mainWindow->nextModInList();
   if (!mod || mod == m_mod) {
@@ -671,7 +678,7 @@ void ModInfoDialog::on_nextButton_clicked()
   update();
 }
 
-void ModInfoDialog::on_prevButton_clicked()
+void ModInfoDialog::onPreviousMod()
 {
   auto mod = m_mainWindow->previousModInList();
   if (!mod || mod == m_mod) {
diff --git a/src/modinfodialog.h b/src/modinfodialog.h
index effb5d98..035eb6d6 100644
--- a/src/modinfodialog.h
+++ b/src/modinfodialog.h
@@ -43,6 +43,9 @@ class ModInfoDialog : public MOBase::TutorableDialog
 {
   Q_OBJECT;
 
+  // creates a tab, it's a friend because it uses a bunch of member variables
+  // to create ModInfoDialogTabContext
+  //
   template <class T>
   friend std::unique_ptr<ModInfoDialogTab> createTab(
     ModInfoDialog& d, ModInfoTabIDs index);
@@ -54,37 +57,67 @@ public:
 
   ~ModInfoDialog();
 
-  void setMod(ModInfo::Ptr mod);
-  void setMod(const QString& name);
-
-  void setTab(ModInfoTabIDs id);
+  // switches to the tab with the given id
+  //
+  void selectTab(ModInfoTabIDs id);
 
+  // updates all tabs, selects the initial tab and opens the dialog
+  //
   int exec() override;
 
+  // saves the dialog state and calls saveState() on all tabs
+  //
   void saveState(Settings& s) const;
+
+  // restores the dialog state and calls restoreState() on all tabs
+  //
   void restoreState(const Settings& s);
 
 signals:
+  // emitted when a tab changes the origin
+  //
   void originModified(int originID);
 
 protected:
+  // forwards to tryClose()
+  //
   void closeEvent(QCloseEvent* e);
 
-private slots:
-  void on_closeButton_clicked();
-  void on_nextButton_clicked();
-  void on_prevButton_clicked();
-
 private:
+  // represents a single tab
+  //
   struct TabInfo
   {
+    // tab implementation
     std::unique_ptr<ModInfoDialogTab> tab;
+
+    // actual position in the tab bar, updated every time a tab is moved
     int realPos;
+
+    // widget used by the QTabWidget for this tab
+    //
+    // because QTabWidget doesn't support simply hiding tabs, they have to be
+    // completely removed from the widget when they don't support the current
+    // mod
+    //
+    // therefore, `widget, `caption` and `icon` are remembered so tabs can be
+    // removed and re-added when navigating between mods
+    //
+    // `widget` is also used figure out which tab is where when they're
+    // re-ordered
     QWidget* widget;
+
+    // caption for this tab, see `widget`
     QString caption;
+
+    // icon for this tab, see `widget`
     QIcon icon;
 
+
     TabInfo(std::unique_ptr<ModInfoDialogTab> tab);
+
+    // returns whether this tab is part of the tab widget
+    //
     bool isVisible() const;
   };
 
@@ -94,28 +127,99 @@ private:
   OrganizerCore* m_core;
   PluginContainer* m_plugin;
   std::vector<TabInfo> m_tabs;
+
+  // initial tab requested by the main window when the dialog is opened; whether
+  // the request can be honoured depends on what tabs are present
   ModInfoTabIDs m_initialTab;
+
+  // set to true when tabs are being removed and re-added while navigating
+  // between mods; since the current index changes while this is happening,
+  // onTabSelectionChanged() will be called repeatedly
+  //
+  // however, it will check this flag and ignore the event so first activations
+  // are not fired incorrectly
   bool m_arrangingTabs;
 
+
+  // creates all the tabs
+  //
   std::vector<TabInfo> createTabs();
+
+
+  // sets the currently selected mod; resets first activation, but doesn't
+  // update anything
+  //
+  void setMod(ModInfo::Ptr mod);
+
+  // sets the currently selected mod, if found; forwards to setMod() above
+  //
+  void setMod(const QString& name);
+
+
+  // returns the currently selected tab, taking re-ordering in to account;
+  // shouldn't be null, but could be
+  //
   TabInfo* currentTab();
-  void restoreTabState(const QString& state);
-  QString saveTabState() const;
+
+  // returns a list of tab object names in their visual order, used by
+  // saveState()
+  //
+  void saveTabOrder(Settings& s) const;
+
+
+  // fully updates the dialog; sets the title, the tab visibility and updates
+  // all the tabs; used when the current mod changes
+  //
+  // see setTabsVisibility() for firstTime
+  //
   void update(bool firstTime=false);
+
+  // builds the list of visible tabs; if the list is different from what's
+  // currently displayed, or firstTime is true, forwards to reAddTabs()
+  void setTabsVisibility(bool firstTime);
+
+  // clears the tab widgets and re-adds the tabs having the visible flag in
+  // the given vector, following the
+  void reAddTabs(const std::vector<bool>& visibility, ModInfoTabIDs sel);
+
   void onDeleteShortcut();
   MOShared::FilesOrigin* getOrigin();
-  void setTabsVisibility(bool firstTime);
   void updateTabs(bool becauseOriginChanged=false);
   void feedFiles(bool becauseOriginChanged);
   void setTabsColors();
   void switchToTab(ModInfoTabIDs id);
-  void reAddTabs(const std::vector<bool>& visibility, ModInfoTabIDs sel);
   std::vector<QString> getOrderedTabNames() const;
   bool tryClose();
 
-  void onOriginModified(int originID);
-  void onTabChanged();
+
+  // called when the user clicks the close button; closing the dialog by other
+  // means ends up in closeEvent(); forwards to tryClose()
+  //
+  void onCloseButton();
+
+  // called when the user clicks the previous button; asks the main window for
+  // the previous mod in the list and loads it
+  //
+  void onPreviousMod();
+
+  // called when the user clicks the next button; asks the main window for the
+  // next mod in the list and loads it
+  //
+  void onNextMod();
+
+  // called when the selects a tab; handles first activation
+  //
+  void onTabSelectionChanged();
+
+  // called when the user re-orders tabs; sets the correct TabInfo::realPos for
+  // all tabs
+  //
   void onTabMoved();
+
+  // called when a tab has modified the origin; emits originModified() and
+  // updates all the tabs that use origin files
+  //
+  void onOriginModified(int originID);
 };
 
 #endif // MODINFODIALOG_H
diff --git a/src/modinfodialog.ui b/src/modinfodialog.ui
index e54b94b3..5b559992 100644
--- a/src/modinfodialog.ui
+++ b/src/modinfodialog.ui
@@ -1260,7 +1260,7 @@ p, li { white-space: pre-wrap; }
    <item>
     <layout class="QHBoxLayout" name="horizontalLayout">
      <item>
-      <widget class="QPushButton" name="prevButton">
+      <widget class="QPushButton" name="previousMod">
        <property name="text">
         <string>Previous</string>
        </property>
@@ -1270,7 +1270,7 @@ p, li { white-space: pre-wrap; }
       </widget>
      </item>
      <item>
-      <widget class="QPushButton" name="nextButton">
+      <widget class="QPushButton" name="nextMod">
        <property name="text">
         <string>Next</string>
        </property>
@@ -1293,7 +1293,7 @@ p, li { white-space: pre-wrap; }
       </spacer>
      </item>
      <item>
-      <widget class="QPushButton" name="closeButton">
+      <widget class="QPushButton" name="close">
        <property name="text">
         <string>Close</string>
        </property>
diff --git a/src/modinfodialogtab.h b/src/modinfodialogtab.h
index 283d9e73..c43fa076 100644
--- a/src/modinfodialogtab.h
+++ b/src/modinfodialogtab.h
@@ -203,13 +203,29 @@ public:
   MOShared::FilesOrigin* origin() const;
 
 
+  // return this tab's ID
+  //
   ModInfoTabIDs tabID() const;
+
+  // returns whether this tab has data; derived classes should call setHasData()
+  //
   bool hasData() const;
 
 signals:
+  // emitted when a tab modified the files in a mod
+  //
   void originModified(int originID);
+
+  // emitted when a tab wants to open a mod by name
+  //
   void modOpen(QString name);
+
+  // emitted when a tab used to have data and is now empty, or vice versa
+  //
   void hasDataChanged();
+
+  // emitted when a tab wants focus
+  //
   void wantsFocus();
 
 protected:
@@ -219,15 +235,27 @@ protected:
 
   OrganizerCore& core();
   PluginContainer& plugin();
-
   QWidget* parentWidget();
 
+  // emits originModified
+  //
   void emitOriginModified();
+
+  // emits modOpen
+  //
   void emitModOpen(QString name);
+
+  // emits hasDataChanged
+  //
   void setHasData(bool b);
 
+  // emits wantsFocus
+  //
   void setFocus();
 
+
+  // saves the sate of the given widget in geometry/modinfodialog_[objectname]
+  //
   // this needs to be a template because saveState() and restoreState() are
   // not in QWidget, but they're in various widgets
   //
@@ -237,6 +265,12 @@ protected:
     s.setValue(settingName(w), w->saveState());
   }
 
+  // restores the sate of the given widget from
+  // geometry/modinfodialog_[objectname]
+  //
+  // this needs to be a template because saveState() and restoreState() are
+  // not in QWidget, but they're in various widgets
+  //
   template <class Widget>
   void restoreWidgetState(const QSettings& s, Widget* w)
   {
@@ -246,16 +280,33 @@ protected:
   }
 
 private:
+  // core
   OrganizerCore& m_core;
+
+  // plugin
   PluginContainer& m_plugin;
+
+  // parent widget, used to display modal dialogs
   QWidget* m_parent;
+
+  // current mod, never null
   ModInfoPtr m_mod;
+
+  // current mod origin, may be null
   MOShared::FilesOrigin* m_origin;
+
+  // tab ID
   ModInfoTabIDs m_tabID;
+
+  // whether the tab has data
   bool m_hasData;
+
+  // true if the tab has never been selected for the current mod
   bool m_firstActivation;
 
 
+  // used by saveWidgetState() and restoreWidgetState()
+  //
   QString settingName(QWidget* w)
   {
     return "geometry/modinfodialog_" + w->objectName();
@@ -263,6 +314,8 @@ private:
 };
 
 
+// the Notes tab
+//
 class NotesTab : public ModInfoDialogTab
 {
 public:
@@ -270,7 +323,13 @@ public:
 
   void clear() override;
   void update() override;
+
+  // returns true, separators can have notes
+  //
   bool canHandleSeparators() const override;
+
+  // returns false, notes don't use files
+  //
   bool usesOriginFiles() const override;
 
 private: