From 8dd6b16fee6e927627fa1adb6b5f2c2fc568ae8f Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 00:21:41 +0100 Subject: [PATCH 1/9] Avoid double lookup --- .../contentselector/model/contentmodel.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/contentselector/model/contentmodel.cpp b/components/contentselector/model/contentmodel.cpp index 46b1015930..97bda943ac 100644 --- a/components/contentselector/model/contentmodel.cpp +++ b/components/contentselector/model/contentmodel.cpp @@ -580,10 +580,10 @@ void ContentSelectorModel::ContentModel::sortFiles() bool ContentSelectorModel::ContentModel::isChecked(const QString& filepath) const { - if (mCheckStates.contains(filepath)) - return (mCheckStates[filepath] == Qt::Checked); - - return false; + const auto it = mCheckStates.find(filepath); + if (it == mCheckStates.end()) + return false; + return it.value() == Qt::Checked; } bool ContentSelectorModel::ContentModel::isEnabled(const QModelIndex& index) const @@ -593,10 +593,10 @@ bool ContentSelectorModel::ContentModel::isEnabled(const QModelIndex& index) con bool ContentSelectorModel::ContentModel::isNew(const QString& filepath) const { - if (mNewFiles.contains(filepath)) - return mNewFiles[filepath]; - - return false; + const auto it = mNewFiles.find(filepath); + if (it == mNewFiles.end()) + return false; + return it.value(); } void ContentSelectorModel::ContentModel::setNew(const QString& filepath, bool isNew) From 5dfb70e8a75be76ff439a44e00c1833526f0296b Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 22:56:56 +0100 Subject: [PATCH 2/9] Name ui elements --- files/ui/datafilespage.ui | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/files/ui/datafilespage.ui b/files/ui/datafilespage.ui index 529f0ca8f2..239df34961 100644 --- a/files/ui/datafilespage.ui +++ b/files/ui/datafilespage.ui @@ -28,7 +28,7 @@ - + <html><head/><body><p><span style=" font-style:italic;">note: content files that are not part of current Content List are </span><span style=" font-style:italic; background-color:#00ff00;">highlighted</span></p></body></html> @@ -49,7 +49,7 @@ - + 0 @@ -208,7 +208,7 @@ - + <html><head/><body><p><span style=" font-style:italic;">note: archives that are not part of current Content List are </span><span style=" font-style:italic; background-color:#00ff00;">highlighted</span></p></body></html> @@ -238,13 +238,13 @@ - + Navigation Mesh Cache - + @@ -294,9 +294,9 @@ - + - + Max size From 0b852edc7f9583f29de6d179792a96715c1de8d7 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 23:03:39 +0100 Subject: [PATCH 3/9] Remove redundant inline --- components/contentselector/model/esmfile.hpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/components/contentselector/model/esmfile.hpp b/components/contentselector/model/esmfile.hpp index bbf275b2c7..f51c6cfca2 100644 --- a/components/contentselector/model/esmfile.hpp +++ b/components/contentselector/model/esmfile.hpp @@ -45,19 +45,19 @@ namespace ContentSelectorModel void setGameFiles(const QStringList& gameFiles); void setDescription(const QString& description); - inline void addGameFile(const QString& name) { mGameFiles.append(name); } + void addGameFile(const QString& name) { mGameFiles.append(name); } QVariant fileProperty(const FileProperty prop) const; - inline QString fileName() const { return mFileName; } - inline QString author() const { return mAuthor; } - inline QDateTime modified() const { return mModified; } + QString fileName() const { return mFileName; } + QString author() const { return mAuthor; } + QDateTime modified() const { return mModified; } ESM::FormatVersion formatVersion() const { return mVersion; } - inline QString filePath() const { return mPath; } + QString filePath() const { return mPath; } /// @note Contains file names, not paths. - inline const QStringList& gameFiles() const { return mGameFiles; } - inline QString description() const { return mDescription; } - inline QString toolTip() const + const QStringList& gameFiles() const { return mGameFiles; } + QString description() const { return mDescription; } + QString toolTip() const { return sToolTip.arg(mAuthor) .arg(mVersion) From ecb9c3526855c62a972ff122800f950e881b23b5 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 23:04:11 +0100 Subject: [PATCH 4/9] Remove unused variable --- apps/launcher/datafilespage.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index bbf6703b7e..e54688a597 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -389,8 +389,6 @@ QStringList Launcher::DataFilesPage::selectedArchivePaths(bool all) const for (int i = 0; i < ui.archiveListWidget->count(); ++i) { const auto* item = ui.archiveListWidget->item(i); - const auto archive = ui.archiveListWidget->item(i)->text(); - if (all || item->checkState() == Qt::Checked) archiveList.append(item->text()); } From 62536d5cf7e4024834bfeb87d3ef00ddf783a831 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 23:05:57 +0100 Subject: [PATCH 5/9] Use static QFile::exists instead of creating object --- apps/launcher/datafilespage.cpp | 5 +---- apps/wizard/existinginstallationpage.cpp | 3 +-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index e54688a597..54dabd3ef9 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -401,11 +401,8 @@ QStringList Launcher::DataFilesPage::selectedFilePaths() const ContentSelectorModel::ContentFileList items = mSelector->selectedFiles(); QStringList filePaths; for (const ContentSelectorModel::EsmFile* item : items) - { - QFile file(item->filePath()); - if (file.exists()) + if (QFile::exists(item->filePath())) filePaths.append(item->filePath()); - } return filePaths; } diff --git a/apps/wizard/existinginstallationpage.cpp b/apps/wizard/existinginstallationpage.cpp index 1f97ad88e0..71ae331a61 100644 --- a/apps/wizard/existinginstallationpage.cpp +++ b/apps/wizard/existinginstallationpage.cpp @@ -50,9 +50,8 @@ bool Wizard::ExistingInstallationPage::validatePage() // Or failed to be detected due to the target being a symlink QString path(field(QLatin1String("installation.path")).toString()); - QFile file(mWizard->mInstallations[path].iniPath); - if (!file.exists()) + if (!QFile::exists(mWizard->mInstallations[path].iniPath)) { QMessageBox msgBox; msgBox.setWindowTitle(tr("Error detecting Morrowind configuration")); From b1765cf05a1036b4f6eafe5125a2fb3b3dc772cd Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 23:07:37 +0100 Subject: [PATCH 6/9] Do single lookup for widget item --- apps/launcher/datafilespage.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index 54dabd3ef9..2bfcabb570 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -377,8 +377,9 @@ QStringList Launcher::DataFilesPage::selectedDirectoriesPaths() const QStringList dirList; for (int i = 0; i < ui.directoryListWidget->count(); ++i) { - if (ui.directoryListWidget->item(i)->flags() & Qt::ItemIsEnabled) - dirList.append(ui.directoryListWidget->item(i)->text()); + const QListWidgetItem* item = ui.directoryListWidget->item(i); + if (item->flags() & Qt::ItemIsEnabled) + dirList.append(item->text()); } return dirList; } From 5a691380eaf5a2c7aab270b01f37d3ee9fc992d8 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 23:24:29 +0100 Subject: [PATCH 7/9] Use single set to check presence of archives --- apps/launcher/datafilespage.cpp | 13 ++++++++----- apps/launcher/datafilespage.hpp | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index 2bfcabb570..8d956e3f24 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -384,13 +384,13 @@ QStringList Launcher::DataFilesPage::selectedDirectoriesPaths() const return dirList; } -QStringList Launcher::DataFilesPage::selectedArchivePaths(bool all) const +QStringList Launcher::DataFilesPage::selectedArchivePaths() const { QStringList archiveList; for (int i = 0; i < ui.archiveListWidget->count(); ++i) { - const auto* item = ui.archiveListWidget->item(i); - if (all || item->checkState() == Qt::Checked) + const QListWidgetItem* item = ui.archiveListWidget->item(i); + if (item->checkState() == Qt::Checked) archiveList.append(item->text()); } return archiveList; @@ -720,6 +720,10 @@ void Launcher::DataFilesPage::addArchivesFromDir(const QString& path) { QDir dir(path, "*.bsa"); + std::unordered_set archives; + for (int i = 0; i < ui.archiveListWidget->count(); ++i) + archives.insert(ui.archiveListWidget->item(i)->text()); + for (const auto& fileinfo : dir.entryInfoList()) { const auto absPath = fileinfo.absoluteFilePath(); @@ -727,9 +731,8 @@ void Launcher::DataFilesPage::addArchivesFromDir(const QString& path) continue; const auto fileName = fileinfo.fileName(); - const auto currentList = selectedArchivePaths(true); - if (!currentList.contains(fileName, Qt::CaseInsensitive)) + if (archives.insert(fileName).second) addArchive(fileName, Qt::Unchecked); } } diff --git a/apps/launcher/datafilespage.hpp b/apps/launcher/datafilespage.hpp index 60082595c9..930e1392aa 100644 --- a/apps/launcher/datafilespage.hpp +++ b/apps/launcher/datafilespage.hpp @@ -138,7 +138,7 @@ namespace Launcher * @return the file paths of all selected content files */ QStringList selectedFilePaths() const; - QStringList selectedArchivePaths(bool all = false) const; + QStringList selectedArchivePaths() const; QStringList selectedDirectoriesPaths() const; class PathIterator From d18d860ea292e7bcb874c113e6ab0e5b5d110ee4 Mon Sep 17 00:00:00 2001 From: elsid Date: Mon, 20 Mar 2023 23:57:53 +0100 Subject: [PATCH 8/9] Catch and log exceptions on loading cell names Instead of terminating the process. --- apps/launcher/utils/cellnameloader.cpp | 42 +++++++++++++++----------- apps/launcher/utils/cellnameloader.hpp | 2 +- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/apps/launcher/utils/cellnameloader.cpp b/apps/launcher/utils/cellnameloader.cpp index 57c009046d..d47e923eb2 100644 --- a/apps/launcher/utils/cellnameloader.cpp +++ b/apps/launcher/utils/cellnameloader.cpp @@ -1,37 +1,45 @@ #include "cellnameloader.hpp" +#include #include #include -QSet CellNameLoader::getCellNames(QStringList& contentPaths) +QSet CellNameLoader::getCellNames(const QStringList& contentPaths) { QSet cellNames; ESM::ESMReader esmReader; // Loop through all content files - for (auto& contentPath : contentPaths) + for (const QString& contentPath : contentPaths) { if (contentPath.endsWith(".omwscripts", Qt::CaseInsensitive)) continue; - esmReader.open(Files::pathFromQString(contentPath)); - - // Loop through all records - while (esmReader.hasMoreRecs()) + try { - ESM::NAME recordName = esmReader.getRecName(); - esmReader.getRecHeader(); + esmReader.open(Files::pathFromQString(contentPath)); - if (isCellRecord(recordName)) + // Loop through all records + while (esmReader.hasMoreRecs()) { - QString cellName = getCellName(esmReader); - if (!cellName.isEmpty()) - { - cellNames.insert(cellName); - } - } + ESM::NAME recordName = esmReader.getRecName(); + esmReader.getRecHeader(); - // Stop loading content for this record and continue to the next - esmReader.skipRecord(); + if (isCellRecord(recordName)) + { + QString cellName = getCellName(esmReader); + if (!cellName.isEmpty()) + { + cellNames.insert(cellName); + } + } + + // Stop loading content for this record and continue to the next + esmReader.skipRecord(); + } + } + catch (const std::exception& e) + { + Log(Debug::Error) << "Failed to get cell names from " << contentPath.toStdString() << ": " << e.what(); } } diff --git a/apps/launcher/utils/cellnameloader.hpp b/apps/launcher/utils/cellnameloader.hpp index 24ff187f41..6c7993dc5a 100644 --- a/apps/launcher/utils/cellnameloader.hpp +++ b/apps/launcher/utils/cellnameloader.hpp @@ -25,7 +25,7 @@ public: * @param contentPaths the file paths of each content file to be examined * @return the names of all cells */ - QSet getCellNames(QStringList& contentPaths); + QSet getCellNames(const QStringList& contentPaths); private: /** From de112a29506863de26ba4ef4571996a43ac5300b Mon Sep 17 00:00:00 2001 From: elsid Date: Tue, 21 Mar 2023 00:42:21 +0100 Subject: [PATCH 9/9] Simplify converting file names into absolute file paths --- apps/launcher/datafilespage.cpp | 42 ++++++++++++++------------ apps/launcher/datafilespage.hpp | 52 --------------------------------- 2 files changed, 23 insertions(+), 71 deletions(-) diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp index 8d956e3f24..f617b07f00 100644 --- a/apps/launcher/datafilespage.cpp +++ b/apps/launcher/datafilespage.cpp @@ -98,6 +98,27 @@ namespace Launcher { return Settings::Manager::getUInt64("max navmeshdb file size", "Navigator") / (1024 * 1024); } + + std::optional findFirstPath(const QStringList& directories, const QString& fileName) + { + for (const QString& directoryPath : directories) + { + const QString filePath = QDir(directoryPath).absoluteFilePath(fileName); + if (QFile::exists(filePath)) + return filePath; + } + return std::nullopt; + } + + QStringList findAllFilePaths(const QStringList& directories, const QStringList& fileNames) + { + QStringList result; + result.reserve(fileNames.size()); + for (const QString& fileName : fileNames) + if (const auto filepath = findFirstPath(directories, fileName)) + result.append(*filepath); + return result; + } } } @@ -305,25 +326,8 @@ void Launcher::DataFilesPage::populateFileViews(const QString& contentModelName) row++; } - PathIterator pathIterator(directories); - - mSelector->setProfileContent(filesInProfile(contentModelName, pathIterator)); -} - -QStringList Launcher::DataFilesPage::filesInProfile(const QString& profileName, PathIterator& pathIterator) -{ - QStringList files = mLauncherSettings.getContentListFiles(profileName); - QStringList filepaths; - - for (const QString& file : files) - { - QString filepath = pathIterator.findFirstPath(file); - - if (!filepath.isEmpty()) - filepaths << filepath; - } - - return filepaths; + mSelector->setProfileContent( + findAllFilePaths(directories, mLauncherSettings.getContentListFiles(contentModelName))); } void Launcher::DataFilesPage::saveSettings(const QString& profile) diff --git a/apps/launcher/datafilespage.hpp b/apps/launcher/datafilespage.hpp index 930e1392aa..fead7d5d93 100644 --- a/apps/launcher/datafilespage.hpp +++ b/apps/launcher/datafilespage.hpp @@ -140,58 +140,6 @@ namespace Launcher QStringList selectedFilePaths() const; QStringList selectedArchivePaths() const; QStringList selectedDirectoriesPaths() const; - - class PathIterator - { - QStringList::ConstIterator mCitEnd; - QStringList::ConstIterator mCitCurrent; - QStringList::ConstIterator mCitBegin; - QString mFile; - QString mFilePath; - - public: - PathIterator(const QStringList& list) - { - mCitBegin = list.constBegin(); - mCitCurrent = mCitBegin; - mCitEnd = list.constEnd(); - } - - QString findFirstPath(const QString& file) - { - mCitCurrent = mCitBegin; - mFile = file; - return path(); - } - - QString findNextPath() { return path(); } - - private: - QString path() - { - bool success = false; - QDir dir; - QFileInfo file; - - while (!success) - { - if (mCitCurrent == mCitEnd) - break; - - dir.setPath(*(mCitCurrent++)); - file.setFile(dir.absoluteFilePath(mFile)); - - success = file.exists(); - } - - if (success) - return file.absoluteFilePath(); - - return ""; - } - }; - - QStringList filesInProfile(const QString& profileName, PathIterator& pathIterator); }; } #endif