From e53905329a7a204ad0ad41600a9fb3040b942991 Mon Sep 17 00:00:00 2001
From: Mikaël Capelle <capelle.mikael@gmail.com>
Date: Mon, 11 Jan 2021 18:55:15 +0100
Subject: Allow collapsible separator in descending priority.

---
 src/modlistbypriorityproxy.cpp | 59 +++++++++++++++++++++++++++++++++++-------
 src/modlistbypriorityproxy.h   |  8 ++++++
 src/modlistsortproxy.cpp       |  7 +++++
 src/modlistview.cpp            |  4 +--
 4 files changed, 66 insertions(+), 12 deletions(-)

(limited to 'src')

diff --git a/src/modlistbypriorityproxy.cpp b/src/modlistbypriorityproxy.cpp
index 480a4105..e6728942 100644
--- a/src/modlistbypriorityproxy.cpp
+++ b/src/modlistbypriorityproxy.cpp
@@ -43,6 +43,11 @@ void ModListByPriorityProxy::setProfile(Profile* profile)
   m_profile = profile;
 }
 
+void ModListByPriorityProxy::setSortOrder(Qt::SortOrder order)
+{
+  m_sortOrder = order;
+}
+
 void ModListByPriorityProxy::buildTree()
 {
   if (!sourceModel()) return;
@@ -56,7 +61,9 @@ void ModListByPriorityProxy::buildTree()
   TreeItem* root = &m_Root;
   std::unique_ptr<TreeItem> overwrite;
   std::vector<std::unique_ptr<TreeItem>> backups;
-  for (auto& [priority, index] : m_profile->getAllIndexesByPriority()) {
+
+  auto fn = [&](const auto& p) {
+    auto& [priority, index] = p;
     ModInfo::Ptr modInfo = ModInfo::getByIndex(index);
 
     TreeItem* item;
@@ -81,11 +88,24 @@ void ModListByPriorityProxy::buildTree()
     }
 
     m_IndexToItem[index] = item;
+  };
+
+  auto& ibp = m_profile->getAllIndexesByPriority();
+  if (m_sortOrder == Qt::AscendingOrder) {
+    std::for_each(ibp.begin(), ibp.end(), fn);
+    m_Root.children.insert(m_Root.children.begin(),
+      std::make_move_iterator(backups.begin()), std::make_move_iterator(backups.end()));
+    m_Root.children.push_back(std::move(overwrite));
+  }
+  else {
+    std::for_each(ibp.rbegin(), ibp.rend(), fn);
+    m_Root.children.insert(m_Root.children.begin(), std::move(overwrite));
+    m_Root.children.insert(m_Root.children.end(),
+      std::make_move_iterator(backups.begin()), std::make_move_iterator(backups.end()));
   }
 
-  m_Root.children.insert(m_Root.children.begin(),
-    std::make_move_iterator(backups.begin()), std::make_move_iterator(backups.end()));
-  m_Root.children.push_back(std::move(overwrite));
+  // we do not really care about their position in the root
+  // as long as those are not in a separator
 
   endResetModel();
 }
@@ -185,7 +205,8 @@ bool ModListByPriorityProxy::canDropMimeData(const QMimeData* data, Qt::DropActi
     int firstRowPriority = INT_MAX;
     for (auto sourceRow : dropInfo.rows()) {
       hasSeparator = hasSeparator || ModInfo::getByIndex(sourceRow)->isSeparator();
-      if (m_profile->getModPriority(sourceRow) < firstRowPriority) {
+      if (m_sortOrder == Qt::AscendingOrder && m_profile->getModPriority(sourceRow) < firstRowPriority
+        || m_sortOrder == Qt::DescendingOrder && m_profile->getModPriority(sourceRow) > firstRowPriority) {
         firstRowIndex = sourceRow;
         firstRowPriority = m_profile->getModPriority(sourceRow);
       }
@@ -193,7 +214,7 @@ bool ModListByPriorityProxy::canDropMimeData(const QMimeData* data, Qt::DropActi
 
     bool firstRowSeparator = firstRowIndex != -1 && ModInfo::getByIndex(firstRowIndex)->isSeparator();
 
-    // row = -1 and invalid parent means we're dropping onto an item, we don't want to drop
+    // row = -1 and valid parent means we're dropping onto an item, we don't want to drop
     // separators onto items or items into their own separator
     if (row == -1 && parent.isValid()) {
       auto* parentItem = static_cast<TreeItem*>(parent.internalPointer());
@@ -216,8 +237,10 @@ bool ModListByPriorityProxy::canDropMimeData(const QMimeData* data, Qt::DropActi
   }
 
   // the row may be outside of the children list if we insert at the end
-  if (!parent.isValid() && row >= m_Root.children.size()) {
-    return false;
+  if (!parent.isValid()) {
+    if (row >= m_Root.children.size()) {
+      return false;
+    }
   }
 
   return QAbstractProxyModel::canDropMimeData(data, action, row, column, parent);
@@ -237,15 +260,25 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
   else {
 
     if (row >= 0) {
+      MOBase::log::debug("row={}, name={}, expand={}, drop={}", row, m_Root.children[row]->mod->name(), m_dropExpanded, m_dropPosition);
       if (!parent.isValid()) {
         if (row < m_Root.children.size()) {
           sourceRow = m_Root.children[row]->index;
           if (row > 0
+            && m_sortOrder == Qt::AscendingOrder
             && m_Root.children[row - 1]->mod->isSeparator()
             && !m_Root.children[row - 1]->children.empty()
             && m_dropExpanded
             && m_dropPosition == ModListView::DropPosition::BelowItem) {
-            sourceRow = m_Root.children[row - 1]->children[0]->index;
+              sourceRow = m_Root.children[row - 1]->children[0]->index;
+          }
+          else if (row > 0
+            && m_sortOrder == Qt::DescendingOrder
+            && m_Root.children[row]->mod->isSeparator()
+            && !m_Root.children[row]->children.empty()
+            && (!m_dropExpanded
+              || m_dropPosition == ModListView::DropPosition::AboveItem)) {
+              sourceRow = m_Root.children[row]->children.back()->index;
           }
         }
         else {
@@ -265,7 +298,13 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
     }
     else if (parent.isValid()) {
       // this is a drop in a separator
-      sourceRow = m_Root.children[parent.row() + 1]->index;
+      if (m_sortOrder == Qt::AscendingOrder) {
+        sourceRow = m_Root.children[parent.row() + 1]->index;
+      }
+      else {
+        auto* item = m_Root.children[parent.row()].get();
+        sourceRow = item->children.empty() ? item->index : item->children.back()->index;
+      }
     }
   }
 
diff --git a/src/modlistbypriorityproxy.h b/src/modlistbypriorityproxy.h
index 74ecc942..380e257b 100644
--- a/src/modlistbypriorityproxy.h
+++ b/src/modlistbypriorityproxy.h
@@ -28,6 +28,13 @@ public:
   ~ModListByPriorityProxy();
 
   void setProfile(Profile* profile);
+
+  // set the sort order but does not refresh the proxy
+  //
+  void setSortOrder(Qt::SortOrder order);
+
+  // refresh the proxy by rebuilding the inner structure
+  //
   void refresh();
 
   void setSourceModel(QAbstractItemModel* sourceModel) override;
@@ -83,6 +90,7 @@ private:
   OrganizerCore& m_core;
   Profile* m_profile;
 
+  Qt::SortOrder m_sortOrder = Qt::AscendingOrder;
   bool m_dropExpanded = false;
   ModListView::DropPosition m_dropPosition = ModListView::DropPosition::OnItem;
 };
diff --git a/src/modlistsortproxy.cpp b/src/modlistsortproxy.cpp
index 9ae37a43..0682b9c3 100644
--- a/src/modlistsortproxy.cpp
+++ b/src/modlistsortproxy.cpp
@@ -617,6 +617,13 @@ bool ModListSortProxy::canDropMimeData(const QMimeData* data, Qt::DropAction act
     }
   }
 
+  // in the regular model, when dropping between rows, the row-value passed to
+  // the sourceModel is inconsistent between ascending and descending ordering.
+  // This should fix that
+  if (sortOrder() == Qt::DescendingOrder) {
+    --row;
+  }
+
   return QSortFilterProxyModel::canDropMimeData(data, action, row, column, parent);
 }
 
diff --git a/src/modlistview.cpp b/src/modlistview.cpp
index 8827733b..1926a3f4 100644
--- a/src/modlistview.cpp
+++ b/src/modlistview.cpp
@@ -683,8 +683,8 @@ void ModListView::updateGroupByProxy()
     nextProxy = m_byNexusIdProxy;
   }
   else if (m_core->settings().interface().collapsibleSeparators()
-    && m_sortProxy->sortColumn() == ModList::COL_PRIORITY
-    && m_sortProxy->sortOrder() == Qt::AscendingOrder) {
+    && m_sortProxy->sortColumn() == ModList::COL_PRIORITY) {
+    m_byPriorityProxy->setSortOrder(m_sortProxy->sortOrder());
     nextProxy = m_byPriorityProxy;
   }
 
-- 
cgit v1.3.1


From 08520822eb91c065a0893da0e2e9d11574450208 Mon Sep 17 00:00:00 2001
From: Mikaël Capelle <capelle.mikael@gmail.com>
Date: Mon, 11 Jan 2021 19:15:39 +0100
Subject: Fix drop in separator and create mod in separator in descending
 priority.

---
 src/modlistbypriorityproxy.cpp |  3 ---
 src/modlistsortproxy.cpp       |  4 ++--
 src/modlistview.cpp            |  5 +++++
 src/modlistview.h              |  3 ++-
 src/modlistviewactions.cpp     | 31 +++++++++++++++++++++----------
 5 files changed, 30 insertions(+), 16 deletions(-)

(limited to 'src')

diff --git a/src/modlistbypriorityproxy.cpp b/src/modlistbypriorityproxy.cpp
index e6728942..eca7108c 100644
--- a/src/modlistbypriorityproxy.cpp
+++ b/src/modlistbypriorityproxy.cpp
@@ -104,9 +104,6 @@ void ModListByPriorityProxy::buildTree()
       std::make_move_iterator(backups.begin()), std::make_move_iterator(backups.end()));
   }
 
-  // we do not really care about their position in the root
-  // as long as those are not in a separator
-
   endResetModel();
 }
 
diff --git a/src/modlistsortproxy.cpp b/src/modlistsortproxy.cpp
index 0682b9c3..bc28490f 100644
--- a/src/modlistsortproxy.cpp
+++ b/src/modlistsortproxy.cpp
@@ -620,7 +620,7 @@ bool ModListSortProxy::canDropMimeData(const QMimeData* data, Qt::DropAction act
   // in the regular model, when dropping between rows, the row-value passed to
   // the sourceModel is inconsistent between ascending and descending ordering.
   // This should fix that
-  if (sortOrder() == Qt::DescendingOrder) {
+  if (sortOrder() == Qt::DescendingOrder && row != -1) {
     --row;
   }
 
@@ -645,7 +645,7 @@ bool ModListSortProxy::dropMimeData(const QMimeData *data, Qt::DropAction action
   // in the regular model, when dropping between rows, the row-value passed to
   // the sourceModel is inconsistent between ascending and descending ordering.
   // This should fix that
-  if (sortOrder() == Qt::DescendingOrder) {
+  if (sortOrder() == Qt::DescendingOrder && row != -1) {
     --row;
   }
 
diff --git a/src/modlistview.cpp b/src/modlistview.cpp
index 1926a3f4..88c588f6 100644
--- a/src/modlistview.cpp
+++ b/src/modlistview.cpp
@@ -209,6 +209,11 @@ int ModListView::sortColumn() const
   return m_sortProxy ? m_sortProxy->sortColumn() : -1;
 }
 
+Qt::SortOrder ModListView::sortOrder() const
+{
+  return m_sortProxy ? m_sortProxy->sortOrder() : Qt::AscendingOrder;
+}
+
 ModListView::GroupByMode ModListView::groupByMode() const
 {
   if (m_sortProxy == nullptr) {
diff --git a/src/modlistview.h b/src/modlistview.h
index 53c71458..13f868c6 100644
--- a/src/modlistview.h
+++ b/src/modlistview.h
@@ -61,9 +61,10 @@ public:
   //
   bool hasCollapsibleSeparators() const;
 
-  // the column by which the mod list is currently sorted
+  // the column/order by which the mod list is currently sorted
   //
   int sortColumn() const;
+  Qt::SortOrder sortOrder() const;
 
   // the current group mode
   //
diff --git a/src/modlistviewactions.cpp b/src/modlistviewactions.cpp
index cc50f009..09142026 100644
--- a/src/modlistviewactions.cpp
+++ b/src/modlistviewactions.cpp
@@ -103,22 +103,33 @@ void ModListViewActions::createEmptyMod(const QModelIndex& index) const
     auto info = ModInfo::getByIndex(mIndex);
     newPriority = m_core.currentProfile()->getModPriority(mIndex);
     if (info->isSeparator()) {
+
+      auto isSeparator = [](const auto& p) {
+        return ModInfo::getByIndex(p.second)->isSeparator();
+      };
+
       auto& ibp = m_core.currentProfile()->getAllIndexesByPriority();
 
-      // start right after the current priority and look for the next
+      // start right after/before the current priority and look for the next
       // separator
-      auto it = ibp.find(newPriority + 1);
-      for (; it != ibp.end(); ++it) {
-        auto info = ModInfo::getByIndex(it->second);
-        if (info->isSeparator()) {
+      if (m_view->sortOrder() == Qt::AscendingOrder) {
+        auto it = std::find_if(ibp.find(newPriority + 1), ibp.end(), isSeparator);
+        if (it != ibp.end()) {
           newPriority = it->first;
-          break;
+        }
+        else {
+          newPriority = -1;
         }
       }
-
-      // no separator found, create at the end
-      if (it == ibp.end()) {
-        newPriority = -1;
+      else {
+        auto it = std::find_if(std::reverse_iterator{ ibp.find(newPriority - 1) }, ibp.rend(), isSeparator);
+        if (it != ibp.rend()) {
+          newPriority = it->first + 1;
+        }
+        else {
+          // create "before" priority 0, i.e. at the end in descending priority.
+          newPriority = 0;
+        }
       }
     }
   }
-- 
cgit v1.3.1


From 49c2c8f3330edccf54d1b5cc9958295288358c1c Mon Sep 17 00:00:00 2001
From: Mikaël Capelle <capelle.mikael@gmail.com>
Date: Mon, 11 Jan 2021 19:44:43 +0100
Subject: Add setting to enable/disable collapsible priority depending on the
 sort order.

---
 src/modlistview.cpp                 |  2 +-
 src/settings.cpp                    | 11 +++++---
 src/settings.h                      |  4 +--
 src/settingsdialog.ui               | 53 ++++++++++++++++++++++++++++++++++++-
 src/settingsdialoguserinterface.cpp | 20 +++++++++-----
 src/settingsdialoguserinterface.h   |  6 +++++
 6 files changed, 82 insertions(+), 14 deletions(-)

(limited to 'src')

diff --git a/src/modlistview.cpp b/src/modlistview.cpp
index 88c588f6..ae905a6e 100644
--- a/src/modlistview.cpp
+++ b/src/modlistview.cpp
@@ -687,7 +687,7 @@ void ModListView::updateGroupByProxy()
   else if (groupIndex == GroupBy::NEXUS_ID) {
     nextProxy = m_byNexusIdProxy;
   }
-  else if (m_core->settings().interface().collapsibleSeparators()
+  else if (m_core->settings().interface().collapsibleSeparators(m_sortProxy->sortOrder())
     && m_sortProxy->sortColumn() == ModList::COL_PRIORITY) {
     m_byPriorityProxy->setSortOrder(m_sortProxy->sortOrder());
     nextProxy = m_byPriorityProxy;
diff --git a/src/settings.cpp b/src/settings.cpp
index 488a9b01..d1f3ef4f 100644
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -2175,14 +2175,17 @@ void InterfaceSettings::setStyleName(const QString& name)
   set(m_Settings, "Settings", "style", name);
 }
 
-bool InterfaceSettings::collapsibleSeparators() const
+bool InterfaceSettings::collapsibleSeparators(Qt::SortOrder order) const
 {
-  return get<bool>(m_Settings, "Settings", "collapsible_separators", true);
+  return get<bool>(m_Settings, "Settings",
+    order == Qt::AscendingOrder ? "collapsible_separators_asc" : "collapsible_separators_dsc",
+    order == Qt::AscendingOrder);
 }
 
-void InterfaceSettings::setCollapsibleSeparators(bool b)
+void InterfaceSettings::setCollapsibleSeparators(bool ascending, bool descending)
 {
-  set(m_Settings, "Settings", "collapsible_separators", b);
+  set(m_Settings, "Settings", "collapsible_separators_asc", ascending);
+  set(m_Settings, "Settings", "collapsible_separators_dsc", descending);
 }
 
 bool InterfaceSettings::collapsibleSeparatorsConflicts() const
diff --git a/src/settings.h b/src/settings.h
index efcfee57..484ce163 100644
--- a/src/settings.h
+++ b/src/settings.h
@@ -623,8 +623,8 @@ public:
 
   // whether to use collapsible separators when possible
   //
-  bool collapsibleSeparators() const;
-  void setCollapsibleSeparators(bool b);
+  bool collapsibleSeparators(Qt::SortOrder order) const;
+  void setCollapsibleSeparators(bool ascending, bool descending);
 
   // whether to display mod conflicts on separators when collapsed
   //
diff --git a/src/settingsdialog.ui b/src/settingsdialog.ui
index d5c93c5c..b23e6d90 100644
--- a/src/settingsdialog.ui
+++ b/src/settingsdialog.ui
@@ -306,7 +306,10 @@ If you disable this feature, MO will only display official DLCs this way. Please
                <bool>false</bool>
               </property>
               <property name="checkable">
-               <bool>true</bool>
+               <bool>false</bool>
+              </property>
+              <property name="checked">
+               <bool>false</bool>
               </property>
               <layout class="QVBoxLayout" name="verticalLayout_20">
                <property name="leftMargin">
@@ -315,6 +318,54 @@ If you disable this feature, MO will only display official DLCs this way. Please
                <property name="rightMargin">
                 <number>7</number>
                </property>
+               <item>
+                <widget class="QWidget" name="widget_8" native="true">
+                 <property name="minimumSize">
+                  <size>
+                   <width>0</width>
+                   <height>0</height>
+                  </size>
+                 </property>
+                 <layout class="QHBoxLayout" name="horizontalLayout_13">
+                  <property name="leftMargin">
+                   <number>0</number>
+                  </property>
+                  <property name="topMargin">
+                   <number>0</number>
+                  </property>
+                  <property name="rightMargin">
+                   <number>0</number>
+                  </property>
+                  <property name="bottomMargin">
+                   <number>0</number>
+                  </property>
+                  <item>
+                   <widget class="QLabel" name="label_18">
+                    <property name="text">
+                     <string>When sorting by</string>
+                    </property>
+                   </widget>
+                  </item>
+                  <item>
+                   <widget class="QCheckBox" name="collapsibleSeparatorsAscBox">
+                    <property name="text">
+                     <string>ascending priority</string>
+                    </property>
+                    <property name="checked">
+                     <bool>true</bool>
+                    </property>
+                   </widget>
+                  </item>
+                  <item>
+                   <widget class="QCheckBox" name="collapsibleSeparatorsDscBox">
+                    <property name="text">
+                     <string>descending  priority</string>
+                    </property>
+                   </widget>
+                  </item>
+                 </layout>
+                </widget>
+               </item>
                <item>
                 <widget class="QCheckBox" name="collapsibleSeparatorsConflictsBox">
                  <property name="toolTip">
diff --git a/src/settingsdialoguserinterface.cpp b/src/settingsdialoguserinterface.cpp
index 5d856395..cdd30f2e 100644
--- a/src/settingsdialoguserinterface.cpp
+++ b/src/settingsdialoguserinterface.cpp
@@ -13,16 +13,15 @@ UserInterfaceSettingsTab::UserInterfaceSettingsTab(Settings& s, SettingsDialog&
 {
 
   // connect before setting to trigger
-  QObject::connect(ui->collapsibleSeparatorsBox, &QGroupBox::toggled, [=](auto&& on) {
-    ui->collapsibleSeparatorsConflictsBox->setEnabled(on);
-    ui->collapsibleSeparatorsPerProfileBox->setEnabled(on);
-  });
+  QObject::connect(ui->collapsibleSeparatorsAscBox, &QCheckBox::toggled, [=] { updateCollapsibleSeparatorsGroup(); });
+  QObject::connect(ui->collapsibleSeparatorsDscBox, &QCheckBox::toggled, [=] { updateCollapsibleSeparatorsGroup(); });
 
   // mod list
   ui->displayForeignBox->setChecked(settings().interface().displayForeign());
   ui->colorSeparatorsBox->setChecked(settings().colors().colorSeparatorScrollbar());
   ui->collapsibleSeparatorsConflictsBox->setChecked(settings().interface().collapsibleSeparatorsConflicts());
-  ui->collapsibleSeparatorsBox->setChecked(settings().interface().collapsibleSeparators());
+  ui->collapsibleSeparatorsAscBox->setChecked(settings().interface().collapsibleSeparators(Qt::AscendingOrder));
+  ui->collapsibleSeparatorsDscBox->setChecked(settings().interface().collapsibleSeparators(Qt::DescendingOrder));
   ui->collapsibleSeparatorsPerProfileBox->setChecked(settings().interface().collapsibleSeparatorsPerProfile());
   ui->saveFiltersBox->setChecked(settings().interface().saveFilters());
 
@@ -42,7 +41,8 @@ void UserInterfaceSettingsTab::update()
   // mod list
   settings().colors().setColorSeparatorScrollbar(ui->colorSeparatorsBox->isChecked());
   settings().interface().setDisplayForeign(ui->displayForeignBox->isChecked());
-  settings().interface().setCollapsibleSeparators(ui->collapsibleSeparatorsBox->isChecked());
+  settings().interface().setCollapsibleSeparators(
+    ui->collapsibleSeparatorsAscBox->isChecked(), ui->collapsibleSeparatorsDscBox->isChecked());
   settings().interface().setCollapsibleSeparatorsConflicts(ui->collapsibleSeparatorsConflictsBox->isChecked());
   settings().interface().setCollapsibleSeparatorsPerProfile(ui->collapsibleSeparatorsPerProfileBox->isChecked());
   settings().interface().setSaveFilters(ui->saveFiltersBox->isChecked());
@@ -54,3 +54,11 @@ void UserInterfaceSettingsTab::update()
   // colors
   ui->colorTable->commitColors();
 }
+
+void UserInterfaceSettingsTab::updateCollapsibleSeparatorsGroup()
+{
+  const auto checked = ui->collapsibleSeparatorsAscBox->isChecked() ||
+    ui->collapsibleSeparatorsDscBox->isChecked();
+  ui->collapsibleSeparatorsConflictsBox->setEnabled(checked);
+  ui->collapsibleSeparatorsPerProfileBox->setEnabled(checked);
+}
diff --git a/src/settingsdialoguserinterface.h b/src/settingsdialoguserinterface.h
index 8b4d48fa..4609ae54 100644
--- a/src/settingsdialoguserinterface.h
+++ b/src/settingsdialoguserinterface.h
@@ -10,6 +10,12 @@ public:
   UserInterfaceSettingsTab(Settings& settings, SettingsDialog& dialog);
 
   void update() override;
+
+protected slots:
+
+  // enable/disable the collapsible separators group depending on
+  // the checkbox states
+  void updateCollapsibleSeparatorsGroup();
 };
 
 #endif // SETTINGSDIALOGGENERAL_H
-- 
cgit v1.3.1


From 16b825bc5d9a03a3d0ef6453e07cf298e23965a2 Mon Sep 17 00:00:00 2001
From: Mikaël Capelle <capelle.mikael@gmail.com>
Date: Mon, 11 Jan 2021 20:57:32 +0100
Subject: Fix drop in first item of separators.

---
 src/modlistbypriorityproxy.cpp | 55 +++++++++++++++++++++++++++---------------
 src/modlistsortproxy.cpp       | 11 ++++++---
 src/modlistsortproxy.h         |  4 +++
 3 files changed, 48 insertions(+), 22 deletions(-)

(limited to 'src')

diff --git a/src/modlistbypriorityproxy.cpp b/src/modlistbypriorityproxy.cpp
index eca7108c..4fd589a4 100644
--- a/src/modlistbypriorityproxy.cpp
+++ b/src/modlistbypriorityproxy.cpp
@@ -257,25 +257,29 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
   else {
 
     if (row >= 0) {
-      MOBase::log::debug("row={}, name={}, expand={}, drop={}", row, m_Root.children[row]->mod->name(), m_dropExpanded, m_dropPosition);
       if (!parent.isValid()) {
         if (row < m_Root.children.size()) {
-          sourceRow = m_Root.children[row]->index;
-          if (row > 0
-            && m_sortOrder == Qt::AscendingOrder
-            && m_Root.children[row - 1]->mod->isSeparator()
-            && !m_Root.children[row - 1]->children.empty()
-            && m_dropExpanded
-            && m_dropPosition == ModListView::DropPosition::BelowItem) {
+          if (m_sortOrder == Qt::AscendingOrder) {
+            sourceRow = m_Root.children[row]->index;
+            if (row > 0
+              && m_sortOrder == Qt::AscendingOrder
+              && m_Root.children[row - 1]->mod->isSeparator()
+              && !m_Root.children[row - 1]->children.empty()
+              && m_dropExpanded
+              && m_dropPosition == ModListView::DropPosition::BelowItem) {
               sourceRow = m_Root.children[row - 1]->children[0]->index;
+            }
           }
-          else if (row > 0
-            && m_sortOrder == Qt::DescendingOrder
-            && m_Root.children[row]->mod->isSeparator()
-            && !m_Root.children[row]->children.empty()
-            && (!m_dropExpanded
-              || m_dropPosition == ModListView::DropPosition::AboveItem)) {
-              sourceRow = m_Root.children[row]->children.back()->index;
+          else {
+            sourceRow = m_Root.children[row - 1]->index;
+            if (row > 0
+              && m_sortOrder == Qt::DescendingOrder
+              && m_Root.children[row - 1]->mod->isSeparator()
+              && !m_Root.children[row - 1]->children.empty()
+              && (!m_dropExpanded
+                || m_dropPosition == ModListView::DropPosition::AboveItem)) {
+              sourceRow = m_Root.children[row - 1]->children.back()->index;
+            }
           }
         }
         else {
@@ -285,12 +289,25 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
       else {
         auto* item = static_cast<TreeItem*>(parent.internalPointer());
 
-        if (row < item->children.size()) {
-          sourceRow = item->children[row]->index;
+        if (m_sortOrder == Qt::DescendingOrder
+          && row == 0 && m_dropPosition == ModListView::DropPosition::AboveItem) {
+          sourceRow = item->index;
         }
-        else if (parent.row() + 1 < m_Root.children.size()) {
-          sourceRow = m_Root.children[parent.row() + 1]->index;
+        else {
+
+          if (m_sortOrder == Qt::DescendingOrder) {
+            row--;
+          }
+
+          if (row < item->children.size()) {
+            sourceRow = item->children[row]->index;
+          }
+          else if (parent.row() + 1 < m_Root.children.size()) {
+            sourceRow = m_Root.children[parent.row() + 1]->index;
+          }
         }
+
+
       }
     }
     else if (parent.isValid()) {
diff --git a/src/modlistsortproxy.cpp b/src/modlistsortproxy.cpp
index bc28490f..1077a833 100644
--- a/src/modlistsortproxy.cpp
+++ b/src/modlistsortproxy.cpp
@@ -18,6 +18,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 */
 
 #include "modlistsortproxy.h"
+#include "modlistbypriorityproxy.h"
 #include "modlistdropinfo.h"
 #include "modinfo.h"
 #include "profile.h"
@@ -279,7 +280,6 @@ bool ModListSortProxy::filterMatchesModOr(ModInfo::Ptr info, bool enabled) const
 
 bool ModListSortProxy::optionsMatchMod(ModInfo::Ptr info, bool) const
 {
-
   return true;
 }
 
@@ -598,6 +598,11 @@ bool ModListSortProxy::filterAcceptsRow(int source_row, const QModelIndex &paren
   }
 }
 
+bool ModListSortProxy::sourceIsByPriorityProxy() const
+{
+  return dynamic_cast<ModListByPriorityProxy*>(sourceModel()) != nullptr;
+}
+
 bool ModListSortProxy::canDropMimeData(const QMimeData* data, Qt::DropAction action, int row, int column, const QModelIndex& parent) const
 {
   ModListDropInfo dropInfo(data, *m_Organizer);
@@ -620,7 +625,7 @@ bool ModListSortProxy::canDropMimeData(const QMimeData* data, Qt::DropAction act
   // in the regular model, when dropping between rows, the row-value passed to
   // the sourceModel is inconsistent between ascending and descending ordering.
   // This should fix that
-  if (sortOrder() == Qt::DescendingOrder && row != -1) {
+  if (sortOrder() == Qt::DescendingOrder && row != -1 && !sourceIsByPriorityProxy()) {
     --row;
   }
 
@@ -645,7 +650,7 @@ bool ModListSortProxy::dropMimeData(const QMimeData *data, Qt::DropAction action
   // in the regular model, when dropping between rows, the row-value passed to
   // the sourceModel is inconsistent between ascending and descending ordering.
   // This should fix that
-  if (sortOrder() == Qt::DescendingOrder && row != -1) {
+  if (sortOrder() == Qt::DescendingOrder && row != -1 && !sourceIsByPriorityProxy()) {
     --row;
   }
 
diff --git a/src/modlistsortproxy.h b/src/modlistsortproxy.h
index 421c82cc..23be77ed 100644
--- a/src/modlistsortproxy.h
+++ b/src/modlistsortproxy.h
@@ -140,6 +140,10 @@ private:
   bool filterMatchesModAnd(ModInfo::Ptr info, bool enabled) const;
   bool filterMatchesModOr(ModInfo::Ptr info, bool enabled) const;
 
+  // check if the source model is the by-priority proxy
+  //
+  bool sourceIsByPriorityProxy() const;
+
 private slots:
 
   void aboutToChangeData();
-- 
cgit v1.3.1


From 748b7e6918c79f1ad9198c4be8016ff379e232ab Mon Sep 17 00:00:00 2001
From: Mikaël Capelle <capelle.mikael@gmail.com>
Date: Mon, 11 Jan 2021 20:58:53 +0100
Subject: Combine nested if clauses.

---
 src/modlistbypriorityproxy.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

(limited to 'src')

diff --git a/src/modlistbypriorityproxy.cpp b/src/modlistbypriorityproxy.cpp
index 4fd589a4..b5d17c46 100644
--- a/src/modlistbypriorityproxy.cpp
+++ b/src/modlistbypriorityproxy.cpp
@@ -234,10 +234,8 @@ bool ModListByPriorityProxy::canDropMimeData(const QMimeData* data, Qt::DropActi
   }
 
   // the row may be outside of the children list if we insert at the end
-  if (!parent.isValid()) {
-    if (row >= m_Root.children.size()) {
-      return false;
-    }
+  if (!parent.isValid() && row >= m_Root.children.size()) {
+    return false;
   }
 
   return QAbstractProxyModel::canDropMimeData(data, action, row, column, parent);
-- 
cgit v1.3.1


From 18f70a2dff138f273a9b6babaec226775a03649f Mon Sep 17 00:00:00 2001
From: Mikaël Capelle <capelle.mikael@gmail.com>
Date: Mon, 11 Jan 2021 21:07:25 +0100
Subject: Add documentation for row computation.

---
 src/modlistbypriorityproxy.cpp | 35 ++++++++++++++++++++++++++++++++---
 src/modlistsortproxy.cpp       | 11 ++++++-----
 2 files changed, 38 insertions(+), 8 deletions(-)

(limited to 'src')

diff --git a/src/modlistbypriorityproxy.cpp b/src/modlistbypriorityproxy.cpp
index b5d17c46..9b5b2f03 100644
--- a/src/modlistbypriorityproxy.cpp
+++ b/src/modlistbypriorityproxy.cpp
@@ -243,8 +243,9 @@ bool ModListByPriorityProxy::canDropMimeData(const QMimeData* data, Qt::DropActi
 
 bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction action, int row, int column, const QModelIndex& parent)
 {
-  // we need to fix the source model row
-  ModListDropInfo dropInfo(data, m_core);
+  // we need to fix the source model row if we are dropping at a
+  // given priority (not a local file)
+  const ModListDropInfo dropInfo(data, m_core);
   int sourceRow = -1;
 
   if (dropInfo.isLocalFileDrop()) {
@@ -257,8 +258,14 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
     if (row >= 0) {
       if (!parent.isValid()) {
         if (row < m_Root.children.size()) {
+
           if (m_sortOrder == Qt::AscendingOrder) {
             sourceRow = m_Root.children[row]->index;
+
+            // fix bug when dropping a mod just below an expanded separator
+            //
+            // by default, Qt consider it's a drop at the end of that separator
+            // but we want to make it a drop at the beginning
             if (row > 0
               && m_sortOrder == Qt::AscendingOrder
               && m_Root.children[row - 1]->mod->isSeparator()
@@ -270,6 +277,9 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
           }
           else {
             sourceRow = m_Root.children[row - 1]->index;
+
+            // fix drop below a collapsed separator or at the end of an expanded
+            // separator, above the next item
             if (row > 0
               && m_sortOrder == Qt::DescendingOrder
               && m_Root.children[row - 1]->mod->isSeparator()
@@ -284,15 +294,21 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
           sourceRow = ModInfo::getNumMods();
         }
       }
+
+      // the parent is valid, we are dropping in a separator
       else {
         auto* item = static_cast<TreeItem*>(parent.internalPointer());
 
+        // we usually need to decrement the row in descending priority, but if
+        // it's the first row, we need to go back to the separator itself
         if (m_sortOrder == Qt::DescendingOrder
           && row == 0 && m_dropPosition == ModListView::DropPosition::AboveItem) {
           sourceRow = item->index;
         }
         else {
 
+          // in descending priority, we decrement the row to fix the drop position
+          // because this is not done by the sort proxy for us
           if (m_sortOrder == Qt::DescendingOrder) {
             row--;
           }
@@ -308,11 +324,24 @@ bool ModListByPriorityProxy::dropMimeData(const QMimeData* data, Qt::DropAction
 
       }
     }
+
+    // row < 0 and valid parent means we are dropping into an item,
+    // which can only be a separator since dropping into non-separators
+    // is disabled
+    //
+    // we want to drop at the end of the separator, so we need to find
+    // the right priority
     else if (parent.isValid()) {
-      // this is a drop in a separator
+
+      // in ascending priority, we take the priority of the next top-level
+      // item, which can be a separator or the overwrite mod, but is guaranteed
+      // to exist
       if (m_sortOrder == Qt::AscendingOrder) {
         sourceRow = m_Root.children[parent.row() + 1]->index;
       }
+
+      // in descending priority, we take the separator itself if it's empty or
+      // its last children
       else {
         auto* item = m_Root.children[parent.row()].get();
         sourceRow = item->children.empty() ? item->index : item->children.back()->index;
diff --git a/src/modlistsortproxy.cpp b/src/modlistsortproxy.cpp
index 1077a833..18e91e6e 100644
--- a/src/modlistsortproxy.cpp
+++ b/src/modlistsortproxy.cpp
@@ -622,9 +622,7 @@ bool ModListSortProxy::canDropMimeData(const QMimeData* data, Qt::DropAction act
     }
   }
 
-  // in the regular model, when dropping between rows, the row-value passed to
-  // the sourceModel is inconsistent between ascending and descending ordering.
-  // This should fix that
+  // see dropMimeData for details
   if (sortOrder() == Qt::DescendingOrder && row != -1 && !sourceIsByPriorityProxy()) {
     --row;
   }
@@ -648,8 +646,11 @@ bool ModListSortProxy::dropMimeData(const QMimeData *data, Qt::DropAction action
   }
 
   // in the regular model, when dropping between rows, the row-value passed to
-  // the sourceModel is inconsistent between ascending and descending ordering.
-  // This should fix that
+  // the sourceModel is inconsistent between ascending and descending ordering
+  //
+  // we want to fix that, but we cannot do it for the by-priority proxy because
+  // it messes up with non top-level items, so we simply forward the row and the
+  // by-priority proxy will fix the row for us
   if (sortOrder() == Qt::DescendingOrder && row != -1 && !sourceIsByPriorityProxy()) {
     --row;
   }
-- 
cgit v1.3.1