From 89bb04e8fd7c807e534781e6f19dab5386ebcccf Mon Sep 17 00:00:00 2001
From: elsid <elsid.mail@gmail.com>
Date: Fri, 28 Jun 2024 22:22:16 +0200
Subject: [PATCH] Exit with fatal error on duplicated instruction code

Instead of causing SIGABRT which is more confusing.
---
 apps/openmw/mwscript/scriptmanagerimp.cpp | 11 ++++-------
 apps/openmw/mwscript/scriptmanagerimp.hpp |  1 -
 components/debug/debugging.cpp            |  2 +-
 components/interpreter/interpreter.hpp    | 24 +++++++++++++----------
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/apps/openmw/mwscript/scriptmanagerimp.cpp b/apps/openmw/mwscript/scriptmanagerimp.cpp
index 14194de61d..82f234bf76 100644
--- a/apps/openmw/mwscript/scriptmanagerimp.cpp
+++ b/apps/openmw/mwscript/scriptmanagerimp.cpp
@@ -30,9 +30,10 @@ namespace MWScript
         , mStore(store)
         , mCompilerContext(compilerContext)
         , mParser(mErrorHandler, mCompilerContext)
-        , mOpcodesInstalled(false)
         , mGlobalScripts(store)
     {
+        installOpcodes(mInterpreter);
+
         mErrorHandler.setWarningsMode(warningsMode);
 
         mScriptBlacklist.resize(scriptBlacklist.size());
@@ -110,14 +111,9 @@ namespace MWScript
         const auto& target = interpreterContext.getTarget();
         if (!iter->second.mProgram.mInstructions.empty()
             && iter->second.mInactive.find(target) == iter->second.mInactive.end())
+        {
             try
             {
-                if (!mOpcodesInstalled)
-                {
-                    installOpcodes(mInterpreter);
-                    mOpcodesInstalled = true;
-                }
-
                 mInterpreter.run(iter->second.mProgram, interpreterContext);
                 return true;
             }
@@ -131,6 +127,7 @@ namespace MWScript
 
                 iter->second.mInactive.insert(target); // don't execute again.
             }
+        }
         return false;
     }
 
diff --git a/apps/openmw/mwscript/scriptmanagerimp.hpp b/apps/openmw/mwscript/scriptmanagerimp.hpp
index de1ce286a6..326ded7bc6 100644
--- a/apps/openmw/mwscript/scriptmanagerimp.hpp
+++ b/apps/openmw/mwscript/scriptmanagerimp.hpp
@@ -42,7 +42,6 @@ namespace MWScript
         Compiler::Context& mCompilerContext;
         Compiler::FileParser mParser;
         Interpreter::Interpreter mInterpreter;
-        bool mOpcodesInstalled;
 
         struct CompiledScript
         {
diff --git a/components/debug/debugging.cpp b/components/debug/debugging.cpp
index f828bfae62..a107584407 100644
--- a/components/debug/debugging.cpp
+++ b/components/debug/debugging.cpp
@@ -468,7 +468,7 @@ int wrapApplication(int (*innerApplication)(int argc, char* argv[]), int argc, c
 #endif
             SDL_ShowSimpleMessageBox(0, (std::string(appName) + ": Fatal error").c_str(), e.what(), nullptr);
 
-        Log(Debug::Error) << "Error: " << e.what();
+        Log(Debug::Error) << "Fatal error: " << e.what();
 
         ret = 1;
     }
diff --git a/components/interpreter/interpreter.hpp b/components/interpreter/interpreter.hpp
index eae3e7b478..381cb5dd6b 100644
--- a/components/interpreter/interpreter.hpp
+++ b/components/interpreter/interpreter.hpp
@@ -1,14 +1,16 @@
 #ifndef INTERPRETER_INTERPRETER_H_INCLUDED
 #define INTERPRETER_INTERPRETER_H_INCLUDED
 
-#include <cassert>
 #include <map>
 #include <memory>
 #include <stack>
+#include <stdexcept>
 #include <utility>
 
-#include "components/interpreter/program.hpp"
+#include <components/misc/strings/format.hpp>
+
 #include "opcodes.hpp"
+#include "program.hpp"
 #include "runtime.hpp"
 #include "types.hpp"
 
@@ -32,11 +34,13 @@ namespace Interpreter
 
         void end();
 
-        template <typename TSeg, typename TOp>
-        void installSegment(TSeg& seg, int code, TOp&& op)
+        template <typename T, typename... Args>
+        void installSegment(auto& segment, std::string_view name, int code, Args&&... args)
         {
-            assert(seg.find(code) == seg.end());
-            seg.emplace(code, std::move(op));
+            if (segment.find(code) != segment.end())
+                throw std::invalid_argument(Misc::StringUtils::format(
+                    "Duplicated interpreter instruction code in segment %s: 0x%x", name, code));
+            segment.emplace(code, std::make_unique<T>(std::forward<Args>(args)...));
         }
 
     public:
@@ -48,25 +52,25 @@ namespace Interpreter
         template <typename T, typename... TArgs>
         void installSegment0(int code, TArgs&&... args)
         {
-            installSegment(mSegment0, code, std::make_unique<T>(std::forward<TArgs>(args)...));
+            installSegment<T>(mSegment0, "0", code, std::forward<TArgs>(args)...);
         }
 
         template <typename T, typename... TArgs>
         void installSegment2(int code, TArgs&&... args)
         {
-            installSegment(mSegment2, code, std::make_unique<T>(std::forward<TArgs>(args)...));
+            installSegment<T>(mSegment2, "2", code, std::forward<TArgs>(args)...);
         }
 
         template <typename T, typename... TArgs>
         void installSegment3(int code, TArgs&&... args)
         {
-            installSegment(mSegment3, code, std::make_unique<T>(std::forward<TArgs>(args)...));
+            installSegment<T>(mSegment3, "3", code, std::forward<TArgs>(args)...);
         }
 
         template <typename T, typename... TArgs>
         void installSegment5(int code, TArgs&&... args)
         {
-            installSegment(mSegment5, code, std::make_unique<T>(std::forward<TArgs>(args)...));
+            installSegment<T>(mSegment5, "5", code, std::forward<TArgs>(args)...);
         }
 
         void run(const Program& program, Context& context);