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.

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

Nested directory and window modality problems in MainWindow::ImportProject #104

Open
rhuizer opened this issue Aug 29, 2018 · 0 comments
Open
Milestone

Comments

@rhuizer
Copy link

rhuizer commented Aug 29, 2018

MainWindow::ImportProject uses recurseAddDir to enumerate files in the resource directories. In theory this should handle subdirectories in the resource directories, but the implementation does not work properly. This creates problems importing certain RPM2K games that do this.

The error dialog that is displayed is modal, with a parent of this, which in turn calls exec that runs the event pump to display the modal QProgressDialog which will be the active modal window. This makes closing the error dialog confusing.

The patch below resolves both issues, the former by handling recurseAddDir paths as relative to the provided base rather than as absolute paths. This allows for clean composition of source and destination paths. Furthermore, it makes the error dialog parent widget the qprogressdialog, which is now explicitly shown before the error dialog is created. This makes the modal hierarchy clear and function intuitively.

diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 0af3693..c7c3c74 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -36,26 +36,23 @@ const std::vector<QString> resource_dirs =
     {BACKDROP, BATTLE, BATTLE2, BATTLECHARSET, BATTLEWEAPON, CHARSET, CHIPSET, FACESET,
     GAMEOVER, MONSTER, MOVIE, MUSIC, PANORAMA, PICTURE, SOUND, SYSTEM, SYSTEM2, TITLE};
 
-static void recurseAddDir(QDir d, QStringList & list) {
-
-    QStringList qsl = d.entryList(QDir::NoDotAndDotDot | QDir::Dirs | QDir::Files);
+static void recurseAddDir(const QString &basedir, QStringList &list, const QString &curdir = QString())
+{
+    QDir cur(basedir + curdir);
+    QStringList qsl = cur.entryList(QDir::NoDotAndDotDot | QDir::Dirs | QDir::Files);
 
     foreach (QString file, qsl) {
-
-        QFileInfo finfo(QString("%1/%2").arg(d.path()).arg(file));
+        QString   path_absolute = cur.filePath(file);
+        QString   path_relative = QDir(curdir).filePath(file);
+        QFileInfo finfo(path_absolute);
 
         if (finfo.isSymLink())
             return;
 
-        if (finfo.isDir()) {
-
-            QString dirname = finfo.fileName();
-            QDir sd(finfo.filePath());
-
-            recurseAddDir(sd, list);
-
-        } else
-            list << QDir::toNativeSeparators(finfo.filePath());
+        if (finfo.isDir())
+            recurseAddDir(basedir, list, path_relative);
+        else
+            list << path_relative;
     }
 }
 
@@ -290,42 +287,43 @@ void MainWindow::ImportProject(QString p_path, QString d_folder, bool convert_xy
     LDB_Reader::SaveXml(mCore->filePath(ROOT, EASY_DB).toStdString());
     LMT_Reader::SaveXml(mCore->filePath(ROOT, EASY_MT).toStdString());
     QStringList entries;
+
     for (const QString& dir : resource_dirs)
-        recurseAddDir(QDir(p_path+dir), entries);
+        recurseAddDir(p_path, entries, dir);
 
     QProgressDialog progress("Importing resources...", "", 0, entries.count(), this);
     progress.setWindowModality(Qt::WindowModal);
+    progress.setFixedSize(progress.size());
+    progress.show();
+
     for (int i = 0; i < entries.count(); i++)
     {
+        QString dest_file = mCore->filePath(entries[i]);
+        QString src_file  = p_path + entries[i];
+
         progress.setValue(i);
-        QFileInfo info(entries[i]);
-        QString dest_file = mCore->filePath(info.dir().dirName()+"/",info.fileName());
-        if (!QFile::copy(entries[i], dest_file))
+
+        if (!QDir::root().mkpath(QFileInfo(dest_file).path()) ||
+            !QFile::copy(src_file, dest_file))
         {
-            QMessageBox box(this);
             QString name = tr("Error");
             QString text = tr("Could not copy file %1 to \n"
-                              "%2").arg(entries[i]).arg(dest_file);
-
-            box.setModal(true);
-            box.setWindowTitle(name);
-            box.setText(QString::fromLatin1("%1").arg(text));
-            box.setIcon(QMessageBox::Critical);
-            box.setStandardButtons(QMessageBox::Ok);
-
-            box.exec();
-
+                              "%2").arg(QDir::toNativeSeparators(src_file))
+                                   .arg(QDir::toNativeSeparators(dest_file));
+            QMessageBox::critical(&progress, name, text);
             on_action_Close_Project_triggered();
             return;
         }
-        if (convert_xyz && info.dir().dirName() != MUSIC && info.dir().dirName() != SOUND)
+
+        if (convert_xyz && !entries[i].startsWith(MUSIC) && !entries[i].startsWith(SOUND))
         {
             QFile file(dest_file);
             file.open(QIODevice::ReadOnly);
             if (file.read(4) == "XYZ1")
             {
-                QString conv_path = mCore->filePath(info.dir().dirName()+"/",
-                                                    info.completeBaseName() + ".png");
+                QFileInfo fi(entries[i]);
+                QString conv_path = mCore->filePath(fi.path() + "/" + fi.completeBaseName() + ".png");
+               // XXX: should handle .png that already exists (both .xyz and .png file in src)
                 if (convertXYZtoPNG(file, conv_path))
                     file.remove();
                 else
@Ghabry Ghabry added this to the 0.1 milestone Mar 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants