Shallow search and lazy loading for Other Logs page (#3664)

flatpak/shared-modules

@@ -1 +1 @@
-Subproject commit 1f8e591b263eef8a0dc04929f2da135af59fac3c
+Subproject commit f5d368a31d6ef046eb2955c74ec6f54f32ed5c4e

launcher/BaseInstance.h

@@ -198,15 +198,10 @@ class BaseInstance : public QObject, public std::enable_shared_from_this<BaseIns
     virtual QProcessEnvironment createEnvironment() = 0;
     virtual QProcessEnvironment createLaunchEnvironment() = 0;
 
-    /*!
-     * Returns a matcher that can maps relative paths within the instance to whether they are 'log files'
-     */
-    virtual IPathMatcher::Ptr getLogFileMatcher() = 0;
-
     /*!
      * Returns the root folder to use for looking up log files
      */
-    virtual QString getLogFileRoot() = 0;
+    virtual QStringList getLogFileSearchPaths() = 0;
 
     virtual QString getStatusbarDescription() = 0;
 

launcher/InstancePageProvider.h

@@ -44,10 +44,7 @@ class InstancePageProvider : protected QObject, public BasePageProvider {
         // values.append(new GameOptionsPage(onesix.get()));
         values.append(new ScreenshotsPage(FS::PathCombine(onesix->gameRoot(), "screenshots")));
         values.append(new InstanceSettingsPage(onesix));
-        auto logMatcher = inst->getLogFileMatcher();
+        values.append(new OtherLogsPage(inst));
-        if (logMatcher) {
-            values.append(new OtherLogsPage(inst, logMatcher));
-        }
         return values;
     }
 

launcher/NullInstance.h

@@ -57,8 +57,7 @@ class NullInstance : public BaseInstance {
     QProcessEnvironment createEnvironment() override { return QProcessEnvironment(); }
     QProcessEnvironment createLaunchEnvironment() override { return QProcessEnvironment(); }
     QMap<QString, QString> getVariables() override { return QMap<QString, QString>(); }
-    IPathMatcher::Ptr getLogFileMatcher() override { return nullptr; }
+    QStringList getLogFileSearchPaths() override { return {}; }
-    QString getLogFileRoot() override { return instanceRoot(); }
     QString typeName() const override { return "Null"; }
     bool canExport() const override { return false; }
     bool canEdit() const override { return false; }

launcher/minecraft/MinecraftInstance.cpp

@@ -1047,19 +1047,9 @@ MessageLevel::Enum MinecraftInstance::guessLevel(const QString& line, MessageLev
     return level;
 }
 
-IPathMatcher::Ptr MinecraftInstance::getLogFileMatcher()
+QStringList MinecraftInstance::getLogFileSearchPaths()
 {
-    auto combined = std::make_shared<MultiMatcher>();
+    return { FS::PathCombine(gameRoot(), "crash-reports"), FS::PathCombine(gameRoot(), "logs"), gameRoot() };
-    combined->add(std::make_shared<RegexpMatcher>(".*\\.log(\\.[0-9]*)?(\\.gz)?$"));
-    combined->add(std::make_shared<RegexpMatcher>("crash-.*\\.txt"));
-    combined->add(std::make_shared<RegexpMatcher>("IDMap dump.*\\.txt$"));
-    combined->add(std::make_shared<RegexpMatcher>("ModLoader\\.txt(\\..*)?$"));
-    return combined;
-}
-
-QString MinecraftInstance::getLogFileRoot()
-{
-    return gameRoot();
 }
 
 QString MinecraftInstance::getStatusbarDescription()

launcher/minecraft/MinecraftInstance.h

@@ -142,9 +142,7 @@ class MinecraftInstance : public BaseInstance {
     /// guess log level from a line of minecraft log
     MessageLevel::Enum guessLevel(const QString& line, MessageLevel::Enum level) override;
 
-    IPathMatcher::Ptr getLogFileMatcher() override;
+    QStringList getLogFileSearchPaths() override;
-
-    QString getLogFileRoot() override;
 
     QString getStatusbarDescription() override;
 

launcher/ui/pages/instance/OtherLogsPage.cpp

@@ -43,16 +43,18 @@
 
 #include <FileSystem.h>
 #include <GZip.h>
+#include <QDir>
+#include <QDirIterator>
+#include <QFileSystemWatcher>
 #include <QShortcut>
-#include "RecursiveFileSystemWatcher.h"
+#include <QUrl>
 
-OtherLogsPage::OtherLogsPage(InstancePtr instance, IPathMatcher::Ptr fileFilter, QWidget* parent)
+OtherLogsPage::OtherLogsPage(InstancePtr instance, QWidget* parent)
     : QWidget(parent)
     , ui(new Ui::OtherLogsPage)
     , m_instance(instance)
-    , m_path(instance->getLogFileRoot())
+    , m_basePath(instance->gameRoot())
-    , m_fileFilter(fileFilter)
+    , m_logSearchPaths(instance->getLogFileSearchPaths())
-    , m_watcher(new RecursiveFileSystemWatcher(this))
     , m_model(new LogModel(this))
 {
     ui->setupUi(this);
@@ -78,11 +80,7 @@ OtherLogsPage::OtherLogsPage(InstancePtr instance, IPathMatcher::Ptr fileFilter,
     m_model->setOverflowMessage(tr("Cannot display this log since the log length surpassed %1 lines.").arg(m_model->getMaxLines()));
     m_proxy->setSourceModel(m_model.get());
 
-    m_watcher->setMatcher(fileFilter);
+    connect(&m_watcher, &QFileSystemWatcher::directoryChanged, this, &OtherLogsPage::populateSelectLogBox);
-    m_watcher->setRootDir(QDir::current().absoluteFilePath(m_path));
-
-    connect(m_watcher, &RecursiveFileSystemWatcher::filesChanged, this, &OtherLogsPage::populateSelectLogBox);
-    populateSelectLogBox();
 
     auto findShortcut = new QShortcut(QKeySequence(QKeySequence::Find), this);
     connect(findShortcut, &QShortcut::activated, this, &OtherLogsPage::findActivated);
@@ -108,29 +106,54 @@ void OtherLogsPage::retranslate()
 
 void OtherLogsPage::openedImpl()
 {
-    m_watcher->enable();
+    const QStringList failedPaths = m_watcher.addPaths(m_logSearchPaths);
+
+    for (const QString& path : m_logSearchPaths) {
+        if (failedPaths.contains(path))
+            qDebug() << "Failed to start watching" << path;
+        else
+            qDebug() << "Started watching" << path;
+    }
+
+    populateSelectLogBox();
 }
+
 void OtherLogsPage::closedImpl()
 {
-    m_watcher->disable();
+    const QStringList failedPaths = m_watcher.removePaths(m_logSearchPaths);
+
+    for (const QString& path : m_logSearchPaths) {
+        if (failedPaths.contains(path))
+            qDebug() << "Failed to stop watching" << path;
+        else
+            qDebug() << "Stopped watching" << path;
+    }
 }
 
 void OtherLogsPage::populateSelectLogBox()
 {
+    const QString prevCurrentFile = m_currentFile;
+
+    ui->selectLogBox->blockSignals(true);
     ui->selectLogBox->clear();
-    ui->selectLogBox->addItems(m_watcher->files());
+    ui->selectLogBox->addItems(getPaths());
-    if (m_currentFile.isEmpty()) {
+    ui->selectLogBox->blockSignals(false);
-        setControlsEnabled(false);
+
-        ui->selectLogBox->setCurrentIndex(-1);
+    if (!prevCurrentFile.isEmpty()) {
-    } else {
+        const int index = ui->selectLogBox->findText(prevCurrentFile);
-        const int index = ui->selectLogBox->findText(m_currentFile);
         if (index != -1) {
+            ui->selectLogBox->blockSignals(true);
             ui->selectLogBox->setCurrentIndex(index);
+            ui->selectLogBox->blockSignals(false);
             setControlsEnabled(true);
+            // don't refresh file
+            return;
         } else {
             setControlsEnabled(false);
         }
     }
+
+    on_selectLogBox_currentIndexChanged(ui->selectLogBox->currentIndex());
 }
 
 void OtherLogsPage::on_selectLogBox_currentIndexChanged(const int index)
@@ -140,7 +163,7 @@ void OtherLogsPage::on_selectLogBox_currentIndexChanged(const int index)
         file = ui->selectLogBox->itemText(index);
     }
 
-    if (file.isEmpty() || !QFile::exists(FS::PathCombine(m_path, file))) {
+    if (file.isEmpty() || !QFile::exists(FS::PathCombine(m_basePath, file))) {
         m_currentFile = QString();
         ui->text->clear();
         setControlsEnabled(false);
@@ -157,8 +180,7 @@ void OtherLogsPage::on_btnReload_clicked()
         setControlsEnabled(false);
         return;
     }
-
+    QFile file(FS::PathCombine(m_basePath, m_currentFile));
-    QFile file(FS::PathCombine(m_path, m_currentFile));
     if (!file.open(QFile::ReadOnly)) {
         setControlsEnabled(false);
         ui->btnReload->setEnabled(true);  // allow reload
@@ -267,7 +289,7 @@ void OtherLogsPage::on_btnDelete_clicked()
                               QMessageBox::Yes, QMessageBox::No) == QMessageBox::No) {
         return;
     }
-    QFile file(FS::PathCombine(m_path, m_currentFile));
+    QFile file(FS::PathCombine(m_basePath, m_currentFile));
 
     if (FS::trash(file.fileName())) {
         return;
@@ -280,7 +302,7 @@ void OtherLogsPage::on_btnDelete_clicked()
 
 void OtherLogsPage::on_btnClean_clicked()
 {
-    auto toDelete = m_watcher->files();
+    auto toDelete = getPaths();
     if (toDelete.isEmpty()) {
         return;
     }
@@ -303,7 +325,9 @@ void OtherLogsPage::on_btnClean_clicked()
     }
     QStringList failed;
     for (auto item : toDelete) {
-        QFile file(FS::PathCombine(m_path, item));
+        QString absolutePath = FS::PathCombine(m_basePath, item);
+        QFile file(absolutePath);
+        qDebug() << "Deleting log" << absolutePath;
         if (FS::trash(file.fileName())) {
             continue;
         }
@@ -357,6 +381,29 @@ void OtherLogsPage::setControlsEnabled(const bool enabled)
     ui->btnClean->setEnabled(enabled);
 }
 
+QStringList OtherLogsPage::getPaths()
+{
+    QDir baseDir(m_basePath);
+
+    QStringList result;
+
+    for (QString searchPath : m_logSearchPaths) {
+        QDir searchDir(searchPath);
+
+        QStringList filters{ "*.log", "*.log.gz" };
+
+        if (searchPath != m_basePath)
+            filters.append("*.txt");
+
+        QStringList entries = searchDir.entryList(filters, QDir::Files | QDir::Readable, QDir::SortFlag::Time);
+
+        for (const QString& name : entries)
+            result.append(baseDir.relativeFilePath(searchDir.filePath(name)));
+    }
+
+    return result;
+}
+
 void OtherLogsPage::on_findButton_clicked()
 {
     auto modifiers = QApplication::keyboardModifiers();

launcher/ui/pages/instance/OtherLogsPage.h

@@ -39,6 +39,7 @@
 
 #include <Application.h>
 #include <pathmatcher/IPathMatcher.h>
+#include <QFileSystemWatcher>
 #include "LogPage.h"
 #include "ui/pages/BasePage.h"
 
@@ -52,7 +53,7 @@ class OtherLogsPage : public QWidget, public BasePage {
     Q_OBJECT
 
    public:
-    explicit OtherLogsPage(InstancePtr instance, IPathMatcher::Ptr fileFilter, QWidget* parent = 0);
+    explicit OtherLogsPage(InstancePtr instance, QWidget* parent = 0);
     ~OtherLogsPage();
 
     QString id() const override { return "logs"; }
@@ -85,13 +86,16 @@ class OtherLogsPage : public QWidget, public BasePage {
    private:
     void setControlsEnabled(bool enabled);
 
+    QStringList getPaths();
+
    private:
     Ui::OtherLogsPage* ui;
     InstancePtr m_instance;
-    QString m_path;
+    /** Path to display log paths relative to. */
+    QString m_basePath;
+    QStringList m_logSearchPaths;
     QString m_currentFile;
-    IPathMatcher::Ptr m_fileFilter;
+    QFileSystemWatcher m_watcher;
-    RecursiveFileSystemWatcher* m_watcher;
 
     LogFormatProxyModel* m_proxy;
     shared_qobject_ptr<LogModel> m_model;

libraries/extra-cmake-modules

@@ -1 +1 @@
-Subproject commit 1f820dc98d0a520c175433bcbb0098327d82aac6
+Subproject commit a3d9394aba4b35789293378e04fb7473d65edf97

libraries/libnbtplusplus

@@ -1 +1 @@
-Subproject commit 531449ba1c930c98e0bcf5d332b237a8566f9d78
+Subproject commit 23b955121b8217c1c348a9ed2483167a6f3ff4ad

libraries/quazip

@@ -1 +1 @@
-Subproject commit 3fd3b299b875fbd2beac4894b8a870d80022cad7
+Subproject commit 8aeb3f7d8254f4bf1f7c6cf2a8f59c2ca141a552