Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore conflicts between hidden files #2148

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Ignore conflicts between hidden files #2148

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e4e03f
Ignore conflicts between hidden files
JonathanFeenstra Oct 16, 2024
a17624d
Optimize checks for .mohidden extension
JonathanFeenstra Dec 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/filetreemodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

using namespace MOBase;
using namespace MOShared;
namespace fs = std::filesystem;

// in mainwindow.cpp
QString UnmanagedModName();
Expand Down Expand Up @@ -969,8 +970,12 @@ void FileTreeModel::updateFileItem(FileTreeItem& item, const MOShared::FileEntry

bool FileTreeModel::shouldShowFile(const FileEntry& file) const
{
if (showConflictsOnly() && (file.getAlternatives().size() == 0)) {
// only conflicts should be shown, but this file is not conflicted
if (showConflictsOnly() &&
((file.getAlternatives().size() == 0) ||
fs::path(file.getName())
.extension()
.compare(ModInfo::s_HiddenExt.toStdWString()) == 0)) {
// only conflicts should be shown, but this file is hidden or not conflicted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a hotpath, I don't know how fast the fs:path conversion to compare the extension is. Might be worth profiling this under a large setup. I know we got massive wins by using a faster string comparison in the past

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think file.getName().ends_with(ModInfo::s_HiddenExt.toStdWString()) would suffice? That's what I used here. I only used the fs::path conversion here to be consistent with similar existing code that I used as a reference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check needs to be case insensitive, which is what can make it costly. If it isn't a problem on a larger setup we don't need to optimize it. Just need to check it isn't a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extension being anything other than fully lowercase would be an unusual edge case, but I suppose we should still account for that possibility. I'm not sure how to go about profiling it though. If we already know it's a hot path, I'd say it's worth optimizing regardless. How about boost::algorithm::iends_with(file.getName(), ModInfo::s_HiddenExt.toStdWString()))? It's case insensitive and doesn't require a fs::path conversion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

QString::fromStdWString(file.getName()).endsWith(ModInfo::s_HiddenExt, Qt::CaseInsensitive))

Not sure why FileEntry uses std::w/string types. Also the less boost usage, the better.

return false;
}

Expand All @@ -980,7 +985,9 @@ bool FileTreeModel::shouldShowFile(const FileEntry& file) const
}

if (!showHiddenFiles() &&
file.getName().ends_with(ModInfo::s_HiddenExt.toStdWString())) {
fs::path(file.getName())
.extension()
.compare(ModInfo::s_HiddenExt.toStdWString()) == 0) {
// hidden files shouldn't be shown, but this file is hidden
return false;
}
Expand All @@ -991,8 +998,10 @@ bool FileTreeModel::shouldShowFile(const FileEntry& file) const
bool FileTreeModel::shouldShowFolder(const DirectoryEntry& dir,
const FileTreeItem* item) const
{
if (!showHiddenFiles() &&
dir.getName().ends_with(ModInfo::s_HiddenExt.toStdWString())) {
if ((!showHiddenFiles() || showConflictsOnly()) &&
fs::path(dir.getName())
.extension()
.compare(ModInfo::s_HiddenExt.toStdWString()) == 0) {
return false;
}

Expand Down
124 changes: 76 additions & 48 deletions src/modinfodialogconflicts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

using namespace MOShared;
using namespace MOBase;
namespace fs = std::filesystem;

// if there are more than 50 selected items in the conflict tree, don't bother
// checking whether menu items apply to them, just show all of them
Expand Down Expand Up @@ -110,7 +111,7 @@ bool ConflictsTab::canHandleUnmanaged() const
return true;
}

void ConflictsTab::changeItemsVisibility(QTreeView* tree, bool visible)
void ConflictsTab::hideItems(QTreeView* tree)
{
bool changed = false;
bool stop = false;
Expand All @@ -119,19 +120,16 @@ void ConflictsTab::changeItemsVisibility(QTreeView* tree, bool visible)

// logging
{
const QString action = (visible ? "unhiding" : "hiding");

QString files;
if (n > max_small_selection)
files = "a lot of";
else
files = QString("%1").arg(n);

log::debug("{} {} conflict files", action, files);
log::debug("hiding {} conflict files", files);
}

QFlags<FileRenamer::RenameFlags> flags =
(visible ? FileRenamer::UNHIDE : FileRenamer::HIDE);
QFlags<FileRenamer::RenameFlags> flags = FileRenamer::HIDE;

if (n > 1) {
flags |= FileRenamer::MULTIPLE;
Expand All @@ -158,25 +156,13 @@ void ConflictsTab::changeItemsVisibility(QTreeView* tree, bool visible)
return false;
}

auto result = FileRenamer::RESULT_CANCEL;

if (visible) {
if (!item->canUnhide()) {
log::debug("cannot unhide {}, skipping", item->relativeName());
return true;
}

result = unhideFile(renamer, item->fileName());

} else {
if (!item->canHide()) {
log::debug("cannot hide {}, skipping", item->relativeName());
return true;
}

result = hideFile(renamer, item->fileName());
if (!item->canHide()) {
log::debug("cannot hide {}, skipping", item->relativeName());
return true;
}

auto result = hideFile(renamer, item->fileName());

switch (result) {
case FileRenamer::RESULT_OK: {
// will trigger a refresh at the end
Expand All @@ -199,7 +185,7 @@ void ConflictsTab::changeItemsVisibility(QTreeView* tree, bool visible)
return true;
});

log::debug("{} conflict files done", (visible ? "unhiding" : "hiding"));
log::debug("hiding conflict files done");

if (changed) {
log::debug("triggering refresh");
Expand Down Expand Up @@ -351,21 +337,12 @@ void ConflictsTab::showContextMenu(const QPoint& pos, QTreeView* tree)
// hide
if (actions.hide) {
connect(actions.hide, &QAction::triggered, [&] {
changeItemsVisibility(tree, false);
hideItems(tree);
});

menu.addAction(actions.hide);
}

// unhide
if (actions.unhide) {
connect(actions.unhide, &QAction::triggered, [&] {
changeItemsVisibility(tree, true);
});

menu.addAction(actions.unhide);
}

if (!menu.isEmpty()) {
if (actions.open || actions.preview || actions.runHooked) {
// bold the first option
Expand All @@ -386,7 +363,6 @@ ConflictsTab::Actions ConflictsTab::createMenuActions(QTreeView* tree)
}

bool enableHide = true;
bool enableUnhide = true;
bool enableRun = true;
bool enableOpen = true;
bool enablePreview = true;
Expand Down Expand Up @@ -421,7 +397,6 @@ ConflictsTab::Actions ConflictsTab::createMenuActions(QTreeView* tree)
}

enableHide = item->canHide();
enableUnhide = item->canUnhide();
enableRun = item->canRun();
enableOpen = item->canOpen();
enablePreview = item->canPreview(plugin());
Expand All @@ -443,19 +418,14 @@ ConflictsTab::Actions ConflictsTab::createMenuActions(QTreeView* tree)
if (n <= max_small_selection) {
// if the number of selected items is low, checking them to accurately
// show the menu items is worth it
enableHide = false;
enableUnhide = false;
enableHide = false;

forEachInSelection(tree, [&](const ConflictItem* item) {
if (item->canHide()) {
enableHide = true;
}

if (item->canUnhide()) {
enableUnhide = true;
}

if (enableHide && enableUnhide && enableGoto) {
if (enableHide && enableGoto) {
// found all, no need to check more
return false;
}
Expand Down Expand Up @@ -487,11 +457,6 @@ ConflictsTab::Actions ConflictsTab::createMenuActions(QTreeView* tree)
actions.hide = new QAction(tr("&Hide"), parentWidget());
actions.hide->setEnabled(enableHide);

// note that it is possible for hidden files to appear if they override other
// hidden files from another mod
actions.unhide = new QAction(tr("&Unhide"), parentWidget());
actions.unhide->setEnabled(enableUnhide);

if (enableGoto && n == 1) {
const auto* item =
model->getItem(static_cast<std::size_t>(modelSel.indexes()[0].row()));
Expand Down Expand Up @@ -633,8 +598,40 @@ bool GeneralConflictsTab::update()

if (m_tab->origin() != nullptr) {
const auto rootPath = m_tab->mod().absolutePath();
const auto hideExt = ModInfo::s_HiddenExt.toStdWString();
std::set<const DirectoryEntry*> checkedDirs;

for (const auto& file : m_tab->origin()->getFiles()) {
const fs::path nameAsPath(file->getName());
if (nameAsPath.extension().wstring().compare(hideExt) == 0) {
// skip hidden file conflicts
continue;
} else {
const DirectoryEntry* parent = file->getParent();
auto hidden = false;
// iterate on all parent direEntries to check for .mohiddden
while (parent != nullptr) {
auto insertResult = checkedDirs.insert(parent);

if (insertResult.second == false) {
// if already present break as we can assume to have checked the parents as
// well
break;
} else {
const fs::path dirPath(parent->getName());
if (dirPath.extension().wstring().compare(hideExt) == 0) {
hidden = true;
break;
}
parent = parent->getParent();
}
}
if (hidden) {
// skip hidden file conflicts
continue;
}
}

// careful: these two strings are moved into createXItem() below
QString relativeName =
QDir::fromNativeSeparators(ToQString(file->getRelativePath()));
Expand Down Expand Up @@ -951,11 +948,42 @@ void AdvancedConflictsTab::update()

if (m_tab->origin() != nullptr) {
const auto rootPath = m_tab->mod().absolutePath();
const auto hideExt = ModInfo::s_HiddenExt.toStdWString();

const auto& files = m_tab->origin()->getFiles();
m_model->reserve(files.size());
std::set<const DirectoryEntry*> checkedDirs;

for (const auto& file : files) {
const fs::path nameAsPath(file->getName());
if (nameAsPath.extension().wstring().compare(hideExt) == 0) {
// skip hidden file conflicts
continue;
} else {
const DirectoryEntry* parent = file->getParent();
auto hidden = false;
// iterate on all parent direEntries to check for .mohiddden
while (parent != nullptr) {
auto insertResult = checkedDirs.insert(parent);

if (insertResult.second == false) {
// if already present break as we can assume to have checked the parents as
// well
break;
} else {
const fs::path dirPath(parent->getName());
if (dirPath.extension().wstring().compare(hideExt) == 0) {
hidden = true;
break;
}
parent = parent->getParent();
}
}
if (hidden) {
// skip hidden file conflicts
continue;
}
}
// careful: these two strings are moved into createItem() below
QString relativeName =
QDir::fromNativeSeparators(ToQString(file->getRelativePath()));
Expand Down
3 changes: 1 addition & 2 deletions src/modinfodialogconflicts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,15 +135,14 @@ class ConflictsTab : public ModInfoDialogTab

void openItem(const ConflictItem* item, bool hooked);
void previewItem(const ConflictItem* item);
void changeItemsVisibility(QTreeView* tree, bool visible);
void hideItems(QTreeView* tree);

void showContextMenu(const QPoint& pos, QTreeView* tree);

private:
struct Actions
{
QAction* hide = nullptr;
QAction* unhide = nullptr;
QAction* open = nullptr;
QAction* runHooked = nullptr;
QAction* preview = nullptr;
Expand Down
50 changes: 27 additions & 23 deletions src/modinfowithconflictinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ ModInfoWithConflictInfo::Conflicts ModInfoWithConflictInfo::doConflictCheck() co

bool providesAnything = false;
bool hasHiddenFiles = false;
bool hasVisibleFiles = false;

std::vector<int> dataIDs;
if (m_Core.directoryStructure()->originExists(L"data")) {
Expand All @@ -120,36 +121,39 @@ ModInfoWithConflictInfo::Conflicts ModInfoWithConflictInfo::doConflictCheck() co

// for all files in this origin
for (FileEntryPtr file : files) {
const fs::path nameAsPath(file->getName());
if (nameAsPath.extension().wstring().compare(hideExt) == 0) {
hasHiddenFiles = true;
// skip hidden file conflicts
continue;
} else {
const DirectoryEntry* parent = file->getParent();
auto hidden = false;

// skip hiidden file check if already found one
if (!hasHiddenFiles) {
const fs::path nameAsPath(file->getName());

if (nameAsPath.extension().wstring().compare(hideExt) == 0) {
hasHiddenFiles = true;
} else {
const DirectoryEntry* parent = file->getParent();

// iterate on all parent direEntries to check for .mohiddden
while (parent != nullptr) {
auto insertResult = checkedDirs.insert(parent);
// iterate on all parent direEntries to check for .mohiddden
while (parent != nullptr) {
auto insertResult = checkedDirs.insert(parent);

if (insertResult.second == false) {
// if already present break as we can assume to have checked the parents
// as well
if (insertResult.second == false) {
// if already present break as we can assume to have checked the parents as
// well
break;
} else {
const fs::path dirPath(parent->getName());
if (dirPath.extension().wstring().compare(hideExt) == 0) {
hasHiddenFiles = hidden = true;
break;
} else {
const fs::path dirPath(parent->getName());
if (dirPath.extension().wstring().compare(hideExt) == 0) {
hasHiddenFiles = true;
break;
}
parent = parent->getParent();
}
parent = parent->getParent();
}
}
if (hidden) {
// skip hidden file conflicts
continue;
}
}

hasVisibleFiles = true;
auto alternatives = file->getAlternatives();
if ((alternatives.size() == 0) ||
std::find(dataIDs.begin(), dataIDs.end(), alternatives.back().originID()) !=
Expand Down Expand Up @@ -223,7 +227,7 @@ ModInfoWithConflictInfo::Conflicts ModInfoWithConflictInfo::doConflictCheck() co
}

if (files.size() != 0) {
if (!providesAnything)
if (hasVisibleFiles && !providesAnything)
conflicts.m_CurrentConflictState = CONFLICT_REDUNDANT;
else if (!conflicts.m_OverwriteList.empty() &&
!conflicts.m_OverwrittenList.empty())
Expand Down
Loading