Resolve merge conflicts from !4938 and !4956

.gitlab-ci.yml

@@ -344,7 +344,7 @@ Ubuntu_GCC_tests_coverage:
     PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip"
   before_script:
     - CI/install_debian_deps.sh gcc openmw-deps openmw-deps-dynamic openmw-coverage
-    - pipx install gcovr
+    - pipx install gcovr==8.3
   coverage: /^\s*lines:\s*\d+.\d+\%/
   artifacts:
     paths: []

CHANGELOG.md

@@ -23,6 +23,7 @@
     Bug #7979: Paralyzed NPCs battlecry
     Bug #7996: Instant magic effects are not always instant
     Bug #8012: Startcombat and Stopcombat do not affect music in the menu mode
+    Bug #8176: Error marker cloning is not thread-safe
     Bug #8245: The console command ShowVars does not list global mwscripts
     Bug #8265: Topics are linked incorrectly
     Bug #8303: On target spells cast by non-actors should fire underwater
@@ -76,6 +77,10 @@
     Bug #8680: Dead ancestor ghosts stop being dust when you rest near them
     Bug #8686: openmw-cs: Crash when smoothing terrain of a not-yet-created cell.
     Bug #8710: Absorb Skill breaks on creatures
+    Bug #8720: Crash due to a malformed Lua UI element
+    Bug #8734: Shield sheathing does not work properly
+    Bug #8757: Crash in Lua coroutine when accessing player
+    Bug #8758: Lua UI crash on creating nested UI elements twice without destroying old parent element
     Feature #2522: Support quick item transfer
     Feature #3740: Gamepad GUI Mode
     Feature #3769: Allow GetSpellEffects on enchantments

CMakeLists.txt

@@ -82,7 +82,7 @@ message(STATUS "Configuring OpenMW...")
 set(OPENMW_VERSION_MAJOR 0)
 set(OPENMW_VERSION_MINOR 51)
 set(OPENMW_VERSION_RELEASE 0)
-set(OPENMW_LUA_API_REVISION 96)
+set(OPENMW_LUA_API_REVISION 99)
 set(OPENMW_POSTPROCESSING_API_REVISION 3)
 
 set(OPENMW_VERSION_COMMITHASH "")
@@ -452,7 +452,7 @@ if(HAVE_MULTIVIEW)
     add_definitions(-DOSG_HAS_MULTIVIEW)
 endif(HAVE_MULTIVIEW)
 
-set(BOOST_COMPONENTS iostreams program_options system)
+set(BOOST_COMPONENTS iostreams program_options)
 
 find_package(Boost 1.70.0 CONFIG REQUIRED COMPONENTS ${BOOST_COMPONENTS} OPTIONAL_COMPONENTS ${BOOST_OPTIONAL_COMPONENTS})
 

apps/bsatool/bsatool.cpp

@@ -170,8 +170,8 @@ int list(std::unique_ptr<File>& bsa, Arguments& info)
             // Long format
             std::ios::fmtflags f(std::cout.flags());
             std::cout << std::setw(50) << std::left << file.name();
-            std::cout << std::setw(8) << std::left << std::dec << file.fileSize;
-            std::cout << "@ 0x" << std::hex << file.offset << std::endl;
+            std::cout << std::setw(8) << std::left << std::dec << file.mFileSize;
+            std::cout << "@ 0x" << std::hex << file.mOffset << std::endl;
             std::cout.flags(f);
         }
         else

apps/components_tests/CMakeLists.txt

@@ -89,6 +89,9 @@ file(GLOB UNITTEST_SRC_FILES
     vfs/testpathutil.cpp
 
     sceneutil/osgacontroller.cpp
+
+    bsa/testbsafile.cpp
+    bsa/testcompressedbsafile.cpp
 )
 
 source_group(apps\\components-tests FILES ${UNITTEST_SRC_FILES})

apps/components_tests/bsa/operators.hpp

@@ -0,0 +1,40 @@
+#ifndef COMPONETS_TESTS_BSA_OPERATORS_H
+#define COMPONETS_TESTS_BSA_OPERATORS_H
+
+#include <components/bsa/bsafile.hpp>
+
+#include <ostream>
+#include <tuple>
+
+namespace Bsa
+{
+    inline auto makeTuple(const BSAFile::Hash& value)
+    {
+        return std::make_tuple(value.mLow, value.mHigh);
+    }
+
+    inline auto makeTuple(const BSAFile::FileStruct& value)
+    {
+        return std::make_tuple(
+            value.mFileSize, value.mOffset, makeTuple(value.mHash), value.mNameOffset, value.mNameSize, value.name());
+    }
+
+    inline std::ostream& operator<<(std::ostream& stream, const BSAFile::Hash& value)
+    {
+        return stream << "Hash { .mLow = " << value.mLow << ", .mHigh = " << value.mHigh << "}";
+    }
+
+    inline std::ostream& operator<<(std::ostream& stream, const BSAFile::FileStruct& value)
+    {
+        return stream << "FileStruct { .mFileSize = " << value.mFileSize << ", .mOffset = " << value.mOffset
+                      << ", .mHash = " << value.mHash << ", .mNameOffset = " << value.mNameOffset
+                      << ", .mNameSize = " << value.mNameSize << ", .name() = " << value.name() << "}";
+    }
+
+    inline bool operator==(const BSAFile::FileStruct& lhs, const BSAFile::FileStruct& rhs)
+    {
+        return makeTuple(lhs) == makeTuple(rhs);
+    }
+}
+
+#endif

apps/components_tests/bsa/testbsafile.cpp

@@ -0,0 +1,356 @@
+#include "operators.hpp"
+
+#include <components/bsa/compressedbsafile.hpp>
+#include <components/files/memorystream.hpp>
+#include <components/testing/util.hpp>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <cstdint>
+#include <cstdlib>
+#include <cstring>
+#include <filesystem>
+#include <format>
+#include <fstream>
+#include <memory>
+#include <sstream>
+#include <vector>
+
+namespace Bsa
+{
+    namespace
+    {
+        using namespace ::testing;
+
+        struct Free
+        {
+            void operator()(void* ptr) const { std::free(ptr); }
+        };
+
+        struct Buffer
+        {
+            std::unique_ptr<char, Free> mData;
+            std::size_t mCapacity;
+        };
+
+        struct Header
+        {
+            uint32_t mFormat;
+            uint32_t mDirSize;
+            uint32_t mFileCount;
+        };
+
+        struct Archive
+        {
+            Header mHeader;
+            std::vector<std::uint32_t> mOffsets;
+            std::vector<char> mStringBuffer;
+            std::vector<BSAFile::Hash> mHashes;
+            std::size_t mTailSize;
+        };
+
+        struct TestBSAFile final : public BSAFile
+        {
+            void readHeader(std::istream& input) override { BSAFile::readHeader(input); }
+
+            void writeHeader() override { throw std::logic_error("TestBSAFile::writeHeader is not implemented"); }
+        };
+
+        void writeArchive(const Archive& value, std::ostream& stream)
+        {
+            stream.write(reinterpret_cast<const char*>(&value.mHeader), sizeof(value.mHeader));
+
+            if (!value.mOffsets.empty())
+                stream.write(reinterpret_cast<const char*>(value.mOffsets.data()),
+                    value.mOffsets.size() * sizeof(std::uint32_t));
+
+            if (!value.mStringBuffer.empty())
+                stream.write(reinterpret_cast<const char*>(value.mStringBuffer.data()), value.mStringBuffer.size());
+
+            for (const BSAFile::Hash& hash : value.mHashes)
+                stream.write(reinterpret_cast<const char*>(&hash), sizeof(BSAFile::Hash));
+
+            const std::size_t chunkSize = 4096;
+            std::vector<char> chunk(chunkSize);
+            for (std::size_t i = 0; i < value.mTailSize; i += chunkSize)
+                stream.write(reinterpret_cast<const char*>(chunk.data()), std::min(chunk.size(), value.mTailSize - i));
+        }
+
+        std::filesystem::path makeOutputPath()
+        {
+            const auto testInfo = UnitTest::GetInstance()->current_test_info();
+            return TestingOpenMW::outputFilePath(
+                std::format("{}.{}.bsa", testInfo->test_suite_name(), testInfo->name()));
+        }
+
+        Buffer makeBsaBuffer(std::uint32_t fileSize, std::uint32_t fileOffset)
+        {
+            std::ostringstream stream;
+
+            const Header header{
+                .mFormat = static_cast<std::uint32_t>(BsaVersion::Uncompressed),
+                .mDirSize = 14,
+                .mFileCount = 1,
+            };
+
+            const BSAFile::Hash hash{
+                .mLow = 0xaaaabbbb,
+                .mHigh = 0xccccdddd,
+            };
+
+            const Archive archive{
+                .mHeader = header,
+                .mOffsets = { fileSize, fileOffset, 0 },
+                .mStringBuffer = { 'a', '\0' },
+                .mHashes = { hash },
+                .mTailSize = 0,
+            };
+
+            writeArchive(archive, stream);
+
+            const std::string data = std::move(stream).str();
+
+            const std::size_t capacity = static_cast<std::size_t>(fileSize) + static_cast<std::size_t>(fileOffset) + 34;
+            std::unique_ptr<char, Free> buffer(reinterpret_cast<char*>(std::malloc(capacity)));
+
+            if (buffer == nullptr)
+                throw std::bad_alloc();
+
+            std::memcpy(buffer.get(), data.data(), data.size());
+
+            return Buffer{ .mData = std::move(buffer), .mCapacity = capacity };
+        }
+
+        TEST(BSAFileTest, shouldHandleEmpty)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+                stream.open(path, std::ios::binary);
+            }
+
+            BSAFile file;
+            EXPECT_THROW(file.open(path), std::runtime_error);
+
+            EXPECT_THAT(file.getList(), IsEmpty());
+        }
+
+        TEST(BSAFileTest, shouldHandleZeroFiles)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+
+                stream.open(path, std::ios::binary);
+
+                const Header header{
+                    .mFormat = static_cast<std::uint32_t>(BsaVersion::Uncompressed),
+                    .mDirSize = 0,
+                    .mFileCount = 0,
+                };
+
+                const Archive archive{
+                    .mHeader = header,
+                    .mOffsets = {},
+                    .mStringBuffer = {},
+                    .mHashes = {},
+                    .mTailSize = 0,
+                };
+
+                writeArchive(archive, stream);
+            }
+
+            BSAFile file;
+            file.open(path);
+
+            EXPECT_THAT(file.getList(), IsEmpty());
+        }
+
+        TEST(BSAFileTest, shouldHandleSingleFile)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+
+                stream.open(path, std::ios::binary);
+
+                const Header header{
+                    .mFormat = static_cast<std::uint32_t>(BsaVersion::Uncompressed),
+                    .mDirSize = 14,
+                    .mFileCount = 1,
+                };
+
+                const BSAFile::Hash hash{
+                    .mLow = 0xaaaabbbb,
+                    .mHigh = 0xccccdddd,
+                };
+
+                const Archive archive{
+                    .mHeader = header,
+                    .mOffsets = { 42, 0, 0 },
+                    .mStringBuffer = { 'a', '\0' },
+                    .mHashes = { hash },
+                    .mTailSize = 42,
+                };
+
+                writeArchive(archive, stream);
+            }
+
+            BSAFile file;
+            file.open(path);
+
+            std::vector<char> namesBuffer = { 'a', '\0' };
+
+            EXPECT_THAT(file.getList(),
+                ElementsAre(BSAFile::FileStruct{
+                    .mFileSize = 42,
+                    .mOffset = 34,
+                    .mHash = BSAFile::Hash{ .mLow = 0xaaaabbbb, .mHigh = 0xccccdddd },
+                    .mNameOffset = 0,
+                    .mNameSize = 1,
+                    .mNamesBuffer = &namesBuffer,
+                }));
+        }
+
+        TEST(BSAFileTest, shouldHandleTwoFiles)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+
+                stream.open(path, std::ios::binary);
+
+                const std::uint32_t fileSize1 = 42;
+                const std::uint32_t fileSize2 = 13;
+
+                const Header header{
+                    .mFormat = static_cast<std::uint32_t>(BsaVersion::Uncompressed),
+                    .mDirSize = 28,
+                    .mFileCount = 2,
+                };
+
+                const BSAFile::Hash hash1{
+                    .mLow = 0xaaaabbbb,
+                    .mHigh = 0xccccdddd,
+                };
+
+                const BSAFile::Hash hash2{
+                    .mLow = 0x11112222,
+                    .mHigh = 0x33334444,
+                };
+
+                const Archive archive{
+                    .mHeader = header,
+                    .mOffsets = { fileSize1, 0, fileSize2, fileSize1, 0, 2 },
+                    .mStringBuffer = { 'a', '\0', 'b', '\0' },
+                    .mHashes = { hash1, hash2 },
+                    .mTailSize = fileSize1 + fileSize2,
+                };
+
+                writeArchive(archive, stream);
+            }
+
+            BSAFile file;
+            file.open(path);
+
+            std::vector<char> namesBuffer = { 'a', '\0', 'b', '\0' };
+
+            EXPECT_THAT(file.getList(),
+                ElementsAre(
+                    BSAFile::FileStruct{
+                        .mFileSize = 42,
+                        .mOffset = 56,
+                        .mHash = BSAFile::Hash{ .mLow = 0xaaaabbbb, .mHigh = 0xccccdddd },
+                        .mNameOffset = 0,
+                        .mNameSize = 1,
+                        .mNamesBuffer = &namesBuffer,
+                    },
+                    BSAFile::FileStruct{
+                        .mFileSize = 13,
+                        .mOffset = 98,
+                        .mHash = BSAFile::Hash{ .mLow = 0x11112222, .mHigh = 0x33334444 },
+                        .mNameOffset = 2,
+                        .mNameSize = 1,
+                        .mNamesBuffer = &namesBuffer,
+                    }));
+        }
+
+        TEST(BSAFileTest, shouldHandleSomewhatLargeFiles)
+        {
+            constexpr std::uint32_t maxUInt32 = std::numeric_limits<uint32_t>::max();
+            constexpr std::uint32_t fileSize = maxUInt32 / 4;
+            constexpr std::uint32_t fileOffset = maxUInt32 / 4 - 34;
+            const Buffer buffer = makeBsaBuffer(fileSize, fileOffset);
+
+            TestBSAFile file;
+            // Use capacity assuming we never read beyond small header.
+            Files::IMemStream stream(buffer.mData.get(), buffer.mCapacity);
+            file.readHeader(stream);
+
+            std::vector<char> namesBuffer = { 'a', '\0' };
+
+            EXPECT_THAT(file.getList(),
+                ElementsAre(BSAFile::FileStruct{
+                    .mFileSize = maxUInt32 / 4,
+                    .mOffset = maxUInt32 / 4,
+                    .mHash = BSAFile::Hash{ .mLow = 0xaaaabbbb, .mHigh = 0xccccdddd },
+                    .mNameOffset = 0,
+                    .mNameSize = 1,
+                    .mNamesBuffer = &namesBuffer,
+                }));
+        }
+
+// std::streambuf in MSVC does not support buffers larger than 2**31 - 1:
+// https://developercommunity.visualstudio.com/t/stdbasic-stringbuf-is-broken/290124
+#ifndef _MSC_VER
+        TEST(BSAFileTest, shouldHandleSingleFileAtTheEndOfLargeFile)
+        {
+            constexpr std::uint32_t maxUInt32 = std::numeric_limits<uint32_t>::max();
+            constexpr std::uint32_t fileSize = maxUInt32;
+            constexpr std::uint32_t fileOffset = maxUInt32 - 34;
+            const Buffer buffer = makeBsaBuffer(fileSize, fileOffset);
+
+            TestBSAFile file;
+            // Use capacity assuming we never read beyond small header.
+            Files::IMemStream stream(buffer.mData.get(), buffer.mCapacity);
+            file.readHeader(stream);
+
+            std::vector<char> namesBuffer = { 'a', '\0' };
+
+            EXPECT_THAT(file.getList(),
+                ElementsAre(BSAFile::FileStruct{
+                    .mFileSize = maxUInt32,
+                    .mOffset = maxUInt32,
+                    .mHash = BSAFile::Hash{ .mLow = 0xaaaabbbb, .mHigh = 0xccccdddd },
+                    .mNameOffset = 0,
+                    .mNameSize = 1,
+                    .mNamesBuffer = &namesBuffer,
+                }));
+        }
+
+        TEST(BSAFileTest, shouldThrowExceptionOnTooBigAbsoluteOffset)
+        {
+            constexpr std::uint32_t maxUInt32 = std::numeric_limits<uint32_t>::max();
+            constexpr std::uint32_t fileSize = maxUInt32;
+            constexpr std::uint32_t fileOffset = maxUInt32 - 34 + 1;
+            const Buffer buffer = makeBsaBuffer(fileSize, fileOffset);
+
+            TestBSAFile file;
+            // Use capacity assuming we never read beyond small header.
+            Files::IMemStream stream(buffer.mData.get(), buffer.mCapacity);
+            EXPECT_THROW(file.readHeader(stream), std::runtime_error);
+
+            EXPECT_THAT(file.getList(), IsEmpty());
+        }
+#endif
+    }
+}

apps/components_tests/bsa/testcompressedbsafile.cpp

@@ -0,0 +1,360 @@
+#include "operators.hpp"
+
+#include <components/bsa/compressedbsafile.hpp>
+#include <components/testing/util.hpp>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <cstdint>
+#include <filesystem>
+#include <format>
+#include <fstream>
+#include <string>
+
+namespace Bsa
+{
+    namespace
+    {
+        using namespace ::testing;
+
+        struct FileRecord
+        {
+            std::uint64_t mHash;
+            std::uint32_t mSize;
+            std::uint32_t mOffset;
+            std::string mName;
+        };
+
+        struct NonSSEFolderRecord
+        {
+            std::uint64_t mHash;
+            std::uint32_t mCount;
+            std::int32_t mOffset;
+            std::string mName;
+            std::vector<FileRecord> mFiles;
+        };
+
+        struct Archive
+        {
+            CompressedBSAFile::Header mHeader;
+            std::vector<NonSSEFolderRecord> mFolders;
+        };
+
+        void writeArchive(const Archive& value, std::ostream& stream)
+        {
+            stream.write(reinterpret_cast<const char*>(&value.mHeader), sizeof(value.mHeader));
+
+            for (const NonSSEFolderRecord& folder : value.mFolders)
+            {
+                stream.write(reinterpret_cast<const char*>(&folder.mHash), sizeof(folder.mHash));
+                stream.write(reinterpret_cast<const char*>(&folder.mCount), sizeof(folder.mCount));
+                stream.write(reinterpret_cast<const char*>(&folder.mOffset), sizeof(folder.mOffset));
+            }
+
+            for (const NonSSEFolderRecord& folder : value.mFolders)
+            {
+                const std::uint8_t folderNameSize = static_cast<std::uint8_t>(folder.mName.size() + 1);
+
+                stream.write(reinterpret_cast<const char*>(&folderNameSize), sizeof(folderNameSize));
+                stream.write(reinterpret_cast<const char*>(folder.mName.data()), folder.mName.size());
+                stream.put('\0');
+
+                for (const FileRecord& file : folder.mFiles)
+                {
+                    stream.write(reinterpret_cast<const char*>(&file.mHash), sizeof(file.mHash));
+                    stream.write(reinterpret_cast<const char*>(&file.mSize), sizeof(file.mSize));
+                    stream.write(reinterpret_cast<const char*>(&file.mOffset), sizeof(file.mOffset));
+                }
+            }
+
+            for (const NonSSEFolderRecord& folder : value.mFolders)
+            {
+                for (const FileRecord& file : folder.mFiles)
+                {
+                    stream.write(reinterpret_cast<const char*>(file.mName.data()), file.mName.size());
+                    stream.put('\0');
+                }
+            }
+        }
+
+        std::filesystem::path makeOutputPath()
+        {
+            const auto testInfo = UnitTest::GetInstance()->current_test_info();
+            return TestingOpenMW::outputFilePath(
+                std::format("{}.{}.bsa", testInfo->test_suite_name(), testInfo->name()));
+        }
+
+        TEST(CompressedBSAFileTest, shouldHandleEmpty)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+                stream.open(path, std::ios::binary);
+            }
+
+            CompressedBSAFile file;
+            EXPECT_THROW(file.open(path), std::runtime_error);
+
+            EXPECT_THAT(file.getList(), IsEmpty());
+        }
+
+        TEST(CompressedBSAFileTest, shouldHandleSingleFile)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+
+                stream.open(path, std::ios::binary);
+
+                const CompressedBSAFile::Header header{
+                    .mFormat = static_cast<std::uint32_t>(BsaVersion::Compressed),
+                    .mVersion = CompressedBSAFile::Version_TES4,
+                    .mFoldersOffset = sizeof(CompressedBSAFile::Header),
+                    .mFlags = CompressedBSAFile::ArchiveFlag_FolderNames | CompressedBSAFile::ArchiveFlag_FileNames,
+                    .mFolderCount = 1,
+                    .mFileCount = 1,
+                    .mFolderNamesLength = 7,
+                    .mFileNamesLength = 9,
+                    .mFileFlags = 0,
+                };
+
+                const FileRecord file{
+                    .mHash = 0xfedcba9876543210,
+                    .mSize = 42,
+                    .mOffset = 0,
+                    .mName = "filename",
+                };
+
+                const NonSSEFolderRecord folder{
+                    .mHash = 0xfedcba9876543210,
+                    .mCount = 1,
+                    .mOffset = 0,
+                    .mName = "folder",
+                    .mFiles = { file },
+                };
+
+                const Archive archive{
+                    .mHeader = header,
+                    .mFolders = { folder },
+                };
+
+                writeArchive(archive, stream);
+            }
+
+            CompressedBSAFile file;
+            file.open(path);
+
+            std::vector<char> namesBuffer;
+            constexpr std::string_view filePath = "folder\\filename";
+            namesBuffer.assign(filePath.begin(), filePath.end());
+            namesBuffer.push_back('\0');
+
+            EXPECT_THAT(file.getList(),
+                ElementsAre(BSAFile::FileStruct{
+                    .mFileSize = 42,
+                    .mOffset = 0,
+                    .mHash = BSAFile::Hash{ .mLow = 0, .mHigh = 0 },
+                    .mNameOffset = 0,
+                    .mNameSize = 15,
+                    .mNamesBuffer = &namesBuffer,
+                }));
+        }
+
+        TEST(CompressedBSAFileTest, shouldHandleEmptyFileName)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+
+                stream.open(path, std::ios::binary);
+
+                const CompressedBSAFile::Header header{
+                    .mFormat = static_cast<std::uint32_t>(BsaVersion::Compressed),
+                    .mVersion = CompressedBSAFile::Version_TES4,
+                    .mFoldersOffset = sizeof(CompressedBSAFile::Header),
+                    .mFlags = CompressedBSAFile::ArchiveFlag_FolderNames | CompressedBSAFile::ArchiveFlag_FileNames,
+                    .mFolderCount = 1,
+                    .mFileCount = 1,
+                    .mFolderNamesLength = 7,
+                    .mFileNamesLength = 1,
+                    .mFileFlags = 0,
+                };
+
+                const FileRecord file{
+                    .mHash = 0xfedcba9876543210,
+                    .mSize = 42,
+                    .mOffset = 0,
+                    .mName = "",
+                };
+
+                const NonSSEFolderRecord folder{
+                    .mHash = 0xfedcba9876543210,
+                    .mCount = 1,
+                    .mOffset = 0,
+                    .mName = "folder",
+                    .mFiles = { file },
+                };
+
+                const Archive archive{
+                    .mHeader = header,
+                    .mFolders = { folder },
+                };
+
+                writeArchive(archive, stream);
+            }
+
+            CompressedBSAFile file;
+            EXPECT_THROW(file.open(path), std::runtime_error);
+        }
+
+        TEST(CompressedBSAFileTest, shouldHandleFoldersWithDuplicateHash)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+
+                stream.open(path, std::ios::binary);
+
+                const CompressedBSAFile::Header header{
+                    .mFormat = static_cast<std::uint32_t>(BsaVersion::Compressed),
+                    .mVersion = CompressedBSAFile::Version_TES4,
+                    .mFoldersOffset = sizeof(CompressedBSAFile::Header),
+                    .mFlags = CompressedBSAFile::ArchiveFlag_FolderNames | CompressedBSAFile::ArchiveFlag_FileNames,
+                    .mFolderCount = 2,
+                    .mFileCount = 2,
+                    .mFolderNamesLength = 16,
+                    .mFileNamesLength = 18,
+                    .mFileFlags = 0,
+                };
+
+                const FileRecord file{
+                    .mHash = 0xfedcba9876543210,
+                    .mSize = 42,
+                    .mOffset = 0,
+                    .mName = "filename",
+                };
+
+                const NonSSEFolderRecord folder1{
+                    .mHash = 0xfedcba9876543210,
+                    .mCount = 1,
+                    .mOffset = 0,
+                    .mName = "folder1",
+                    .mFiles = { file },
+                };
+
+                const NonSSEFolderRecord folder2{
+                    .mHash = 0xfedcba9876543210,
+                    .mCount = 1,
+                    .mOffset = 0,
+                    .mName = "folder2",
+                    .mFiles = { file },
+                };
+
+                const Archive archive{
+                    .mHeader = header,
+                    .mFolders = { folder1, folder2 },
+                };
+
+                writeArchive(archive, stream);
+            }
+
+            CompressedBSAFile file;
+            file.open(path);
+
+            std::vector<char> namesBuffer;
+            constexpr std::string_view filePath = "folder2\\filename";
+            namesBuffer.assign(filePath.begin(), filePath.end());
+            namesBuffer.push_back('\0');
+
+            EXPECT_THAT(file.getList(),
+                ElementsAre(BSAFile::FileStruct{
+                    .mFileSize = 42,
+                    .mOffset = 0,
+                    .mHash = BSAFile::Hash{ .mLow = 0, .mHigh = 0 },
+                    .mNameOffset = 0,
+                    .mNameSize = 16,
+                    .mNamesBuffer = &namesBuffer,
+                }));
+        }
+
+        TEST(CompressedBSAFileTest, shouldHandleFilesWithDuplicateHash)
+        {
+            const std::filesystem::path path = makeOutputPath();
+
+            {
+                std::ofstream stream;
+                stream.exceptions(std::ifstream::failbit | std::ifstream::badbit);
+
+                stream.open(path, std::ios::binary);
+
+                const CompressedBSAFile::Header header{
+                    .mFormat = static_cast<std::uint32_t>(BsaVersion::Compressed),
+                    .mVersion = CompressedBSAFile::Version_TES4,
+                    .mFoldersOffset = sizeof(CompressedBSAFile::Header),
+                    .mFlags = CompressedBSAFile::ArchiveFlag_FolderNames | CompressedBSAFile::ArchiveFlag_FileNames,
+                    .mFolderCount = 1,
+                    .mFileCount = 2,
+                    .mFolderNamesLength = 9,
+                    .mFileNamesLength = 18,
+                    .mFileFlags = 0,
+                };
+
+                const FileRecord file1{
+                    .mHash = 0xfedcba9876543210,
+                    .mSize = 42,
+                    .mOffset = 0,
+                    .mName = "filename1",
+                };
+
+                const FileRecord file2{
+                    .mHash = 0xfedcba9876543210,
+                    .mSize = 13,
+                    .mOffset = 0,
+                    .mName = "filename2",
+                };
+
+                const NonSSEFolderRecord folder{
+                    .mHash = 0xfedcba9876543210,
+                    .mCount = 2,
+                    .mOffset = 0,
+                    .mName = "folder",
+                    .mFiles = { file1, file2 },
+                };
+
+                const Archive archive{
+                    .mHeader = header,
+                    .mFolders = { folder },
+                };
+
+                writeArchive(archive, stream);
+            }
+
+            CompressedBSAFile file;
+            file.open(path);
+
+            std::vector<char> namesBuffer;
+            constexpr std::string_view filePath = "folder\\filename2";
+            namesBuffer.assign(filePath.begin(), filePath.end());
+            namesBuffer.push_back('\0');
+
+            EXPECT_THAT(file.getList(),
+                ElementsAre(BSAFile::FileStruct{
+                    .mFileSize = 13,
+                    .mOffset = 0,
+                    .mHash = BSAFile::Hash{ .mLow = 0, .mHigh = 0 },
+                    .mNameOffset = 0,
+                    .mNameSize = 16,
+                    .mNamesBuffer = &namesBuffer,
+                }));
+        }
+    }
+}

apps/opencs/model/world/nestedtableproxymodel.cpp

@@ -13,7 +13,9 @@ CSMWorld::NestedTableProxyModel::NestedTableProxyModel(
 {
     const int parentRow = parent.row();
 
-    mId = std::string(parentModel->index(parentRow, 0).data().toString().toUtf8());
+    const QByteArray utf8 = parentModel->index(parentRow, 0).data().toString().toUtf8();
+
+    mId = std::string(utf8.constData(), utf8.size());
 
     QAbstractProxyModel::setSourceModel(parentModel);
 

apps/opencs/view/world/util.cpp

@@ -362,8 +362,8 @@ void CSVWorld::CommandDelegate::setEditorData(QWidget* editor, const QModelIndex
     if (!n.isEmpty())
     {
         if (!variant.isValid())
-            variant = QVariant(editor->property(n).metaType(), (const void*)nullptr);
-        editor->setProperty(n, variant);
+            variant = QVariant(editor->property(n.constData()).metaType(), (const void*)nullptr);
+        editor->setProperty(n.constData(), variant);
     }
 }
 

apps/openmw/engine.cpp

@@ -342,10 +342,10 @@ bool OMW::Engine::frame(unsigned frameNumber, float frametime)
     mViewer->eventTraversal();
     mViewer->updateTraversal();
 
-    // update GUI by world data
+    // update focus object for GUI
     {
-        ScopedProfile<UserStatsType::WindowManager> profile(frameStart, frameNumber, *timer, *stats);
-        mWorld->updateWindowManager();
+        ScopedProfile<UserStatsType::Focus> profile(frameStart, frameNumber, *timer, *stats);
+        mWorld->updateFocusObject();
     }
 
     // if there is a separate Lua thread, it starts the update now

apps/openmw/mwbase/world.hpp

@@ -257,10 +257,10 @@ namespace MWBase
             = 0;
         ///< @param changeEvent If false, do not trigger cell change flag or detect worldspace changes
 
-        virtual MWWorld::Ptr getFacedObject() = 0;
+        virtual MWWorld::Ptr getFocusObject() = 0;
         ///< Return pointer to the object the player is looking at, if it is within activation range
 
-        virtual float getDistanceToFacedObject() = 0;
+        virtual float getDistanceToFocusObject() = 0;
 
         virtual float getMaxActivationDistance() const = 0;
 

apps/openmw/mwclass/door.cpp

@@ -124,7 +124,7 @@ namespace MWClass
 
         // make door glow if player activates it with telekinesis
         if (actor == MWMechanics::getPlayer()
-            && MWBase::Environment::get().getWorld()->getDistanceToFacedObject()
+            && MWBase::Environment::get().getWorld()->getDistanceToFocusObject()
                 > MWBase::Environment::get().getWorld()->getMaxActivationDistance())
         {
             MWRender::Animation* animation = MWBase::Environment::get().getWorld()->getAnimation(ptr);
@@ -184,7 +184,7 @@ namespace MWClass
             if (ptr.getCellRef().getTeleport())
             {
                 if (actor == MWMechanics::getPlayer()
-                    && MWBase::Environment::get().getWorld()->getDistanceToFacedObject()
+                    && MWBase::Environment::get().getWorld()->getDistanceToFocusObject()
                         > MWBase::Environment::get().getWorld()->getMaxActivationDistance())
                 {
                     // player activated teleport door with telekinesis

apps/openmw/mwgui/hud.cpp

@@ -225,7 +225,7 @@ namespace MWGui
             if (!winMgr->isConsoleMode() && (mode != GM_Container) && (mode != GM_Inventory))
                 return;
 
-            MWWorld::Ptr object = MWBase::Environment::get().getWorld()->getFacedObject();
+            MWWorld::Ptr object = MWBase::Environment::get().getWorld()->getFocusObject();
 
             if (winMgr->isConsoleMode())
                 winMgr->setConsoleSelectedObject(object);

apps/openmw/mwgui/itemview.cpp

@@ -82,7 +82,9 @@ namespace MWGui
 
         if (Settings::gui().mControllerMenus)
         {
-            mControllerFocus = std::clamp(mControllerFocus, 0, mItemCount - 1);
+            mControllerFocus = -1;
+            if (mItemCount > 0)
+                mControllerFocus = std::clamp(mControllerFocus, 0, mItemCount - 1);
             updateControllerFocus(-1, mControllerFocus);
         }
 

apps/openmw/mwlua/cellbindings.cpp

@@ -42,7 +42,10 @@
 
 #include <components/misc/convert.hpp>
 
+#include <components/translation/translation.hpp>
+
 #include "../mwbase/environment.hpp"
+#include "../mwbase/windowmanager.hpp"
 #include "../mwbase/world.hpp"
 #include "../mwworld/cellstore.hpp"
 #include "../mwworld/worldmodel.hpp"
@@ -87,12 +90,15 @@ namespace MWLua
         };
 
         cellT["name"] = sol::readonly_property([](const CellT& c) { return c.mStore->getCell()->getNameId(); });
-        cellT["id"]
-            = sol::readonly_property([](const CellT& c) { return c.mStore->getCell()->getId().serializeText(); });
-        cellT["region"] = sol::readonly_property(
-            [](const CellT& c) -> std::string { return c.mStore->getCell()->getRegion().serializeText(); });
-        cellT["worldSpaceId"] = sol::readonly_property(
-            [](const CellT& c) -> std::string { return c.mStore->getCell()->getWorldSpace().serializeText(); });
+        cellT["displayName"] = sol::readonly_property([](const CellT& c) -> std::string_view {
+            const auto& storage = MWBase::Environment::get().getWindowManager()->getTranslationDataStorage();
+            return storage.translateCellName(c.mStore->getCell()->getNameId());
+        });
+        cellT["id"] = sol::readonly_property([](const CellT& c) -> ESM::RefId { return c.mStore->getCell()->getId(); });
+        cellT["region"]
+            = sol::readonly_property([](const CellT& c) -> ESM::RefId { return c.mStore->getCell()->getRegion(); });
+        cellT["worldSpaceId"]
+            = sol::readonly_property([](const CellT& c) -> ESM::RefId { return c.mStore->getCell()->getWorldSpace(); });
         cellT["gridX"] = sol::readonly_property([](const CellT& c) { return c.mStore->getCell()->getGridX(); });
         cellT["gridY"] = sol::readonly_property([](const CellT& c) { return c.mStore->getCell()->getGridY(); });
         cellT["hasWater"] = sol::readonly_property([](const CellT& c) { return c.mStore->getCell()->hasWater(); });

apps/openmw/mwlua/objectbindings.cpp

@@ -331,10 +331,9 @@ namespace MWLua
                 return LuaUtil::Box{ bb.center(), bb._max - bb.center() };
             };
 
-            objectT["type"]
-                = sol::readonly_property([types = getTypeToPackageTable(context.sol())](const ObjectT& o) mutable {
-                      return types[getLiveCellRefType(o.ptr().mRef)];
-                  });
+            objectT["type"] = sol::readonly_property(
+                [types = getTypeToPackageTable(context.sol())](
+                    const ObjectT& o) -> sol::object { return types[getLiveCellRefType(o.ptr().mRef)]; });
 
             objectT["count"] = sol::readonly_property([](const ObjectT& o) { return o.ptr().getCellRef().getCount(); });
             objectT[sol::meta_function::equal_to] = [](const ObjectT& a, const ObjectT& b) { return a.id() == b.id(); };

apps/openmw/mwlua/regionbindings.cpp

@@ -46,7 +46,7 @@ namespace MWLua
         regionT["mapColor"] = sol::readonly_property(
             [](const ESM::Region& rec) -> Misc::Color { return Misc::Color::fromRGB(rec.mMapColor); });
         regionT["sleepList"]
-            = sol::readonly_property([](const ESM::Region& rec) { return LuaUtil::serializeRefId(rec.mSleepList); });
+            = sol::readonly_property([](const ESM::Region& rec) -> ESM::RefId { return rec.mSleepList; });
 
         regionT["weatherProbabilities"] = sol::readonly_property([lua = lua.lua_state()](const ESM::Region& rec) {
             sol::table res(lua, sol::create);

apps/openmw/mwlua/types/activator.cpp

@@ -52,8 +52,7 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Activator& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Activator& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property([](const ESM::Activator& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["mwscript"]
+            = sol::readonly_property([](const ESM::Activator& rec) -> ESM::RefId { return rec.mScript; });
     }
 }

apps/openmw/mwlua/types/actor.cpp

@@ -4,6 +4,7 @@
 
 #include <components/detournavigator/agentbounds.hpp>
 #include <components/lua/luastate.hpp>
+#include <components/misc/finitevalues.hpp>
 #include <components/settings/values.hpp>
 
 #include "apps/openmw/mwbase/environment.hpp"
@@ -396,8 +397,8 @@ namespace MWLua
             const int actorsProcessingRange = Settings::game().mActorsProcessingRange;
             const osg::Vec3f playerPos = player.getRefData().getPosition().asVec3();
 
-            const float dist = (playerPos - target.getRefData().getPosition().asVec3()).length();
-            return dist <= actorsProcessingRange;
+            const float dist = (playerPos - target.getRefData().getPosition().asVec3()).length2();
+            return dist <= (actorsProcessingRange * actorsProcessingRange);
         };
 
         actor["isDead"] = [](const Object& o) {
@@ -427,7 +428,7 @@ namespace MWLua
             {
                 for (auto& [key, value] : damageLua.value())
                 {
-                    damageCpp[key.as<std::string>()] = value.as<float>();
+                    damageCpp[key.as<std::string>()] = value.as<Misc::FiniteFloat>();
                 }
             }
             std::string sourceTypeStr = options.get_or<std::string>("sourceType", "unspecified");

apps/openmw/mwlua/types/apparatus.cpp

@@ -42,9 +42,8 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Apparatus& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Apparatus& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property([](const ESM::Apparatus& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["mwscript"]
+            = sol::readonly_property([](const ESM::Apparatus& rec) -> ESM::RefId { return rec.mScript; });
         record["icon"] = sol::readonly_property([vfs](const ESM::Apparatus& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
@@ -54,4 +53,4 @@ namespace MWLua
         record["quality"]
             = sol::readonly_property([](const ESM::Apparatus& rec) -> float { return rec.mData.mQuality; });
     }
-}
+}

apps/openmw/mwlua/types/armor.cpp

@@ -102,10 +102,8 @@ namespace MWLua
         record["icon"] = sol::readonly_property([vfs](const ESM::Armor& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
-        record["enchant"] = sol::readonly_property(
-            [](const ESM::Armor& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mEnchant); });
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Armor& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["enchant"] = sol::readonly_property([](const ESM::Armor& rec) -> ESM::RefId { return rec.mEnchant; });
+        record["mwscript"] = sol::readonly_property([](const ESM::Armor& rec) -> ESM::RefId { return rec.mScript; });
         record["weight"] = sol::readonly_property([](const ESM::Armor& rec) -> float { return rec.mData.mWeight; });
         record["value"] = sol::readonly_property([](const ESM::Armor& rec) -> int { return rec.mData.mValue; });
         record["type"] = sol::readonly_property([](const ESM::Armor& rec) -> int { return rec.mData.mType; });

apps/openmw/mwlua/types/book.cpp

@@ -106,21 +106,18 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Book& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Book& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Book& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["mwscript"] = sol::readonly_property([](const ESM::Book& rec) -> ESM::RefId { return rec.mScript; });
         record["icon"] = sol::readonly_property([vfs](const ESM::Book& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
         record["text"] = sol::readonly_property([](const ESM::Book& rec) -> std::string { return rec.mText; });
-        record["enchant"] = sol::readonly_property(
-            [](const ESM::Book& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mEnchant); });
+        record["enchant"] = sol::readonly_property([](const ESM::Book& rec) -> ESM::RefId { return rec.mEnchant; });
         record["isScroll"] = sol::readonly_property([](const ESM::Book& rec) -> bool { return rec.mData.mIsScroll; });
         record["value"] = sol::readonly_property([](const ESM::Book& rec) -> int { return rec.mData.mValue; });
         record["weight"] = sol::readonly_property([](const ESM::Book& rec) -> float { return rec.mData.mWeight; });
         record["enchantCapacity"]
             = sol::readonly_property([](const ESM::Book& rec) -> float { return rec.mData.mEnchant * 0.1f; });
-        record["skill"] = sol::readonly_property([](const ESM::Book& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(ESM::Skill::indexToRefId(rec.mData.mSkillId));
-        });
+        record["skill"] = sol::readonly_property(
+            [](const ESM::Book& rec) -> ESM::RefId { return ESM::Skill::indexToRefId(rec.mData.mSkillId); });
     }
-}
+}

apps/openmw/mwlua/types/clothing.cpp

@@ -97,12 +97,8 @@ namespace MWLua
         record["icon"] = sol::readonly_property([vfs](const ESM::Clothing& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
-        record["enchant"] = sol::readonly_property([](const ESM::Clothing& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mEnchant);
-        });
-        record["mwscript"] = sol::readonly_property([](const ESM::Clothing& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["enchant"] = sol::readonly_property([](const ESM::Clothing& rec) -> ESM::RefId { return rec.mEnchant; });
+        record["mwscript"] = sol::readonly_property([](const ESM::Clothing& rec) -> ESM::RefId { return rec.mScript; });
         record["weight"] = sol::readonly_property([](const ESM::Clothing& rec) -> float { return rec.mData.mWeight; });
         record["value"] = sol::readonly_property([](const ESM::Clothing& rec) -> int { return rec.mData.mValue; });
         record["type"] = sol::readonly_property([](const ESM::Clothing& rec) -> int { return rec.mData.mType; });

apps/openmw/mwlua/types/container.cpp

@@ -52,9 +52,8 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Container& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Container& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property([](const ESM::Container& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["mwscript"]
+            = sol::readonly_property([](const ESM::Container& rec) -> ESM::RefId { return rec.mScript; });
         record["weight"] = sol::readonly_property([](const ESM::Container& rec) -> float { return rec.mWeight; });
         record["isOrganic"] = sol::readonly_property(
             [](const ESM::Container& rec) -> bool { return rec.mFlags & ESM::Container::Organic; });

apps/openmw/mwlua/types/creature.cpp

@@ -40,9 +40,7 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Creature& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Creature& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property([](const ESM::Creature& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["mwscript"] = sol::readonly_property([](const ESM::Creature& rec) -> ESM::RefId { return rec.mScript; });
         record["baseCreature"] = sol::readonly_property(
             [](const ESM::Creature& rec) -> std::string { return rec.mOriginal.serializeText(); });
         record["soulValue"] = sol::readonly_property([](const ESM::Creature& rec) -> int { return rec.mData.mSoul; });
@@ -78,4 +76,4 @@ namespace MWLua
 
         addActorServicesBindings<ESM::Creature>(record, context);
     }
-}
+}

apps/openmw/mwlua/types/door.cpp

@@ -30,7 +30,6 @@ namespace sol
 
 namespace MWLua
 {
-
     static const MWWorld::Ptr& doorPtr(const Object& o)
     {
         return verifyType(ESM::REC_DOOR, o.ptr());
@@ -110,8 +109,7 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Door& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Door& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Door& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["mwscript"] = sol::readonly_property([](const ESM::Door& rec) -> ESM::RefId { return rec.mScript; });
         record["openSound"] = sol::readonly_property(
             [](const ESM::Door& rec) -> std::string { return rec.mOpenSound.serializeText(); });
         record["closeSound"] = sol::readonly_property(
@@ -154,4 +152,4 @@ namespace MWLua
         record["closeSound"] = sol::readonly_property(
             [](const ESM4::Door& rec) -> std::string { return ESM::RefId(rec.mCloseSound).serializeText(); });
     }
-}
+}

apps/openmw/mwlua/types/ingredient.cpp

@@ -34,9 +34,8 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Ingredient& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Ingredient& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property([](const ESM::Ingredient& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["mwscript"]
+            = sol::readonly_property([](const ESM::Ingredient& rec) -> ESM::RefId { return rec.mScript; });
         record["icon"] = sol::readonly_property([vfs](const ESM::Ingredient& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
@@ -64,4 +63,4 @@ namespace MWLua
             return res;
         });
     }
-}
+}

apps/openmw/mwlua/types/light.cpp

@@ -105,8 +105,7 @@ namespace MWLua
         });
         record["sound"]
             = sol::readonly_property([](const ESM::Light& rec) -> std::string { return rec.mSound.serializeText(); });
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Light& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["mwscript"] = sol::readonly_property([](const ESM::Light& rec) -> ESM::RefId { return rec.mScript; });
         record["weight"] = sol::readonly_property([](const ESM::Light& rec) -> float { return rec.mData.mWeight; });
         record["value"] = sol::readonly_property([](const ESM::Light& rec) -> int { return rec.mData.mValue; });
         record["duration"] = sol::readonly_property([](const ESM::Light& rec) -> int { return rec.mData.mTime; });

apps/openmw/mwlua/types/lockpick.cpp

@@ -33,9 +33,7 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Lockpick& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Lockpick& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property([](const ESM::Lockpick& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["mwscript"] = sol::readonly_property([](const ESM::Lockpick& rec) -> ESM::RefId { return rec.mScript; });
         record["icon"] = sol::readonly_property([vfs](const ESM::Lockpick& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
@@ -46,4 +44,4 @@ namespace MWLua
         record["quality"]
             = sol::readonly_property([](const ESM::Lockpick& rec) -> float { return rec.mData.mQuality; });
     }
-}
+}

apps/openmw/mwlua/types/misc.cpp

@@ -71,10 +71,8 @@ namespace MWLua
 
             object.ptr().getCellRef().setSoul(creature);
         };
-        miscellaneous["getSoul"] = [](const Object& object) -> sol::optional<std::string> {
-            ESM::RefId soul = object.ptr().getCellRef().getSoul();
-            return LuaUtil::serializeRefId(soul);
-        };
+        miscellaneous["getSoul"]
+            = [](const Object& object) -> ESM::RefId { return object.ptr().getCellRef().getSoul(); };
         miscellaneous["soul"] = miscellaneous["getSoul"]; // for compatibility; should be removed later
 
         sol::usertype<ESM::Miscellaneous> record = context.sol().new_usertype<ESM::Miscellaneous>("ESM3_Miscellaneous");
@@ -84,9 +82,8 @@ namespace MWLua
             [](const ESM::Miscellaneous& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Miscellaneous& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property([](const ESM::Miscellaneous& rec) -> sol::optional<std::string> {
-            return LuaUtil::serializeRefId(rec.mScript);
-        });
+        record["mwscript"]
+            = sol::readonly_property([](const ESM::Miscellaneous& rec) -> ESM::RefId { return rec.mScript; });
         record["icon"] = sol::readonly_property([vfs](const ESM::Miscellaneous& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
@@ -96,4 +93,4 @@ namespace MWLua
         record["weight"]
             = sol::readonly_property([](const ESM::Miscellaneous& rec) -> float { return rec.mData.mWeight; });
     }
-}
+}

apps/openmw/mwlua/types/npc.cpp

@@ -208,16 +208,15 @@ namespace MWLua
             = sol::readonly_property([](const ESM::NPC& rec) -> std::string { return rec.mRace.serializeText(); });
         record["class"]
             = sol::readonly_property([](const ESM::NPC& rec) -> std::string { return rec.mClass.serializeText(); });
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::NPC& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["mwscript"] = sol::readonly_property([](const ESM::NPC& rec) -> ESM::RefId { return rec.mScript; });
         record["hair"]
             = sol::readonly_property([](const ESM::NPC& rec) -> std::string { return rec.mHair.serializeText(); });
         record["baseDisposition"]
             = sol::readonly_property([](const ESM::NPC& rec) -> int { return (int)rec.mNpdt.mDisposition; });
         record["head"]
             = sol::readonly_property([](const ESM::NPC& rec) -> std::string { return rec.mHead.serializeText(); });
-        record["primaryFaction"] = sol::readonly_property(
-            [](const ESM::NPC& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mFaction); });
+        record["primaryFaction"]
+            = sol::readonly_property([](const ESM::NPC& rec) -> ESM::RefId { return rec.mFaction; });
         record["primaryFactionRank"] = sol::readonly_property([](const ESM::NPC& rec, sol::this_state s) -> int64_t {
             if (rec.mFaction.empty())
                 return 0;

apps/openmw/mwlua/types/potion.cpp

@@ -82,8 +82,7 @@ namespace MWLua
         record["icon"] = sol::readonly_property([vfs](const ESM::Potion& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Potion& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["mwscript"] = sol::readonly_property([](const ESM::Potion& rec) -> ESM::RefId { return rec.mScript; });
         record["weight"] = sol::readonly_property([](const ESM::Potion& rec) -> float { return rec.mData.mWeight; });
         record["value"] = sol::readonly_property([](const ESM::Potion& rec) -> int { return rec.mData.mValue; });
         record["effects"] = sol::readonly_property([lua = lua.lua_state()](const ESM::Potion& rec) -> sol::table {
@@ -93,4 +92,4 @@ namespace MWLua
             return res;
         });
     }
-}
+}

apps/openmw/mwlua/types/probe.cpp

@@ -33,8 +33,7 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Probe& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Probe& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Probe& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["mwscript"] = sol::readonly_property([](const ESM::Probe& rec) -> ESM::RefId { return rec.mScript; });
         record["icon"] = sol::readonly_property([vfs](const ESM::Probe& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
@@ -43,4 +42,4 @@ namespace MWLua
         record["weight"] = sol::readonly_property([](const ESM::Probe& rec) -> float { return rec.mData.mWeight; });
         record["quality"] = sol::readonly_property([](const ESM::Probe& rec) -> float { return rec.mData.mQuality; });
     }
-}
+}

apps/openmw/mwlua/types/repair.cpp

@@ -33,8 +33,7 @@ namespace MWLua
             = sol::readonly_property([](const ESM::Repair& rec) -> std::string { return rec.mId.serializeText(); });
         record["name"] = sol::readonly_property([](const ESM::Repair& rec) -> std::string { return rec.mName; });
         addModelProperty(record);
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Repair& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["mwscript"] = sol::readonly_property([](const ESM::Repair& rec) -> ESM::RefId { return rec.mScript; });
         record["icon"] = sol::readonly_property([vfs](const ESM::Repair& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
@@ -43,4 +42,4 @@ namespace MWLua
         record["weight"] = sol::readonly_property([](const ESM::Repair& rec) -> float { return rec.mData.mWeight; });
         record["quality"] = sol::readonly_property([](const ESM::Repair& rec) -> float { return rec.mData.mQuality; });
     }
-}
+}

apps/openmw/mwlua/types/weapon.cpp

@@ -135,10 +135,8 @@ namespace MWLua
         record["icon"] = sol::readonly_property([vfs](const ESM::Weapon& rec) -> std::string {
             return Misc::ResourceHelpers::correctIconPath(rec.mIcon, vfs);
         });
-        record["enchant"] = sol::readonly_property(
-            [](const ESM::Weapon& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mEnchant); });
-        record["mwscript"] = sol::readonly_property(
-            [](const ESM::Weapon& rec) -> sol::optional<std::string> { return LuaUtil::serializeRefId(rec.mScript); });
+        record["enchant"] = sol::readonly_property([](const ESM::Weapon& rec) -> ESM::RefId { return rec.mEnchant; });
+        record["mwscript"] = sol::readonly_property([](const ESM::Weapon& rec) -> ESM::RefId { return rec.mScript; });
         record["isMagical"] = sol::readonly_property(
             [](const ESM::Weapon& rec) -> bool { return rec.mData.mFlags & ESM::Weapon::Magical; });
         record["isSilver"] = sol::readonly_property(

apps/openmw/mwlua/uibindings.cpp

@@ -22,21 +22,6 @@ namespace MWLua
 {
     namespace
     {
-        template <typename Fn>
-        void wrapAction(const std::shared_ptr<LuaUi::Element>& element, Fn&& fn)
-        {
-            try
-            {
-                fn();
-            }
-            catch (...)
-            {
-                // prevent any actions on a potentially corrupted widget
-                element->mRoot = nullptr;
-                throw;
-            }
-        }
-
         const std::unordered_map<MWGui::GuiMode, std::string_view> modeToName{
             { MWGui::GM_Inventory, "Interface" },
             { MWGui::GM_Container, "Container" },
@@ -142,7 +127,7 @@ namespace MWLua
 
         api["create"] = [luaManager = context.mLuaManager, menu](const sol::table& layout) {
             auto element = LuaUi::Element::make(layout, menu);
-            luaManager->addAction([element] { wrapAction(element, [&] { element->create(); }); }, "Create UI");
+            luaManager->addAction([element] { element->create(); }, "Create UI");
             return element;
         };
 
@@ -330,14 +315,13 @@ namespace MWLua
                 if (element->mState != LuaUi::Element::Created)
                     return;
                 element->mState = LuaUi::Element::Update;
-                luaManager->addAction([element] { wrapAction(element, [&] { element->update(); }); }, "Update UI");
+                luaManager->addAction([element] { element->update(); }, "Update UI");
             };
             uiElement["destroy"] = [luaManager = context.mLuaManager](const std::shared_ptr<LuaUi::Element>& element) {
                 if (element->mState == LuaUi::Element::Destroyed)
                     return;
                 element->mState = LuaUi::Element::Destroy;
-                luaManager->addAction(
-                    [element] { wrapAction(element, [&] { LuaUi::Element::erase(element.get()); }); }, "Destroy UI");
+                luaManager->addAction([element] { LuaUi::Element::erase(element.get()); }, "Destroy UI");
             };
 
             auto uiLayer = context.sol().new_usertype<LuaUi::Layer>("UiLayer");

apps/openmw/mwlua/weatherbindings.cpp

@@ -161,6 +161,8 @@ namespace MWLua
         weatherT["cloudsMaximumPercent"]
             = sol::property([](const MWWorld::Weather& w) { return w.mCloudsMaximumPercent; },
                 [](MWWorld::Weather& w, const FiniteFloat cloudsMaximumPercent) {
+                    if (cloudsMaximumPercent <= 0.f)
+                        throw std::runtime_error("Value must be greater than 0");
                     w.mCloudsMaximumPercent = cloudsMaximumPercent;
                 });
         weatherT["isStorm"] = sol::property([](const MWWorld::Weather& w) { return w.mIsStorm; },
@@ -172,7 +174,11 @@ namespace MWLua
         weatherT["rainSpeed"] = sol::property([](const MWWorld::Weather& w) { return w.mRainSpeed; },
             [](MWWorld::Weather& w, const FiniteFloat rainSpeed) { w.mRainSpeed = rainSpeed; });
         weatherT["rainEntranceSpeed"] = sol::property([](const MWWorld::Weather& w) { return w.mRainEntranceSpeed; },
-            [](MWWorld::Weather& w, const FiniteFloat rainEntranceSpeed) { w.mRainEntranceSpeed = rainEntranceSpeed; });
+            [](MWWorld::Weather& w, const FiniteFloat rainEntranceSpeed) {
+                if (rainEntranceSpeed <= 0.f)
+                    throw std::runtime_error("Value must be greater than 0");
+                w.mRainEntranceSpeed = rainEntranceSpeed;
+            });
         weatherT["rainEffect"] = sol::property(
             [](const MWWorld::Weather& w) -> sol::optional<std::string> {
                 if (w.mRainEffect.empty())
@@ -193,7 +199,7 @@ namespace MWLua
         weatherT["rainMinHeight"] = sol::property([](const MWWorld::Weather& w) { return w.mRainMinHeight; },
             [](MWWorld::Weather& w, const FiniteFloat rainMinHeight) { w.mRainMinHeight = rainMinHeight; });
         weatherT["rainLoopSoundID"]
-            = sol::property([](const MWWorld::Weather& w) { return LuaUtil::serializeRefId(w.mRainLoopSoundID); },
+            = sol::property([](const MWWorld::Weather& w) -> ESM::RefId { return w.mRainLoopSoundID; },
                 [](MWWorld::Weather& w, sol::optional<std::string_view> rainLoopSoundID) {
                     w.mRainLoopSoundID = ESM::RefId::deserializeText(rainLoopSoundID.value_or(""));
                 });
@@ -203,7 +209,7 @@ namespace MWLua
                     w.mSunDiscSunsetColor = sunDiscSunsetColor.toVec();
                 });
         weatherT["ambientLoopSoundID"]
-            = sol::property([](const MWWorld::Weather& w) { return LuaUtil::serializeRefId(w.mAmbientLoopSoundID); },
+            = sol::property([](const MWWorld::Weather& w) -> ESM::RefId { return w.mAmbientLoopSoundID; },
                 [](MWWorld::Weather& w, sol::optional<std::string_view> ambientLoopSoundId) {
                     w.mAmbientLoopSoundID = ESM::RefId::deserializeText(ambientLoopSoundId.value_or(""));
                 });

apps/openmw/mwmechanics/character.cpp

@@ -1073,23 +1073,17 @@ namespace MWMechanics
         std::string_view action = evt.substr(groupname.size() + 2);
         if (action == "equip attach")
         {
-            if (mUpperBodyState == UpperBodyState::Equipping)
-            {
-                if (groupname == "shield")
-                    mAnimation->showCarriedLeft(true);
-                else
-                    mAnimation->showWeapons(true);
-            }
+            if (groupname == "shield")
+                mAnimation->showCarriedLeft(true);
+            else if (mUpperBodyState == UpperBodyState::Equipping)
+                mAnimation->showWeapons(true);
         }
         else if (action == "unequip detach")
         {
-            if (mUpperBodyState == UpperBodyState::Unequipping)
-            {
-                if (groupname == "shield")
-                    mAnimation->showCarriedLeft(false);
-                else
-                    mAnimation->showWeapons(false);
-            }
+            if (groupname == "shield")
+                mAnimation->showCarriedLeft(false);
+            else if (mUpperBodyState == UpperBodyState::Unequipping)
+                mAnimation->showWeapons(false);
         }
         else if (action == "chop hit" || action == "slash hit" || action == "thrust hit" || action == "hit")
         {
@@ -1393,7 +1387,7 @@ namespace MWMechanics
                 // We can not play un-equip animation if weapon changed since last update
                 if (!weaponChanged)
                 {
-                    // Note: we do not disable unequipping animation automatically to avoid body desync
+                    // Note: we do not disable the weapon unequipping animation automatically to avoid body desync
                     weapgroup = getWeaponAnimation(mWeaponType);
                     int unequipMask = MWRender::BlendMask_All;
                     mUpperBodyState = UpperBodyState::Unequipping;
@@ -1402,6 +1396,7 @@ namespace MWMechanics
                         && !(mWeaponType == ESM::Weapon::None && weaptype == ESM::Weapon::Spell))
                     {
                         unequipMask = unequipMask | ~MWRender::BlendMask_LeftArm;
+                        mAnimation->disable("shield");
                         playBlendedAnimation("shield", Priority_Block, MWRender::BlendMask_LeftArm, true, 1.0f,
                             "unequip start", "unequip stop", 0.0f, 0);
                     }
@@ -1459,6 +1454,7 @@ namespace MWMechanics
                             if (useShieldAnims && weaptype != ESM::Weapon::Spell)
                             {
                                 equipMask = equipMask | ~MWRender::BlendMask_LeftArm;
+                                mAnimation->disable("shield");
                                 playBlendedAnimation("shield", Priority_Block, MWRender::BlendMask_LeftArm, true, 1.0f,
                                     "equip start", "equip stop", 0.0f, 0);
                             }
@@ -1746,7 +1742,7 @@ namespace MWMechanics
             {
                 // TODO: this will only work for the player, and needs to be fixed if NPCs should ever use
                 // lockpicks/probes.
-                MWWorld::Ptr target = world->getFacedObject();
+                MWWorld::Ptr target = world->getFocusObject();
 
                 if (!target.isEmpty())
                 {

apps/openmw/mwmechanics/spelleffects.cpp

@@ -45,18 +45,16 @@ namespace
             + Misc::Rng::rollDice(static_cast<int>(effect.mMaxMagnitude - effect.mMinMagnitude + 1), prng);
     }
 
-    void modifyAiSetting(const MWWorld::Ptr& target, const ESM::ActiveEffect& effect,
-        ESM::MagicEffect::Effects creatureEffect, MWMechanics::AiSetting setting, float magnitude, bool& invalid)
+    ESM::ActiveEffect::Flags modifyAiSetting(const MWWorld::Ptr& target, const ESM::ActiveEffect& effect,
+        ESM::MagicEffect::Effects creatureEffect, MWMechanics::AiSetting setting, float magnitude)
     {
         if (target == MWMechanics::getPlayer() || (effect.mEffectId == creatureEffect) == target.getClass().isNpc())
-            invalid = true;
-        else
-        {
-            auto& creatureStats = target.getClass().getCreatureStats(target);
-            auto stat = creatureStats.getAiSetting(setting);
-            stat.setModifier(static_cast<int>(stat.getModifier() + magnitude));
-            creatureStats.setAiSetting(setting, stat);
-        }
+            return ESM::ActiveEffect::Flag_Invalid;
+        auto& creatureStats = target.getClass().getCreatureStats(target);
+        auto stat = creatureStats.getAiSetting(setting);
+        stat.setModifier(static_cast<int>(stat.getModifier() + magnitude));
+        creatureStats.setAiSetting(setting, stat);
+        return ESM::ActiveEffect::Flag_Applied;
     }
 
     void adjustDynamicStat(const MWWorld::Ptr& target, int index, float magnitude, bool allowDecreaseBelowZero = false,
@@ -421,12 +419,12 @@ namespace
 namespace MWMechanics
 {
 
-    void applyMagicEffect(const MWWorld::Ptr& target, const MWWorld::Ptr& caster,
-        const ActiveSpells::ActiveSpellParams& spellParams, ESM::ActiveEffect& effect, bool& invalid,
-        bool& receivedMagicDamage, bool& affectedHealth, bool& recalculateMagicka)
+    ESM::ActiveEffect::Flags applyMagicEffect(const MWWorld::Ptr& target, const MWWorld::Ptr& caster,
+        const ActiveSpells::ActiveSpellParams& spellParams, ESM::ActiveEffect& effect, bool& receivedMagicDamage,
+        bool& affectedHealth, bool& recalculateMagicka)
     {
         const auto world = MWBase::Environment::get().getWorld();
-        bool godmode = target == getPlayer() && world->getGodModeState();
+        const bool godmode = target == getPlayer() && world->getGodModeState();
         switch (effect.mEffectId)
         {
             case ESM::MagicEffect::CureCommonDisease:
@@ -470,7 +468,7 @@ namespace MWMechanics
             case ESM::MagicEffect::AlmsiviIntervention:
             case ESM::MagicEffect::DivineIntervention:
                 if (target != getPlayer())
-                    invalid = true;
+                    return ESM::ActiveEffect::Flag_Invalid;
                 else if (world->isTeleportingEnabled())
                 {
                     std::string_view marker
@@ -495,7 +493,7 @@ namespace MWMechanics
                 break;
             case ESM::MagicEffect::Mark:
                 if (target != getPlayer())
-                    invalid = true;
+                    return ESM::ActiveEffect::Flag_Invalid;
                 else if (world->isTeleportingEnabled())
                     world->getPlayer().markPosition(target.getCell(), target.getRefData().getPosition());
                 else if (caster == getPlayer())
@@ -503,7 +501,7 @@ namespace MWMechanics
                 break;
             case ESM::MagicEffect::Recall:
                 if (target != getPlayer())
-                    invalid = true;
+                    return ESM::ActiveEffect::Flag_Invalid;
                 else if (world->isTeleportingEnabled())
                 {
                     MWWorld::CellStore* markedCell = nullptr;
@@ -529,7 +527,7 @@ namespace MWMechanics
             case ESM::MagicEffect::CommandHumanoid:
                 if (caster.isEmpty() || !caster.getClass().isActor() || target == getPlayer()
                     || (effect.mEffectId == ESM::MagicEffect::CommandCreature) == target.getClass().isNpc())
-                    invalid = true;
+                    return ESM::ActiveEffect::Flag_Invalid;
                 else if (effect.mMagnitude >= target.getClass().getCreatureStats(target).getLevel())
                 {
                     MWMechanics::AiFollow package(caster, true);
@@ -537,41 +535,37 @@ namespace MWMechanics
                 }
                 break;
             case ESM::MagicEffect::ExtraSpell:
-                if (target.getClass().hasInventoryStore(target))
+                if (!target.getClass().hasInventoryStore(target))
+                    return ESM::ActiveEffect::Flag_Invalid;
+                if (target != getPlayer())
                 {
-                    if (target != getPlayer())
+                    auto& store = target.getClass().getInventoryStore(target);
+                    for (int slot = 0; slot < MWWorld::InventoryStore::Slots; ++slot)
                     {
-                        auto& store = target.getClass().getInventoryStore(target);
-                        for (int slot = 0; slot < MWWorld::InventoryStore::Slots; ++slot)
+                        // Unequip everything except weapons, torches, and pants
+                        switch (slot)
                         {
-                            // Unequip everything except weapons, torches, and pants
-                            switch (slot)
+                            case MWWorld::InventoryStore::Slot_Ammunition:
+                            case MWWorld::InventoryStore::Slot_CarriedRight:
+                            case MWWorld::InventoryStore::Slot_Pants:
+                                continue;
+                            case MWWorld::InventoryStore::Slot_CarriedLeft:
                             {
-                                case MWWorld::InventoryStore::Slot_Ammunition:
-                                case MWWorld::InventoryStore::Slot_CarriedRight:
-                                case MWWorld::InventoryStore::Slot_Pants:
+                                auto carried = store.getSlot(slot);
+                                if (carried == store.end() || carried.getType() != MWWorld::ContainerStore::Type_Armor)
                                     continue;
-                                case MWWorld::InventoryStore::Slot_CarriedLeft:
-                                {
-                                    auto carried = store.getSlot(slot);
-                                    if (carried == store.end()
-                                        || carried.getType() != MWWorld::ContainerStore::Type_Armor)
-                                        continue;
-                                    [[fallthrough]];
-                                }
-                                default:
-                                    store.unequipSlot(slot);
+                                [[fallthrough]];
                             }
+                            default:
+                                store.unequipSlot(slot);
                         }
                     }
                 }
-                else
-                    invalid = true;
                 break;
             case ESM::MagicEffect::TurnUndead:
                 if (target.getClass().isNpc()
                     || target.get<ESM::Creature>()->mBase->mData.mType != ESM::Creature::Undead)
-                    invalid = true;
+                    return ESM::ActiveEffect::Flag_Invalid;
                 else
                 {
                     auto& creatureStats = target.getClass().getCreatureStats(target);
@@ -582,32 +576,33 @@ namespace MWMechanics
                 break;
             case ESM::MagicEffect::FrenzyCreature:
             case ESM::MagicEffect::FrenzyHumanoid:
-                modifyAiSetting(
-                    target, effect, ESM::MagicEffect::FrenzyCreature, AiSetting::Fight, effect.mMagnitude, invalid);
-                break;
+                return modifyAiSetting(
+                    target, effect, ESM::MagicEffect::FrenzyCreature, AiSetting::Fight, effect.mMagnitude);
             case ESM::MagicEffect::CalmCreature:
             case ESM::MagicEffect::CalmHumanoid:
-                modifyAiSetting(
-                    target, effect, ESM::MagicEffect::CalmCreature, AiSetting::Fight, -effect.mMagnitude, invalid);
-                if (!invalid && effect.mMagnitude > 0)
+            {
+                ESM::ActiveEffect::Flags applied = modifyAiSetting(
+                    target, effect, ESM::MagicEffect::CalmCreature, AiSetting::Fight, -effect.mMagnitude);
+                if (applied != ESM::ActiveEffect::Flag_Applied)
+                    return applied;
+                if (effect.mMagnitude > 0)
                 {
                     auto& creatureStats = target.getClass().getCreatureStats(target);
                     creatureStats.getAiSequence().stopCombat();
                 }
                 break;
+            }
             case ESM::MagicEffect::DemoralizeCreature:
             case ESM::MagicEffect::DemoralizeHumanoid:
-                modifyAiSetting(
-                    target, effect, ESM::MagicEffect::DemoralizeCreature, AiSetting::Flee, effect.mMagnitude, invalid);
-                break;
+                return modifyAiSetting(
+                    target, effect, ESM::MagicEffect::DemoralizeCreature, AiSetting::Flee, effect.mMagnitude);
             case ESM::MagicEffect::RallyCreature:
             case ESM::MagicEffect::RallyHumanoid:
-                modifyAiSetting(
-                    target, effect, ESM::MagicEffect::RallyCreature, AiSetting::Flee, -effect.mMagnitude, invalid);
-                break;
+                return modifyAiSetting(
+                    target, effect, ESM::MagicEffect::RallyCreature, AiSetting::Flee, -effect.mMagnitude);
             case ESM::MagicEffect::Charm:
                 if (!target.getClass().isNpc())
-                    invalid = true;
+                    return ESM::ActiveEffect::Flag_Invalid;
                 break;
             case ESM::MagicEffect::Sound:
                 if (target == getPlayer())
@@ -644,16 +639,12 @@ namespace MWMechanics
             case ESM::MagicEffect::SummonCreature04:
             case ESM::MagicEffect::SummonCreature05:
                 if (!target.isInCell())
-                    invalid = true;
-                else
-                    effect.mArg = summonCreature(effect.mEffectId, target);
+                    return ESM::ActiveEffect::Flag_Invalid;
+                effect.mArg = summonCreature(effect.mEffectId, target);
                 break;
             case ESM::MagicEffect::BoundGloves:
                 if (!target.getClass().hasInventoryStore(target))
-                {
-                    invalid = true;
-                    break;
-                }
+                    return ESM::ActiveEffect::Flag_Invalid;
                 addBoundItem(ESM::RefId::stringRefId(world->getStore()
                                                          .get<ESM::GameSetting>()
                                                          .find("sMagicBoundRightGauntletID")
@@ -673,10 +664,7 @@ namespace MWMechanics
             case ESM::MagicEffect::BoundShield:
             {
                 if (!target.getClass().hasInventoryStore(target))
-                {
-                    invalid = true;
-                    break;
-                }
+                    return ESM::ActiveEffect::Flag_Invalid;
                 const std::string& item = sBoundItemsMap.at(effect.mEffectId);
                 const MWWorld::Store<ESM::GameSetting>& gmst = world->getStore().get<ESM::GameSetting>();
                 const ESM::RefId itemId = ESM::RefId::stringRefId(gmst.find(item)->mValue.getString());
@@ -715,9 +703,7 @@ namespace MWMechanics
                     damageAttribute(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::DamageSkill:
-                if (!target.getClass().isNpc())
-                    invalid = true;
-                else if (!godmode)
+                if (!godmode && target.getClass().isNpc())
                 {
                     // Damage Skill abilities reduce base skill :todd:
                     if (spellParams.hasFlag(ESM::ActiveSpells::Flag_AffectsBaseValues))
@@ -735,9 +721,7 @@ namespace MWMechanics
                 restoreAttribute(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::RestoreSkill:
-                if (!target.getClass().isNpc())
-                    invalid = true;
-                else
+                if (target.getClass().isNpc())
                     restoreSkill(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::RestoreHealth:
@@ -767,7 +751,9 @@ namespace MWMechanics
             case ESM::MagicEffect::DrainHealth:
             case ESM::MagicEffect::DrainMagicka:
             case ESM::MagicEffect::DrainFatigue:
-                if (!godmode)
+                if (godmode)
+                    return ESM::ActiveEffect::Flag_Remove;
+                else
                 {
                     int index = effect.mEffectId - ESM::MagicEffect::DrainHealth;
                     // Unlike Absorb and Damage effects Drain effects can bring stats below zero
@@ -786,8 +772,9 @@ namespace MWMechanics
                         target, effect.mEffectId - ESM::MagicEffect::FortifyHealth, effect.mMagnitude, false, true);
                 break;
             case ESM::MagicEffect::DrainAttribute:
-                if (!godmode)
-                    damageAttribute(target, effect, effect.mMagnitude);
+                if (godmode)
+                    return ESM::ActiveEffect::Flag_Remove;
+                damageAttribute(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::FortifyAttribute:
                 // Abilities affect base stats, but not for drain
@@ -803,23 +790,23 @@ namespace MWMechanics
                     fortifyAttribute(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::DrainSkill:
-                if (!target.getClass().isNpc())
-                    invalid = true;
-                else if (!godmode)
-                    damageSkill(target, effect, effect.mMagnitude);
+                if (godmode || !target.getClass().isNpc())
+                    return ESM::ActiveEffect::Flag_Remove;
+                damageSkill(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::FortifySkill:
-                if (!target.getClass().isNpc())
-                    invalid = true;
-                else if (spellParams.hasFlag(ESM::ActiveSpells::Flag_AffectsBaseValues))
+                if (target.getClass().isNpc())
                 {
-                    // Abilities affect base stats, but not for drain
-                    auto& npcStats = target.getClass().getNpcStats(target);
-                    auto& skill = npcStats.getSkill(effect.getSkillOrAttribute());
-                    skill.setBase(skill.getBase() + effect.mMagnitude);
+                    if (spellParams.hasFlag(ESM::ActiveSpells::Flag_AffectsBaseValues))
+                    {
+                        // Abilities affect base stats, but not for drain
+                        auto& npcStats = target.getClass().getNpcStats(target);
+                        auto& skill = npcStats.getSkill(effect.getSkillOrAttribute());
+                        skill.setBase(skill.getBase() + effect.mMagnitude);
+                    }
+                    else
+                        fortifySkill(target, effect, effect.mMagnitude);
                 }
-                else
-                    fortifySkill(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::FortifyMaximumMagicka:
                 recalculateMagicka = true;
@@ -827,7 +814,9 @@ namespace MWMechanics
             case ESM::MagicEffect::AbsorbHealth:
             case ESM::MagicEffect::AbsorbMagicka:
             case ESM::MagicEffect::AbsorbFatigue:
-                if (!godmode)
+                if (godmode)
+                    return ESM::ActiveEffect::Flag_Remove;
+                else
                 {
                     int index = effect.mEffectId - ESM::MagicEffect::AbsorbHealth;
                     adjustDynamicStat(target, index, -effect.mMagnitude);
@@ -838,7 +827,9 @@ namespace MWMechanics
                 }
                 break;
             case ESM::MagicEffect::AbsorbAttribute:
-                if (!godmode)
+                if (godmode)
+                    return ESM::ActiveEffect::Flag_Remove;
+                else
                 {
                     damageAttribute(target, effect, effect.mMagnitude);
                     if (!caster.isEmpty())
@@ -846,11 +837,12 @@ namespace MWMechanics
                 }
                 break;
             case ESM::MagicEffect::AbsorbSkill:
-                if (!target.getClass().isNpc())
-                    invalid = true;
-                else if (!godmode)
+                if (godmode)
+                    return ESM::ActiveEffect::Flag_Remove;
+                else
                 {
-                    damageSkill(target, effect, effect.mMagnitude);
+                    if (target.getClass().isNpc())
+                        damageSkill(target, effect, effect.mMagnitude);
                     if (!caster.isEmpty() && caster.getClass().isNpc())
                         fortifySkill(caster, effect, effect.mMagnitude);
                 }
@@ -858,10 +850,7 @@ namespace MWMechanics
             case ESM::MagicEffect::DisintegrateArmor:
             {
                 if (!target.getClass().hasInventoryStore(target))
-                {
-                    invalid = true;
-                    break;
-                }
+                    return ESM::ActiveEffect::Flag_Invalid;
                 if (godmode)
                     break;
                 static const std::array<int, 9> priorities{
@@ -884,18 +873,18 @@ namespace MWMechanics
             }
             case ESM::MagicEffect::DisintegrateWeapon:
                 if (!target.getClass().hasInventoryStore(target))
-                {
-                    invalid = true;
-                    break;
-                }
+                    return ESM::ActiveEffect::Flag_Invalid;
                 if (!godmode)
                     disintegrateSlot(target, MWWorld::InventoryStore::Slot_CarriedRight, effect.mMagnitude);
                 break;
         }
+        return ESM::ActiveEffect::Flag_Applied;
     }
 
     bool shouldRemoveEffect(const MWWorld::Ptr& target, const ESM::ActiveEffect& effect)
     {
+        if (effect.mFlags & ESM::ActiveEffect::Flag_Invalid)
+            return true;
         const auto world = MWBase::Environment::get().getWorld();
         switch (effect.mEffectId)
         {
@@ -937,7 +926,7 @@ namespace MWMechanics
         ActiveSpells::ActiveSpellParams& spellParams, ESM::ActiveEffect& effect, float dt, bool playNonLooping)
     {
         const auto world = MWBase::Environment::get().getWorld();
-        bool invalid = false;
+        int32_t applied = ESM::ActiveEffect::Flag_Remove;
         bool receivedMagicDamage = false;
         bool recalculateMagicka = false;
         bool affectedHealth = false;
@@ -947,8 +936,8 @@ namespace MWMechanics
             for (auto& otherEffect : spellParams.getEffects())
             {
                 if (isCorprusEffect(otherEffect))
-                    applyMagicEffect(target, caster, spellParams, otherEffect, invalid, receivedMagicDamage,
-                        affectedHealth, recalculateMagicka);
+                    applyMagicEffect(target, caster, spellParams, otherEffect, receivedMagicDamage, affectedHealth,
+                        recalculateMagicka);
             }
             if (target == getPlayer())
                 MWBase::Environment::get().getWindowManager()->messageBox("#{sMagicCorprusWorsens}");
@@ -990,7 +979,7 @@ namespace MWMechanics
                 }
             }
             else
-                invalid = true;
+                applied |= ESM::ActiveEffect::Flag_Invalid;
         }
         else if (effect.mEffectId == ESM::MagicEffect::Open)
         {
@@ -1022,11 +1011,11 @@ namespace MWMechanics
                 }
             }
             else
-                invalid = true;
+                applied |= ESM::ActiveEffect::Flag_Invalid;
         }
         else if (!target.getClass().isActor())
         {
-            invalid = true;
+            applied |= ESM::ActiveEffect::Flag_Invalid;
         }
         else
         {
@@ -1042,7 +1031,7 @@ namespace MWMechanics
             auto& stats = target.getClass().getCreatureStats(target);
             auto& magnitudes = stats.getMagicEffects();
             if (!spellParams.hasFlag(ESM::ActiveSpells::Flag_AffectsBaseValues)
-                && !(effect.mFlags & ESM::ActiveEffect::Flag_Applied))
+                && !(effect.mFlags & ESM::ActiveEffect::Flag_Remove))
             {
                 MagicApplicationResult::Type result
                     = applyProtections(target, caster, spellParams, effect, magicEffect);
@@ -1052,7 +1041,7 @@ namespace MWMechanics
             float oldMagnitude = 0.f;
             if (effect.mFlags & ESM::ActiveEffect::Flag_Applied)
                 oldMagnitude = effect.mMagnitude;
-            else
+            else if (!(effect.mFlags & ESM::ActiveEffect::Flag_Remove))
             {
                 bool isTemporary = spellParams.hasFlag(ESM::ActiveSpells::Flag_Temporary);
                 bool isEquipment = spellParams.hasFlag(ESM::ActiveSpells::Flag_Equipment);
@@ -1087,25 +1076,27 @@ namespace MWMechanics
                 }
             }
             if (effect.mEffectId == ESM::MagicEffect::Corprus)
+            {
                 spellParams.worsen();
+                applied |= ESM::ActiveEffect::Flag_Applied;
+            }
             else
-                applyMagicEffect(target, caster, spellParams, effect, invalid, receivedMagicDamage, affectedHealth,
-                    recalculateMagicka);
+                applied |= applyMagicEffect(
+                    target, caster, spellParams, effect, receivedMagicDamage, affectedHealth, recalculateMagicka);
             effect.mMagnitude = magnitude;
             magnitudes.add(EffectKey(effect.mEffectId, effect.getSkillOrAttribute()),
                 EffectParam(effect.mMagnitude - oldMagnitude));
         }
         effect.mTimeLeft -= dt;
-        if (invalid)
+        if (applied & ESM::ActiveEffect::Flag_Invalid)
         {
             effect.mTimeLeft = 0;
-            effect.mFlags |= ESM::ActiveEffect::Flag_Remove;
             auto anim = world->getAnimation(target);
             if (anim)
                 anim->removeEffect(ESM::MagicEffect::indexToName(effect.mEffectId));
+            // Note that we can't return REMOVED here because the effect still needs to be detectable
         }
-        else
-            effect.mFlags |= ESM::ActiveEffect::Flag_Applied | ESM::ActiveEffect::Flag_Remove;
+        effect.mFlags |= applied;
         if (recalculateMagicka)
             target.getClass().getCreatureStats(target).recalculateMagicka();
         return { MagicApplicationResult::Type::APPLIED, receivedMagicDamage, affectedHealth };
@@ -1116,7 +1107,6 @@ namespace MWMechanics
     {
         const auto world = MWBase::Environment::get().getWorld();
         auto& magnitudes = target.getClass().getCreatureStats(target).getMagicEffects();
-        bool invalid;
         switch (effect.mEffectId)
         {
             case ESM::MagicEffect::CommandCreature:
@@ -1144,18 +1134,16 @@ namespace MWMechanics
             break;
             case ESM::MagicEffect::FrenzyCreature:
             case ESM::MagicEffect::FrenzyHumanoid:
-                modifyAiSetting(
-                    target, effect, ESM::MagicEffect::FrenzyCreature, AiSetting::Fight, -effect.mMagnitude, invalid);
+                modifyAiSetting(target, effect, ESM::MagicEffect::FrenzyCreature, AiSetting::Fight, -effect.mMagnitude);
                 break;
             case ESM::MagicEffect::CalmCreature:
             case ESM::MagicEffect::CalmHumanoid:
-                modifyAiSetting(
-                    target, effect, ESM::MagicEffect::CalmCreature, AiSetting::Fight, effect.mMagnitude, invalid);
+                modifyAiSetting(target, effect, ESM::MagicEffect::CalmCreature, AiSetting::Fight, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::DemoralizeCreature:
             case ESM::MagicEffect::DemoralizeHumanoid:
                 modifyAiSetting(
-                    target, effect, ESM::MagicEffect::DemoralizeCreature, AiSetting::Flee, -effect.mMagnitude, invalid);
+                    target, effect, ESM::MagicEffect::DemoralizeCreature, AiSetting::Flee, -effect.mMagnitude);
                 break;
             case ESM::MagicEffect::NightEye:
             {
@@ -1174,8 +1162,7 @@ namespace MWMechanics
             break;
             case ESM::MagicEffect::RallyCreature:
             case ESM::MagicEffect::RallyHumanoid:
-                modifyAiSetting(
-                    target, effect, ESM::MagicEffect::RallyCreature, AiSetting::Flee, effect.mMagnitude, invalid);
+                modifyAiSetting(target, effect, ESM::MagicEffect::RallyCreature, AiSetting::Flee, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::Sound:
                 if (magnitudes.getOrDefault(effect.mEffectId).getModifier() <= 0.f && target == getPlayer())
@@ -1275,18 +1262,22 @@ namespace MWMechanics
                     fortifyAttribute(target, effect, -effect.mMagnitude);
                 break;
             case ESM::MagicEffect::DrainSkill:
-                restoreSkill(target, effect, effect.mMagnitude);
+                if (target.getClass().isNpc())
+                    restoreSkill(target, effect, effect.mMagnitude);
                 break;
             case ESM::MagicEffect::FortifySkill:
-                // Abilities affect base stats, but not for drain
-                if (spellParams.hasFlag(ESM::ActiveSpells::Flag_AffectsBaseValues))
+                if (target.getClass().isNpc())
                 {
-                    auto& npcStats = target.getClass().getNpcStats(target);
-                    auto& skill = npcStats.getSkill(effect.getSkillOrAttribute());
-                    skill.setBase(skill.getBase() - effect.mMagnitude);
+                    // Abilities affect base stats, but not for drain
+                    if (spellParams.hasFlag(ESM::ActiveSpells::Flag_AffectsBaseValues))
+                    {
+                        auto& npcStats = target.getClass().getNpcStats(target);
+                        auto& skill = npcStats.getSkill(effect.getSkillOrAttribute());
+                        skill.setBase(skill.getBase() - effect.mMagnitude);
+                    }
+                    else
+                        fortifySkill(target, effect, -effect.mMagnitude);
                 }
-                else
-                    fortifySkill(target, effect, -effect.mMagnitude);
                 break;
             case ESM::MagicEffect::FortifyMaximumMagicka:
                 target.getClass().getCreatureStats(target).recalculateMagicka();
@@ -1301,8 +1292,9 @@ namespace MWMechanics
             break;
             case ESM::MagicEffect::AbsorbSkill:
             {
+                if (target.getClass().isNpc())
+                    restoreSkill(target, effect, effect.mMagnitude);
                 const auto caster = world->searchPtrViaActorId(spellParams.getCasterActorId());
-                restoreSkill(target, effect, effect.mMagnitude);
                 if (!caster.isEmpty() && caster.getClass().isNpc())
                     fortifySkill(caster, effect, -effect.mMagnitude);
             }

apps/openmw/mwscript/miscextensions.cpp

@@ -626,7 +626,7 @@ namespace MWScript
                 {
                     for (const auto& effect : spell.getEffects())
                     {
-                        if (effect.mFlags & ESM::ActiveEffect::Flag_Applied && effect.mEffectId == key)
+                        if (effect.mFlags & ESM::ActiveEffect::Flag_Remove && effect.mEffectId == key)
                         {
                             runtime.push(1);
                             return;

apps/openmw/mwworld/actiontrap.cpp

@@ -18,7 +18,7 @@ namespace MWWorld
         // Using activation distance as the trap range.
 
         if (actor == MWBase::Environment::get().getWorld()->getPlayerPtr()
-            && MWBase::Environment::get().getWorld()->getDistanceToFacedObject()
+            && MWBase::Environment::get().getWorld()->getDistanceToFocusObject()
                 > trapRange) // player activated object outside range of trap
         {
             MWMechanics::CastSpell cast(mTrapSource, mTrapSource);

apps/openmw/mwworld/player.cpp

@@ -193,7 +193,7 @@ namespace MWWorld
         if (playerStats.isParalyzed() || playerStats.getKnockedDown() || playerStats.isDead())
             return;
 
-        MWWorld::Ptr toActivate = MWBase::Environment::get().getWorld()->getFacedObject();
+        MWWorld::Ptr toActivate = MWBase::Environment::get().getWorld()->getFocusObject();
 
         if (toActivate.isEmpty())
             return;

apps/openmw/mwworld/worldimp.cpp

@@ -261,7 +261,7 @@ namespace MWWorld
         , mActivationDistanceOverride(activationDistanceOverride)
         , mStartCell(startCell)
         , mSwimHeightScale(0.f)
-        , mDistanceToFacedObject(-1.f)
+        , mDistanceToFocusObject(-1.f)
         , mTeleportEnabled(true)
         , mLevitationEnabled(true)
         , mGoToJail(false)
@@ -999,33 +999,33 @@ namespace MWWorld
         return static_cast<float>(iMaxActivateDist);
     }
 
-    MWWorld::Ptr World::getFacedObject()
+    MWWorld::Ptr World::getFocusObject()
     {
-        MWWorld::Ptr facedObject;
+        MWWorld::Ptr focusObject;
 
         if (MWBase::Environment::get().getStateManager()->getState() == MWBase::StateManager::State_NoGame)
-            return facedObject;
+            return focusObject;
 
         if (MWBase::Environment::get().getWindowManager()->isGuiMode()
             && MWBase::Environment::get().getWindowManager()->isConsoleMode())
-            facedObject = getFacedObject(getMaxActivationDistance() * 50, false);
+            focusObject = getFocusObject(getMaxActivationDistance() * 50, false);
         else
         {
             float activationDistance = getActivationDistancePlusTelekinesis();
 
-            facedObject = getFacedObject(activationDistance, true);
+            focusObject = getFocusObject(activationDistance, true);
 
-            if (!facedObject.isEmpty() && !facedObject.getClass().allowTelekinesis(facedObject)
-                && mDistanceToFacedObject > getMaxActivationDistance()
+            if (!focusObject.isEmpty() && !focusObject.getClass().allowTelekinesis(focusObject)
+                && mDistanceToFocusObject > getMaxActivationDistance()
                 && !MWBase::Environment::get().getWindowManager()->isGuiMode())
                 return nullptr;
         }
-        return facedObject;
+        return focusObject;
     }
 
-    float World::getDistanceToFacedObject()
+    float World::getDistanceToFocusObject()
     {
-        return mDistanceToFacedObject;
+        return mDistanceToFocusObject;
     }
 
     osg::Matrixf World::getActorHeadTransform(const MWWorld::ConstPtr& actor) const
@@ -1772,12 +1772,12 @@ namespace MWWorld
         MWBase::Environment::get().getSoundManager()->setListenerPosDir(listenerPos, forward, up, underwater);
     }
 
-    void World::updateWindowManager()
+    void World::updateFocusObject()
     {
         try
         {
             // inform the GUI about focused object
-            MWWorld::Ptr object = getFacedObject();
+            MWWorld::Ptr object = getFocusObject();
 
             // retrieve the object's top point's screen position so we know where to place the floating label
             if (!object.isEmpty())
@@ -1798,15 +1798,15 @@ namespace MWWorld
         }
         catch (std::exception& e)
         {
-            Log(Debug::Error) << "Error updating window manager: " << e.what();
+            Log(Debug::Error) << "Error updating focus object: " << e.what();
         }
     }
 
-    MWWorld::Ptr World::getFacedObject(float maxDistance, bool ignorePlayer)
+    MWWorld::Ptr World::getFocusObject(float maxDistance, bool ignorePlayer)
     {
         const float camDist = mRendering->getCamera()->getCameraDistance();
         maxDistance += camDist;
-        MWWorld::Ptr facedObject;
+        MWWorld::Ptr focusObject;
         MWRender::RenderingManager::RayResult rayToObject;
 
         if (MWBase::Environment::get().getWindowManager()->isGuiMode())
@@ -1818,14 +1818,14 @@ namespace MWWorld
         else
             rayToObject = mRendering->castCameraToViewportRay(0.5f, 0.5f, maxDistance, ignorePlayer);
 
-        facedObject = rayToObject.mHitObject;
-        if (facedObject.isEmpty() && rayToObject.mHitRefnum.isSet())
-            facedObject = MWBase::Environment::get().getWorldModel()->getPtr(rayToObject.mHitRefnum);
+        focusObject = rayToObject.mHitObject;
+        if (focusObject.isEmpty() && rayToObject.mHitRefnum.isSet())
+            focusObject = MWBase::Environment::get().getWorldModel()->getPtr(rayToObject.mHitRefnum);
         if (rayToObject.mHit)
-            mDistanceToFacedObject = (rayToObject.mRatio * maxDistance) - camDist;
+            mDistanceToFocusObject = (rayToObject.mRatio * maxDistance) - camDist;
         else
-            mDistanceToFacedObject = -1;
-        return facedObject;
+            mDistanceToFocusObject = -1;
+        return focusObject;
     }
 
     bool World::castRenderingRay(MWPhysics::RayCastingResult& res, const osg::Vec3f& from, const osg::Vec3f& to,
@@ -2997,7 +2997,7 @@ namespace MWWorld
         else
         {
             if (casterIsPlayer)
-                target = getFacedObject();
+                target = getFocusObject();
 
             if (target.isEmpty() || !target.getClass().hasToolTip(target))
             {

apps/openmw/mwworld/worldimp.hpp

@@ -121,7 +121,7 @@ namespace MWWorld
 
         float mSwimHeightScale;
 
-        float mDistanceToFacedObject;
+        float mDistanceToFocusObject;
 
         bool mTeleportEnabled;
         bool mLevitationEnabled;
@@ -152,7 +152,7 @@ namespace MWWorld
 
         void preloadSpells();
 
-        MWWorld::Ptr getFacedObject(float maxDistance, bool ignorePlayer = true);
+        MWWorld::Ptr getFocusObject(float maxDistance, bool ignorePlayer = true);
 
         void PCDropped(const Ptr& item);
 
@@ -349,10 +349,10 @@ namespace MWWorld
             bool changeEvent = true) override;
         ///< @param changeEvent If false, do not trigger cell change flag or detect worldspace changes
 
-        MWWorld::Ptr getFacedObject() override;
+        MWWorld::Ptr getFocusObject() override;
         ///< Return pointer to the object the player is looking at, if it is within activation range
 
-        float getDistanceToFacedObject() override;
+        float getDistanceToFocusObject() override;
 
         /// @note No-op for items in containers. Use ContainerStore::removeItem instead.
         void deleteObject(const Ptr& ptr) override;
@@ -419,7 +419,7 @@ namespace MWWorld
         void updatePhysics(
             float duration, bool paused, osg::Timer_t frameStart, unsigned int frameNumber, osg::Stats& stats);
 
-        void updateWindowManager();
+        void updateFocusObject();
 
         MWWorld::Ptr placeObject(
             const MWWorld::Ptr& object, float cursorX, float cursorY, int amount, bool copy = true) override;

apps/openmw/profile.hpp

@@ -37,7 +37,7 @@ namespace OMW
         PhysicsWorker,
         World,
         Gui,
-        WindowManager,
+        Focus,
         Lua,
         Number,
     };
@@ -73,7 +73,7 @@ namespace OMW
     inline const UserStats UserStatsValue<UserStatsType::World>::sValue{ "World", "world" };
 
     template <>
-    inline const UserStats UserStatsValue<UserStatsType::Gui>::sValue{ "Gui", "gui" };
+    inline const UserStats UserStatsValue<UserStatsType::Gui>::sValue{ "GUI", "gui" };
 
     template <>
     inline const UserStats UserStatsValue<UserStatsType::Lua>::sValue{ "Lua", "lua" };
@@ -82,7 +82,7 @@ namespace OMW
     inline const UserStats UserStatsValue<UserStatsType::LuaSyncUpdate>::sValue{ "LuaSync", "luasyncupdate" };
 
     template <>
-    inline const UserStats UserStatsValue<UserStatsType::WindowManager>::sValue{ "WindowManager", "windowmanager" };
+    inline const UserStats UserStatsValue<UserStatsType::Focus>::sValue{ "Focus", "focusobject" };
 
     template <UserStatsType type>
     struct ForEachUserStatsValue

components/CMakeLists.txt

@@ -321,7 +321,7 @@ ENDIF()
 add_component_dir (files
     linuxpath androidpath windowspath macospath fixedpath multidircollection collections configurationmanager
     constrainedfilestream memorystream hash configfileparser openfile constrainedfilestreambuf conversion
-    istreamptr streamwithbuffer
+    istreamptr streamwithbuffer utils
     )
 
 if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "MSVC" AND NOT CMAKE_CXX_COMPILER_FRONTEND_VARIANT STREQUAL "MSVC")
@@ -592,7 +592,6 @@ target_link_libraries(components
     ${OSG_LIBRARIES}
     ${OPENTHREADS_LIBRARIES}
 
-    Boost::system
     Boost::program_options
     Boost::iostreams
 

components/bsa/ba2dx10file.cpp

@@ -4,13 +4,15 @@
 #include <cassert>
 #include <cstring>
 #include <filesystem>
-#include <fstream>
+#include <format>
+#include <istream>
 
 #include <zlib.h>
 
 #include <components/esm/fourcc.hpp>
 #include <components/files/constrainedfilestream.hpp>
 #include <components/files/conversion.hpp>
+#include <components/files/utils.hpp>
 #include <components/misc/strings/lower.hpp>
 
 #include "ba2file.hpp"
@@ -73,19 +75,11 @@ namespace Bsa
     }
 
     /// Read header information from the input source
-    void BA2DX10File::readHeader()
+    void BA2DX10File::readHeader(std::istream& input)
     {
         assert(!mIsLoaded);
 
-        std::ifstream input(mFilepath, std::ios_base::binary);
-
-        // Total archive size
-        std::streamoff fsize = 0;
-        if (input.seekg(0, std::ios_base::end))
-        {
-            fsize = input.tellg();
-            input.seekg(0);
-        }
+        const std::streamsize fsize = Files::getStreamSizeLeft(input);
 
         if (fsize < 24) // header is 24 bytes
             fail("File too small to be a valid BSA archive");
@@ -135,23 +129,22 @@ namespace Bsa
             std::vector<char> fileName;
             uint16_t fileNameSize;
             input.read(reinterpret_cast<char*>(&fileNameSize), sizeof(uint16_t));
-            fileName.resize(fileNameSize);
-            input.read(fileName.data(), fileName.size());
-            fileName.push_back('\0');
+            fileName.resize(fileNameSize + 1);
+            input.read(fileName.data(), fileNameSize);
             mFileNames.push_back(std::move(fileName));
-            mFiles[i].setNameInfos(0, &mFileNames.back());
+            mFiles[i].mNameOffset = 0;
+            mFiles[i].mNameSize = fileNameSize;
+            mFiles[i].mNamesBuffer = &mFileNames.back();
         }
-
-        mIsLoaded = true;
     }
 
-    std::optional<BA2DX10File::FileRecord> BA2DX10File::getFileRecord(const std::string& str) const
+    std::optional<BA2DX10File::FileRecord> BA2DX10File::getFileRecord(std::string_view str) const
     {
         for (const auto c : str)
         {
             if (((static_cast<unsigned>(c) >> 7U) & 1U) != 0U)
             {
-                fail("File record " + str + " contains unicode characters, refusing to load.");
+                fail(std::format("File record {} contains unicode characters, refusing to load.", str));
             }
         }
 
@@ -161,7 +154,7 @@ namespace Bsa
         // Force-convert the path into something UNIX can handle first
         // to make sure std::filesystem::path doesn't think the entire path is the filename on Linux
         // and subsequently purge it to determine the file folder.
-        std::string path = str;
+        std::string path(str);
         std::replace(path.begin(), path.end(), '\\', '/');
 #endif
 

components/bsa/ba2dx10file.hpp

@@ -41,7 +41,7 @@ namespace Bsa
 
         std::list<std::vector<char>> mFileNames;
 
-        std::optional<FileRecord> getFileRecord(const std::string& str) const;
+        std::optional<FileRecord> getFileRecord(std::string_view str) const;
 
         Files::IStreamPtr getFile(const FileRecord& fileRecord);
 
@@ -57,7 +57,7 @@ namespace Bsa
         virtual ~BA2DX10File();
 
         /// Read header information from the input source
-        void readHeader() override;
+        void readHeader(std::istream& stream) override;
 
         Files::IStreamPtr getFile(const char* filePath);
         Files::IStreamPtr getFile(const FileStruct* fileStruct);

components/bsa/ba2gnrlfile.cpp

@@ -3,6 +3,7 @@
 #include <algorithm>
 #include <cassert>
 #include <filesystem>
+#include <format>
 #include <fstream>
 
 #include <zlib.h>
@@ -10,6 +11,7 @@
 #include <components/esm/fourcc.hpp>
 #include <components/files/constrainedfilestream.hpp>
 #include <components/files/conversion.hpp>
+#include <components/files/utils.hpp>
 #include <components/misc/strings/lower.hpp>
 
 #include "ba2file.hpp"
@@ -61,26 +63,18 @@ namespace Bsa
             mFolders[dirHash][{ nameHash, extHash }] = file;
 
             FileStruct fileStruct{};
-            fileStruct.fileSize = file.size;
-            fileStruct.offset = file.offset;
+            fileStruct.mFileSize = file.size;
+            fileStruct.mOffset = file.offset;
             mFiles.push_back(fileStruct);
         }
     }
 
     /// Read header information from the input source
-    void BA2GNRLFile::readHeader()
+    void BA2GNRLFile::readHeader(std::istream& input)
     {
         assert(!mIsLoaded);
 
-        std::ifstream input(mFilepath, std::ios_base::binary);
-
-        // Total archive size
-        std::streamoff fsize = 0;
-        if (input.seekg(0, std::ios_base::end))
-        {
-            fsize = input.tellg();
-            input.seekg(0);
-        }
+        const std::streamsize fsize = Files::getStreamSizeLeft(input);
 
         if (fsize < 24) // header is 24 bytes
             fail("File too small to be a valid BSA archive");
@@ -126,23 +120,22 @@ namespace Bsa
             std::vector<char> fileName;
             uint16_t fileNameSize;
             input.read(reinterpret_cast<char*>(&fileNameSize), sizeof(uint16_t));
-            fileName.resize(fileNameSize);
-            input.read(fileName.data(), fileName.size());
-            fileName.push_back('\0');
+            fileName.resize(fileNameSize + 1);
+            input.read(fileName.data(), fileNameSize);
             mFileNames.push_back(std::move(fileName));
-            mFiles[i].setNameInfos(0, &mFileNames.back());
+            mFiles[i].mNameOffset = 0;
+            mFiles[i].mNameSize = fileNameSize;
+            mFiles[i].mNamesBuffer = &mFileNames.back();
         }
-
-        mIsLoaded = true;
     }
 
-    BA2GNRLFile::FileRecord BA2GNRLFile::getFileRecord(const std::string& str) const
+    BA2GNRLFile::FileRecord BA2GNRLFile::getFileRecord(std::string_view str) const
     {
         for (const auto c : str)
         {
             if (((static_cast<unsigned>(c) >> 7U) & 1U) != 0U)
             {
-                fail("File record " + str + " contains unicode characters, refusing to load.");
+                fail(std::format("File record {} contains unicode characters, refusing to load.", str));
             }
         }
 
@@ -152,7 +145,7 @@ namespace Bsa
         // Force-convert the path into something UNIX can handle first
         // to make sure std::filesystem::path doesn't think the entire path is the filename on Linux
         // and subsequently purge it to determine the file folder.
-        std::string path = str;
+        std::string path(str);
         std::replace(path.begin(), path.end(), '\\', '/');
 #endif
 

components/bsa/ba2gnrlfile.hpp

@@ -29,7 +29,7 @@ namespace Bsa
 
         std::list<std::vector<char>> mFileNames;
 
-        FileRecord getFileRecord(const std::string& str) const;
+        FileRecord getFileRecord(std::string_view str) const;
 
         Files::IStreamPtr getFile(const FileRecord& fileRecord);
 
@@ -45,7 +45,7 @@ namespace Bsa
         virtual ~BA2GNRLFile();
 
         /// Read header information from the input source
-        void readHeader() override;
+        void readHeader(std::istream& input) override;
 
         Files::IStreamPtr getFile(const char* filePath);
         Files::IStreamPtr getFile(const FileStruct* fileStruct);

components/bsa/bsafile.cpp

@@ -25,12 +25,17 @@
 
 #include <algorithm>
 #include <cassert>
+#include <cerrno>
 #include <cstring>
 #include <filesystem>
+#include <format>
 #include <fstream>
+#include <istream>
+#include <system_error>
 
 #include <components/esm/fourcc.hpp>
 #include <components/files/constrainedfilestream.hpp>
+#include <components/files/utils.hpp>
 
 using namespace Bsa;
 
@@ -54,7 +59,7 @@ BSAFile::Hash getHash(const std::string& name)
         sum ^= (((unsigned)(name[i])) << (off & 0x1F));
         off += 8;
     }
-    hash.low = sum;
+    hash.mLow = sum;
 
     for (sum = off = 0; i < name.size(); i++)
     {
@@ -64,12 +69,12 @@ BSAFile::Hash getHash(const std::string& name)
         sum = (sum << (32 - n)) | (sum >> n); // binary "rotate right"
         off += 8;
     }
-    hash.high = sum;
+    hash.mHigh = sum;
     return hash;
 }
 
 /// Read header information from the input source
-void BSAFile::readHeader()
+void BSAFile::readHeader(std::istream& input)
 {
     /*
      * The layout of a BSA archive is as follows:
@@ -103,27 +108,24 @@ void BSAFile::readHeader()
      */
     assert(!mIsLoaded);
 
-    std::ifstream input(mFilepath, std::ios_base::binary);
-
     // Total archive size
-    std::streamoff fsize = 0;
-    if (input.seekg(0, std::ios_base::end))
-    {
-        fsize = input.tellg();
-        input.seekg(0);
-    }
+    const std::streamsize fsize = Files::getStreamSizeLeft(input);
 
     if (fsize < 12)
         fail("File too small to be a valid BSA archive");
 
     // Get essential header numbers
-    size_t dirsize, filenum;
+    std::streamsize dirsize;
+    std::streamsize filenum;
     {
         // First 12 bytes
         uint32_t head[3];
 
         input.read(reinterpret_cast<char*>(head), 12);
 
+        if (input.fail())
+            fail(std::format("Failed to read head: {}", std::generic_category().message(errno)));
+
         if (head[0] != 0x100)
             fail("Unrecognized BSA header");
 
@@ -138,62 +140,83 @@ void BSAFile::readHeader()
     // Each file must take up at least 21 bytes of data in the bsa. So
     // if files*21 overflows the file size then we are guaranteed that
     // the archive is corrupt.
-    if ((filenum * 21 > unsigned(fsize - 12)) || (dirsize + 8 * filenum > unsigned(fsize - 12)))
+    if (filenum * 21 > fsize - 12 || dirsize + 8 * filenum > fsize - 12)
         fail("Directory information larger than entire archive");
 
     // Read the offset info into a temporary buffer
     std::vector<uint32_t> offsets(3 * filenum);
     input.read(reinterpret_cast<char*>(offsets.data()), 12 * filenum);
 
+    if (input.fail())
+        fail(std::format("Failed to read offsets: {}", std::generic_category().message(errno)));
+
     // Read the string table
     mStringBuf.resize(dirsize - 12 * filenum);
     input.read(mStringBuf.data(), mStringBuf.size());
 
+    if (input.fail())
+        fail(std::format("Failed to read string table: {}", std::generic_category().message(errno)));
+
     // Check our position
     assert(input.tellg() == std::streampos(12 + dirsize));
     std::vector<Hash> hashes(filenum);
     static_assert(sizeof(Hash) == 8);
     input.read(reinterpret_cast<char*>(hashes.data()), 8 * filenum);
 
+    if (input.fail())
+        fail(std::format("Failed to read hashes: {}", std::generic_category().message(errno)));
+
     // Calculate the offset of the data buffer. All file offsets are
     // relative to this. 12 header bytes + directory + hash table
     // (skipped)
-    size_t fileDataOffset = 12 + dirsize + 8 * filenum;
+    const std::streamsize fileDataOffset = 12 + dirsize + 8 * filenum;
 
     // Set up the the FileStruct table
-    mFiles.resize(filenum);
+    mFiles.reserve(filenum);
     size_t endOfNameBuffer = 0;
-    for (size_t i = 0; i < filenum; i++)
+    for (std::streamsize i = 0; i < filenum; i++)
     {
-        FileStruct& fs = mFiles[i];
-        fs.fileSize = offsets[i * 2];
-        fs.offset = static_cast<uint32_t>(offsets[i * 2 + 1] + fileDataOffset);
-        auto namesOffset = offsets[2 * filenum + i];
-        fs.setNameInfos(namesOffset, &mStringBuf);
-        fs.hash = hashes[i];
+        const uint32_t fileSize = offsets[i * 2];
+        const std::streamsize offset = static_cast<std::streamsize>(offsets[i * 2 + 1]) + fileDataOffset;
 
-        if (namesOffset >= mStringBuf.size())
-        {
+        if (fileSize + offset > fsize)
+            fail(std::format("Archive contains offsets outside itself: {} + {} > {}", fileSize, offset, fsize));
+
+        if (offset > std::numeric_limits<uint32_t>::max())
+            fail(std::format(
+                "Absolute file {} offset is too large: {} > {}", i, offset, std::numeric_limits<uint32_t>::max()));
+
+        const uint32_t nameOffset = offsets[2 * filenum + i];
+
+        if (nameOffset >= mStringBuf.size())
             fail("Archive contains names offset outside itself");
-        }
-        const void* end = std::memchr(fs.name(), '\0', mStringBuf.size() - namesOffset);
-        if (!end)
-        {
+
+        const char* const begin = mStringBuf.data() + nameOffset;
+        const char* const end = reinterpret_cast<const char*>(std::memchr(begin, '\0', mStringBuf.size() - nameOffset));
+
+        if (end == nullptr)
             fail("Archive contains non-zero terminated string");
-        }
 
-        endOfNameBuffer = std::max(endOfNameBuffer, namesOffset + std::strlen(fs.name()) + 1);
+        const std::size_t nameSize = end - begin;
+
+        FileStruct fs;
+
+        fs.mFileSize = fileSize;
+        fs.mOffset = static_cast<uint32_t>(offset);
+        fs.mHash = hashes[i];
+        fs.mNameOffset = nameOffset;
+        fs.mNameSize = static_cast<uint32_t>(nameSize);
+        fs.mNamesBuffer = &mStringBuf;
+
+        mFiles.push_back(fs);
+
+        endOfNameBuffer = std::max(endOfNameBuffer, nameOffset + nameSize + 1);
         assert(endOfNameBuffer <= mStringBuf.size());
-
-        if (fs.offset + fs.fileSize > fsize)
-            fail("Archive contains offsets outside itself");
     }
     mStringBuf.resize(endOfNameBuffer);
 
     std::sort(mFiles.begin(), mFiles.end(),
-        [](const FileStruct& left, const FileStruct& right) { return left.offset < right.offset; });
-
-    mIsLoaded = true;
+        [](const FileStruct& left, const FileStruct& right) { return left.mOffset < right.mOffset; });
 }
 
 /// Write header information to the output sink
@@ -203,7 +226,7 @@ void Bsa::BSAFile::writeHeader()
 
     uint32_t head[3];
     head[0] = 0x100;
-    auto fileDataOffset = mFiles.empty() ? 12 : mFiles.front().offset;
+    auto fileDataOffset = mFiles.empty() ? 12 : mFiles.front().mOffset;
     head[1] = static_cast<uint32_t>(fileDataOffset - 12 - 8 * mFiles.size());
 
     output.seekp(0, std::ios_base::end);
@@ -213,7 +236,7 @@ void Bsa::BSAFile::writeHeader()
     output.write(reinterpret_cast<char*>(head), 12);
 
     std::sort(mFiles.begin(), mFiles.end(), [](const FileStruct& left, const FileStruct& right) {
-        return std::make_pair(left.hash.low, left.hash.high) < std::make_pair(right.hash.low, right.hash.high);
+        return std::make_pair(left.mHash.mLow, left.mHash.mHigh) < std::make_pair(right.mHash.mLow, right.mHash.mHigh);
     });
 
     size_t filenum = mFiles.size();
@@ -222,10 +245,10 @@ void Bsa::BSAFile::writeHeader()
     for (size_t i = 0; i < filenum; i++)
     {
         auto& f = mFiles[i];
-        offsets[i * 2] = f.fileSize;
-        offsets[i * 2 + 1] = f.offset - fileDataOffset;
-        offsets[2 * filenum + i] = f.namesOffset;
-        hashes[i] = f.hash;
+        offsets[i * 2] = f.mFileSize;
+        offsets[i * 2 + 1] = f.mOffset - fileDataOffset;
+        offsets[2 * filenum + i] = f.mNameOffset;
+        hashes[i] = f.mHash;
     }
     output.write(reinterpret_cast<char*>(offsets.data()), sizeof(uint32_t) * offsets.size());
     output.write(reinterpret_cast<char*>(mStringBuf.data()), mStringBuf.size());
@@ -241,7 +264,11 @@ void BSAFile::open(const std::filesystem::path& file)
 
     mFilepath = file;
     if (std::filesystem::exists(file))
-        readHeader();
+    {
+        std::ifstream input(mFilepath, std::ios_base::binary);
+        readHeader(input);
+        mIsLoaded = true;
+    }
     else
     {
         {
@@ -265,7 +292,7 @@ void Bsa::BSAFile::close()
 
 Files::IStreamPtr Bsa::BSAFile::getFile(const FileStruct* file)
 {
-    return Files::openConstrainedFileStream(mFilepath, file->offset, file->fileSize);
+    return Files::openConstrainedFileStream(mFilepath, file->mOffset, file->mFileSize);
 }
 
 void Bsa::BSAFile::addFile(const std::string& filename, std::istream& file)
@@ -281,37 +308,41 @@ void Bsa::BSAFile::addFile(const std::string& filename, std::istream& file)
 
     FileStruct newFile;
     file.seekg(0, std::ios::end);
-    newFile.fileSize = static_cast<uint32_t>(file.tellg());
-    newFile.setNameInfos(mStringBuf.size(), &mStringBuf);
-    newFile.hash = getHash(filename);
+    newFile.mFileSize = static_cast<uint32_t>(file.tellg());
+    newFile.mHash = getHash(filename);
 
     if (mFiles.empty())
-        newFile.offset = static_cast<uint32_t>(newStartOfDataBuffer);
+        newFile.mOffset = static_cast<uint32_t>(newStartOfDataBuffer);
     else
     {
         std::vector<char> buffer;
-        while (mFiles.front().offset < newStartOfDataBuffer)
+        while (mFiles.front().mOffset < newStartOfDataBuffer)
         {
             FileStruct& firstFile = mFiles.front();
-            buffer.resize(firstFile.fileSize);
+            buffer.resize(firstFile.mFileSize);
 
-            stream.seekg(firstFile.offset, std::ios::beg);
-            stream.read(buffer.data(), firstFile.fileSize);
+            stream.seekg(firstFile.mOffset, std::ios::beg);
+            stream.read(buffer.data(), firstFile.mFileSize);
 
             stream.seekp(0, std::ios::end);
-            firstFile.offset = static_cast<uint32_t>(stream.tellp());
+            firstFile.mOffset = static_cast<uint32_t>(stream.tellp());
 
-            stream.write(buffer.data(), firstFile.fileSize);
+            stream.write(buffer.data(), firstFile.mFileSize);
 
             // ensure sort order is preserved
             std::rotate(mFiles.begin(), mFiles.begin() + 1, mFiles.end());
         }
         stream.seekp(0, std::ios::end);
-        newFile.offset = static_cast<uint32_t>(stream.tellp());
+        newFile.mOffset = static_cast<uint32_t>(stream.tellp());
     }
 
+    newFile.mNameOffset = mStringBuf.size();
+    newFile.mNameSize = filename.size();
+    newFile.mNamesBuffer = &mStringBuf;
+
     mStringBuf.insert(mStringBuf.end(), filename.begin(), filename.end());
     mStringBuf.push_back('\0');
+
     mFiles.push_back(newFile);
 
     mHasChanged = true;

components/bsa/bsafile.hpp

@@ -26,6 +26,7 @@
 
 #include <cstdint>
 #include <filesystem>
+#include <iosfwd>
 #include <string>
 #include <vector>
 
@@ -54,30 +55,25 @@ namespace Bsa
 #pragma pack(1)
         struct Hash
         {
-            uint32_t low, high;
+            uint32_t mLow;
+            uint32_t mHigh;
         };
 #pragma pack(pop)
 
         /// Represents one file entry in the archive
         struct FileStruct
         {
-            void setNameInfos(size_t index, std::vector<char>* stringBuf)
-            {
-                namesOffset = static_cast<uint32_t>(index);
-                namesBuffer = stringBuf;
-            }
-
             // File size and offset in file. We store the offset from the
             // beginning of the file, not the offset into the data buffer
             // (which is what is stored in the archive.)
-            uint32_t fileSize, offset;
-            Hash hash;
+            uint32_t mFileSize = 0;
+            uint32_t mOffset = 0;
+            Hash mHash{};
+            uint32_t mNameOffset = 0;
+            uint32_t mNameSize = 0;
+            std::vector<char>* mNamesBuffer = nullptr;
 
-            // Zero-terminated file name
-            const char* name() const { return &(*namesBuffer)[namesOffset]; }
-
-            uint32_t namesOffset = 0;
-            std::vector<char>* namesBuffer = nullptr;
+            std::string_view name() const { return std::string_view(mNamesBuffer->data() + mNameOffset, mNameSize); }
         };
         typedef std::vector<FileStruct> FileList;
 
@@ -100,7 +96,7 @@ namespace Bsa
         [[noreturn]] void fail(const std::string& msg) const;
 
         /// Read header information from the input source
-        virtual void readHeader();
+        virtual void readHeader(std::istream& input);
         virtual void writeHeader();
 
     public:
@@ -151,7 +147,6 @@ namespace Bsa
         // checks version of BSA from file header
         static BsaVersion detectVersion(const std::filesystem::path& filePath);
     };
-
 }
 
 #endif

components/bsa/compressedbsafile.cpp

@@ -26,14 +26,18 @@
 
 #include <algorithm>
 #include <cassert>
+#include <cerrno>
 #include <filesystem>
-#include <fstream>
+#include <format>
+#include <istream>
+#include <system_error>
 
 #include <lz4frame.h>
 #include <zlib.h>
 
 #include <components/files/constrainedfilestream.hpp>
 #include <components/files/conversion.hpp>
+#include <components/files/utils.hpp>
 #include <components/misc/strings/lower.hpp>
 
 #include "memorystream.hpp"
@@ -41,19 +45,11 @@
 namespace Bsa
 {
     /// Read header information from the input source
-    void CompressedBSAFile::readHeader()
+    void CompressedBSAFile::readHeader(std::istream& input)
     {
         assert(!mIsLoaded);
 
-        std::ifstream input(mFilepath, std::ios_base::binary);
-
-        // Total archive size
-        std::streamoff fsize = 0;
-        if (input.seekg(0, std::ios_base::end))
-        {
-            fsize = input.tellg();
-            input.seekg(0);
-        }
+        const std::streamsize fsize = Files::getStreamSizeLeft(input);
 
         if (fsize < 36) // Header is 36 bytes
             fail("File too small to be a valid BSA archive");
@@ -69,8 +65,8 @@ namespace Bsa
             mHeader.mFlags &= (~ArchiveFlag_EmbeddedNames);
 
         input.seekg(mHeader.mFoldersOffset);
-        if (input.bad())
-            fail("Invalid compressed BSA folder record offset");
+        if (input.fail())
+            fail("Failed to read compressed BSA folder record offset: " + std::generic_category().message(errno));
 
         struct FlatFolderRecord
         {
@@ -81,9 +77,12 @@ namespace Bsa
         };
 
         std::vector<std::pair<FlatFolderRecord, std::vector<FileRecord>>> folders;
-        folders.resize(mHeader.mFolderCount);
-        for (auto& [folder, filelist] : folders)
+        folders.reserve(mHeader.mFolderCount);
+
+        for (std::uint32_t i = 0; i < mHeader.mFolderCount; ++i)
         {
+            FlatFolderRecord folder;
+
             input.read(reinterpret_cast<char*>(&folder.mHash), 8);
             input.read(reinterpret_cast<char*>(&folder.mCount), 4);
             if (mHeader.mVersion == Version_SSE) // SSE
@@ -96,10 +95,13 @@ namespace Bsa
             {
                 input.read(reinterpret_cast<char*>(&folder.mOffset), 4);
             }
-        }
 
-        if (input.bad())
-            fail("Failed to read compressed BSA folder records: input error");
+            if (input.fail())
+                fail(std::format(
+                    "Failed to read compressed BSA folder record: {}", std::generic_category().message(errno)));
+
+            folders.emplace_back(std::move(folder), std::vector<FileRecord>());
+        }
 
         // file record blocks
         if ((mHeader.mFlags & ArchiveFlag_FolderNames) == 0)
@@ -126,20 +128,29 @@ namespace Bsa
                 mHeader.mFolderNamesLength -= size;
             }
 
-            filelist.resize(folder.mCount);
-            for (auto& file : filelist)
+            filelist.reserve(folder.mCount);
+
+            for (std::uint32_t i = 0; i < folder.mCount; ++i)
             {
+                FileRecord file;
+
                 input.read(reinterpret_cast<char*>(&file.mHash), 8);
                 input.read(reinterpret_cast<char*>(&file.mSize), 4);
                 input.read(reinterpret_cast<char*>(&file.mOffset), 4);
+
+                if (input.fail())
+                    fail(std::format("Failed to read compressed BSA folder file record: {}",
+                        std::generic_category().message(errno)));
+
+                filelist.push_back(std::move(file));
             }
         }
 
         if (mHeader.mFolderNamesLength != 0)
             input.ignore(mHeader.mFolderNamesLength);
 
-        if (input.bad())
-            fail("Failed to read compressed BSA file records: input error");
+        if (input.fail())
+            fail(std::format("Failed to read compressed BSA file records: {}", std::generic_category().message(errno)));
 
         if ((mHeader.mFlags & ArchiveFlag_FileNames) != 0)
         {
@@ -168,36 +179,42 @@ namespace Bsa
         if (mHeader.mFileNamesLength != 0)
             input.ignore(mHeader.mFileNamesLength);
 
-        if (input.bad())
-            fail("Failed to read compressed BSA filenames: input error");
+        if (input.fail())
+            fail(std::format("Failed to read compressed BSA filenames: {}", std::generic_category().message(errno)));
 
         for (auto& [folder, filelist] : folders)
         {
             std::map<std::uint64_t, FileRecord> fileMap;
-            for (const auto& file : filelist)
+
+            for (auto& file : filelist)
                 fileMap[file.mHash] = std::move(file);
-            auto& folderMap = mFolders[folder.mHash];
-            folderMap = FolderRecord{ folder.mCount, folder.mOffset, std::move(fileMap) };
-            for (auto& [hash, fileRec] : folderMap.mFiles)
-            {
-                FileStruct fileStruct{};
-                fileStruct.fileSize = fileRec.mSize & (~FileSizeFlag_Compression);
-                fileStruct.offset = fileRec.mOffset;
-                fileStruct.setNameInfos(0, &fileRec.mName);
-                mFiles.emplace_back(fileStruct);
-            }
+
+            mFolders[folder.mHash] = FolderRecord{ folder.mCount, folder.mOffset, folder.mName, std::move(fileMap) };
         }
 
-        mIsLoaded = true;
+        for (auto& [folderHash, folderRecord] : mFolders)
+        {
+            for (auto& [fileHash, fileRecord] : folderRecord.mFiles)
+            {
+                FileStruct fileStruct{};
+                fileStruct.mFileSize = fileRecord.mSize & (~FileSizeFlag_Compression);
+                fileStruct.mOffset = fileRecord.mOffset;
+                fileStruct.mNameOffset = 0;
+                fileStruct.mNameSize
+                    = fileRecord.mName.empty() ? 0 : static_cast<uint32_t>(fileRecord.mName.size() - 1);
+                fileStruct.mNamesBuffer = &fileRecord.mName;
+                mFiles.push_back(fileStruct);
+            }
+        }
     }
 
-    CompressedBSAFile::FileRecord CompressedBSAFile::getFileRecord(const std::string& str) const
+    CompressedBSAFile::FileRecord CompressedBSAFile::getFileRecord(std::string_view str) const
     {
         for (const auto c : str)
         {
             if (((static_cast<unsigned>(c) >> 7U) & 1U) != 0U)
             {
-                fail("File record " + str + " contains unicode characters, refusing to load.");
+                fail(std::format("File record {} contains unicode characters, refusing to load.", str));
             }
         }
 
@@ -207,7 +224,7 @@ namespace Bsa
         // Force-convert the path into something UNIX can handle first
         // to make sure std::filesystem::path doesn't think the entire path is the filename on Linux
         // and subsequently purge it to determine the file folder.
-        std::string path = str;
+        std::string path(str);
         std::replace(path.begin(), path.end(), '\\', '/');
 #endif
 

components/bsa/compressedbsafile.hpp

@@ -36,7 +36,7 @@ namespace Bsa
 {
     class CompressedBSAFile : private BSAFile
     {
-    private:
+    public:
         enum ArchiveFlags
         {
             ArchiveFlag_FolderNames = 0x0001,
@@ -89,8 +89,6 @@ namespace Bsa
             std::uint32_t mFileFlags;
         };
 
-        Header mHeader;
-
         struct FileRecord
         {
             std::uint64_t mHash;
@@ -103,12 +101,15 @@ namespace Bsa
         {
             std::uint32_t mCount;
             std::int64_t mOffset;
+            std::string mName;
             std::map<std::uint64_t, FileRecord> mFiles;
         };
 
+    private:
+        Header mHeader;
         std::map<std::uint64_t, FolderRecord> mFolders;
 
-        FileRecord getFileRecord(const std::string& str) const;
+        FileRecord getFileRecord(std::string_view str) const;
 
         /// \brief Normalizes given filename or folder and generates format-compatible hash.
         static std::uint64_t generateHash(const std::filesystem::path& stem, std::string extension);
@@ -124,7 +125,7 @@ namespace Bsa
         virtual ~CompressedBSAFile() = default;
 
         /// Read header information from the input source
-        void readHeader() override;
+        void readHeader(std::istream& input) override;
 
         Files::IStreamPtr getFile(const char* filePath);
         Files::IStreamPtr getFile(const FileStruct* fileStruct);

components/esm3/activespells.hpp

@@ -26,7 +26,8 @@ namespace ESM
             Flag_Remove = 1 << 1,
             Flag_Ignore_Resistances = 1 << 2,
             Flag_Ignore_Reflect = 1 << 3,
-            Flag_Ignore_SpellAbsorption = 1 << 4
+            Flag_Ignore_SpellAbsorption = 1 << 4,
+            Flag_Invalid = 1 << 5
         };
 
         int32_t mEffectId;

components/files/qtconversion.cpp

@@ -1,8 +1,9 @@
-
 #include "qtconversion.hpp"
 
 #include <components/misc/strings/conversion.hpp>
 
+#include <string_view>
+
 QString Files::pathToQString(const std::filesystem::path& path)
 {
     const auto tmp = path.u8string();
@@ -17,12 +18,10 @@ QString Files::pathToQString(std::filesystem::path&& path)
 
 std::filesystem::path Files::pathFromQString(QStringView path)
 {
-    const auto tmp = path.toUtf8();
-    return std::filesystem::path{ Misc::StringUtils::stringToU8String(tmp) };
+    return std::filesystem::path(std::u16string_view(path.utf16(), path.size()));
 }
 
 std::filesystem::path Files::pathFromQString(QString&& path)
 {
-    const auto tmp = path.toUtf8();
-    return std::filesystem::path{ Misc::StringUtils::stringToU8String(tmp) };
+    return std::filesystem::path(path.toStdU16String());
 }

components/files/utils.hpp

@@ -0,0 +1,38 @@
+#ifndef COMPONENTS_FILES_UTILS_H
+#define COMPONENTS_FILES_UTILS_H
+
+#include <cerrno>
+#include <format>
+#include <istream>
+#include <stdexcept>
+#include <system_error>
+
+namespace Files
+{
+    inline std::streamsize getStreamSizeLeft(std::istream& stream)
+    {
+        const auto begin = stream.tellg();
+        if (stream.fail())
+            throw std::runtime_error(
+                std::format("Failed to get current file position: {}", std::generic_category().message(errno)));
+
+        stream.seekg(0, std::ios_base::end);
+        if (stream.fail())
+            throw std::runtime_error(
+                std::format("Failed to seek end file position: {}", std::generic_category().message(errno)));
+
+        const auto end = stream.tellg();
+        if (stream.fail())
+            throw std::runtime_error(
+                std::format("Failed to get current file position: {}", std::generic_category().message(errno)));
+
+        stream.seekg(begin);
+        if (stream.fail())
+            throw std::runtime_error(
+                std::format("Failed to seek original file position: {}", std::generic_category().message(errno)));
+
+        return end - begin;
+    }
+}
+
+#endif

components/lua/util.hpp

@@ -20,13 +20,15 @@ namespace LuaUtil
     {
         return i + 1;
     }
-
-    inline sol::optional<std::string> serializeRefId(ESM::RefId id)
-    {
-        if (id.empty())
-            return sol::nullopt;
-        return id.serializeText();
-    }
 }
 
-#endif
+// ADL-based customization point for sol2 to automatically convert ESM::RefId
+// Empty RefIds are converted to nil, non-empty ones are serialized to strings
+inline int sol_lua_push(sol::types<ESM::RefId>, lua_State* L, const ESM::RefId& id)
+{
+    if (id.empty())
+        return sol::stack::push(L, sol::lua_nil);
+    return sol::stack::push(L, id.serializeText());
+}
+
+#endif

components/lua_ui/element.cpp

@@ -296,6 +296,7 @@ namespace LuaUi
                     children.erase(it);
                     parent->setChildren(children);
                     mRoot = nullptr;
+                    mState = New;
                     throw;
                 }
                 parent->setChildren(children);

components/lua_ui/widget.cpp

@@ -105,7 +105,7 @@ namespace LuaUi
             if (ext->mParent)
             {
                 auto children = ext->mParent->children();
-                std::erase(children, this);
+                std::erase(children, ext);
                 ext->mParent->setChildren(children);
             }
             ext->detachFromParent();

components/vfs/qtconversion.cpp

@@ -1,8 +1,9 @@
-
 #include "qtconversion.hpp"
 
 #include <components/misc/strings/conversion.hpp>
 
+#include <string_view>
+
 QString VFS::Path::normalizedToQString(NormalizedView path)
 {
     return QString::fromUtf8(path.value().data(), path.value().size());
@@ -15,12 +16,12 @@ QString VFS::Path::normalizedToQString(Normalized&& path)
 
 VFS::Path::Normalized VFS::Path::normalizedFromQString(QStringView path)
 {
-    const auto tmp = path.toUtf8();
-    return Normalized{ tmp };
+    const QByteArray tmp = path.toUtf8();
+    return Normalized(std::string_view(tmp.constData(), tmp.size()));
 }
 
 VFS::Path::Normalized VFS::Path::normalizedFromQString(QString&& path)
 {
-    const auto tmp = path.toUtf8();
-    return Normalized{ tmp };
+    const QByteArray tmp = std::move(path).toUtf8();
+    return Normalized(std::string_view(tmp.constData(), tmp.size()));
 }

docs/source/manuals/openmw-cs/record-filters.rst

@@ -241,7 +241,7 @@ Logical Expressions
    In this particular case only one argument can evaluate to ``true``, but one
    can write expressions where multiple arguments can be ``true`` at a time.
 
-``or(<expr1>, <expr2>, ..., <exprN>)``
+``and(<expr1>, <expr2>, ..., <exprN>)``
    Logical conjunction, evaluates to ``true`` if and only if all arguments
    evaluate to ``true`` as well, otherwise the expression evaluates to
    ``false``.

docs/source/reference/modding/extended.rst

@@ -165,6 +165,7 @@ The minimum you need is the ``xbase_anim_sh.nif`` file from the `Weapon Sheathin
 
     [Game]
     weapon sheathing = true
+    use additional anim sources = true
 
 The ``xbase_anim_sh.nif`` contains default placement points for different weapon types.
 That way you'll get Gothic-style weapon sheathing for all biped actors (without quivers and scabbards).
@@ -204,6 +205,28 @@ It is important to make sure the names of empty nodes start with ``"Bip01 "``, o
 An example of a mod which uses this feature is `Weapon Sheathing`_.
 
 
+Shield sheathing support
+------------------------
+
+The minimum you need is the ``xbase_anim_sh.nif`` file from the `Weapon Sheathing`_ mod and this line in your settings.cfg:
+
+.. code-block:: ini
+    :caption: settings.cfg
+
+    [Game]
+    shield sheathing = true
+    use additional anim sources = true
+
+The ``xbase_anim_sh.nif`` contains default placement points for shields (a ``"Bip01 AttachShield"`` node). 
+You also may use meshes with ``_sh`` suffix (with ``Bip01 Sheath`` node) to tweak how particular shield looks in the sheathed mode. A stub sheath means that the shield should be excluded from this feature. 
+When a two-handed weapon is equipped, a shield is hidden when this feature is enabled.
+This feature also supports shield equipping and unequipping animations. It is a ``Shield`` group (with ``Equip Start``, ``Equip Attach``, ``Equip Stop``, ``Unequip Start``, ``Unequip Attach`` and ``Unequip Stop`` keys).
+Note that equip and unequip animation blocks should not overlap each other and weapon equip/unequip animations. 
+Basically, you need to avoid situations when you play an animation block where you need both to attach and detach the shield.
+
+An example of a mod which uses this feature is `Weapon Sheathing`_.
+
+
 Skeleton extensions
 -------------------
 

files/lua_api/openmw/core.lua

@@ -297,13 +297,14 @@
 -- A cell of the game world.
 -- @type Cell
 -- @field #string name Name of the cell (can be empty string).
+-- @field #string displayName Human-readable cell name (takes into account *.cel file localizations). Can be empty string.
 -- @field #string id Unique record ID of the cell, based on cell name for interiors and the worldspace for exteriors, or the formID of the cell for ESM4 cells.
--- @field #string region Region of the cell.
+-- @field #string region Region of the cell (can be nil).
 -- @field #boolean isExterior Whether the cell is an exterior cell. "Exterior" means grid of cells where the player can seamless walk from one cell to another without teleports. QuasiExterior (interior with sky) is not an exterior.
 -- @field #boolean isQuasiExterior (DEPRECATED, use `hasTag("QuasiExterior")`) Whether the cell is a quasi exterior (like interior but with the sky and the weather).
 -- @field #number gridX Index of the cell by X (only for exteriors).
 -- @field #number gridY Index of the cell by Y (only for exteriors).
--- @field #string worldSpaceId Id of the world space.
+-- @field #string worldSpaceId Id of the world space (can be nil).
 -- @field #boolean hasWater True if the cell contains water.
 -- @field #number waterLevel The water level of the cell. (nil if cell has no water).
 -- @field #boolean hasSky True if in this cell sky should be rendered.
@@ -1305,24 +1306,24 @@
 -- Weather data
 -- @type WeatherRecord
 -- @field #string recordId
--- @field #number scriptId
--- @field #string name
--- @field #number windSpeed
+-- @field #number scriptId Read-only ID used in mwscript and dialogue
+-- @field #string name Read-only weather name
+-- @field #number windSpeed Affects the angle of falling rain
 -- @field #number cloudSpeed
 -- @field #string cloudTexture
--- @field #number cloudsMaximumPercent
--- @field #boolean isStorm
+-- @field #number cloudsMaximumPercent Affects the speed of weather transitions (0, 1]
+-- @field #boolean isStorm Controls whether the weather is considered a storm for animation and movement purposes
 -- @field openmw.util#Vector3 stormDirection
--- @field #number glareView
--- @field #number rainSpeed
--- @field #number rainEntranceSpeed
+-- @field #number glareView Strength of the sun glare [0, 1]
+-- @field #number rainSpeed The speed at which rain falls
+-- @field #number rainEntranceSpeed The number of seconds between rain particle batches being created
 -- @field #string rainEffect Will return nil if weather has no rainEffect
--- @field #number rainMaxRaindrops
--- @field #number rainDiameter
--- @field #number rainMaxHeight
--- @field #number rainMinHeight
+-- @field #number rainMaxRaindrops The maximum number of rain particle batches to create every rainEntranceSpeed
+-- @field #number rainDiameter The area around the player to spawn rain in
+-- @field #number rainMaxHeight The maximum height relative to the player to spawn rain at
+-- @field #number rainMinHeight The minimum height relative to the player to spawn rain at
 -- @field #string rainLoopSoundID
--- @field #table  thunderSoundID An array containing the recordIds of the thunder sounds
+-- @field #table thunderSoundID A read-only array containing the recordIds of the thunder sounds
 -- @field #string ambientLoopSoundID
 -- @field #string particleEffect Will return nil if weather has no particleEffect
 -- @field #number distantLandFogFactor

files/lua_api/openmw/world.lua

@@ -166,18 +166,19 @@
 -- potion = world.createObject('Generated:0x0', 1)
 
 ---
--- Creates a custom record in the world database; String ids that came from ESM3 content files are lower-cased.
+-- Creates a custom record in the world database; string IDs that came from ESM3 content files are lower-cased.
 -- Eventually meant to support all records, but the current
 -- set of supported types is limited to:
 --
--- * @{openmw.types#PotionRecord},
+-- * @{openmw.types#ActivatorRecord},
 -- * @{openmw.types#ArmorRecord},
 -- * @{openmw.types#BookRecord},
--- * @{openmw.types#MiscellaneousRecord},
 -- * @{openmw.types#ClothingRecord},
--- * @{openmw.types#WeaponRecord},
--- * @{openmw.types#ActivatorRecord},
--- * @{openmw.types#LightRecord}
+-- * @{openmw.types#LightRecord},
+-- * @{openmw.types#MiscellaneousRecord},
+-- * @{openmw.types#NpcRecord},
+-- * @{openmw.types#PotionRecord},
+-- * @{openmw.types#WeaponRecord}
 -- @function [parent=#world] createRecord
 -- @param #any record A record to be registered in the database. Must be one of the supported types. The id field is not used, one will be generated for you.
 -- @return #any A new record added to the database. The type is the same as the input's.

scripts/data/integration_tests/test_lua_api/global.lua

@@ -7,6 +7,13 @@ local vfs = require('openmw.vfs')
 local world = require('openmw.world')
 local I = require('openmw.interfaces')
 
+testing.registerGlobalTest('crash in lua coroutine when accessing type (#8757)', function()
+    local co = coroutine.wrap(function()
+        testing.expectEqual(tostring(world.players[1].type), 'Player')
+    end)
+    co()
+end)
+
 testing.registerGlobalTest('timers', function()
     testing.expectAlmostEqual(core.getGameTimeScale(), 30, 'incorrect getGameTimeScale() result')
     testing.expectAlmostEqual(core.getSimulationTimeScale(), 1, 'incorrect getSimulationTimeScale result')

scripts/data/integration_tests/test_lua_api/menu.lua

@@ -61,6 +61,7 @@ local function registerGlobalTest(name, description)
    end)
 end
 
+registerGlobalTest('crash in lua coroutine when accessing type (#8757)')
 registerGlobalTest('timers')
 registerGlobalTest('teleport')
 registerGlobalTest('getGMST')