From 7d9ad88bc5f43c224e28ec67522ba0bac6d0d3c6 Mon Sep 17 00:00:00 2001 From: Michael M Date: Sat, 19 Aug 2017 15:47:19 -0700 Subject: [PATCH 1/2] Add initial paths to GameTracker after construction It's strange to see GameTracker add its own initial paths in construction, because you might expect a race condition where the GameLoaded signal is emitted before it gets connected to in GameListModel. In fact, this doesn't happen, but only because of how it abuses the Qt signals mechanism to load files asynchronously: GameLoader emits a GameLoaded signal which gets forwarded to the GameTracker::GameLoaded signal _after_ control returns to the event loop, at which point GameListModel has connected. This commit moves the logic of adding initial paths out of GameTracker to a point after the signals are connected, which is more obvious and doesn't rely on how GameTracker implements concurrency. --- Source/Core/DolphinQt2/GameList/GameListModel.cpp | 3 +++ Source/Core/DolphinQt2/GameList/GameTracker.cpp | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/DolphinQt2/GameList/GameListModel.cpp b/Source/Core/DolphinQt2/GameList/GameListModel.cpp index 860fbbe9d0..44561c7f1e 100644 --- a/Source/Core/DolphinQt2/GameList/GameListModel.cpp +++ b/Source/Core/DolphinQt2/GameList/GameListModel.cpp @@ -17,6 +17,9 @@ GameListModel::GameListModel(QObject* parent) : QAbstractTableModel(parent) connect(this, &GameListModel::DirectoryAdded, &m_tracker, &GameTracker::AddDirectory); connect(this, &GameListModel::DirectoryRemoved, &m_tracker, &GameTracker::RemoveDirectory); + for (const QString& dir : Settings::Instance().GetPaths()) + m_tracker.AddDirectory(dir); + connect(&Settings::Instance(), &Settings::ThemeChanged, [this] { // Tell the view to repaint. The signal 'dataChanged' also seems like it would work here, but // unfortunately it won't cause a repaint until the view is focused. diff --git a/Source/Core/DolphinQt2/GameList/GameTracker.cpp b/Source/Core/DolphinQt2/GameList/GameTracker.cpp index fb832512b5..6050feaa8d 100644 --- a/Source/Core/DolphinQt2/GameList/GameTracker.cpp +++ b/Source/Core/DolphinQt2/GameList/GameTracker.cpp @@ -28,9 +28,6 @@ GameTracker::GameTracker(QObject* parent) : QFileSystemWatcher(parent) connect(m_loader, &GameLoader::GameLoaded, this, &GameTracker::GameLoaded); m_loader_thread.start(); - - for (QString dir : Settings::Instance().GetPaths()) - AddDirectory(dir); } GameTracker::~GameTracker() From 623026f42066be41790b2625b334f9a3b3912f04 Mon Sep 17 00:00:00 2001 From: Michael M Date: Sat, 19 Aug 2017 15:47:48 -0700 Subject: [PATCH 2/2] Connect to Settings::PathAdded/Removed directly in GameListModel It no longer makes sense to connect these signals in GameList, since GameListModel is now owned by Settings. --- Source/Core/DolphinQt2/GameList/GameList.cpp | 2 -- Source/Core/DolphinQt2/GameList/GameListModel.cpp | 4 ++-- Source/Core/DolphinQt2/GameList/GameListModel.h | 4 ---- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/Source/Core/DolphinQt2/GameList/GameList.cpp b/Source/Core/DolphinQt2/GameList/GameList.cpp index 3e71ac7aa8..c51708980e 100644 --- a/Source/Core/DolphinQt2/GameList/GameList.cpp +++ b/Source/Core/DolphinQt2/GameList/GameList.cpp @@ -49,8 +49,6 @@ GameList::GameList(QWidget* parent) : QStackedWidget(parent) connect(m_list, &QTableView::doubleClicked, this, &GameList::GameSelected); connect(m_grid, &QListView::doubleClicked, this, &GameList::GameSelected); - connect(&Settings::Instance(), &Settings::PathAdded, m_model, &GameListModel::DirectoryAdded); - connect(&Settings::Instance(), &Settings::PathRemoved, m_model, &GameListModel::DirectoryRemoved); connect(m_model, &QAbstractItemModel::rowsInserted, this, &GameList::ConsiderViewChange); connect(m_model, &QAbstractItemModel::rowsRemoved, this, &GameList::ConsiderViewChange); diff --git a/Source/Core/DolphinQt2/GameList/GameListModel.cpp b/Source/Core/DolphinQt2/GameList/GameListModel.cpp index 44561c7f1e..0cf62c92e0 100644 --- a/Source/Core/DolphinQt2/GameList/GameListModel.cpp +++ b/Source/Core/DolphinQt2/GameList/GameListModel.cpp @@ -14,8 +14,8 @@ GameListModel::GameListModel(QObject* parent) : QAbstractTableModel(parent) { connect(&m_tracker, &GameTracker::GameLoaded, this, &GameListModel::UpdateGame); connect(&m_tracker, &GameTracker::GameRemoved, this, &GameListModel::RemoveGame); - connect(this, &GameListModel::DirectoryAdded, &m_tracker, &GameTracker::AddDirectory); - connect(this, &GameListModel::DirectoryRemoved, &m_tracker, &GameTracker::RemoveDirectory); + connect(&Settings::Instance(), &Settings::PathAdded, &m_tracker, &GameTracker::AddDirectory); + connect(&Settings::Instance(), &Settings::PathRemoved, &m_tracker, &GameTracker::RemoveDirectory); for (const QString& dir : Settings::Instance().GetPaths()) m_tracker.AddDirectory(dir); diff --git a/Source/Core/DolphinQt2/GameList/GameListModel.h b/Source/Core/DolphinQt2/GameList/GameListModel.h index 1230573d98..7e3d926f86 100644 --- a/Source/Core/DolphinQt2/GameList/GameListModel.h +++ b/Source/Core/DolphinQt2/GameList/GameListModel.h @@ -48,10 +48,6 @@ public: void UpdateGame(QSharedPointer game); void RemoveGame(const QString& path); -signals: - void DirectoryAdded(const QString& dir); - void DirectoryRemoved(const QString& dir); - private: // Index in m_games, or -1 if it isn't found int FindGame(const QString& path) const;