From cd7941dc9fcf9473626947c1baa11b04f08968ea Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 31 Jul 2024 00:04:38 +0100 Subject: [PATCH] Some launcher fixes I tried to fix https://gitlab.com/OpenMW/openmw/-/issues/8080 by making it so that instead of crashing, we showed an error. In doing so, I discovered some problems with plugin sorting and the refresh button, like: * it forgetting the non-user content files somewhere * nothing guaranteeing that built-in content files stay at the top of the list and them only being there because the first data directory that provides them is usually the first data directory * it forgetting the non-user content files somewhere else * it looking like it'd forget any kind of non-user setting under certain circumstances I fixed those problems too --- components/config/gamesettings.hpp | 12 +++-- .../contentselector/model/contentmodel.cpp | 52 +++++++++++++------ .../contentselector/model/contentmodel.hpp | 5 +- components/contentselector/model/esmfile.hpp | 3 ++ .../contentselector/view/contentselector.cpp | 3 +- files/lang/components_de.ts | 4 ++ files/lang/components_en.ts | 4 ++ files/lang/components_fr.ts | 4 ++ files/lang/components_ru.ts | 4 ++ files/lang/components_sv.ts | 4 ++ 10 files changed, 72 insertions(+), 23 deletions(-) diff --git a/components/config/gamesettings.hpp b/components/config/gamesettings.hpp index 7627d5153a..89139ff41a 100644 --- a/components/config/gamesettings.hpp +++ b/components/config/gamesettings.hpp @@ -40,9 +40,8 @@ namespace Config inline void setValue(const QString& key, const SettingValue& value) { - mSettings.remove(key); + remove(key); mSettings.insert(key, value); - mUserSettings.remove(key); if (isUserSetting(value)) mUserSettings.insert(key, value); } @@ -63,7 +62,14 @@ namespace Config inline void remove(const QString& key) { - mSettings.remove(key); + // simplify to removeIf when Qt5 goes + for (auto itr = mSettings.lowerBound(key); itr != mSettings.upperBound(key);) + { + if (isUserSetting(*itr)) + itr = mSettings.erase(itr); + else + ++itr; + } mUserSettings.remove(key); } diff --git a/components/contentselector/model/contentmodel.cpp b/components/contentselector/model/contentmodel.cpp index 06248bf0af..8a29b0391d 100644 --- a/components/contentselector/model/contentmodel.cpp +++ b/components/contentselector/model/contentmodel.cpp @@ -19,9 +19,10 @@ #include #include -ContentSelectorModel::ContentModel::ContentModel(QObject* parent, QIcon& warningIcon, bool showOMWScripts) +ContentSelectorModel::ContentModel::ContentModel(QObject* parent, QIcon& warningIcon, QIcon& errorIcon, bool showOMWScripts) : QAbstractTableModel(parent) , mWarningIcon(warningIcon) + , mErrorIcon(errorIcon) , mShowOMWScripts(showOMWScripts) , mMimeType("application/omwcontent") , mMimeTypes(QStringList() << mMimeType) @@ -169,7 +170,12 @@ QVariant ContentSelectorModel::ContentModel::data(const QModelIndex& index, int { case Qt::DecorationRole: { - return isLoadOrderError(file) ? mWarningIcon : QVariant(); + if (file->isMissing()) + return mErrorIcon; + else if (isLoadOrderError(file)) + return mWarningIcon; + else + return QVariant(); } case Qt::FontRole: @@ -595,10 +601,32 @@ void ContentSelectorModel::ContentModel::sortFiles() { emit layoutAboutToBeChanged(); - int firstModifiable = 0; - while (firstModifiable < mFiles.size() - && (mFiles.at(firstModifiable)->builtIn() || mFiles.at(firstModifiable)->fromAnotherConfigFile())) - ++firstModifiable; + // make both Qt5 (int) and Qt6 (qsizetype aka size_t) happy + using index_t = ContentFileList::size_type; + + // ensure built-in are first + index_t firstModifiable = 0; + for (index_t i = 0; i < mFiles.length(); ++i) + { + if (mFiles.at(i)->builtIn()) + mFiles.move(i, firstModifiable++); + } + + // then non-user content + for (const auto& filename : mNonUserContent) + { + const EsmFile* file = item(filename); + int filePosition = indexFromItem(file).row(); + if (filePosition >= 0) + mFiles.move(filePosition, firstModifiable++); + else + { + // the file is not in the VFS, and will be displayed with an error + auto missingFile = std::make_unique(filename); + missingFile->setFromAnotherConfigFile(true); + mFiles.insert(firstModifiable++, missingFile.release()); + } + } // For the purposes of dependency sort we'll hallucinate that Bloodmoon is dependent on Tribunal const EsmFile* tribunalFile = item("Tribunal.esm"); @@ -669,20 +697,10 @@ void ContentSelectorModel::ContentModel::setNonUserContent(const QStringList& fi { mNonUserContent.clear(); for (const auto& file : fileList) - mNonUserContent.insert(file.toLower()); + mNonUserContent.append(file.toLower()); for (auto* file : mFiles) file->setFromAnotherConfigFile(mNonUserContent.contains(file->fileName().toLower())); - auto insertPosition - = std::ranges::find_if(mFiles, [](const EsmFile* file) { return !file->builtIn(); }) - mFiles.begin(); - - for (const auto& filepath : fileList) - { - const EsmFile* file = item(filepath); - int filePosition = indexFromItem(file).row(); - mFiles.move(filePosition, insertPosition++); - } - sortFiles(); } diff --git a/components/contentselector/model/contentmodel.hpp b/components/contentselector/model/contentmodel.hpp index 3cc05fd3cb..467a9c032a 100644 --- a/components/contentselector/model/contentmodel.hpp +++ b/components/contentselector/model/contentmodel.hpp @@ -25,7 +25,7 @@ namespace ContentSelectorModel { Q_OBJECT public: - explicit ContentModel(QObject* parent, QIcon& warningIcon, bool showOMWScripts); + explicit ContentModel(QObject* parent, QIcon& warningIcon, QIcon& errorIcon, bool showOMWScripts); ~ContentModel(); void setEncoding(const QString& encoding); @@ -86,12 +86,13 @@ namespace ContentSelectorModel const EsmFile* mGameFile; ContentFileList mFiles; - QSet mNonUserContent; + QStringList mNonUserContent; std::set mCheckedFiles; QHash mNewFiles; QSet mPluginsWithLoadOrderError; QString mEncoding; QIcon mWarningIcon; + QIcon mErrorIcon; bool mShowOMWScripts; QString mErrorToolTips[ContentSelectorModel::LoadOrderError::ErrorCode_LoadOrder] diff --git a/components/contentselector/model/esmfile.hpp b/components/contentselector/model/esmfile.hpp index 4703df562c..28b4bd2822 100644 --- a/components/contentselector/model/esmfile.hpp +++ b/components/contentselector/model/esmfile.hpp @@ -55,12 +55,15 @@ namespace ContentSelectorModel QString filePath() const { return mPath; } bool builtIn() const { return mBuiltIn; } bool fromAnotherConfigFile() const { return mFromAnotherConfigFile; } + bool isMissing() const { return mPath.isEmpty(); } /// @note Contains file names, not paths. const QStringList& gameFiles() const { return mGameFiles; } QString description() const { return mDescription; } QString toolTip() const { + if (isMissing()) + return tr("This file is specified in a non-user config file, but does not exist in the VFS."); QString tooltip = mTooltipTemlate.arg(mAuthor) .arg(mVersion) .arg(mModified.toString(Qt::ISODate)) diff --git a/components/contentselector/view/contentselector.cpp b/components/contentselector/view/contentselector.cpp index ab12f45fd7..2bcb73689e 100644 --- a/components/contentselector/view/contentselector.cpp +++ b/components/contentselector/view/contentselector.cpp @@ -32,7 +32,8 @@ ContentSelectorView::ContentSelector::~ContentSelector() = default; void ContentSelectorView::ContentSelector::buildContentModel(bool showOMWScripts) { QIcon warningIcon(ui->addonView->style()->standardIcon(QStyle::SP_MessageBoxWarning)); - mContentModel = new ContentSelectorModel::ContentModel(this, warningIcon, showOMWScripts); + QIcon errorIcon(ui->addonView->style()->standardIcon(QStyle::SP_MessageBoxCritical)); + mContentModel = new ContentSelectorModel::ContentModel(this, warningIcon, errorIcon, showOMWScripts); } void ContentSelectorView::ContentSelector::buildGameFileView() diff --git a/files/lang/components_de.ts b/files/lang/components_de.ts index 76b90229fc..acd8ee94ba 100644 --- a/files/lang/components_de.ts +++ b/files/lang/components_de.ts @@ -37,6 +37,10 @@ <br/><b>This content file cannot be disabled because it is enabled in a config file other than the user one.</b><br/> + + <b>This file is specified in a non-user config file, but does not exist in the VFS.</b> + + ContentSelectorView::ContentSelector diff --git a/files/lang/components_en.ts b/files/lang/components_en.ts index fdc5b37d36..6539c3fe26 100644 --- a/files/lang/components_en.ts +++ b/files/lang/components_en.ts @@ -37,6 +37,10 @@ <b>Author:</b> %1<br/><b>Format version:</b> %2<br/><b>Modified:</b> %3<br/><b>Path:</b><br/>%4<br/><br/><b>Description:</b><br/>%5<br/><br/><b>Dependencies: </b>%6<br/> + + <b>This file is specified in a non-user config file, but does not exist in the VFS.</b> + + ContentSelectorView::ContentSelector diff --git a/files/lang/components_fr.ts b/files/lang/components_fr.ts index 706dc1c988..ac1e650f9b 100644 --- a/files/lang/components_fr.ts +++ b/files/lang/components_fr.ts @@ -37,6 +37,10 @@ <br/><b>This content file cannot be disabled because it is enabled in a config file other than the user one.</b><br/> <br/><b>Ce fichier de contenu ne peut être désactivé, car il est activé par un fichier de configuration non contrôlé par l'utilisateur.</b><br/> + + <b>This file is specified in a non-user config file, but does not exist in the VFS.</b> + + ContentSelectorView::ContentSelector diff --git a/files/lang/components_ru.ts b/files/lang/components_ru.ts index 10cbcedeae..0ef32ed965 100644 --- a/files/lang/components_ru.ts +++ b/files/lang/components_ru.ts @@ -37,6 +37,10 @@ <br/><b>This content file cannot be disabled because it is enabled in a config file other than the user one.</b><br/> <br/><b>Этот файл данных не может быть отключен, потому что он включен в файле с настройками, не являющемся пользовательским.</b><br/> + + <b>This file is specified in a non-user config file, but does not exist in the VFS.</b> + + ContentSelectorView::ContentSelector diff --git a/files/lang/components_sv.ts b/files/lang/components_sv.ts index 8682a569bd..a18186e5b6 100644 --- a/files/lang/components_sv.ts +++ b/files/lang/components_sv.ts @@ -37,6 +37,10 @@ <br/><b>This content file cannot be disabled because it is enabled in a config file other than the user one.</b><br/> <br/><b>Denna innehållsfil kan inte inaktiveras då den är en aktiverad i en annan konfigurationsfil än användarens.</b><br/> + + <b>This file is specified in a non-user config file, but does not exist in the VFS.</b> + + ContentSelectorView::ContentSelector