some refactoring

updateTabs() creates a list of tabs to update instead of checking for each file
comments

1 files changed, 196 insertions, 109 deletions

diff --git a/src/modinfodialog.cpp b/src/modinfodialog.cpp
index adbd22df..16690019 100644
--- a/src/modinfodialog.cpp
+++ b/src/modinfodialog.cpp
@@ -33,6 +33,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 
 using namespace MOBase;
 using namespace MOShared;
+namespace fs = std::filesystem;
 
 const int max_scan_for_context_menu = 50;
 
@@ -140,15 +141,51 @@ ModInfoDialog::ModInfoDialog(
   connect(sc, &QShortcut::activated, [&]{ onDeleteShortcut(); });
 
   setMod(mod);
-  m_tabs = createTabs();
+  createTabs();
 
-  for (int i=0; i<ui->tabWidget->count(); ++i) {
-    if (static_cast<std::size_t>(i) >= m_tabs.size()) {
-      qCritical() << "mod info dialog has more tabs than expected";
-      break;
-    }
+  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;
+
+template <class T>
+std::unique_ptr<ModInfoDialogTab> createTab(ModInfoDialog& d, ModInfoTabIDs id)
+{
+  return std::make_unique<T>(ModInfoDialogTabContext(
+    *d.m_core, *d.m_plugin, &d, d.ui.get(), id, d.m_mod, d.getOrigin()));
+}
+
+void ModInfoDialog::createTabs()
+{
+  m_tabs.clear();
+
+  m_tabs.push_back(createTab<TextFilesTab>(*this, ModInfoTabIDs::TextFiles));
+  m_tabs.push_back(createTab<IniFilesTab>(*this, ModInfoTabIDs::IniFiles));
+  m_tabs.push_back(createTab<ImagesTab>(*this, ModInfoTabIDs::Images));
+  m_tabs.push_back(createTab<ESPsTab>(*this, ModInfoTabIDs::Esps));
+  m_tabs.push_back(createTab<ConflictsTab>(*this, ModInfoTabIDs::Conflicts));
+  m_tabs.push_back(createTab<CategoriesTab>(*this, ModInfoTabIDs::Categories));
+  m_tabs.push_back(createTab<NexusTab>(*this, ModInfoTabIDs::Nexus));
+  m_tabs.push_back(createTab<NotesTab>(*this, ModInfoTabIDs::Notes));
+  m_tabs.push_back(createTab<FileTreeTab>(*this, ModInfoTabIDs::Filetree));
+
+  // check for tabs in the ui not having a corresponding tab in the list
+  int count = ui->tabWidget->count();
+  if (count < 0 || count > static_cast<int>(m_tabs.size())) {
+    qCritical() << "mod info dialog has more tabs than expected";
+    count = static_cast<int>(m_tabs.size());
+  }
 
+  // for each tab in the widget; connects the widgets with the tab objects
+  //
+  for (int i=0; i<count; ++i) {
     auto& tabInfo = m_tabs[static_cast<std::size_t>(i)];
+
+    // remembering tab information so tabs can be removed and re-added
     tabInfo.widget = ui->tabWidget->widget(i);
     tabInfo.caption = ui->tabWidget->tabText(i);
     tabInfo.icon = ui->tabWidget->tabIcon(i);
@@ -169,50 +206,18 @@ ModInfoDialog::ModInfoDialog(
       tabInfo.tab.get(), &ModInfoDialogTab::wantsFocus,
       [&, id=tabInfo.tab->tabID()]{ switchToTab(id); });
   }
-
-  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;
-
-template <class T>
-std::unique_ptr<ModInfoDialogTab> createTab(ModInfoDialog& d, ModInfoTabIDs id)
-{
-  return std::make_unique<T>(ModInfoDialogTabContext(
-    *d.m_core, *d.m_plugin, &d, d.ui.get(), id, d.m_mod, d.getOrigin()));
-}
-
-std::vector<ModInfoDialog::TabInfo> ModInfoDialog::createTabs()
-{
-  std::vector<TabInfo> v;
-
-  v.push_back(createTab<TextFilesTab>(*this, ModInfoTabIDs::TextFiles));
-  v.push_back(createTab<IniFilesTab>(*this, ModInfoTabIDs::IniFiles));
-  v.push_back(createTab<ImagesTab>(*this, ModInfoTabIDs::Images));
-  v.push_back(createTab<ESPsTab>(*this, ModInfoTabIDs::Esps));
-  v.push_back(createTab<ConflictsTab>(*this, ModInfoTabIDs::Conflicts));
-  v.push_back(createTab<CategoriesTab>(*this, ModInfoTabIDs::Categories));
-  v.push_back(createTab<NexusTab>(*this, ModInfoTabIDs::Nexus));
-  v.push_back(createTab<NotesTab>(*this, ModInfoTabIDs::Notes));
-  v.push_back(createTab<FileTreeTab>(*this, ModInfoTabIDs::Filetree));
-
-  return v;
 }
 
 int ModInfoDialog::exec()
 {
+  // whether to select the first tab; if the main window requested a specific
+  // tab, it is selected when encountered in update()
   const auto selectFirst = (m_initialTab == ModInfoTabIDs::None);
 
   update(true);
 
-  if (selectFirst) {
-    if (ui->tabWidget->count() > 0) {
-      ui->tabWidget->setCurrentIndex(0);
-    }
+  if (selectFirst && ui->tabWidget->count() > 0) {
+    ui->tabWidget->setCurrentIndex(0);
   }
 
   return TutorableDialog::exec();
@@ -223,6 +228,8 @@ void ModInfoDialog::setMod(ModInfo::Ptr mod)
   Q_ASSERT(mod);
   m_mod = mod;
 
+  // resetting the first activation flag so selecting tabs will trigger it
+  // again
   for (auto& tabInfo : m_tabs) {
     tabInfo.tab->resetFirstActivation();
   }
@@ -248,6 +255,7 @@ void ModInfoDialog::setMod(const QString& name)
 void ModInfoDialog::selectTab(ModInfoTabIDs id)
 {
   if (!isVisible()) {
+    // can't select a tab if the dialog hasn't been properly updated yet
     m_initialTab = id;
     return;
   }
@@ -262,42 +270,55 @@ ModInfoDialog::TabInfo* ModInfoDialog::currentTab()
     return nullptr;
   }
 
+  // looking for the actual tab at that position
   for (auto& tabInfo : m_tabs) {
     if (tabInfo.realPos == index) {
       return &tabInfo;
     }
   }
 
-  qCritical() << "tab index " << index << " not found";
   return nullptr;
 }
 
 void ModInfoDialog::update(bool firstTime)
 {
+  // remembering the current selection, will be restored if the tab still
+  // exists
   const int oldTab = ui->tabWidget->currentIndex();
 
   setWindowTitle(m_mod->name());
+
+  // rebuilding the tab widget if needed depending on what tabs are valid for
+  // the current mod
   setTabsVisibility(firstTime);
 
+  // updating the data in all tabs
   updateTabs();
 
+  // switching to the initial tab, if any
   if (m_initialTab != ModInfoTabIDs::None) {
     switchToTab(m_initialTab);
     m_initialTab = ModInfoTabIDs::None;
   }
 
   if (ui->tabWidget->currentIndex() == oldTab) {
-    // manually fire activated() because the tab index hasn't been changed
     if (auto* tabInfo=currentTab()) {
+      // activated() has to be fired manually because the tab index hasn't been
+      // changed
       tabInfo->tab->activated();
+    } else {
+      qCritical() << "tab index " << oldTab << " not found";
     }
   }
 }
 
 void ModInfoDialog::setTabsVisibility(bool firstTime)
 {
+  // this flag is picked up by onTabSelectionChanged() to avoid triggering
+  // activation events while moving tabs around
   QScopedValueRollback arrangingTabs(m_arrangingTabs, true);
 
+  // one bool per tab to indicate whether the tab should be visible
   std::vector<bool> visibility(m_tabs.size());
 
   bool changed = false;
@@ -307,14 +328,16 @@ void ModInfoDialog::setTabsVisibility(bool firstTime)
 
     bool visible = true;
 
+    // a tab is visible if it can handle the current mod
     if (m_mod->hasFlag(ModInfo::FLAG_FOREIGN)) {
       visible = tabInfo.tab->canHandleUnmanaged();
     } else if (m_mod->hasFlag(ModInfo::FLAG_SEPARATOR)) {
       visible = tabInfo.tab->canHandleSeparators();
     }
 
+    // if the visibility of this tab is changing, set changed to true because
+    // the tabs have to be rebuilt
     const auto currentlyVisible = (ui->tabWidget->indexOf(tabInfo.widget) != -1);
-
     if (visible != currentlyVisible) {
       changed = true;
     }
@@ -322,25 +345,23 @@ void ModInfoDialog::setTabsVisibility(bool firstTime)
     visibility[i] = visible;
   }
 
+  // the tabs have to be rebuilt the first time the dialog is shown, or when
+  // the visibility of any tab has changed
   if (!firstTime && !changed) {
     return;
   }
 
-  // something changed; save the current order (if necessary) because some tabs
-  // will be removed and others added
+  // 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();
+  // remember selection, if any
   auto sel = ModInfoTabIDs::None;
-
-  for (const auto& tabInfo : m_tabs) {
-    if (tabInfo.realPos == selIndex) {
-      sel = tabInfo.tab->tabID();
-      break;
-    }
+  if (const auto* tabInfo=currentTab()) {
+    sel = tabInfo->tab->tabID();
   }
 
+  // removes all tabs and re-adds the visible ones
   reAddTabs(visibility, sel);
 }
 
@@ -352,21 +373,31 @@ void ModInfoDialog::reAddTabs(
   // ordered tab names from settings
   const auto orderedNames = getOrderedTabNames();
 
+  // whether the tabs can be sorted; if the object name of a tab widget is not
+  // found in orderedNames, the list cannot be sorted safely
   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 (!visibility[i]) {
+      // this tab is not visible, skip it
+      continue;
+    }
+
+    // this tab is visible
+    visibleTabs.push_back(&m_tabs[i]);
+
+    if (canSort) {
+      // make sure the widget object name is found in the list
+      const auto objectName = m_tabs[i].widget->objectName();
+      auto itor = std::find(orderedNames.begin(), orderedNames.end(), objectName);
 
-      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;
-        }
+      if (itor == orderedNames.end()) {
+        // this shouldn't happen, it means there's a tab in the UI that's no
+        // in the list
+        qCritical() << "can't sort tabs, '" << objectName << "' not found";
+        canSort = false;
       }
     }
   }
@@ -374,10 +405,16 @@ void ModInfoDialog::reAddTabs(
   // 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());
+      // looking the names in the ordered list
+      auto aItor = std::find(
+        orderedNames.begin(), orderedNames.end(),
+        a->widget->objectName());
 
-      // this was checked above
+      auto bItor = std::find(
+        orderedNames.begin(), orderedNames.end(),
+        b->widget->objectName());
+
+      // this shouldn't happen, it was checked above
       Q_ASSERT(aItor != orderedNames.end() && bItor != orderedNames.end());
 
       return (aItor < bItor);
@@ -397,9 +434,13 @@ void ModInfoDialog::reAddTabs(
   for (std::size_t i=0; i<visibleTabs.size(); ++i) {
     auto& tabInfo = *visibleTabs[i];
 
+    // remembering real position
     tabInfo.realPos = static_cast<int>(i);
+
+    // adding tab
     ui->tabWidget->addTab(tabInfo.widget, tabInfo.icon, tabInfo.caption);
 
+    // selecting
     if (tabInfo.tab->tabID() == sel) {
       ui->tabWidget->setCurrentIndex(static_cast<int>(i));
     }
@@ -410,59 +451,72 @@ void ModInfoDialog::updateTabs(bool becauseOriginChanged)
 {
   auto* origin = getOrigin();
 
+  // list of tabs that should be updated
+  std::vector<TabInfo*> interestedTabs;
+
   for (auto& tabInfo : m_tabs) {
+    // don't touch invisible tabs
     if (!tabInfo.isVisible()) {
       continue;
     }
 
+    // updateTabs() is also called from onOriginModified() to update all the
+    // tabs that depend on the origin; if updateTabs() is called because the
+    // origin changed, but the tab doesn't use origin files, it can be safely
+    // skipped
+    //
+    // this happens for tabs like notes and categories, which don't need to
+    // be updated when files change
     if (becauseOriginChanged && !tabInfo.tab->usesOriginFiles()) {
       continue;
     }
 
-    tabInfo.tab->setMod(m_mod, origin);
-    tabInfo.tab->clear();
+    // this tab should be updated
+    interestedTabs.push_back(&tabInfo);
   }
 
 
-  feedFiles(becauseOriginChanged);
+  for (auto* tabInfo : interestedTabs) {
+    // set the current mod
+    tabInfo->tab->setMod(m_mod, origin);
 
-  for (auto& tabInfo : m_tabs) {
-    if (tabInfo.isVisible()) {
-      tabInfo.tab->update();
-    }
+    // clear
+    tabInfo->tab->clear();
   }
 
+  // feed all the files from the filesystem
+  feedFiles(interestedTabs);
+
+  // call update() on all tabs
+  for (auto* tabInfo : interestedTabs) {
+    tabInfo->tab->update();
+  }
+
+  // update the text colours
   setTabsColors();
 }
 
-void ModInfoDialog::feedFiles(bool becauseOriginChanged)
+void ModInfoDialog::feedFiles(std::vector<TabInfo*>& interestedTabs)
 {
-  namespace fs = std::filesystem;
-
   const auto rootPath = m_mod->absolutePath();
+  if (rootPath.isEmpty()) {
+    return;
+  }
 
-  if (rootPath.length() > 0) {
-    fs::path path(rootPath.toStdWString());
-    for (const auto& entry : fs::recursive_directory_iterator(path)) {
-      if (!entry.is_regular_file()) {
-        continue;
-      }
-
-      const auto fileName = QString::fromWCharArray(
-        entry.path().native().c_str());
+  const fs::path fsPath(rootPath.toStdWString());
 
-      for (auto& tabInfo : m_tabs) {
-        if (!tabInfo.isVisible()) {
-          continue;
-        }
+  for (const auto& entry : fs::recursive_directory_iterator(fsPath)) {
+    if (!entry.is_regular_file()) {
+      // skip directories
+      continue;
+    }
 
-        if (becauseOriginChanged && !tabInfo.tab->usesOriginFiles()) {
-          continue;
-        }
+    const auto filePath = QString::fromStdWString(entry.path().native());
 
-        if (tabInfo.tab->feedFile(rootPath, fileName)) {
-          break;
-        }
+    // for each tab
+    for (auto* tabInfo : interestedTabs) {
+      if (tabInfo->tab->feedFile(rootPath, filePath)) {
+        break;
       }
     }
   }
@@ -470,41 +524,53 @@ void ModInfoDialog::feedFiles(bool becauseOriginChanged)
 
 void ModInfoDialog::setTabsColors()
 {
+  const auto p = m_mainWindow->palette();
+
   for (const auto& tabInfo : m_tabs) {
-    const auto c = tabInfo.tab->hasData() ?
+    if (!tabInfo.isVisible()) {
+      // don't bother with invisible tabs
+      continue;
+    }
+
+    const QColor color = tabInfo.tab->hasData() ?
       QColor::Invalid :
-      m_mainWindow->palette().color(QPalette::Disabled, QPalette::WindowText);
+      p.color(QPalette::Disabled, QPalette::WindowText);
 
-    ui->tabWidget->tabBar()->setTabTextColor(tabInfo.realPos, c);
+    ui->tabWidget->tabBar()->setTabTextColor(tabInfo.realPos, color);
   }
 }
 
 void ModInfoDialog::switchToTab(ModInfoTabIDs id)
 {
+  // look a tab with the given id
   for (const auto& tabInfo : m_tabs) {
     if (tabInfo.tab->tabID() == id) {
+      // use realPos to select the proper tab in the widget
       ui->tabWidget->setCurrentIndex(tabInfo.realPos);
       return;
     }
   }
 
+  // this could happen if the tab is not visible right now
   qDebug()
-    << "can't switch to tab ID " << static_cast<int>(id) << ", not available";
+    << "can't switch to tab ID " << static_cast<int>(id)
+    << ", not available";
 }
 
 MOShared::FilesOrigin* ModInfoDialog::getOrigin()
 {
-  MOShared::FilesOrigin* origin = nullptr;
-
   auto* ds = m_core->directoryStructure();
-  if (ds->originExists(ToWString(m_mod->name()))) {
-    auto* origin = &ds->getOriginByName(ToWString(m_mod->name()));
-    if (!origin->isDisabled()) {
-      return origin;
-    }
+
+  if (!ds->originExists(m_mod->name().toStdWString())) {
+    return nullptr;
   }
 
-  return nullptr;
+  auto* origin = &ds->getOriginByName(m_mod->name().toStdWString());
+  if (origin->isDisabled()) {
+    return nullptr;
+  }
+
+  return origin;
 }
 
 void ModInfoDialog::saveState(Settings& s) const
@@ -520,6 +586,7 @@ void ModInfoDialog::saveState(Settings& s) const
   s.directInterface().remove("mod_info_conflicts_noconflict");
   s.directInterface().remove("mod_info_conflicts_overwritten");
 
+  // save state for each tab
   for (const auto& tabInfo : m_tabs) {
     tabInfo.tab->saveState(s);
   }
@@ -527,6 +594,10 @@ void ModInfoDialog::saveState(Settings& s) const
 
 void ModInfoDialog::restoreState(const Settings& s)
 {
+  // tab order is not restored here, it will be picked up if tabs have to be
+  // removed and re-added
+
+  // restore state for each tab
   for (const auto& tabInfo : m_tabs) {
     tabInfo.tab->restoreState(s);
   }
@@ -536,6 +607,13 @@ 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
+    //
+    // if not all tabs are visible, it becomes very difficult to figure out in
+    // what order the user wants these tabs to be, so just avoid saving it
+    // completely
+    //
+    // this means that reordering tabs when not all tabs are visible is not
+    // saved, but it's better than breaking everything
     return;
   }
 
@@ -559,7 +637,7 @@ std::vector<QString> ModInfoDialog::getOrderedTabNames() const
   std::vector<QString> v;
 
   if (settings.contains("mod_info_tabs")) {
-    // old byte array
+    // old byte array from 2.2.0
     QDataStream stream(settings.value("mod_info_tabs").toByteArray());
 
     int count = 0;
@@ -587,12 +665,16 @@ std::vector<QString> ModInfoDialog::getOrderedTabNames() const
 
 void ModInfoDialog::onOriginModified(int originID)
 {
+  // tell the main window the origin changed
   emit originModified(originID);
+
+  // update tabs that depend on the origin
   updateTabs(true);
 }
 
 void ModInfoDialog::onDeleteShortcut()
 {
+  // forward the request to the current tab
   if (auto* tabInfo=currentTab()) {
     tabInfo->tab->deleteRequested();
   }
@@ -616,6 +698,10 @@ void ModInfoDialog::onCloseButton()
 
 bool ModInfoDialog::tryClose()
 {
+  // cancel the close if any tab returns false; for example. this can happen if
+  // a tab has unsaved content, pops a confirmation dialog, and the user clicks
+  // cancel
+
   for (auto& tabInfo : m_tabs) {
     if (!tabInfo.tab->canClose()) {
       return false;
@@ -634,6 +720,7 @@ void ModInfoDialog::onTabSelectionChanged()
     return;
   }
 
+  // this will call firstActivation() on the tab if needed
   if (auto* tabInfo=currentTab()) {
     tabInfo->tab->activated();
   }