diff --git a/src/downloadlist.cpp b/src/downloadlist.cpp index b461be505..bf72c42cd 100644 --- a/src/downloadlist.cpp +++ b/src/downloadlist.cpp @@ -35,8 +35,10 @@ DownloadList::DownloadList(OrganizerCore& core, QObject* parent) : QAbstractTableModel(parent), m_manager(*core.downloadManager()), m_settings(core.settings()) { - connect(&m_manager, SIGNAL(update(int)), this, SLOT(update(int))); - connect(&m_manager, SIGNAL(aboutToUpdate()), this, SLOT(aboutToUpdate())); + connect(&m_manager, &DownloadManager::aboutToResetModel, this, + &DownloadList::onAboutToResetModel); + connect(&m_manager, &DownloadManager::modelReset, this, &DownloadList::onModelReset); + connect(&m_manager, &DownloadManager::rowChanged, this, &DownloadList::onRowChanged); } int DownloadList::rowCount(const QModelIndex& parent) const @@ -56,7 +58,9 @@ int DownloadList::columnCount(const QModelIndex&) const QModelIndex DownloadList::index(int row, int column, const QModelIndex&) const { - return createIndex(row, column, row); + // Embed the stable DownloadID in internalId() so any consumer of the index + // can identify the download without having to track row shifts. + return createIndex(row, column, m_manager.downloadIDAtRow(row)); } QModelIndex DownloadList::parent(const QModelIndex&) const @@ -110,14 +114,14 @@ QVariant DownloadList::data(const QModelIndex& index, int role) const bool pendingDownload = index.row() >= m_manager.numTotalDownloads(); if (role == Qt::DisplayRole) { if (pendingDownload) { - std::tuple nexusids = + const DownloadManager::PendingDownload pending = m_manager.getPendingDownload(index.row() - m_manager.numTotalDownloads()); switch (index.column()) { case COL_NAME: return tr("< game %1 mod %2 file %3 >") - .arg(std::get<0>(nexusids)) - .arg(std::get<1>(nexusids)) - .arg(std::get<2>(nexusids)); + .arg(pending.gameName) + .arg(pending.modID) + .arg(pending.fileID); case COL_SIZE: return tr("Unknown"); case COL_STATUS: @@ -239,16 +243,19 @@ QVariant DownloadList::data(const QModelIndex& index, int role) const return QVariant(); } -void DownloadList::aboutToUpdate() +void DownloadList::onAboutToResetModel() { - emit beginResetModel(); + beginResetModel(); } -void DownloadList::update(int row) +void DownloadList::onModelReset() { - if (row < 0) - emit endResetModel(); - else if (row < this->rowCount()) + endResetModel(); +} + +void DownloadList::onRowChanged(int row) +{ + if (row < this->rowCount()) emit dataChanged( this->index(row, 0, QModelIndex()), this->index(row, this->columnCount(QModelIndex()) - 1, QModelIndex())); diff --git a/src/downloadlist.h b/src/downloadlist.h index c9adaa7f2..538da71ac 100644 --- a/src/downloadlist.h +++ b/src/downloadlist.h @@ -83,16 +83,20 @@ class DownloadList : public QAbstractTableModel // bool lessThanPredicate(const QModelIndex& left, const QModelIndex& right); -public slots: +private slots: /** - * @brief used to inform the model that data has changed - * - * @param row the row that changed. This can be negative to update the whole view - **/ - void update(int row); + * @brief full reset (row count changed). Drops selection and scroll state. + */ + void onAboutToResetModel(); + void onModelReset(); - void aboutToUpdate(); + /** + * @brief single-row data change (row count unchanged). Preserves view state. + * + * @param row the row that changed + */ + void onRowChanged(int row); private: DownloadManager& m_manager; diff --git a/src/downloadmanager.cpp b/src/downloadmanager.cpp index c5d4c13ee..d8f199465 100644 --- a/src/downloadmanager.cpp +++ b/src/downloadmanager.cpp @@ -35,9 +35,9 @@ along with Mod Organizer. If not, see . #include #include -#include #include #include +#include #include #include #include @@ -55,15 +55,20 @@ using namespace MOBase; static const char UNFINISHED[] = ".unfinished"; -unsigned int DownloadManager::DownloadInfo::s_NextDownloadID = 1U; -int DownloadManager::m_DirWatcherDisabler = 0; +DownloadManager::DownloadID DownloadManager::DownloadInfo::s_NextDownloadID = 1U; + +DownloadManager::DownloadID DownloadManager::DownloadInfo::newDownloadID() +{ + return s_NextDownloadID++; +} DownloadManager::DownloadInfo* DownloadManager::DownloadInfo::createNew(const ModRepositoryFileInfo* fileInfo, - const QStringList& URLs) + const QStringList& URLs, + std::optional reservedID) { DownloadInfo* info = new DownloadInfo; - info->m_DownloadID = s_NextDownloadID++; + info->m_DownloadID = reservedID.has_value() ? *reservedID : newDownloadID(); info->m_StartTime.start(); info->m_PreResumeSize = 0LL; info->m_Progress = std::make_pair(0, "0.0 B/s "); @@ -84,21 +89,22 @@ DownloadManager::DownloadInfo::createFromMeta(const QString& filePath, bool show const QString outputDirectory, std::optional fileSize) { - DownloadInfo* info = new DownloadInfo; - QString metaFileName = filePath + ".meta"; QFileInfo metaFileInfo(metaFileName); if (QDir::fromNativeSeparators(metaFileInfo.path()) .compare(QDir::fromNativeSeparators(outputDirectory), Qt::CaseInsensitive) != 0) return nullptr; + QSettings metaFile(metaFileName, QSettings::IniFormat); - if (!showHidden && metaFile.value("removed", false).toBool()) { + const bool hidden = metaFile.value("removed", false).toBool(); + if (!showHidden && hidden) { return nullptr; - } else { - info->m_Hidden = metaFile.value("removed", false).toBool(); } + DownloadInfo* info = new DownloadInfo; + info->m_Hidden = hidden; + QString fileName = QFileInfo(filePath).fileName(); if (fileName.endsWith(UNFINISHED)) { @@ -119,7 +125,7 @@ DownloadManager::DownloadInfo::createFromMeta(const QString& filePath, bool show } } - info->m_DownloadID = s_NextDownloadID++; + info->m_DownloadID = newDownloadID(); info->m_Output.setFileName(filePath); info->m_TotalSize = fileSize ? *fileSize : QFileInfo(filePath).size(); info->m_PreResumeSize = info->m_TotalSize; @@ -156,33 +162,77 @@ DownloadManager::DownloadInfo::createFromMeta(const QString& filePath, bool show return info; } -ScopedDisableDirWatcher::ScopedDisableDirWatcher(DownloadManager* downloadManager) +DirWatcherManager::DirWatcherManager(QObject* parent) : QObject(parent) { - m_downloadManager = downloadManager; - m_downloadManager->startDisableDirWatcher(); - log::debug("Scoped Disable DirWatcher: Started"); + connect(&m_watcher, &QFileSystemWatcher::directoryChanged, this, + &DirWatcherManager::onDirectoryChanged); } -ScopedDisableDirWatcher::~ScopedDisableDirWatcher() +void DirWatcherManager::setPath(const QString& path) { - m_downloadManager->endDisableDirWatcher(); - m_downloadManager = nullptr; - log::debug("Scoped Disable DirWatcher: Stopped"); + const QStringList existing = m_watcher.directories(); + if (!existing.isEmpty()) { + m_watcher.removePaths(existing); + } + m_watcher.addPath(path); } -void DownloadManager::startDisableDirWatcher() +bool DirWatcherManager::isSuspended() const { - DownloadManager::m_DirWatcherDisabler++; + return m_suspendDepth > 0; } -void DownloadManager::endDisableDirWatcher() +void DirWatcherManager::onDirectoryChanged(const QString&) { - if (DownloadManager::m_DirWatcherDisabler > 0) { - if (DownloadManager::m_DirWatcherDisabler == 1) - QCoreApplication::processEvents(); - DownloadManager::m_DirWatcherDisabler--; - } else { - DownloadManager::m_DirWatcherDisabler = 0; + if (!isSuspended()) { + emit directoryChanged(); + } +} + +void DirWatcherManager::releaseSuspension() +{ + --m_suspendDepth; +} + +DirWatcherManager::Guard::Guard(DirWatcherManager& manager) : m_manager(manager) +{ + ++m_manager.m_suspendDepth; +} + +DirWatcherManager::Guard::~Guard() +{ + // Queued so file system notifications already posted during the + // suspension are dispatched (and filtered) before the suspension lifts. + QMetaObject::invokeMethod(&m_manager, &DirWatcherManager::releaseSuspension, + Qt::QueuedConnection); +} + +DirWatcherManager::Guard DirWatcherManager::scopedGuard() +{ + return Guard(*this); +} + +DownloadManager::ModelResetGuard::ModelResetGuard(DownloadManager& manager) + : m_manager(manager) +{ + if (m_manager.m_modelResetDepth++ == 0) { + emit m_manager.aboutToResetModel(); + } +} + +DownloadManager::ModelResetGuard::~ModelResetGuard() +{ + if (--m_manager.m_modelResetDepth == 0) { + emit m_manager.modelReset(); + } +} + +void DownloadManager::notifyRowChanged(int row) +{ + // suppressed while a reset is in progress; the outer reset will refresh + // all rows on release + if (m_modelResetDepth == 0) { + emit rowChanged(row); } } @@ -225,15 +275,12 @@ QString DownloadManager::DownloadInfo::currentURL() } DownloadManager::DownloadManager(NexusInterface* nexusInterface, QObject* parent) - : m_NexusInterface(nexusInterface), m_DirWatcher(), m_ShowHidden(false), + : m_NexusInterface(nexusInterface), m_DirWatcher(this), m_ShowHidden(false), m_ParentWidget(nullptr) { m_OrganizerCore = dynamic_cast(parent); - connect(&m_DirWatcher, SIGNAL(directoryChanged(QString)), this, - SLOT(directoryChanged(QString))); - m_TimeoutTimer.setSingleShot(false); - // connect(&m_TimeoutTimer, SIGNAL(timeout()), this, SLOT(checkDownloadTimeout())); - m_TimeoutTimer.start(5 * 1000); + connect(&m_DirWatcher, &DirWatcherManager::directoryChanged, this, + &DownloadManager::refreshList); } DownloadManager::~DownloadManager() @@ -243,6 +290,7 @@ DownloadManager::~DownloadManager() delete *iter; } m_ActiveDownloads.clear(); + m_ByID.clear(); } void DownloadManager::setParentWidget(QWidget* w) @@ -274,49 +322,44 @@ bool DownloadManager::downloadsInProgressNoPause() void DownloadManager::pauseAll() { - - // first loop: pause all downloads - for (int i = 0; i < m_ActiveDownloads.count(); ++i) { - if (m_ActiveDownloads[i]->m_State < STATE_READY) { - pauseDownload(i); + for (DownloadInfo* info : m_ActiveDownloads) { + if (info->m_State < STATE_READY) { + pauseDownload(info->m_DownloadID); } } - ::Sleep(100); - - bool done = false; - QTime startTime = QTime::currentTime(); - // further loops: busy waiting for all downloads to complete. This could be neater... - while (!done && (startTime.secsTo(QTime::currentTime()) < 5)) { - QCoreApplication::processEvents(); - done = true; - foreach (DownloadInfo* info, m_ActiveDownloads) { - if ((info->m_State < STATE_CANCELED) || - (info->m_State == STATE_FETCHINGFILEINFO) || - (info->m_State == STATE_FETCHINGMODINFO) || - (info->m_State == STATE_FETCHINGMODINFO_MD5)) { - done = false; - break; + auto stillTransitioning = [this] { + for (const DownloadInfo* info : m_ActiveDownloads) { + if (info->m_State < STATE_CANCELED || info->m_State == STATE_FETCHINGFILEINFO || + info->m_State == STATE_FETCHINGMODINFO || + info->m_State == STATE_FETCHINGMODINFO_MD5) { + return true; } } - if (!done) { - ::Sleep(100); + return false; + }; + + QEventLoop loop; + QTimer::singleShot(5000, &loop, &QEventLoop::quit); + const auto conn = connect(this, &DownloadManager::stateChanged, &loop, [&] { + if (!stillTransitioning()) { + loop.quit(); } + }); + if (stillTransitioning()) { + loop.exec(); } + disconnect(conn); } void DownloadManager::setOutputDirectory(const QString& outputDirectory, const bool refresh) { - QStringList directories = m_DirWatcher.directories(); - if (directories.length() != 0) { - m_DirWatcher.removePaths(directories); - } m_OutputDirectory = QDir::fromNativeSeparators(outputDirectory); if (refresh) { refreshList(); } - m_DirWatcher.addPath(m_OutputDirectory); + m_DirWatcher.setPath(m_OutputDirectory); } void DownloadManager::setShowHidden(bool showHidden) @@ -334,18 +377,16 @@ void DownloadManager::refreshList() { TimeThis tt("DownloadManager::refreshList()"); - try { - emit aboutToUpdate(); - // avoid triggering other refreshes - ScopedDisableDirWatcher scopedDirWatcher(this); - - int downloadsBefore = m_ActiveDownloads.size(); + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); + ModelResetGuard guard(*this); + try { // remove finished downloads for (QVector::iterator iter = m_ActiveDownloads.begin(); iter != m_ActiveDownloads.end();) { if (((*iter)->m_State == STATE_READY) || ((*iter)->m_State == STATE_INSTALLED) || ((*iter)->m_State == STATE_UNINSTALLED)) { + m_ByID.remove((*iter)->m_DownloadID); delete *iter; iter = m_ActiveDownloads.erase(iter); } else { @@ -430,15 +471,13 @@ void DownloadManager::refreshList() } cx.self.m_ActiveDownloads.push_front(info); + cx.self.m_ByID.insert(info->m_DownloadID, info); cx.seen.insert(std::move(lc)); cx.seen.insert( QFileInfo(info->m_Output.fileName()).fileName().toLower().toStdWString()); }); log::debug("saw {} downloads", m_ActiveDownloads.size()); - - emit update(-1); - } catch (const std::bad_alloc&) { reportError(tr("Memory allocation error (in refreshing directory).")); } @@ -447,7 +486,7 @@ void DownloadManager::refreshList() void DownloadManager::queryDownloadListInfo() { int incompleteCount = 0; - for (size_t i = 0; i < m_ActiveDownloads.size(); i++) { + for (int i = 0; i < m_ActiveDownloads.size(); i++) { if (isInfoIncomplete(i)) { incompleteCount++; } @@ -463,19 +502,21 @@ void DownloadManager::queryDownloadListInfo() QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { TimeThis tt("DownloadManager::queryDownloadListInfo()"); log::info("Querying metadata for every download with incomplete info..."); - startDisableDirWatcher(); - for (size_t i = 0; i < m_ActiveDownloads.size(); i++) { - if (isInfoIncomplete(i)) { - queryInfoMd5(i, false); + { + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); + for (int i = 0; i < m_ActiveDownloads.size(); i++) { + if (isInfoIncomplete(i)) { + queryInfoMd5(i, false); + } } } - endDisableDirWatcher(); log::info("Metadata has been retrieved successfully!"); } } bool DownloadManager::addDownload(const QStringList& URLs, QString gameName, int modID, - int fileID, const ModRepositoryFileInfo* fileInfo) + int fileID, const ModRepositoryFileInfo* fileInfo, + std::optional reservedID) { QString fileName = QFileInfo(URLs.first()).fileName(); if (fileName.isEmpty()) { @@ -513,7 +554,7 @@ bool DownloadManager::addDownload(const QStringList& URLs, QString gameName, int QNetworkRequest::AlwaysNetwork); request.setHttp2Configuration(h2Conf); return addDownload(m_NexusInterface->getAccessManager()->get(request), URLs, fileName, - gameName, modID, fileID, fileInfo); + gameName, modID, fileID, fileInfo, reservedID); } bool DownloadManager::addDownload(QNetworkReply* reply, @@ -530,11 +571,12 @@ bool DownloadManager::addDownload(QNetworkReply* reply, bool DownloadManager::addDownload(QNetworkReply* reply, const QStringList& URLs, const QString& fileName, QString gameName, int modID, - int fileID, const ModRepositoryFileInfo* fileInfo) + int fileID, const ModRepositoryFileInfo* fileInfo, + std::optional reservedID) { // download invoked from an already open network reply (i.e. download link in the // browser) - DownloadInfo* newDownload = DownloadInfo::createNew(fileInfo, URLs); + DownloadInfo* newDownload = DownloadInfo::createNew(fileInfo, URLs, reservedID); QString baseName = fileName; if (!fileInfo->fileName.isEmpty()) { @@ -553,40 +595,77 @@ bool DownloadManager::addDownload(QNetworkReply* reply, const QStringList& URLs, baseName.truncate(queryIndex); } - startDisableDirWatcher(); - newDownload->setName(getDownloadFileName(baseName), false); - endDisableDirWatcher(); + bool renameToUnique = false; + if (QFile::exists(m_OutputDirectory + "/" + MOBase::sanitizeFileName(baseName))) { + if (QMessageBox::question( + m_ParentWidget, tr("Download again?"), + tr("A file with the same name \"%1\" has already been downloaded. " + "Do you want to download it again? The new file will receive a " + "different name.") + .arg(baseName), + QMessageBox::Yes | QMessageBox::No) == QMessageBox::No) { + cancelPendingDownload(newDownload, reply); + return false; + } + renameToUnique = true; + } - startDownload(reply, newDownload, false); - // emit update(-1); - return true; + try { + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); + newDownload->setName(getDownloadFileName(baseName, renameToUnique), false); + } catch (const std::exception& e) { + log::error("exception preparing download: {}", e.what()); + cancelPendingDownload(newDownload, reply); + return false; + } + + return startDownload(reply, newDownload, false); } -void DownloadManager::removePending(QString gameName, int modID, int fileID) +void DownloadManager::notifyPendingDownloadFailed(const QString& gameName, int modID, + int fileID) { - QString gameShortName = gameName; - QStringList games(m_ManagedGame->validShortNames()); - games += m_ManagedGame->gameShortName(); - for (auto game : games) { - MOBase::IPluginGame* gamePlugin = m_OrganizerCore->getGame(game); - if (gamePlugin != nullptr && - gamePlugin->gameNexusName().compare(gameName, Qt::CaseInsensitive) == 0) { - gameShortName = gamePlugin->gameShortName(); - break; + for (const PendingDownload& pending : m_PendingDownloads) { + if (pending.gameName.compare(gameName, Qt::CaseInsensitive) == 0 && + pending.modID == modID && pending.fileID == fileID) { + m_DownloadFailed(pending.reservedID); + return; } } - emit aboutToUpdate(); - for (auto iter : m_PendingDownloads) { - if (gameShortName.compare(std::get<0>(iter), Qt::CaseInsensitive) == 0 && - (std::get<1>(iter) == modID) && (std::get<2>(iter) == fileID)) { - m_PendingDownloads.removeAt(m_PendingDownloads.indexOf(iter)); +} + +void DownloadManager::cancelPendingDownload(DownloadInfo* newDownload, + QNetworkReply* reply) +{ + notifyPendingDownloadFailed(newDownload->m_FileInfo->gameName, + newDownload->m_FileInfo->modID, + newDownload->m_FileInfo->fileID); + removePending(newDownload->m_FileInfo->gameName, newDownload->m_FileInfo->modID, + newDownload->m_FileInfo->fileID); + delete newDownload; + if (reply != nullptr) { + reply->abort(); + reply->deleteLater(); + } +} + +void DownloadManager::removePending(QString gameName, int modID, int fileID) +{ + QString gameShortName = getValidGameShortName(gameName); + + for (int i = 0; i < m_PendingDownloads.size(); ++i) { + const PendingDownload& pending = m_PendingDownloads.at(i); + if (gameShortName.compare(pending.gameName, Qt::CaseInsensitive) == 0 && + pending.modID == modID && pending.fileID == fileID) { + // only emit a reset if we actually have a row to remove + ModelResetGuard guard(*this); + m_PendingDownloads.removeAt(i); break; } } - emit update(-1); } -void DownloadManager::startDownload(QNetworkReply* reply, DownloadInfo* newDownload, +bool DownloadManager::startDownload(QNetworkReply* reply, DownloadInfo* newDownload, bool resume) { reply->setReadBufferSize( @@ -603,15 +682,21 @@ void DownloadManager::startDownload(QNetworkReply* reply, DownloadInfo* newDownl } newDownload->m_StartTime.start(); - createMetaFile(newDownload); if (!newDownload->m_Output.open(mode)) { reportError(tr("failed to download %1: could not open output file: %2") .arg(reply->url().toString()) .arg(newDownload->m_Output.fileName())); - return; + if (!resume) { + cancelPendingDownload(newDownload, reply); + } + return false; } + // Write the .meta only once we know the download is actually going to start. + // Otherwise a failed start leaves an orphan .meta on disk. + createMetaFile(newDownload); + connect(newDownload->m_Reply, SIGNAL(downloadProgress(qint64, qint64)), this, SLOT(downloadProgress(qint64, qint64))); connect(newDownload->m_Reply, SIGNAL(errorOccurred(QNetworkReply::NetworkError)), @@ -619,60 +704,35 @@ void DownloadManager::startDownload(QNetworkReply* reply, DownloadInfo* newDownl connect(newDownload->m_Reply, SIGNAL(readyRead()), this, SLOT(downloadReadyRead())); connect(newDownload->m_Reply, SIGNAL(metaDataChanged()), this, SLOT(metaDataChanged())); + connect(newDownload->m_Reply, SIGNAL(finished()), this, SLOT(onReplyFinished())); if (!resume) { newDownload->m_PreResumeSize = newDownload->m_Output.size(); - removePending(newDownload->m_FileInfo->gameName, newDownload->m_FileInfo->modID, - newDownload->m_FileInfo->fileID); - - emit aboutToUpdate(); - m_ActiveDownloads.append(newDownload); - - emit update(-1); + { + // coalesce the pending removal and the active append into one reset + ModelResetGuard guard(*this); + removePending(newDownload->m_FileInfo->gameName, newDownload->m_FileInfo->modID, + newDownload->m_FileInfo->fileID); + m_ActiveDownloads.append(newDownload); + m_ByID.insert(newDownload->m_DownloadID, newDownload); + } emit downloadAdded(); + } - if (QFile::exists(m_OutputDirectory + "/" + newDownload->m_FileName)) { - setState(newDownload, STATE_PAUSING); - QCoreApplication::processEvents(); - if (QMessageBox::question( - m_ParentWidget, tr("Download again?"), - tr("A file with the same name \"%1\" has already been downloaded. " - "Do you want to download it again? The new file will receive a " - "different name.") - .arg(newDownload->m_FileName), - QMessageBox::Yes | QMessageBox::No) == QMessageBox::No) { - if (reply->isFinished()) - setState(newDownload, STATE_CANCELED); - else - setState(newDownload, STATE_CANCELING); - } else { - startDisableDirWatcher(); - newDownload->setName(getDownloadFileName(newDownload->m_FileName, true), true); - endDisableDirWatcher(); - if (newDownload->m_State == STATE_PAUSED) - resumeDownload(indexByInfo(newDownload)); - else - setState(newDownload, STATE_DOWNLOADING); - } - } else - connect(newDownload->m_Reply, SIGNAL(finished()), this, SLOT(downloadFinished())); - - QCoreApplication::processEvents(); - - if (newDownload->m_State != STATE_DOWNLOADING && - newDownload->m_State != STATE_READY && - newDownload->m_State != STATE_FETCHINGMODINFO && reply->isFinished()) { - int index = indexByInfo(newDownload); - if (index >= 0) { - downloadFinished(index); - } - return; - } - } else - connect(newDownload->m_Reply, SIGNAL(finished()), this, SLOT(downloadFinished())); + // Reply may have finished before we connected; defer a manual finish via a + // queued call rather than reentering startDownload synchronously. + if (reply->isFinished()) { + QMetaObject::invokeMethod( + this, + [this, id = newDownload->m_DownloadID] { + finishDownload(id); + }, + Qt::QueuedConnection); + } + return true; } -void DownloadManager::addNXMDownload(const QString& url) +DownloadManager::DownloadID DownloadManager::addNXMDownload(const QString& url) { NXMUrl nxmInfo(url); @@ -681,7 +741,7 @@ void DownloadManager::addNXMDownload(const QString& url) tr("This is a Nexus collections link. These are not yet supported by MO.")); QMessageBox::information(m_ParentWidget, tr("Collections Not Supported"), collectionStr, QMessageBox::Ok); - return; + return 0; } QStringList validGames; @@ -717,14 +777,15 @@ void DownloadManager::addNXMDownload(const QString& url) .arg(nxmInfo.game()) .arg(m_ManagedGame->gameShortName()), QMessageBox::Ok); - return; + return 0; } - for (auto tuple : m_PendingDownloads) { - if (std::get<0>(tuple).compare(foundGame->gameShortName(), Qt::CaseInsensitive) == - 0, - std::get<1>(tuple) == nxmInfo.modId() && - std::get<2>(tuple) == nxmInfo.fileId()) { + // If a pending entry already covers this file, return its id so callers that + // poll by id see the same download regardless of how many times they queue. + for (const PendingDownload& pending : m_PendingDownloads) { + if (pending.gameName.compare(foundGame->gameShortName(), Qt::CaseInsensitive) == + 0 && + pending.modID == nxmInfo.modId() && pending.fileID == nxmInfo.fileId()) { const auto infoStr = tr("There is already a download queued for this file.\n\nMod %1\nFile %2") .arg(nxmInfo.modId()) @@ -735,7 +796,7 @@ void DownloadManager::addNXMDownload(const QString& url) QMessageBox::information(m_ParentWidget, tr("Already Queued"), infoStr, QMessageBox::Ok); - return; + return pending.reservedID; } } @@ -784,17 +845,21 @@ void DownloadManager::addNXMDownload(const QString& url) log::debug("{}", debugStr); QMessageBox::information(m_ParentWidget, tr("Already Started"), infoStr, QMessageBox::Ok); - return; + return download->m_DownloadID; } } } - emit aboutToUpdate(); + // Reserve the id now (before any async work) so the caller can refer to the + // eventual download before the Nexus API responds and a DownloadInfo exists. + const DownloadID reservedID = DownloadInfo::newDownloadID(); - m_PendingDownloads.append( - std::make_tuple(foundGame->gameShortName(), nxmInfo.modId(), nxmInfo.fileId())); - - emit update(-1); + { + ModelResetGuard guard(*this); + PendingDownload pending{foundGame->gameShortName(), nxmInfo.modId(), + nxmInfo.fileId(), reservedID}; + m_PendingDownloads.append(pending); + } emit downloadAdded(); ModRepositoryFileInfo* info = new ModRepositoryFileInfo(); @@ -806,12 +871,12 @@ void DownloadManager::addNXMDownload(const QString& url) m_RequestIDs.insert(m_NexusInterface->requestFileInfo( foundGame->gameShortName(), nxmInfo.modId(), nxmInfo.fileId(), this, QVariant::fromValue(test), "")); + return reservedID; } void DownloadManager::removeFile(int index, bool deleteFile) { - // Avoid triggering refreshes from DirWatcher - ScopedDisableDirWatcher scopedDirWatcher(this); + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); if (index >= m_ActiveDownloads.size()) { throw MyException(tr("remove: invalid download index %1").arg(index)); @@ -845,39 +910,7 @@ void DownloadManager::removeFile(int index, bool deleteFile) if (!download->m_Hidden) metaSettings.setValue("removed", true); } - m_DownloadRemoved(index); -} - -class LessThanWrapper -{ -public: - LessThanWrapper(DownloadManager* manager) : m_Manager(manager) {} - bool operator()(int LHS, int RHS) - { - return m_Manager->getFileName(LHS).compare(m_Manager->getFileName(RHS), - Qt::CaseInsensitive) < 0; - } - -private: - DownloadManager* m_Manager; -}; - -bool DownloadManager::ByName(int LHS, int RHS) -{ - return m_ActiveDownloads[LHS]->m_FileName < m_ActiveDownloads[RHS]->m_FileName; -} - -void DownloadManager::refreshAlphabeticalTranslation() -{ - m_AlphabeticalTranslation.clear(); - int pos = 0; - for (QVector::iterator iter = m_ActiveDownloads.begin(); - iter != m_ActiveDownloads.end(); ++iter, ++pos) { - m_AlphabeticalTranslation.push_back(pos); - } - - std::sort(m_AlphabeticalTranslation.begin(), m_AlphabeticalTranslation.end(), - LessThanWrapper(this)); + m_DownloadRemoved(download->m_DownloadID); } void DownloadManager::restoreDownload(int index) @@ -906,23 +939,31 @@ void DownloadManager::restoreDownload(int index) QString filePath = m_OutputDirectory + "/" + download->m_FileName; - // avoid dirWatcher triggering refreshes - startDisableDirWatcher(); - QSettings metaSettings(filePath.append(".meta"), QSettings::IniFormat); - metaSettings.setValue("removed", false); + { + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); + QSettings metaSettings(filePath.append(".meta"), QSettings::IniFormat); + metaSettings.setValue("removed", false); + } - endDisableDirWatcher(); + notifyRowChanged(index); } } } void DownloadManager::removeDownload(int index, bool deleteFile) { - try { - // avoid dirWatcher triggering refreshes - ScopedDisableDirWatcher scopedDirWatcher(this); + // validate before entering the guard so we don't emit an empty reset on + // early return + if (index >= m_ActiveDownloads.size()) { + reportError(tr("remove: invalid download index %1").arg(index)); + return; + } - emit aboutToUpdate(); + // coalesce the removal and the subsequent refresh into one reset + ModelResetGuard guard(*this); + + try { + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); if (index < 0) { bool removeAll = (index == -1); @@ -935,6 +976,7 @@ void DownloadManager::removeDownload(int index, bool deleteFile) if ((removeAll && (downloadState >= STATE_READY)) || (removeState == downloadState)) { removeFile(index, deleteFile); + m_ByID.remove((*iter)->m_DownloadID); delete *iter; iter = m_ActiveDownloads.erase(iter); } else { @@ -943,44 +985,40 @@ void DownloadManager::removeDownload(int index, bool deleteFile) } } } else { - if (index >= m_ActiveDownloads.size()) { - reportError(tr("remove: invalid download index %1").arg(index)); - // emit update(-1); - return; - } - + DownloadInfo* info = m_ActiveDownloads.at(index); removeFile(index, deleteFile); - delete m_ActiveDownloads.at(index); + m_ByID.remove(info->m_DownloadID); + delete info; m_ActiveDownloads.erase(m_ActiveDownloads.begin() + index); } - emit update(-1); } catch (const std::exception& e) { log::error("failed to remove download: {}", e.what()); } + refreshList(); } -void DownloadManager::cancelDownload(int index) +void DownloadManager::cancelDownload(DownloadID id) { - if ((index < 0) || (index >= m_ActiveDownloads.size())) { - reportError(tr("cancel: invalid download index %1").arg(index)); + DownloadInfo* info = m_ByID.value(id, nullptr); + if (info == nullptr) { + reportError(tr("cancel: invalid download id %1").arg(id)); return; } - if (m_ActiveDownloads.at(index)->m_State == STATE_DOWNLOADING) { - setState(m_ActiveDownloads.at(index), STATE_CANCELING); + if (info->m_State == STATE_DOWNLOADING) { + setState(info, STATE_CANCELING); } } -void DownloadManager::pauseDownload(int index) +void DownloadManager::pauseDownload(DownloadID id) { - if ((index < 0) || (index >= m_ActiveDownloads.size())) { - reportError(tr("pause: invalid download index %1").arg(index)); + DownloadInfo* info = m_ByID.value(id, nullptr); + if (info == nullptr) { + reportError(tr("pause: invalid download id %1").arg(id)); return; } - DownloadInfo* info = m_ActiveDownloads.at(index); - if (info->m_State == STATE_DOWNLOADING) { if ((info->m_Reply != nullptr) && (info->m_Reply->isRunning())) { setState(info, STATE_PAUSING); @@ -994,30 +1032,30 @@ void DownloadManager::pauseDownload(int index) } } -void DownloadManager::resumeDownload(int index) +void DownloadManager::resumeDownload(DownloadID id) { - if ((index < 0) || (index >= m_ActiveDownloads.size())) { - reportError(tr("resume: invalid download index %1").arg(index)); + DownloadInfo* info = m_ByID.value(id, nullptr); + if (info == nullptr) { + reportError(tr("resume: invalid download id %1").arg(id)); return; } - DownloadInfo* info = m_ActiveDownloads[index]; - info->m_Tries = AUTOMATIC_RETRIES; - resumeDownloadInt(index); + info->m_Tries = AUTOMATIC_RETRIES; + resumeDownloadInt(id); } -void DownloadManager::resumeDownloadInt(int index) +void DownloadManager::resumeDownloadInt(DownloadID id) { - if ((index < 0) || (index >= m_ActiveDownloads.size())) { - reportError(tr("resume (int): invalid download index %1").arg(index)); + DownloadInfo* info = m_ByID.value(id, nullptr); + if (info == nullptr) { + reportError(tr("resume (int): invalid download id %1").arg(id)); return; } - DownloadInfo* info = m_ActiveDownloads[index]; // Check for finished download; if (info->m_TotalSize <= info->m_Output.size() && info->m_Reply != nullptr && info->m_Reply->isFinished() && info->m_State != STATE_ERROR) { setState(info, STATE_DOWNLOADING); - downloadFinished(index); + finishDownload(info->m_DownloadID); return; } @@ -1057,20 +1095,11 @@ void DownloadManager::resumeDownloadInt(int index) log::debug("resume at {} bytes", info->m_ResumePos); startDownload(m_NexusInterface->getAccessManager()->get(request), info, true); } - emit update(index); } -DownloadManager::DownloadInfo* DownloadManager::downloadInfoByID(unsigned int id) +DownloadManager::DownloadInfo* DownloadManager::downloadInfoByID(DownloadID id) { - auto iter = std::find_if(m_ActiveDownloads.begin(), m_ActiveDownloads.end(), - [id](DownloadInfo* info) { - return info->m_DownloadID == id; - }); - if (iter != m_ActiveDownloads.end()) { - return *iter; - } else { - return nullptr; - } + return m_ByID.value(id, nullptr); } void DownloadManager::queryInfo(int index) @@ -1281,6 +1310,7 @@ void DownloadManager::openMetaFile(int index) shell::Open(metaPath); return; } else { + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); QSettings metaFile(metaPath, QSettings::IniFormat); metaFile.setValue("removed", false); } @@ -1326,7 +1356,7 @@ int DownloadManager::numPendingDownloads() const return m_PendingDownloads.size(); } -std::tuple DownloadManager::getPendingDownload(int index) +DownloadManager::PendingDownload DownloadManager::getPendingDownload(int index) { if ((index < 0) || (index >= m_PendingDownloads.size())) { throw MyException(tr("get pending: invalid download index %1").arg(index)); @@ -1508,8 +1538,7 @@ void DownloadManager::markInstalled(int index) throw MyException(tr("mark installed: invalid download index %1").arg(index)); } - // Avoid triggering refreshes from DirWatcher - ScopedDisableDirWatcher scopedDirWatcher(this); + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); DownloadInfo* info = m_ActiveDownloads.at(index); QSettings metaFile(info->m_Output.fileName() + ".meta", QSettings::IniFormat); @@ -1527,8 +1556,7 @@ void DownloadManager::markInstalled(QString fileName) } else { DownloadInfo* info = getDownloadInfo(fileName); if (info != nullptr) { - // Avoid triggering refreshes from DirWatcher - ScopedDisableDirWatcher scopedDirWatcher(this); + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); QSettings metaFile(info->m_Output.fileName() + ".meta", QSettings::IniFormat); metaFile.setValue("installed", true); @@ -1549,8 +1577,7 @@ void DownloadManager::markUninstalled(int index) throw MyException(tr("mark uninstalled: invalid download index %1").arg(index)); } - // Avoid triggering refreshes from DirWatcher - ScopedDisableDirWatcher scopedDirWatcher(this); + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); DownloadInfo* info = m_ActiveDownloads.at(index); QSettings metaFile(info->m_Output.fileName() + ".meta", QSettings::IniFormat); @@ -1569,8 +1596,7 @@ void DownloadManager::markUninstalled(QString fileName) DownloadInfo* info = getDownloadInfo(filePath); if (info != nullptr) { - // Avoid triggering refreshes from DirWatcher - ScopedDisableDirWatcher scopedDirWatcher(this); + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); QSettings metaFile(info->m_Output.fileName() + ".meta", QSettings::IniFormat); metaFile.setValue("uninstalled", true); @@ -1581,15 +1607,16 @@ void DownloadManager::markUninstalled(QString fileName) QString DownloadManager::getDownloadFileName(const QString& baseName, bool rename) const { - QString fullPath = m_OutputDirectory + "/" + MOBase::sanitizeFileName(baseName); + const QString sanitized = MOBase::sanitizeFileName(baseName); + QString fullPath = m_OutputDirectory + "/" + sanitized; if (QFile::exists(fullPath) && rename) { int i = 1; while (QFile::exists( - QString("%1/%2_%3").arg(m_OutputDirectory).arg(i).arg(baseName))) { + QString("%1/%2_%3").arg(m_OutputDirectory).arg(i).arg(sanitized))) { ++i; } - fullPath = QString("%1/%2_%3").arg(m_OutputDirectory).arg(i).arg(baseName); + fullPath = QString("%1/%2_%3").arg(m_OutputDirectory).arg(i).arg(sanitized); } return fullPath; } @@ -1612,53 +1639,55 @@ QString DownloadManager::getFileNameFromNetworkReply(QNetworkReply* reply) void DownloadManager::setState(DownloadManager::DownloadInfo* info, DownloadManager::DownloadState state) { - int row = 0; - for (int i = 0; i < m_ActiveDownloads.size(); ++i) { - if (m_ActiveDownloads[i] == info) { - row = i; - break; - } - } - info->m_State = state; + // Cache the id before reply->abort(): aborting fires signals synchronously + // and a slot may erase info, leaving this pointer dangling. The id is stable + // and m_ByID.contains() lets us check whether info is still present afterward. + const DownloadID id = info->m_DownloadID; + info->m_State = state; switch (state) { case STATE_PAUSED: { info->m_Reply->abort(); info->m_Output.close(); - m_DownloadPaused(row); + if (m_ByID.contains(id)) + m_DownloadPaused(id); } break; case STATE_ERROR: { info->m_Reply->abort(); info->m_Output.close(); - m_DownloadFailed(row); + if (m_ByID.contains(id)) + m_DownloadFailed(id); } break; case STATE_CANCELED: { info->m_Reply->abort(); - m_DownloadFailed(row); + if (m_ByID.contains(id)) + m_DownloadFailed(id); } break; case STATE_FETCHINGMODINFO: { m_RequestIDs.insert(m_NexusInterface->requestDescription( - info->m_FileInfo->gameName, info->m_FileInfo->modID, this, info->m_DownloadID, - QString())); + info->m_FileInfo->gameName, info->m_FileInfo->modID, this, id, QString())); } break; case STATE_FETCHINGFILEINFO: { - m_RequestIDs.insert(m_NexusInterface->requestFiles(info->m_FileInfo->gameName, - info->m_FileInfo->modID, this, - info->m_DownloadID, QString())); + m_RequestIDs.insert(m_NexusInterface->requestFiles( + info->m_FileInfo->gameName, info->m_FileInfo->modID, this, id, QString())); } break; case STATE_FETCHINGMODINFO_MD5: { log::debug("Searching {} for MD5 of {}", info->m_GamesToQuery[0], QString(info->m_Hash.toHex())); m_RequestIDs.insert(m_NexusInterface->requestInfoFromMd5( - info->m_GamesToQuery[0], info->m_Hash, this, info->m_DownloadID, QString())); + info->m_GamesToQuery[0], info->m_Hash, this, id, QString())); } break; case STATE_READY: { createMetaFile(info); - m_DownloadComplete(row); + if (m_ByID.contains(id)) + m_DownloadComplete(id); } break; default: /* NOP */ break; } - emit stateChanged(row, state); + if (const int row = indexByInfo(info); row >= 0) { + emit stateChanged(row, state); + notifyRowChanged(row); + } } DownloadManager::DownloadInfo* DownloadManager::findDownload(QObject* reply, @@ -1694,7 +1723,6 @@ void DownloadManager::downloadProgress(qint64 bytesReceived, qint64 bytesTotal) if (bytesTotal > info->m_TotalSize) { info->m_TotalSize = bytesTotal; } - int oldProgress = info->m_Progress.first; info->m_Progress.first = ((info->m_ResumePos + bytesReceived) * 100) / (info->m_ResumePos + bytesTotal); @@ -1718,7 +1746,7 @@ void DownloadManager::downloadProgress(qint64 bytesReceived, qint64 bytesTotal) TaskProgressManager::instance().updateProgress(info->m_TaskProgressId, bytesReceived, bytesTotal); - emit update(index); + notifyRowChanged(index); } } } catch (const std::bad_alloc&) { @@ -1737,8 +1765,7 @@ void DownloadManager::downloadReadyRead() void DownloadManager::createMetaFile(DownloadInfo* info) { - // Avoid triggering refreshes from DirWatcher - ScopedDisableDirWatcher scopedDirWatcher(this); + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); QSettings metaFile(QString("%1.meta").arg(info->m_Output.fileName()), QSettings::IniFormat); @@ -1764,13 +1791,6 @@ void DownloadManager::createMetaFile(DownloadInfo* info) metaFile.setValue("paused", (info->m_State == DownloadManager::STATE_PAUSED) || (info->m_State == DownloadManager::STATE_ERROR)); metaFile.setValue("removed", info->m_Hidden); - - // slightly hackish... - for (int i = 0; i < m_ActiveDownloads.size(); ++i) { - if (m_ActiveDownloads[i] == info) { - emit update(i); - } - } } void DownloadManager::nxmDescriptionAvailable(QString, int, QVariant userData, @@ -1928,15 +1948,7 @@ void DownloadManager::nxmFileInfoAvailable(QString gameName, int modID, int file info->repository = "Nexus"; - QStringList games(m_ManagedGame->validShortNames()); - games += m_ManagedGame->gameShortName(); - for (auto game : games) { - MOBase::IPluginGame* gamePlugin = m_OrganizerCore->getGame(game); - if (gamePlugin != nullptr && - gamePlugin->gameNexusName().compare(gameName, Qt::CaseInsensitive) == 0) { - info->gameName = gamePlugin->gameShortName(); - } - } + info->gameName = getValidGameShortName(gameName); info->modID = modID; info->fileID = fileID; @@ -1981,9 +1993,12 @@ bool ServerByPreference(const ServerList::container& preferredServers, int DownloadManager::startDownloadURLs(const QStringList& urls) { + const DownloadID reservedID = DownloadInfo::newDownloadID(); ModRepositoryFileInfo info; - addDownload(urls, "", -1, -1, &info); - return m_ActiveDownloads.size() - 1; + if (!addDownload(urls, "", -1, -1, &info, reservedID)) { + return 0; + } + return static_cast(reservedID); } int DownloadManager::startDownloadURLWithMeta(const QString& url, const QString& name, @@ -2005,15 +2020,17 @@ int DownloadManager::startDownloadURLWithMeta(const QString& url, const QString& int DownloadManager::startDownloadNexusFile(const QString& gameName, int modID, int fileID) { - int newID = m_ActiveDownloads.size(); - addNXMDownload( - QString("nxm://%1/mods/%2/files/%3").arg(gameName).arg(modID).arg(fileID)); - return newID; + return static_cast(addNXMDownload( + QString("nxm://%1/mods/%2/files/%3").arg(gameName).arg(modID).arg(fileID))); } QString DownloadManager::downloadPath(int id) { - return getFilePath(id); + DownloadInfo* info = downloadInfoByID(static_cast(id)); + if (info == nullptr) { + return QString(); + } + return m_OutputDirectory + "/" + info->m_FileName; } boost::signals2::connection @@ -2060,6 +2077,32 @@ int DownloadManager::indexByInfo(const DownloadInfo* info) const return -1; } +DownloadManager::DownloadID DownloadManager::downloadIDAtRow(int row) const +{ + if (row >= 0 && row < m_ActiveDownloads.size()) { + return m_ActiveDownloads[row]->m_DownloadID; + } + const int pendingRow = row - m_ActiveDownloads.size(); + if (pendingRow >= 0 && pendingRow < m_PendingDownloads.size()) { + return m_PendingDownloads[pendingRow].reservedID; + } + return 0; +} + +int DownloadManager::rowForDownloadID(DownloadID id) const +{ + if (DownloadInfo* info = m_ByID.value(id, nullptr)) { + return indexByInfo(info); + } + const int base = m_ActiveDownloads.size(); + for (int i = 0; i < m_PendingDownloads.size(); ++i) { + if (m_PendingDownloads[i].reservedID == id) { + return base + i; + } + } + return -1; +} + void DownloadManager::nxmDownloadURLsAvailable(QString gameName, int modID, int fileID, QVariant userData, QVariant resultData, int requestID) @@ -2077,6 +2120,7 @@ void DownloadManager::nxmDownloadURLsAvailable(QString gameName, int modID, int qobject_cast(qvariant_cast(userData)); QVariantList resultList = resultData.toList(); if (resultList.length() == 0) { + notifyPendingDownloadFailed(gameName, modID, fileID); removePending(gameName, modID, fileID); emit showMessage(tr("No download server available. Please try again later.")); return; @@ -2094,7 +2138,18 @@ void DownloadManager::nxmDownloadURLsAvailable(QString gameName, int modID, int foreach (const QVariant& server, resultList) { URLs.append(server.toMap()["URI"].toString()); } - addDownload(URLs, gameName, modID, fileID, info); + + // Carry the pending entry's reserved id into the DownloadInfo so callers who + // received it from addNXMDownload can continue to identify the download. + std::optional reservedID; + for (const PendingDownload& pending : m_PendingDownloads) { + if (pending.gameName.compare(gameName, Qt::CaseInsensitive) == 0 && + pending.modID == modID && pending.fileID == fileID) { + reservedID = pending.reservedID; + break; + } + } + addDownload(URLs, gameName, modID, fileID, info, reservedID); } void DownloadManager::nxmFileInfoFromMd5Available(QString gameName, QVariant userData, @@ -2180,19 +2235,7 @@ void DownloadManager::nxmFileInfoFromMd5Available(QString gameName, QVariant use info->m_FileInfo->uploader = modDetails["uploaded_by"].toString(); info->m_FileInfo->uploaderUrl = modDetails["uploaded_users_profile_url"].toString(); - QString gameShortName = gameName; - QStringList games(m_ManagedGame->validShortNames()); - games += m_ManagedGame->gameShortName(); - for (auto game : games) { - MOBase::IPluginGame* gamePlugin = m_OrganizerCore->getGame(game); - if (gamePlugin != nullptr && - gamePlugin->gameNexusName().compare(gameName, Qt::CaseInsensitive) == 0) { - gameShortName = gamePlugin->gameShortName(); - break; - } - } - - info->m_FileInfo->gameName = gameShortName; + info->m_FileInfo->gameName = getValidGameShortName(gameName); // If the file description is not present, send another query to get it if (!fileDetails["description"].isValid()) { @@ -2238,156 +2281,161 @@ void DownloadManager::nxmRequestFailed(QString gameName, int modID, int fileID, } else { info->m_State = STATE_READY; queryInfo(index); - emit update(index); return; } } if (info->m_FileInfo->modID == modID) { if (info->m_State < STATE_FETCHINGMODINFO) { + ModelResetGuard guard(*this); + m_ByID.remove(info->m_DownloadID); m_ActiveDownloads.erase(iter); delete info; } else { setState(info, STATE_READY); } - emit update(index); break; } } + notifyPendingDownloadFailed(gameName, modID, fileID); removePending(gameName, modID, fileID); emit showMessage(tr("Failed to request file info from nexus: %1").arg(errorString)); } -void DownloadManager::downloadFinished(int index) +void DownloadManager::onReplyFinished() { - DownloadInfo* info; - if (index > 0) - info = m_ActiveDownloads[index]; - else { - info = findDownload(this->sender(), &index); - if (info == nullptr && index == 0) { - info = m_ActiveDownloads[index]; - } + DownloadInfo* info = findDownload(this->sender()); + if (info == nullptr) { + log::warn("finished signal from unknown reply"); + return; } + finishDownload(info->m_DownloadID); +} - if (info != nullptr) { - QNetworkReply* reply = info->m_Reply; - QByteArray data; - if (reply->isOpen() && info->m_HasData) { - data = reply->readAll(); - info->m_Output.write(data); - } - info->m_Output.close(); - TaskProgressManager::instance().forgetMe(info->m_TaskProgressId); - - bool error = false; - if ((info->m_State != STATE_CANCELING) && (info->m_State != STATE_PAUSING)) { - bool textData = reply->header(QNetworkRequest::ContentTypeHeader) - .toString() - .startsWith("text", Qt::CaseInsensitive); - if (textData) +void DownloadManager::finishDownload(DownloadID id) +{ + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); + + DownloadInfo* info = m_ByID.value(id, nullptr); + if (info == nullptr) { + log::warn("no download with id {}", id); + return; + } + int index = indexByInfo(info); + + QNetworkReply* reply = info->m_Reply; + QByteArray data; + if (reply->isOpen() && info->m_HasData) { + data = reply->readAll(); + info->m_Output.write(data); + } + info->m_Output.close(); + TaskProgressManager::instance().forgetMe(info->m_TaskProgressId); + + bool error = false; + if ((info->m_State != STATE_CANCELING) && (info->m_State != STATE_PAUSING)) { + bool textData = reply->header(QNetworkRequest::ContentTypeHeader) + .toString() + .startsWith("text", Qt::CaseInsensitive); + if (textData) + emit showMessage( + tr("Warning: Content type is: %1") + .arg(reply->header(QNetworkRequest::ContentTypeHeader).toString())); + if ((info->m_Output.size() == 0) || + ((reply->error() != QNetworkReply::NoError) && + (reply->error() != QNetworkReply::OperationCanceledError))) { + if (reply->error() == QNetworkReply::UnknownContentError) emit showMessage( - tr("Warning: Content type is: %1") - .arg(reply->header(QNetworkRequest::ContentTypeHeader).toString())); - if ((info->m_Output.size() == 0) || - ((reply->error() != QNetworkReply::NoError) && - (reply->error() != QNetworkReply::OperationCanceledError))) { - if (reply->error() == QNetworkReply::UnknownContentError) - emit showMessage( - tr("Download header content length: %1 downloaded file size: %2") - .arg(reply->header(QNetworkRequest::ContentLengthHeader).toLongLong()) - .arg(info->m_Output.size())); - if (info->m_Tries == 0) { - emit showMessage(tr("Download failed: %1 (%2)") - .arg(reply->errorString()) - .arg(reply->error())); - } - error = true; - setState(info, STATE_ERROR); + tr("Download header content length: %1 downloaded file size: %2") + .arg(reply->header(QNetworkRequest::ContentLengthHeader).toLongLong()) + .arg(info->m_Output.size())); + if (info->m_Tries == 0) { + emit showMessage(tr("Download failed: %1 (%2)") + .arg(reply->errorString()) + .arg(reply->error())); } + error = true; + setState(info, STATE_ERROR); } + } - if (info->m_State == STATE_CANCELING) { - setState(info, STATE_CANCELED); - } else if (info->m_State == STATE_PAUSING) { - if (info->m_Output.isOpen() && info->m_HasData) { - info->m_Output.write(info->m_Reply->readAll()); - } - setState(info, STATE_PAUSED); + if (info->m_State == STATE_CANCELING) { + setState(info, STATE_CANCELED); + } else if (info->m_State == STATE_PAUSING) { + if (info->m_Output.isOpen() && info->m_HasData) { + info->m_Output.write(info->m_Reply->readAll()); } + setState(info, STATE_PAUSED); + } - if (info->m_State == STATE_CANCELED || (info->m_Tries == 0 && error)) { - emit aboutToUpdate(); + if (info->m_State == STATE_CANCELED || (info->m_Tries == 0 && error)) { + { + ModelResetGuard guard(*this); info->m_Output.remove(); + m_ByID.remove(info->m_DownloadID); delete info; m_ActiveDownloads.erase(m_ActiveDownloads.begin() + index); - if (error) - emit showMessage( - tr("We were unable to download the file due to errors after four retries. " - "There may be an issue with the Nexus servers.")); - emit update(-1); - } else if (info->isPausedState() || info->m_State == STATE_PAUSING) { - emit aboutToUpdate(); - info->m_Output.close(); - createMetaFile(info); - emit update(index); - } else { - emit aboutToUpdate(); - QString url = info->m_Urls[info->m_CurrentUrl]; - if (info->m_FileInfo->userData.contains("downloadMap")) { - foreach (const QVariant& server, - info->m_FileInfo->userData["downloadMap"].toList()) { - QVariantMap serverMap = server.toMap(); - if (serverMap["URI"].toString() == url) { - int deltaTime = info->m_StartTime.elapsed() / 1000; - if (deltaTime > 5) { - emit downloadSpeed(serverMap["short_name"].toString(), - (info->m_TotalSize - info->m_PreResumeSize) / - deltaTime); - } // no division by zero please! Also, if the download is shorter than a - // few seconds, the result is way to inprecise - break; - } + } + if (error) + emit showMessage( + tr("We were unable to download the file due to errors after four retries. " + "There may be an issue with the Nexus servers.")); + } else if (info->isPausedState() || info->m_State == STATE_PAUSING) { + info->m_Output.close(); + createMetaFile(info); + } else { + QString url = info->m_Urls[info->m_CurrentUrl]; + if (info->m_FileInfo->userData.contains("downloadMap")) { + foreach (const QVariant& server, + info->m_FileInfo->userData["downloadMap"].toList()) { + QVariantMap serverMap = server.toMap(); + if (serverMap["URI"].toString() == url) { + int deltaTime = info->m_StartTime.elapsed() / 1000; + if (deltaTime > 5) { + emit downloadSpeed(serverMap["short_name"].toString(), + (info->m_TotalSize - info->m_PreResumeSize) / deltaTime); + } // no division by zero please! Also, if the download is shorter than a + // few seconds, the result is way to inprecise + break; } } + } - bool isNexus = info->m_FileInfo->repository == "Nexus"; - // need to change state before changing the file name, otherwise .unfinished is - // appended - if (isNexus) { - setState(info, STATE_FETCHINGMODINFO); - } else { - setState(info, STATE_NOFETCH); - } - - QString newName = getFileNameFromNetworkReply(reply); - QString oldName = QFileInfo(info->m_Output).fileName(); - - startDisableDirWatcher(); - if (!newName.isEmpty() && (oldName.isEmpty())) { - info->setName(getDownloadFileName(newName), true); - } else { - info->setName(m_OutputDirectory + "/" + info->m_FileName, - true); // don't rename but remove the ".unfinished" extension - } - endDisableDirWatcher(); + bool isNexus = info->m_FileInfo->repository == "Nexus"; + // need to change state before changing the file name, otherwise .unfinished is + // appended + if (isNexus) { + setState(info, STATE_FETCHINGMODINFO); + } else { + setState(info, STATE_NOFETCH); + } - if (!isNexus) { - setState(info, STATE_READY); - } + QString newName = getFileNameFromNetworkReply(reply); + QString oldName = QFileInfo(info->m_Output).fileName(); - emit update(index); + if (!newName.isEmpty() && (oldName.isEmpty())) { + info->setName(getDownloadFileName(newName), true); + } else { + info->setName(m_OutputDirectory + "/" + info->m_FileName, + true); // don't rename but remove the ".unfinished" extension } - reply->close(); - reply->deleteLater(); - if ((info->m_Tries > 0) && error) { - --info->m_Tries; - resumeDownloadInt(index); + if (!isNexus) { + setState(info, STATE_READY); } - } else { - log::warn("no download index {}", index); + + notifyRowChanged(index); + } + reply->close(); + reply->deleteLater(); + + // The CANCELED / retries-exhausted branch above may have deleted info; use + // m_ByID to check liveness before reading any more fields or retrying. + if (DownloadInfo* aliveInfo = m_ByID.value(id, nullptr); + aliveInfo != nullptr && error && aliveInfo->m_Tries > 0) { + --aliveInfo->m_Tries; + resumeDownloadInt(id); } } @@ -2409,10 +2457,11 @@ void DownloadManager::metaDataChanged() if (info != nullptr) { QString newName = getFileNameFromNetworkReply(info->m_Reply); if (!newName.isEmpty() && (info->m_FileName.isEmpty())) { - startDisableDirWatcher(); - info->setName(getDownloadFileName(newName), true); - endDisableDirWatcher(); - refreshAlphabeticalTranslation(); + { + DirWatcherManager::Guard dirWatcherGuard = m_DirWatcher.scopedGuard(); + info->setName(getDownloadFileName(newName), true); + } + notifyRowChanged(index); if (!info->m_Output.isOpen() && !info->m_Output.open(QIODevice::WriteOnly | QIODevice::Append)) { reportError(tr("failed to re-open %1").arg(info->m_FileName)); @@ -2424,33 +2473,11 @@ void DownloadManager::metaDataChanged() } } -void DownloadManager::directoryChanged(const QString&) -{ - if (DownloadManager::m_DirWatcherDisabler == 0) - refreshList(); -} - void DownloadManager::managedGameChanged(MOBase::IPluginGame const* managedGame) { m_ManagedGame = managedGame; } -void DownloadManager::checkDownloadTimeout() -{ - for (int i = 0; i < m_ActiveDownloads.size(); ++i) { - if (m_ActiveDownloads[i]->m_StartTime.elapsed() - - m_ActiveDownloads[i]->m_DownloadTimeLast > - 5 * 1000 && - m_ActiveDownloads[i]->m_State == STATE_DOWNLOADING && - m_ActiveDownloads[i]->m_Reply != nullptr && - m_ActiveDownloads[i]->m_Reply->isOpen()) { - pauseDownload(i); - downloadFinished(i); - resumeDownload(i); - } - } -} - void DownloadManager::writeData(DownloadInfo* info) { if (info != nullptr) { @@ -2471,3 +2498,18 @@ void DownloadManager::writeData(DownloadInfo* info) } } } + +QString DownloadManager::getValidGameShortName(const QString& gameNexusName) const +{ + QStringList games(m_ManagedGame->validShortNames()); + games.prepend(m_ManagedGame->gameShortName()); + for (auto& game : games) { + MOBase::IPluginGame* gamePlugin = m_OrganizerCore->getGame(game); + if (gamePlugin != nullptr && + gamePlugin->gameNexusName().compare(gameNexusName, Qt::CaseInsensitive) == 0) { + return gamePlugin->gameShortName(); + } + } + + return gameNexusName; +} diff --git a/src/downloadmanager.h b/src/downloadmanager.h index ad93b87d2..18a3b5a03 100644 --- a/src/downloadmanager.h +++ b/src/downloadmanager.h @@ -24,6 +24,7 @@ along with Mod Organizer. If not, see . #include #include #include +#include #include #include #include @@ -31,7 +32,6 @@ along with Mod Organizer. If not, see . #include #include #include -#include #include #include #include @@ -40,6 +40,7 @@ along with Mod Organizer. If not, see . #include #include #include +#include #include using namespace boost::accumulators; @@ -52,6 +53,60 @@ class NexusInterface; class PluginContainer; class OrganizerCore; +/** + * @brief QFileSystemWatcher with a nestable RAII suspension scope. + * + * Forwards directoryChanged() only while no Guard is alive. Use a Guard to + * bracket filesystem writes that would otherwise trigger a spurious refresh. + */ +class DirWatcherManager : public QObject +{ + Q_OBJECT + +public: + explicit DirWatcherManager(QObject* parent = nullptr); + + /// Set the directory being watched (replaces any previous path). + void setPath(const QString& path); + + /// True while one or more Guards are alive. + bool isSuspended() const; + + /** + * @brief RAII suspension guard. Nests safely; the only way to suspend + * forwarding. + */ + class [[nodiscard]] Guard + { + public: + explicit Guard(DirWatcherManager& manager); + ~Guard(); + Guard(const Guard&) = delete; + Guard& operator=(const Guard&) = delete; + Guard(Guard&&) = delete; + Guard& operator=(Guard&&) = delete; + + private: + DirWatcherManager& m_manager; + }; + + /// Returns a suspension Guard bound to the caller's scope. + [[nodiscard]] Guard scopedGuard(); + +signals: + /// Emitted when the watched directory changes and no Guard is active. + void directoryChanged(); + +private slots: + void onDirectoryChanged(const QString&); + +private: + void releaseSuspension(); + + QFileSystemWatcher m_watcher; + int m_suspendDepth = 0; +}; + /*! * \brief manages downloading of files and provides progress information for gui *elements @@ -61,6 +116,25 @@ class DownloadManager : public QObject Q_OBJECT public: + /** + * @brief RAII full-reset guard. Use when the row count changes; drops + * view selection/scroll state. Nests safely: inner guards coalesce into + * the outermost scope so only one reset is emitted. + */ + class [[nodiscard]] ModelResetGuard + { + public: + explicit ModelResetGuard(DownloadManager& manager); + ~ModelResetGuard(); + ModelResetGuard(const ModelResetGuard&) = delete; + ModelResetGuard& operator=(const ModelResetGuard&) = delete; + ModelResetGuard(ModelResetGuard&&) = delete; + ModelResetGuard& operator=(ModelResetGuard&&) = delete; + + private: + DownloadManager& m_manager; + }; + enum DownloadState { STATE_STARTED = 0, @@ -79,6 +153,32 @@ class DownloadManager : public QObject STATE_UNINSTALLED }; + /** + * @brief Stable identifier for a download. + * + * Monotonically increasing within a session and never reused. Distinct from + * row indices, which are positional and shift as the list is mutated. + */ + using DownloadID = unsigned int; + + /** + * @brief A download that has been requested but has not yet produced a + * DownloadInfo. + * + * Created when the user initiates an NXM download; drained either when the + * Nexus API returns the actual download URL (at which point a DownloadInfo + * is created using reservedID as its download id so that external references + * handed out before the download existed remain valid) or when the request + * is cancelled or fails. + */ + struct PendingDownload + { + QString gameName; + int modID; + int fileID; + DownloadID reservedID; + }; + private: struct DownloadInfo { @@ -87,7 +187,7 @@ class DownloadManager : public QObject accumulator_set> m_DownloadTimeAcc; qint64 m_DownloadLast; qint64 m_DownloadTimeLast; - unsigned int m_DownloadID; + DownloadID m_DownloadID; QString m_FileName; QFile m_Output; QNetworkReply* m_Reply; @@ -116,8 +216,26 @@ class DownloadManager : public QObject bool m_Hidden; + /** + * @brief Issue a new download id. + * + * The only supported way to obtain one; ids are monotonically increasing + * within a session and never reused. + */ + static DownloadID newDownloadID(); + + /** + * @brief Create a new DownloadInfo for a fresh download. + * + * When reservedID is provided it is used as the download id. Callers that + * need to hand out an id before the DownloadInfo exists (e.g. the NXM flow + * reserves an id when the request is queued, long before the Nexus API + * returns the actual URL) should reserve via newDownloadID() and pass it + * here. Otherwise a fresh id is drawn internally. + */ static DownloadInfo* createNew(const MOBase::ModRepositoryFileInfo* fileInfo, - const QStringList& URLs); + const QStringList& URLs, + std::optional reservedID = {}); static DownloadInfo* createFromMeta(const QString& filePath, bool showHidden, const QString outputDirectory, std::optional fileSize = {}); @@ -132,14 +250,14 @@ class DownloadManager : public QObject **/ void setName(QString newName, bool renameFile); - unsigned int downloadID() { return m_DownloadID; } + DownloadID downloadID() { return m_DownloadID; } bool isPausedState(); QString currentURL(); private: - static unsigned int s_NextDownloadID; + static DownloadID s_NextDownloadID; private: DownloadInfo() @@ -191,19 +309,6 @@ class DownloadManager : public QObject **/ void setOutputDirectory(const QString& outputDirectory, const bool refresh = true); - /** - * @brief disables feedback from the downlods fileSystemWhatcher untill - *disableDownloadsWatcherEnd() is called - * - **/ - static void startDisableDirWatcher(); - - /** - * @brief re-enables feedback from the downlods fileSystemWhatcher after - *disableDownloadsWatcherStart() was called - **/ - static void endDisableDirWatcher(); - /** * @return current download directory **/ @@ -239,18 +344,22 @@ class DownloadManager : public QObject bool addDownload(QNetworkReply* reply, const QStringList& URLs, const QString& fileName, QString gameName, int modID, int fileID = 0, const MOBase::ModRepositoryFileInfo* fileInfo = - new MOBase::ModRepositoryFileInfo()); + new MOBase::ModRepositoryFileInfo(), + std::optional reservedID = {}); /** * @brief start a download using a nxm-link * - * starts a download using a nxm-link. The download manager will first query the nexus - * page for file information. + * Starts a download using a nxm-link. The download manager will first query the + * nexus page for file information. The returned id identifies the eventual + * download; it is reserved immediately so external references remain valid even + * before the Nexus API responds. * @param url a nxm link looking like this: nxm://skyrim/mods/1234/files/4711 + * @return the reserved download id * @todo the game name encoded into the link is currently ignored, all downloads are *incorrectly assumed to be for the identified game **/ - void addNXMDownload(const QString& url); + DownloadID addNXMDownload(const QString& url); /** * @brief retrieve the total number of downloads, both finished and unfinished @@ -271,9 +380,24 @@ class DownloadManager : public QObject * @brief retrieve the info of a pending download * @param index index of the pending download (index in the range [0, * numPendingDownloads()[) - * @return pair of modid, fileid + * @return the PendingDownload entry at the given index + */ + PendingDownload getPendingDownload(int index); + + /** + * @brief Resolve a view row to a stable DownloadID. + * + * Rows cover active downloads followed by pending ones. Returns 0 if the row + * is out of range. */ - std::tuple getPendingDownload(int index); + DownloadID downloadIDAtRow(int row) const; + + /** + * @brief Resolve a stable DownloadID to its current view row. + * + * @return the current row, or -1 if no download with that id is tracked. + */ + int rowForDownloadID(DownloadID id) const; /** * @brief retrieve the full path to the download specified by index @@ -408,6 +532,12 @@ class DownloadManager : public QObject */ void queryDownloadListInfo(); + /** + * @return the directory watcher for the downloads folder; call + * scopedGuard() on it to suspend across filesystem writes. + */ + DirWatcherManager& dirWatcher() { return m_DirWatcher; } + public: // IDownloadManager interface: int startDownloadURLs(const QStringList& urls); int startDownloadURLWithMeta(const QString& url, const QString& name, @@ -435,16 +565,38 @@ class DownloadManager : public QObject void pauseAll(); + /** + * @brief notify the UI that a single row's data changed. Preserves view + * state; prefer over ModelResetGuard when the row count is unchanged. + * + * @param row the row that changed. This corresponds to the download index + */ + void notifyRowChanged(int row); + Q_SIGNALS: - void aboutToUpdate(); + /** + * @brief emitted before the download list model is about to be reset + * + * Emitted by ModelResetGuard on construction. Views should call + * beginResetModel() in response. + */ + void aboutToResetModel(); /** - * @brief signals that the specified download has changed + * @brief emitted after the download list model has been reset + * + * Emitted by ModelResetGuard on destruction. Views should call + * endResetModel() in response. + */ + void modelReset(); + + /** + * @brief signals that the specified download row's data has changed * * @param row the row that changed. This corresponds to the download index - **/ - void update(int row); + */ + void rowChanged(int row); /** * @brief signals the ui that a message should be displayed @@ -492,13 +644,13 @@ public slots: * @brief cancel the specified download. This will lead to the corresponding file to *be deleted * - * @param index index of the download to cancel + * @param id id of the download to cancel **/ - void cancelDownload(int index); + void cancelDownload(DownloadID id); - void pauseDownload(int index); + void pauseDownload(DownloadID id); - void resumeDownload(int index); + void resumeDownload(DownloadID id); void queryInfo(int index); @@ -538,11 +690,23 @@ private slots: void downloadProgress(qint64 bytesReceived, qint64 bytesTotal); void downloadReadyRead(); - void downloadFinished(int index = 0); + /** + * @brief Slot wired to QNetworkReply::finished(). + * + * Resolves the originating reply through sender() and then dispatches to + * finishDownload. Use the public finishDownload directly for non-slot calls. + */ + void onReplyFinished(); + + /** + * @brief Run the post-download bookkeeping for the given download. + * + * Writes any remaining data, transitions the download's state, and emits + * the appropriate plugin signals. + */ + void finishDownload(DownloadID id); void downloadError(QNetworkReply::NetworkError error); void metaDataChanged(); - void directoryChanged(const QString& dirctory); - void checkDownloadTimeout(); private: void createMetaFile(DownloadInfo* info); @@ -561,8 +725,15 @@ private slots: QString getDownloadFileName(const QString& baseName, bool rename = false) const; private: - void startDownload(QNetworkReply* reply, DownloadInfo* newDownload, bool resume); - void resumeDownloadInt(int index); + /** + * @brief Begin downloading into newDownload from reply. + * + * On the !resume path newDownload becomes owned by m_ActiveDownloads on + * success; on failure (e.g. the output file cannot be opened) it is deleted + * before returning. Returns whether the download actually started. + */ + bool startDownload(QNetworkReply* reply, DownloadInfo* newDownload, bool resume); + void resumeDownloadInt(DownloadID id); /** * @brief start a download from a url @@ -573,7 +744,8 @@ private slots: *only happens if there is a duplicate and the user decides not to download again **/ bool addDownload(const QStringList& URLs, QString gameName, int modID, int fileID, - const MOBase::ModRepositoryFileInfo* fileInfo); + const MOBase::ModRepositoryFileInfo* fileInfo, + std::optional reservedID = {}); // important: the caller has to lock the list-mutex, otherwise the // DownloadInfo-pointer might get invalidated at any time @@ -581,22 +753,38 @@ private slots: void removeFile(int index, bool deleteFile); - void refreshAlphabeticalTranslation(); - - bool ByName(int LHS, int RHS); - QString getFileNameFromNetworkReply(QNetworkReply* reply); void setState(DownloadInfo* info, DownloadManager::DownloadState state); - DownloadInfo* downloadInfoByID(unsigned int id); + DownloadInfo* downloadInfoByID(DownloadID id); void removePending(QString gameName, int modID, int fileID); + /** + * @brief Fire onDownloadFailed for a pending entry, if any matches. + * + * Used on Nexus API failures so callers holding a reserved id from + * addNXMDownload do not wait indefinitely for a result. No-op if no pending + * entry matches the (gameName, modID, fileID) triple. + */ + void notifyPendingDownloadFailed(const QString& gameName, int modID, int fileID); + + /** + * @brief Roll back a download that has not yet been activated. + * + * Ensures a caller awaiting the reservedID receives an onDownloadFailed + * callback. Must not be called once the download has been registered as + * active. + */ + void cancelPendingDownload(DownloadInfo* newDownload, QNetworkReply* reply); + static QString getFileTypeString(int fileType); void writeData(DownloadInfo* info); + QString getValidGameShortName(const QString& gameNexusName) const; + private: static const int AUTOMATIC_RETRIES = 3; @@ -606,45 +794,30 @@ private slots: OrganizerCore* m_OrganizerCore; QWidget* m_ParentWidget; - QVector> m_PendingDownloads; + QVector m_PendingDownloads; QVector m_ActiveDownloads; + // Secondary index into m_ActiveDownloads keyed by m_DownloadID; kept in sync + // with every m_ActiveDownloads mutation. + QHash m_ByID; + QString m_OutputDirectory; std::set m_RequestIDs; - QVector m_AlphabeticalTranslation; - QFileSystemWatcher m_DirWatcher; + DirWatcherManager m_DirWatcher; + + // nesting depth of active ModelResetGuard scopes; see its docs + int m_modelResetDepth = 0; SignalDownloadCallback m_DownloadComplete; SignalDownloadCallback m_DownloadPaused; SignalDownloadCallback m_DownloadFailed; SignalDownloadCallback m_DownloadRemoved; - // The dirWatcher is actually triggering off normal Mo operations such as deleting - // downloads or editing .meta files so it needs to be disabled during operations that - // are known to cause the creation or deletion of files in the Downloads folder. - // Notably using QSettings to edit a file creates a temporarily .lock file that causes - // the Watcher to trigger multiple listRefreshes freezing the ui. - static int m_DirWatcherDisabler; - - std::map m_DownloadFails; - bool m_ShowHidden; MOBase::IPluginGame const* m_ManagedGame; - - QTimer m_TimeoutTimer; -}; - -class ScopedDisableDirWatcher -{ -public: - ScopedDisableDirWatcher(DownloadManager* downloadManager); - ~ScopedDisableDirWatcher(); - -private: - DownloadManager* m_downloadManager; }; #endif // DOWNLOADMANAGER_H diff --git a/src/downloadstab.cpp b/src/downloadstab.cpp index 4f988bf09..ee0c250c3 100644 --- a/src/downloadstab.cpp +++ b/src/downloadstab.cpp @@ -49,12 +49,17 @@ DownloadsTab::DownloadsTab(OrganizerCore& core, Ui::MainWindow* mwui) SLOT(removeDownload(int, bool))); connect(ui.list, SIGNAL(restoreDownload(int)), m_core.downloadManager(), SLOT(restoreDownload(int))); - connect(ui.list, SIGNAL(cancelDownload(int)), m_core.downloadManager(), - SLOT(cancelDownload(int))); - connect(ui.list, SIGNAL(pauseDownload(int)), m_core.downloadManager(), - SLOT(pauseDownload(int))); - connect(ui.list, &DownloadListView::resumeDownload, [&](int i) { - resumeDownload(i); + // The view reports actions by row; translate to DownloadID at the boundary. + connect(ui.list, &DownloadListView::cancelDownload, this, [this](int row) { + auto* dm = m_core.downloadManager(); + dm->cancelDownload(dm->downloadIDAtRow(row)); + }); + connect(ui.list, &DownloadListView::pauseDownload, this, [this](int row) { + auto* dm = m_core.downloadManager(); + dm->pauseDownload(dm->downloadIDAtRow(row)); + }); + connect(ui.list, &DownloadListView::resumeDownload, this, [this](int row) { + resumeDownload(row); }); } @@ -105,6 +110,7 @@ void DownloadsTab::queryInfos() void DownloadsTab::resumeDownload(int downloadIndex) { m_core.loggedInAction(ui.list, [this, downloadIndex] { - m_core.downloadManager()->resumeDownload(downloadIndex); + auto* dm = m_core.downloadManager(); + dm->resumeDownload(dm->downloadIDAtRow(downloadIndex)); }); } diff --git a/src/modlistviewactions.cpp b/src/modlistviewactions.cpp index 6b26a1e33..9f2896785 100644 --- a/src/modlistviewactions.cpp +++ b/src/modlistviewactions.cpp @@ -818,11 +818,13 @@ void ModListViewActions::removeMods(const QModelIndexList& indices) const QMessageBox::Yes | QMessageBox::No) == QMessageBox::Yes) { // use mod names instead of indexes because those become invalid during the // removal - DownloadManager::startDisableDirWatcher(); - for (QString name : modNames) { - m_core.modList()->removeRowForce(ModInfo::getIndex(name), QModelIndex()); + { + DirWatcherManager::Guard dirWatcherGuard = + m_core.downloadManager()->dirWatcher().scopedGuard(); + for (QString name : modNames) { + m_core.modList()->removeRowForce(ModInfo::getIndex(name), QModelIndex()); + } } - DownloadManager::endDisableDirWatcher(); } } else if (!indices.isEmpty()) { m_core.modList()->removeRow(indices[0].data(ModList::IndexRole).toInt(), diff --git a/src/organizercore.cpp b/src/organizercore.cpp index a8fc67c27..dff3485eb 100644 --- a/src/organizercore.cpp +++ b/src/organizercore.cpp @@ -866,7 +866,8 @@ OrganizerCore::doInstall(const QString& archivePath, GuessedValue modNa ModInfo::Ptr OrganizerCore::installDownload(int index, int priority) { - ScopedDisableDirWatcher scopedDirwatcher(&m_DownloadManager); + DirWatcherManager::Guard dirWatcherGuard = + m_DownloadManager.dirWatcher().scopedGuard(); try { QString fileName = m_DownloadManager.getFilePath(index);