From bf9f737c609a2831ad52aab68030fb888c64bd01 Mon Sep 17 00:00:00 2001
From: FearlessTobi <thm.frey@gmail.com>
Date: Mon, 18 Jan 2021 01:22:54 +0100
Subject: [PATCH] game_list: Fix folder reordering

The bug(s) happened because we swapped the contents on values.game_dirs, but the pointer each item had to their respective game_dir wasn't updated. This made it so that the item had the wrong game_dir associated with it after a "move up" or "move down" operation. It can be observed by choosing "open directory location" after such operation.

Changed from raw pointer to an index because it's equivalent but a bit clearer, but the change is not essential.

Co-Authored-By: Vitor K <29167336+vitor-k@users.noreply.github.com>
---
 src/yuzu/game_list.cpp | 58 ++++++++++++++++++++++--------------------
 src/yuzu/game_list_p.h |  2 +-
 2 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/src/yuzu/game_list.cpp b/src/yuzu/game_list.cpp
index 37b0d1a0e..9afd5b45f 100644
--- a/src/yuzu/game_list.cpp
+++ b/src/yuzu/game_list.cpp
@@ -173,8 +173,8 @@ void GameList::OnItemExpanded(const QModelIndex& item) {
         return;
     }
 
-    auto* game_dir = item.data(GameListDir::GameDirRole).value<UISettings::GameDir*>();
-    game_dir->expanded = tree_view->isExpanded(item);
+    UISettings::values.game_dirs[item.data(GameListDir::GameDirRole).toInt()].expanded =
+        tree_view->isExpanded(item);
 }
 
 // Event in order to filter the gamelist after editing the searchfield
@@ -262,9 +262,9 @@ void GameList::OnUpdateThemedIcons() {
                 Qt::DecorationRole);
             break;
         case GameListItemType::CustomDir: {
-            const UISettings::GameDir* game_dir =
-                child->data(GameListDir::GameDirRole).value<UISettings::GameDir*>();
-            const QString icon_name = QFileInfo::exists(game_dir->path)
+            const UISettings::GameDir& game_dir =
+                UISettings::values.game_dirs[child->data(GameListDir::GameDirRole).toInt()];
+            const QString icon_name = QFileInfo::exists(game_dir.path)
                                           ? QStringLiteral("folder")
                                           : QStringLiteral("bad_folder");
             child->setData(
@@ -366,7 +366,7 @@ void GameList::AddDirEntry(GameListDir* entry_items) {
     item_model->invisibleRootItem()->appendRow(entry_items);
     tree_view->setExpanded(
         entry_items->index(),
-        entry_items->data(GameListDir::GameDirRole).value<UISettings::GameDir*>()->expanded);
+        UISettings::values.game_dirs[entry_items->data(GameListDir::GameDirRole).toInt()].expanded);
 }
 
 void GameList::AddEntry(const QList<QStandardItem*>& entry_items, GameListDir* parent) {
@@ -549,7 +549,7 @@ void GameList::AddGamePopup(QMenu& context_menu, u64 program_id, const std::stri
 
 void GameList::AddCustomDirPopup(QMenu& context_menu, QModelIndex selected) {
     UISettings::GameDir& game_dir =
-        *selected.data(GameListDir::GameDirRole).value<UISettings::GameDir*>();
+        UISettings::values.game_dirs[selected.data(GameListDir::GameDirRole).toInt()];
 
     QAction* deep_scan = context_menu.addAction(tr("Scan Subfolders"));
     QAction* delete_dir = context_menu.addAction(tr("Remove Game Directory"));
@@ -568,8 +568,7 @@ void GameList::AddCustomDirPopup(QMenu& context_menu, QModelIndex selected) {
 }
 
 void GameList::AddPermDirPopup(QMenu& context_menu, QModelIndex selected) {
-    UISettings::GameDir& game_dir =
-        *selected.data(GameListDir::GameDirRole).value<UISettings::GameDir*>();
+    const int game_dir_index = selected.data(GameListDir::GameDirRole).toInt();
 
     QAction* move_up = context_menu.addAction(tr("\u25B2 Move Up"));
     QAction* move_down = context_menu.addAction(tr("\u25bc Move Down"));
@@ -580,34 +579,39 @@ void GameList::AddPermDirPopup(QMenu& context_menu, QModelIndex selected) {
     move_up->setEnabled(row > 0);
     move_down->setEnabled(row < item_model->rowCount() - 2);
 
-    connect(move_up, &QAction::triggered, [this, selected, row, &game_dir] {
-        // find the indices of the items in settings and swap them
-        std::swap(UISettings::values.game_dirs[UISettings::values.game_dirs.indexOf(game_dir)],
-                  UISettings::values.game_dirs[UISettings::values.game_dirs.indexOf(
-                      *selected.sibling(row - 1, 0)
-                           .data(GameListDir::GameDirRole)
-                           .value<UISettings::GameDir*>())]);
+    connect(move_up, &QAction::triggered, [this, selected, row, game_dir_index] {
+        const int other_index = selected.sibling(row - 1, 0).data(GameListDir::GameDirRole).toInt();
+        // swap the items in the settings
+        std::swap(UISettings::values.game_dirs[game_dir_index],
+                  UISettings::values.game_dirs[other_index]);
+        // swap the indexes held by the QVariants
+        item_model->setData(selected, QVariant(other_index), GameListDir::GameDirRole);
+        item_model->setData(selected.sibling(row - 1, 0), QVariant(game_dir_index),
+                            GameListDir::GameDirRole);
         // move the treeview items
         QList<QStandardItem*> item = item_model->takeRow(row);
         item_model->invisibleRootItem()->insertRow(row - 1, item);
-        tree_view->setExpanded(selected, game_dir.expanded);
+        tree_view->setExpanded(selected, UISettings::values.game_dirs[game_dir_index].expanded);
     });
 
-    connect(move_down, &QAction::triggered, [this, selected, row, &game_dir] {
-        // find the indices of the items in settings and swap them
-        std::swap(UISettings::values.game_dirs[UISettings::values.game_dirs.indexOf(game_dir)],
-                  UISettings::values.game_dirs[UISettings::values.game_dirs.indexOf(
-                      *selected.sibling(row + 1, 0)
-                           .data(GameListDir::GameDirRole)
-                           .value<UISettings::GameDir*>())]);
+    connect(move_down, &QAction::triggered, [this, selected, row, game_dir_index] {
+        const int other_index = selected.sibling(row + 1, 0).data(GameListDir::GameDirRole).toInt();
+        // swap the items in the settings
+        std::swap(UISettings::values.game_dirs[game_dir_index],
+                  UISettings::values.game_dirs[other_index]);
+        // swap the indexes held by the QVariants
+        item_model->setData(selected, QVariant(other_index), GameListDir::GameDirRole);
+        item_model->setData(selected.sibling(row + 1, 0), QVariant(game_dir_index),
+                            GameListDir::GameDirRole);
         // move the treeview items
         const QList<QStandardItem*> item = item_model->takeRow(row);
         item_model->invisibleRootItem()->insertRow(row + 1, item);
-        tree_view->setExpanded(selected, game_dir.expanded);
+        tree_view->setExpanded(selected, UISettings::values.game_dirs[game_dir_index].expanded);
     });
 
-    connect(open_directory_location, &QAction::triggered,
-            [this, game_dir] { emit OpenDirectory(game_dir.path); });
+    connect(open_directory_location, &QAction::triggered, [this, game_dir_index] {
+        emit OpenDirectory(UISettings::values.game_dirs[game_dir_index].path);
+    });
 }
 
 void GameList::LoadCompatibilityList() {
diff --git a/src/yuzu/game_list_p.h b/src/yuzu/game_list_p.h
index df935022d..f25445f18 100644
--- a/src/yuzu/game_list_p.h
+++ b/src/yuzu/game_list_p.h
@@ -230,7 +230,7 @@ public:
         setData(type(), TypeRole);
 
         UISettings::GameDir* game_dir = &directory;
-        setData(QVariant::fromValue(game_dir), GameDirRole);
+        setData(QVariant(UISettings::values.game_dirs.indexOf(directory)), GameDirRole);
 
         const int icon_size = std::min(static_cast<int>(UISettings::values.icon_size), 64);
         switch (dir_type) {