Merge branch 'handle_bad_navmeshtool_message_magic_48' into 'openmw-48'

Stop updating navmeshtool progress on first bad message (0.48) See merge request OpenMW/openmw!2660
OpenMW · Feb 2, 2023 · 3dd8fd2 · 3dd8fd2
2 parents 7211779 + f23866b
commit 3dd8fd2
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 13 deletions.
diff --git a/apps/launcher/datafilespage.cpp b/apps/launcher/datafilespage.cpp
@@ -11,6 +11,7 @@
 #include <algorithm>
 
 #include <apps/launcher/utils/cellnameloader.hpp>
+
 #include <components/files/configurationmanager.hpp>
 
 #include <components/contentselector/model/esmfile.hpp>
@@ -22,6 +23,7 @@
 #include <components/settings/settings.hpp>
 #include <components/bsa/compressedbsafile.hpp>
 #include <components/navmeshtool/protocol.hpp>
+#include <components/debug/debuglog.hpp>
 #include <components/vfs/bsaarchive.hpp>
 
 #include "utils/textinputdialog.hpp"
@@ -787,6 +789,7 @@ void Launcher::DataFilesPage::startNavMeshTool()
     ui.navMeshLogPlainTextEdit->clear();
     ui.navMeshProgressBar->setValue(0);
     ui.navMeshProgressBar->setMaximum(1);
+    ui.navMeshProgressBar->resetFormat();
 
     mNavMeshToolProgress = NavMeshToolProgress {};
 
@@ -813,6 +816,8 @@ void Launcher::DataFilesPage::readNavMeshToolStderr()
 
 void Launcher::DataFilesPage::updateNavMeshProgress(int minDataSize)
 {
+    if (!mNavMeshToolProgress.mEnabled)
+        return;
     QProcess& process = *mNavMeshToolInvoker->getProcess();
     mNavMeshToolProgress.mMessagesData.append(process.readAllStandardError());
     if (mNavMeshToolProgress.mMessagesData.size() < minDataSize)
@@ -826,14 +831,23 @@ void Launcher::DataFilesPage::updateNavMeshProgress(int minDataSize)
         ui.navMeshProgressBar->maximum(),
         ui.navMeshProgressBar->value(),
     };
-    while (true)
+    try
+    {
+        while (true)
+        {
+            NavMeshTool::Message message;
+            const std::byte* const nextPosition = NavMeshTool::deserialize(position, end, message);
+            if (nextPosition == position)
+                break;
+            position = nextPosition;
+            handle = std::visit(handle, NavMeshTool::decode(message));
+        }
+    }
+    catch (const std::exception& e)
     {
-        NavMeshTool::Message message;
-        const std::byte* const nextPosition = NavMeshTool::deserialize(position, end, message);
-        if (nextPosition == position)
-            break;
-        position = nextPosition;
-        handle = std::visit(handle, NavMeshTool::decode(message));
+        Log(Debug::Error) << "Failed to deserialize navmeshtool message: " << e.what();
+        mNavMeshToolProgress.mEnabled = false;
+        ui.navMeshProgressBar->setFormat("Failed to update progress: " + QString(e.what()));
     }
     if (position != begin)
         mNavMeshToolProgress.mMessagesData = mNavMeshToolProgress.mMessagesData.mid(position - begin);
@@ -861,7 +875,10 @@ void Launcher::DataFilesPage::navMeshToolFinished(int exitCode, QProcess::ExitSt
     updateNavMeshProgress(0);
     ui.navMeshLogPlainTextEdit->appendPlainText(QString::fromUtf8(mNavMeshToolInvoker->getProcess()->readAllStandardOutput()));
     if (exitCode == 0 && exitStatus == QProcess::ExitStatus::NormalExit)
+    {
         ui.navMeshProgressBar->setValue(ui.navMeshProgressBar->maximum());
+        ui.navMeshProgressBar->resetFormat();
+    }
     ui.cancelNavMeshButton->setEnabled(false);
     ui.navMeshProgressBar->setEnabled(false);
 }
diff --git a/apps/launcher/datafilespage.hpp b/apps/launcher/datafilespage.hpp
@@ -83,6 +83,7 @@ namespace Launcher
     private:
         struct NavMeshToolProgress
         {
+            bool mEnabled = true;
             QByteArray mLogData;
             QByteArray mMessagesData;
             std::map<std::uint64_t, std::string> mWorldspaces;

diff --git a/components/navmeshtool/protocol.cpp b/components/navmeshtool/protocol.cpp
@@ -5,13 +5,30 @@
 #include <components/serialization/binarywriter.hpp>
 #include <components/serialization/binaryreader.hpp>
 
+#include <iomanip>
+#include <sstream>
 #include <stdexcept>
 #include <string>
 
 namespace NavMeshTool
 {
     namespace
     {
+        std::string formatMagic(const char (&value)[std::size(messageMagic)])
+        {
+            std::ostringstream stream;
+            for (const char v : value)
+            {
+                if (std::isprint(v) && !std::isspace(v))
+                    stream << '\'' << v << '\'';
+                else
+                    stream << "0x" << std::hex << std::uppercase << std::setfill('0') << std::setw(2)
+                           << static_cast<int>(v);
+                stream << ' ';
+            }
+            return stream.str();
+        }
+
         template <Serialization::Mode mode>
         struct Format : Serialization::Format<mode, Format<mode>>
         {
@@ -29,7 +46,7 @@ namespace NavMeshTool
                     char magic[std::size(messageMagic)];
                     visitor(*this, magic);
                     if (std::memcmp(magic, messageMagic, sizeof(magic)) != 0)
-                        throw BadMessageMagic();
+                        throw std::runtime_error("Bad navmeshtool message magic: " + formatMagic(magic));
                 }
                 visitor(*this, value.mType);
                 visitor(*this, value.mSize);

diff --git a/components/navmeshtool/protocol.hpp b/components/navmeshtool/protocol.hpp
@@ -11,11 +11,6 @@ namespace NavMeshTool
 {
     inline constexpr char messageMagic[] = {'n', 'v', 't', 'm'};
 
-    struct BadMessageMagic : std::runtime_error
-    {
-        BadMessageMagic() : std::runtime_error("Bad Message magic") {}
-    };
-
     enum class MessageType : std::uint64_t
     {
         ExpectedCells = 1,