From 2c6d11f95e6f2e29e68bfbefe6163aa338905b10 Mon Sep 17 00:00:00 2001 From: Alexei Kotov Date: Sat, 17 May 2025 19:02:51 +0300 Subject: [PATCH] Make the horrifying content model flags() game search less horrifying Properly exit early and cache game file check result --- .../contentselector/model/contentmodel.cpp | 37 +++++++------------ components/contentselector/model/esmfile.cpp | 10 ++--- components/contentselector/model/esmfile.hpp | 1 + 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/components/contentselector/model/contentmodel.cpp b/components/contentselector/model/contentmodel.cpp index 4309850f2d..30bfeffe6e 100644 --- a/components/contentselector/model/contentmodel.cpp +++ b/components/contentselector/model/contentmodel.cpp @@ -116,37 +116,26 @@ Qt::ItemFlags ContentSelectorModel::ContentModel::flags(const QModelIndex& index if (file == mGameFile) return Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsUserCheckable; - Qt::ItemFlags returnFlags; + // files with no dependencies can always be checked + if (file->gameFiles().empty()) + return Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsUserCheckable | Qt::ItemIsDragEnabled; - // addon can be checked if its gamefile is - // ... special case, addon with no dependency can be used with any gamefile. - bool gamefileChecked = false; - bool noGameFiles = true; - for (const QString& fileName : file->gameFiles()) + // Show the file if the game it is for is enabled. + // NB: The file may theoretically depend on multiple games. + // Early exit means that a file is visible only if its earliest found game dependency is enabled. + // This can be counterintuitive, but it is okay for non-bizarre content setups. And also faster. + for (const EsmFile* depFile : mFiles) { - for (QListIterator dependencyIter(mFiles); dependencyIter.hasNext(); dependencyIter.next()) + if (depFile->isGameFile() && file->gameFiles().contains(depFile->fileName(), Qt::CaseInsensitive)) { - // compare filenames only. Multiple instances - // of the filename (with different paths) is not relevant here. - EsmFile* depFile = dependencyIter.peekNext(); - if (!depFile->isGameFile() || depFile->fileName().compare(fileName, Qt::CaseInsensitive) != 0) - continue; - - noGameFiles = false; - if (depFile->builtIn() || depFile->fromAnotherConfigFile() || mCheckedFiles.contains(depFile)) - { - gamefileChecked = true; + if (!depFile->builtIn() && !depFile->fromAnotherConfigFile() && !mCheckedFiles.contains(depFile)) break; - } + + return Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsUserCheckable | Qt::ItemIsDragEnabled; } } - if (gamefileChecked || noGameFiles) - { - returnFlags = Qt::ItemIsEnabled | Qt::ItemIsSelectable | Qt::ItemIsUserCheckable | Qt::ItemIsDragEnabled; - } - - return returnFlags; + return Qt::NoItemFlags; } QVariant ContentSelectorModel::ContentModel::data(const QModelIndex& index, int role) const diff --git a/components/contentselector/model/esmfile.cpp b/components/contentselector/model/esmfile.cpp index 02ac0efa70..c9df0a39e9 100644 --- a/components/contentselector/model/esmfile.cpp +++ b/components/contentselector/model/esmfile.cpp @@ -2,13 +2,15 @@ ContentSelectorModel::EsmFile::EsmFile(const QString& fileName, ModelItem* parent) : ModelItem(parent) - , mFileName(fileName) { + setFileName(fileName); } void ContentSelectorModel::EsmFile::setFileName(const QString& fileName) { mFileName = fileName; + mHasGameExtension = (mFileName.endsWith(QLatin1String(".esm"), Qt::CaseInsensitive) + || mFileName.endsWith(QLatin1String(".omwgame"), Qt::CaseInsensitive)); } void ContentSelectorModel::EsmFile::setAuthor(const QString& author) @@ -53,9 +55,7 @@ void ContentSelectorModel::EsmFile::setFromAnotherConfigFile(bool fromAnotherCon bool ContentSelectorModel::EsmFile::isGameFile() const { - return (mGameFiles.size() == 0) - && (mFileName.endsWith(QLatin1String(".esm"), Qt::CaseInsensitive) - || mFileName.endsWith(QLatin1String(".omwgame"), Qt::CaseInsensitive)); + return mHasGameExtension && mGameFiles.empty(); } QVariant ContentSelectorModel::EsmFile::fileProperty(const FileProperty prop) const @@ -108,7 +108,7 @@ void ContentSelectorModel::EsmFile::setFileProperty(const FileProperty prop, con switch (prop) { case FileProperty_FileName: - mFileName = value; + setFileName(value); break; case FileProperty_Author: diff --git a/components/contentselector/model/esmfile.hpp b/components/contentselector/model/esmfile.hpp index d040dbd04d..505deb45d6 100644 --- a/components/contentselector/model/esmfile.hpp +++ b/components/contentselector/model/esmfile.hpp @@ -102,6 +102,7 @@ namespace ContentSelectorModel QString mToolTip; bool mBuiltIn = false; bool mFromAnotherConfigFile = false; + bool mHasGameExtension = false; }; }