From 3b2d9161c4189bb3dac9ad07f5a396269f4caadd Mon Sep 17 00:00:00 2001
From: "florent.teppe" <teppe.florent@hotmail.fr>
Date: Mon, 2 Jan 2023 19:20:03 +0100
Subject: [PATCH] Applied review advice

better deals with templated functions meant for esm3 used by esm4s.
simplified loading of esm4 and dealing with unhandled record types.
---
 apps/esmtool/tes4.cpp             |  3 +--
 apps/openmw/mwworld/esmloader.cpp |  7 ++-----
 apps/openmw/mwworld/esmstore.cpp  | 11 ++++-------
 apps/openmw/mwworld/store.cpp     |  8 ++++----
 components/esm4/loadcell.cpp      |  2 +-
 5 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/apps/esmtool/tes4.cpp b/apps/esmtool/tes4.cpp
index 50c7d24b7c..78e5931c0e 100644
--- a/apps/esmtool/tes4.cpp
+++ b/apps/esmtool/tes4.cpp
@@ -184,7 +184,7 @@ namespace EsmTool
             if constexpr (hasFormId<T>)
                 std::cout << "\n  FormId: " << value.mFormId;
             if constexpr (hasRefId<T>)
-                std::cout << "\n  RefId: " << value.mId;
+                std::cout << "\n  FormId: " << value.mId;
             if constexpr (hasFlags<T>)
                 std::cout << "\n  Record flags: " << recordFlags(value.mFlags);
             if constexpr (hasEditorId<T>)
@@ -196,7 +196,6 @@ namespace EsmTool
             if constexpr (hasKf<T>)
                 std::cout << "\n  Kf:" << WriteArray("\n  - ", value.mKf);
             std::cout << '\n';
-            return;
         }
 
         bool readRecord(const Params& params, ESM4::Reader& reader)
diff --git a/apps/openmw/mwworld/esmloader.cpp b/apps/openmw/mwworld/esmloader.cpp
index c529a9834c..be2b3abda9 100644
--- a/apps/openmw/mwworld/esmloader.cpp
+++ b/apps/openmw/mwworld/esmloader.cpp
@@ -28,10 +28,6 @@ namespace MWWorld
     {
 
         auto stream = Files::openBinaryInputFileStream(filepath);
-        if (!stream->is_open())
-        {
-            throw std::runtime_error(std::string("File Failed to open file: ") + std::strerror(errno) + "\n");
-        }
         const ESM::Format format = ESM::readFormat(*stream);
         stream->seekg(0);
 
@@ -64,13 +60,14 @@ namespace MWWorld
             }
             case ESM::Format::Tes4:
             {
-                if (mEncoder)
+                if (mEncoder != nullptr)
                 {
                     ESM4::Reader readerESM4(std::move(stream), filepath);
                     auto statelessEncoder = mEncoder->getStatelessEncoder();
                     readerESM4.setEncoder(&statelessEncoder);
                     mStore.loadESM4(readerESM4, listener, mDialogue);
                 }
+                break;
             }
         }
     }
diff --git a/apps/openmw/mwworld/esmstore.cpp b/apps/openmw/mwworld/esmstore.cpp
index 31c788283d..6f39a3e602 100644
--- a/apps/openmw/mwworld/esmstore.cpp
+++ b/apps/openmw/mwworld/esmstore.cpp
@@ -197,7 +197,7 @@ namespace MWWorld
         };
 
         template <typename T>
-        static void typedReadRecordESM4(ESM4::Reader& reader, ESMStore& stores, Store<T>& store, int& found)
+        static bool typedReadRecordESM4(ESM4::Reader& reader, Store<T>& store)
         {
             auto recordType = static_cast<ESM4::RecordTypes>(reader.hdr().record.typeId);
 
@@ -212,20 +212,17 @@ namespace MWWorld
                         T value;
                         value.load(reader);
                         store.insertStatic(value);
-                        found++;
+                        return true;
                     }
                 }
             }
+            return false;
         }
 
         static bool readRecord(ESM4::Reader& reader, ESMStore& store)
         {
-            int found = 0;
-            std::apply([&reader, &store, &found](
-                           auto&... x) { (ESMStoreImp::typedReadRecordESM4(reader, store, x, found), ...); },
+            return std::apply([&reader](auto&... x) { return (ESMStoreImp::typedReadRecordESM4(reader, x) || ...); },
                 store.mStoreImp->mStores);
-            assert(found <= 1);
-            return found;
         }
     };
 
diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp
index 71161fa6a4..60179f01b5 100644
--- a/apps/openmw/mwworld/store.cpp
+++ b/apps/openmw/mwworld/store.cpp
@@ -164,8 +164,9 @@ namespace MWWorld
             std::stringstream msg;
             if constexpr (!ESM::isESM4Rec(T::sRecordId))
             {
-                msg << T::getRecordType() << " '" << id << "' not found";
+                msg << T::getRecordType();
             }
+            msg << " '" << id << "' not found";
             throw std::runtime_error(msg.str());
         }
         return ptr;
@@ -298,13 +299,12 @@ namespace MWWorld
     {
         for (typename Dynamic::const_iterator iter(mDynamic.begin()); iter != mDynamic.end(); ++iter)
         {
-            writer.startRecord(T::sRecordId);
             if constexpr (!ESM::isESM4Rec(T::sRecordId))
             {
+                writer.startRecord(T::sRecordId);
                 iter->second.save(writer);
+                writer.endRecord(T::sRecordId);
             }
-
-            writer.endRecord(T::sRecordId);
         }
     }
     template <typename T>
diff --git a/components/esm4/loadcell.cpp b/components/esm4/loadcell.cpp
index adf8a13180..0b829d657f 100644
--- a/components/esm4/loadcell.cpp
+++ b/components/esm4/loadcell.cpp
@@ -36,9 +36,9 @@
 #include <stdexcept>
 
 #include "reader.hpp"
+// #include "writer.hpp"
 
 #include <components/esm/refid.hpp>
-// #include "writer.hpp"
 
 // TODO: Try loading only EDID and XCLC (along with mFormId, mFlags and mParent)
 //