From 32f3a16db333806ed5b76736ade3eb5189d542dd Mon Sep 17 00:00:00 2001 From: Petr Mikheev Date: Sat, 26 Aug 2023 13:46:43 +0200 Subject: [PATCH] Change FormId::toString to be consistent with RefId. Remove FormIdRefId. --- apps/opencs/model/world/columnimp.cpp | 2 + apps/openmw/mwlua/luabindings.cpp | 4 +- apps/openmw/mwlua/nearbybindings.cpp | 4 +- apps/openmw_test_suite/esm/testrefid.cpp | 37 ++++++++--------- components/CMakeLists.txt | 1 - components/esm/formid.cpp | 29 ++++++++++++- components/esm/formid.hpp | 2 +- components/esm/formidrefid.cpp | 38 ----------------- components/esm/formidrefid.hpp | 52 ------------------------ components/esm/refid.cpp | 31 +++++--------- components/esm/refid.hpp | 22 ++++------ components/esm3/esmreader.cpp | 2 +- components/esm3/esmwriter.cpp | 4 +- 13 files changed, 71 insertions(+), 157 deletions(-) delete mode 100644 components/esm/formidrefid.cpp delete mode 100644 components/esm/formidrefid.hpp diff --git a/apps/opencs/model/world/columnimp.cpp b/apps/opencs/model/world/columnimp.cpp index b514550479..981ec5278d 100644 --- a/apps/opencs/model/world/columnimp.cpp +++ b/apps/opencs/model/world/columnimp.cpp @@ -22,6 +22,8 @@ namespace CSMWorld std::string operator()(ESM::StringRefId value) const { return value.getValue(); } + std::string operator()(ESM::FormId value) const { return value.toString("FormId:"); } + std::string operator()(ESM::IndexRefId value) const { switch (value.getRecordType()) diff --git a/apps/openmw/mwlua/luabindings.cpp b/apps/openmw/mwlua/luabindings.cpp index 25a232bf6c..65b6977982 100644 --- a/apps/openmw/mwlua/luabindings.cpp +++ b/apps/openmw/mwlua/luabindings.cpp @@ -268,9 +268,9 @@ namespace MWLua }; api["getObjectByFormId"] = [](std::string_view formIdStr) -> GObject { ESM::RefId refId = ESM::RefId::deserializeText(formIdStr); - if (!refId.is()) + if (!refId.is()) throw std::runtime_error("FormId expected, got " + std::string(formIdStr) + "; use core.getFormId"); - return GObject(refId.getIf()->getValue()); + return GObject(*refId.getIf()); }; // Creates a new record in the world database. diff --git a/apps/openmw/mwlua/nearbybindings.cpp b/apps/openmw/mwlua/nearbybindings.cpp index c4c2521c80..86c8ef31e8 100644 --- a/apps/openmw/mwlua/nearbybindings.cpp +++ b/apps/openmw/mwlua/nearbybindings.cpp @@ -126,9 +126,9 @@ namespace MWLua api["getObjectByFormId"] = [](std::string_view formIdStr) -> LObject { ESM::RefId refId = ESM::RefId::deserializeText(formIdStr); - if (!refId.is()) + if (!refId.is()) throw std::runtime_error("FormId expected, got " + std::string(formIdStr) + "; use core.getFormId"); - return LObject(refId.getIf()->getValue()); + return LObject(*refId.getIf()); }; api["activators"] = LObjectList{ objectLists->getActivatorsInScene() }; diff --git a/apps/openmw_test_suite/esm/testrefid.cpp b/apps/openmw_test_suite/esm/testrefid.cpp index 4e9d297bc4..168f9e0b9e 100644 --- a/apps/openmw_test_suite/esm/testrefid.cpp +++ b/apps/openmw_test_suite/esm/testrefid.cpp @@ -9,6 +9,8 @@ #include #include +#include "../testing_util.hpp" + MATCHER(IsPrint, "") { return std::isprint(arg) != 0; @@ -38,6 +40,12 @@ namespace ESM EXPECT_FALSE(refId.empty()); } + TEST(ESMRefIdTest, FormIdRefIdMustHaveContentFile) + { + EXPECT_TRUE(RefId(FormId()).empty()); + EXPECT_ERROR(RefId(FormId{ .mIndex = 1, .mContentFile = -1 }), "RefId can't be a generated FormId"); + } + TEST(ESMRefIdTest, defaultConstructedIsEqualToItself) { const RefId refId; @@ -104,11 +112,10 @@ namespace ESM EXPECT_EQ(stringRefId, refId); } - TEST(ESMRefIdTest, equalityIsDefinedForFormRefIdAndRefId) + TEST(ESMRefIdTest, equalityIsDefinedForFormIdAndRefId) { - const FormIdRefId formIdRefId({ .mIndex = 42, .mContentFile = 0 }); - const RefId refId = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); - EXPECT_EQ(formIdRefId, refId); + const FormId formId{ .mIndex = 42, .mContentFile = 0 }; + EXPECT_EQ(formId, RefId(formId)); } TEST(ESMRefIdTest, stringRefIdIsEqualToItself) @@ -139,9 +146,9 @@ namespace ESM TEST(ESMRefIdTest, lessThanIsDefinedForFormRefIdAndRefId) { - const FormIdRefId formIdRefId({ .mIndex = 13, .mContentFile = 0 }); - const RefId refId = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); - EXPECT_LT(formIdRefId, refId); + const FormId formId{ .mIndex = 13, .mContentFile = 0 }; + const RefId refId = RefId(FormId{ .mIndex = 42, .mContentFile = 0 }); + EXPECT_LT(formId, refId); } TEST(ESMRefIdTest, stringRefIdHasCaseInsensitiveHash) @@ -191,11 +198,6 @@ namespace ESM EXPECT_FALSE(formIdRefId < stringView); } - TEST(ESMRefIdTest, formIdRefIdIndexShouldHaveOnly24SignificantBits) - { - EXPECT_THROW(FormIdRefId(FormId{ .mIndex = 1 << 25, .mContentFile = 0 }), std::invalid_argument); - } - TEST(ESMRefIdTest, canBeUsedAsMapKeyWithLookupByStringView) { const std::map> map({ { RefId::stringRefId("a"), 42 } }); @@ -256,8 +258,6 @@ namespace ESM { RefId::stringRefId("foo"), "foo" }, { RefId::stringRefId(std::string({ 'a', 0, -1, '\n', '\t' })), { 'a', 0, -1, '\n', '\t' } }, { RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }), "0x2a" }, - { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = std::numeric_limits::min() }), - "0xff80000000ffffff" }, { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = std::numeric_limits::max() }), "0x7fffffffffffff" }, { RefId::generated(42), "0x2a" }, @@ -302,8 +302,6 @@ namespace ESM { RefId::stringRefId("\xff\x9b"), "\"\\xff\\x9b\"" }, { RefId::stringRefId("\xd0\xd0"), "\"\\xd0\\xd0\"" }, { RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }), "FormId:0x2a" }, - { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = std::numeric_limits::min() }), - "FormId:0xff80000000ffffff" }, { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = std::numeric_limits::max() }), "FormId:0x7fffffffffffff" }, { RefId::generated(42), "Generated:0x2a" }, @@ -344,9 +342,6 @@ namespace ESM { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = 0x1abc }), "FormId:0x1abcffffff" }, { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = std::numeric_limits::max() }), "FormId:0x7fffffffffffff" }, - { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = -1 }), "FormId:0xffffffffffffffff" }, - { RefId::formIdRefId({ .mIndex = 0xffffff, .mContentFile = std::numeric_limits::min() }), - "FormId:0xff80000000ffffff" }, { RefId::generated(0), "Generated:0x0" }, { RefId::generated(1), "Generated:0x1" }, { RefId::generated(0x1f), "Generated:0x1f" }, @@ -390,9 +385,9 @@ namespace ESM }; template <> - struct GenerateRefId + struct GenerateRefId { - static RefId call() { return RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); } + static RefId call() { return FormId{ .mIndex = 42, .mContentFile = 0 }; } }; template <> diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index 628a97265a..9a073b5ca1 100644 --- a/components/CMakeLists.txt +++ b/components/CMakeLists.txt @@ -120,7 +120,6 @@ add_component_dir (to_utf8 add_component_dir(esm attr common defs esmcommon records util luascripts format refid esmbridge esmterrain formid - formidrefid stringrefid generatedrefid indexrefid diff --git a/components/esm/formid.cpp b/components/esm/formid.cpp index a0b4c56a70..85ef3818e8 100644 --- a/components/esm/formid.cpp +++ b/components/esm/formid.cpp @@ -1,8 +1,33 @@ #include "formid.hpp" -std::string ESM::FormId::toString() const +#include +#include + +std::string ESM::FormId::toString(std::string_view prefix) const { - return std::to_string(mIndex) + "_" + std::to_string(mContentFile); + std::string res; + res.resize(prefix.length() + 20); + std::memcpy(res.data(), prefix.data(), prefix.size()); + char* buf = res.data() + prefix.size(); + uint64_t value; + if (hasContentFile()) + { + if ((mIndex & 0xff000000) != 0) + throw std::invalid_argument("Invalid FormId index value: " + std::to_string(mIndex)); + value = mIndex | (static_cast(mContentFile) << 24); + } + else + { + *(buf++) = '@'; + value = mIndex | (static_cast(-mContentFile - 1) << 32); + } + *(buf++) = '0'; + *(buf++) = 'x'; + const auto r = std::to_chars(buf, res.data() + res.size(), value, 16); + if (r.ec != std::errc()) + throw std::system_error(std::make_error_code(r.ec), "ESM::FormId::toString failed"); + res.resize(r.ptr - res.data()); + return res; } uint32_t ESM::FormId::toUint32() const diff --git a/components/esm/formid.hpp b/components/esm/formid.hpp index 78fb948e5e..e9416e35c7 100644 --- a/components/esm/formid.hpp +++ b/components/esm/formid.hpp @@ -21,7 +21,7 @@ namespace ESM // Zero is used in ESM4 as a null reference constexpr bool isZeroOrUnset() const { return mIndex == 0 && (mContentFile == 0 || mContentFile == -1); } - std::string toString() const; + std::string toString(std::string_view prefix = "") const; FormId32 toUint32() const; static constexpr FormId fromUint32(FormId32 v) { return { v & 0xffffff, static_cast(v >> 24) }; } }; diff --git a/components/esm/formidrefid.cpp b/components/esm/formidrefid.cpp deleted file mode 100644 index d8d049d203..0000000000 --- a/components/esm/formidrefid.cpp +++ /dev/null @@ -1,38 +0,0 @@ -#include "formidrefid.hpp" - -#include - -#include "serializerefid.hpp" - -namespace ESM -{ - namespace - { - std::uint64_t truncate(FormId value) - { - return (static_cast(value.mContentFile) << 24) | value.mIndex; - } - } - - std::string FormIdRefId::toString() const - { - std::string result; - const std::uint64_t v = truncate(mValue); - result.resize(getHexIntegralSizeWith0x(v), '\0'); - serializeHexIntegral(v, 0, result); - return result; - } - - std::string FormIdRefId::toDebugString() const - { - std::string result; - const std::uint64_t v = truncate(mValue); - serializeRefIdValue(v, formIdRefIdPrefix, result); - return result; - } - - std::ostream& operator<<(std::ostream& stream, FormIdRefId value) - { - return stream << value.toDebugString(); - } -} diff --git a/components/esm/formidrefid.hpp b/components/esm/formidrefid.hpp deleted file mode 100644 index 10db8c918b..0000000000 --- a/components/esm/formidrefid.hpp +++ /dev/null @@ -1,52 +0,0 @@ -#ifndef OPENMW_COMPONENTS_ESM_FORMIDREFID_HPP -#define OPENMW_COMPONENTS_ESM_FORMIDREFID_HPP - -#include -#include -#include - -#include - -namespace ESM -{ - class FormIdRefId - { - public: - constexpr FormIdRefId() = default; - - constexpr explicit FormIdRefId(ESM::FormId value) - : mValue(value) - { - if ((mValue.mIndex & 0xff000000) != 0) - throw std::invalid_argument("Invalid FormIdRefId index value: " + std::to_string(mValue.mIndex)); - } - - ESM::FormId getValue() const { return mValue; } - - std::string toString() const; - - std::string toDebugString() const; - - constexpr bool operator==(FormIdRefId rhs) const noexcept { return mValue == rhs.mValue; } - - constexpr bool operator<(FormIdRefId rhs) const noexcept { return mValue < rhs.mValue; } - - friend std::ostream& operator<<(std::ostream& stream, FormIdRefId value); - - friend struct std::hash; - - private: - ESM::FormId mValue; - }; -} - -namespace std -{ - template <> - struct hash - { - std::size_t operator()(ESM::FormIdRefId value) const noexcept { return std::hash{}(value.mValue); } - }; -} - -#endif diff --git a/components/esm/refid.cpp b/components/esm/refid.cpp index 03d7cb06eb..89df886974 100644 --- a/components/esm/refid.cpp +++ b/components/esm/refid.cpp @@ -116,6 +116,8 @@ namespace ESM std::string operator()(ESM::StringRefId v) const { return Misc::StringUtils::lowerCase(v.getValue()); } + std::string operator()(ESM::FormId v) const { return v.toString(formIdRefIdPrefix); } + template std::string operator()(const T& v) const { @@ -124,21 +126,6 @@ namespace ESM }; } - std::string EmptyRefId::toString() const - { - return std::string(); - } - - std::string EmptyRefId::toDebugString() const - { - return "Empty{}"; - } - - std::ostream& operator<<(std::ostream& stream, EmptyRefId value) - { - return stream << value.toDebugString(); - } - bool RefId::operator==(std::string_view rhs) const { return std::visit(IsEqualToString{ rhs }, mValue); @@ -154,11 +141,6 @@ namespace ESM return std::visit(IsGreaterThanString{ lhs }, rhs.mValue); } - std::ostream& operator<<(std::ostream& stream, RefId value) - { - return std::visit([&](auto v) -> std::ostream& { return stream << v; }, value.mValue); - } - RefId RefId::stringRefId(std::string_view value) { if (value.empty()) @@ -178,7 +160,14 @@ namespace ESM std::string RefId::toDebugString() const { - return std::visit([](auto v) { return v.toDebugString(); }, mValue); + return std::visit( + [](auto v) { + if constexpr (std::is_same_v) + return v.toString(formIdRefIdPrefix); + else + return v.toDebugString(); + }, + mValue); } bool RefId::startsWith(std::string_view prefix) const diff --git a/components/esm/refid.hpp b/components/esm/refid.hpp index ccc0417633..174652419a 100644 --- a/components/esm/refid.hpp +++ b/components/esm/refid.hpp @@ -11,7 +11,7 @@ #include #include "esm3exteriorcellrefid.hpp" -#include "formidrefid.hpp" +#include "formid.hpp" #include "generatedrefid.hpp" #include "indexrefid.hpp" #include "stringrefid.hpp" @@ -24,11 +24,9 @@ namespace ESM constexpr bool operator<(EmptyRefId /*rhs*/) const { return false; } - std::string toString() const; - - std::string toDebugString() const; + std::string toString() const { return ""; } - friend std::ostream& operator<<(std::ostream& stream, EmptyRefId value); + std::string toDebugString() const { return "Empty{}"; } }; enum class RefIdType : std::uint8_t @@ -48,8 +46,7 @@ namespace ESM class RefId { public: - using Value - = std::variant; + using Value = std::variant; // Constructs RefId from a serialized string containing byte by byte copy of RefId::mValue. static ESM::RefId deserialize(std::string_view value); @@ -85,17 +82,14 @@ namespace ESM { } - constexpr RefId(FormIdRefId value) - : mValue(value) - { - } - constexpr RefId(FormId value) { if (value.isZeroOrUnset()) mValue = EmptyRefId(); + else if (value.hasContentFile()) + mValue = value; else - mValue = FormIdRefId(value); + throw std::logic_error("RefId can't be a generated FormId"); } constexpr RefId(GeneratedRefId value) noexcept @@ -164,7 +158,7 @@ namespace ESM friend bool operator<(std::string_view lhs, RefId rhs); - friend std::ostream& operator<<(std::ostream& stream, RefId value); + friend std::ostream& operator<<(std::ostream& stream, RefId value) { return stream << value.toDebugString(); } template friend constexpr auto visit(F&& f, T&&... v) diff --git a/components/esm3/esmreader.cpp b/components/esm3/esmreader.cpp index 7eeb21d6b6..77468f22d5 100644 --- a/components/esm3/esmreader.cpp +++ b/components/esm3/esmreader.cpp @@ -498,7 +498,7 @@ namespace ESM FormId formId{}; getTSized<8>(formId); if (applyContentFileMapping(formId)) - return RefId::formIdRefId(formId); + return RefId(formId); else return RefId(); // content file was removed from load order } diff --git a/components/esm3/esmwriter.cpp b/components/esm3/esmwriter.cpp index 94cd1a8009..66788aa924 100644 --- a/components/esm3/esmwriter.cpp +++ b/components/esm3/esmwriter.cpp @@ -43,10 +43,10 @@ namespace ESM mWriter.write(v.getValue().data(), v.getValue().size()); } - void operator()(FormIdRefId v) const + void operator()(FormId v) const { mWriter.writeT(RefIdType::FormId); - mWriter.writeT(v.getValue()); + mWriter.writeT(v); } void operator()(GeneratedRefId v) const