From 7db14b3392f20b095d1984362f4ec0d66c89bc3f Mon Sep 17 00:00:00 2001 From: elsid Date: Thu, 20 Apr 2023 00:24:49 +0200 Subject: [PATCH] Add more tests for printing RefId and clarify some constants --- apps/openmw_test_suite/esm/testrefid.cpp | 87 ++++++++++++++++++------ components/esm/esm3exteriorcellrefid.cpp | 10 +-- components/esm/generatedrefid.cpp | 2 +- components/esm/indexrefid.cpp | 11 +-- components/esm/serializerefid.hpp | 16 +++-- 5 files changed, 93 insertions(+), 33 deletions(-) diff --git a/apps/openmw_test_suite/esm/testrefid.cpp b/apps/openmw_test_suite/esm/testrefid.cpp index 11ac7a6477..763484867a 100644 --- a/apps/openmw_test_suite/esm/testrefid.cpp +++ b/apps/openmw_test_suite/esm/testrefid.cpp @@ -1,11 +1,17 @@ #include +#include #include #include #include #include +MATCHER(IsPrint, "") +{ + return std::isprint(arg) != 0; +} + namespace ESM { namespace @@ -26,7 +32,7 @@ namespace ESM TEST(ESMRefIdTest, formIdRefIdIsNotEmpty) { - const RefId refId = RefId::formIdRefId({ 42, 0 }); + const RefId refId = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); EXPECT_FALSE(refId.empty()); } @@ -53,7 +59,7 @@ namespace ESM TEST(ESMRefIdTest, defaultConstructedIsNotEqualToFormIdRefId) { const RefId a; - const RefId b = RefId::formIdRefId({ 42, 0 }); + const RefId b = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); EXPECT_NE(a, b); } @@ -98,8 +104,8 @@ namespace ESM TEST(ESMRefIdTest, equalityIsDefinedForFormRefIdAndRefId) { - const FormIdRefId formIdRefId({ 42, 0 }); - const RefId refId = RefId::formIdRefId({ 42, 0 }); + const FormIdRefId formIdRefId({ .mIndex = 42, .mContentFile = 0 }); + const RefId refId = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); EXPECT_EQ(formIdRefId, refId); } @@ -125,8 +131,8 @@ namespace ESM TEST(ESMRefIdTest, lessThanIsDefinedForFormRefIdAndRefId) { - const FormIdRefId formIdRefId({ 13, 0 }); - const RefId refId = RefId::formIdRefId({ 42, 0 }); + const FormIdRefId formIdRefId({ .mIndex = 13, .mContentFile = 0 }); + const RefId refId = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); EXPECT_LT(formIdRefId, refId); } @@ -164,14 +170,14 @@ namespace ESM TEST(ESMRefIdTest, stringRefIdHasStrongOrderWithFormId) { const RefId stringRefId = RefId::stringRefId("a"); - const RefId formIdRefId = RefId::formIdRefId({ 42, 0 }); + const RefId formIdRefId = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); EXPECT_TRUE(stringRefId < formIdRefId); EXPECT_FALSE(formIdRefId < stringRefId); } TEST(ESMRefIdTest, formIdRefIdHasStrongOrderWithStringView) { - const RefId formIdRefId = RefId::formIdRefId({ 42, 0 }); + const RefId formIdRefId = RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); const std::string_view stringView = "42"; EXPECT_TRUE(stringView < formIdRefId); EXPECT_FALSE(formIdRefId < stringView); @@ -197,7 +203,7 @@ namespace ESM TEST(ESMRefIdTest, stringRefIdIsNotEqualToFormId) { const RefId stringRefId = RefId::stringRefId("\0"); - const RefId formIdRefId = RefId::formIdRefId({ 0, 0 }); + const RefId formIdRefId = RefId::formIdRefId({ .mIndex = 0, .mContentFile = 0 }); EXPECT_NE(stringRefId, formIdRefId); } @@ -240,10 +246,19 @@ namespace ESM { RefId(), std::string() }, { RefId::stringRefId("foo"), "foo" }, { RefId::stringRefId(std::string({ 'a', 0, -1, '\n', '\t' })), { 'a', 0, -1, '\n', '\t' } }, - { RefId::formIdRefId({ 42, 0 }), "0x2a" }, + { 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" }, + { RefId::generated(std::numeric_limits::max()), "0xffffffffffffffff" }, { RefId::index(REC_ARMO, 42), "ARMO:0x2a" }, { RefId::esm3ExteriorCell(-13, 42), "-13:42" }, + { RefId::esm3ExteriorCell(std::numeric_limits::min(), std::numeric_limits::min()), + "-2147483648:-2147483648" }, + { RefId::esm3ExteriorCell(std::numeric_limits::max(), std::numeric_limits::max()), + "2147483647:2147483647" }, }; INSTANTIATE_TEST_SUITE_P(ESMRefIdToString, ESMRefIdToStringTest, ValuesIn(toStringParams)); @@ -273,10 +288,20 @@ namespace ESM { RefId::stringRefId("foo"), "\"foo\"" }, { RefId::stringRefId("BAR"), "\"BAR\"" }, { RefId::stringRefId(std::string({ 'a', 0, -1, '\n', '\t' })), "\"a\\x0\\xFF\\xA\\x9\"" }, - { RefId::formIdRefId({ 42, 0 }), "FormId:0x2a" }, + { 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" }, + { RefId::generated(std::numeric_limits::max()), "Generated:0xffffffffffffffff" }, { RefId::index(REC_ARMO, 42), "Index:ARMO:0x2a" }, + { RefId::index(REC_ARMO, std::numeric_limits::max()), "Index:ARMO:0xffffffff" }, { RefId::esm3ExteriorCell(-13, 42), "Esm3ExteriorCell:-13:42" }, + { RefId::esm3ExteriorCell(std::numeric_limits::min(), std::numeric_limits::min()), + "Esm3ExteriorCell:-2147483648:-2147483648" }, + { RefId::esm3ExteriorCell(std::numeric_limits::max(), std::numeric_limits::max()), + "Esm3ExteriorCell:2147483647:2147483647" }, }; INSTANTIATE_TEST_SUITE_P(ESMRefIdToDebugString, ESMRefIdToDebugStringTest, ValuesIn(toDebugStringParams)); @@ -300,11 +325,16 @@ namespace ESM { RefId::stringRefId("foo"), "foo" }, { RefId::stringRefId("BAR"), "bar" }, { RefId::stringRefId(std::string({ 'a', 0, -1, '\n', '\t' })), { 'a', 0, -1, '\n', '\t' } }, - { RefId::formIdRefId({ 0, 0 }), "FormId:0x0" }, - { RefId::formIdRefId({ 1, 0 }), "FormId:0x1" }, - { RefId::formIdRefId({ 0x1f, 0 }), "FormId:0x1f" }, - { RefId::formIdRefId({ 0x1f, 2 }), "FormId:0x200001f" }, - { RefId::formIdRefId({ 0xffffff, 0x1abc }), "FormId:0x1abcffffff" }, + { RefId::formIdRefId({ .mIndex = 0, .mContentFile = 0 }), "FormId:0x0" }, + { RefId::formIdRefId({ .mIndex = 1, .mContentFile = 0 }), "FormId:0x1" }, + { RefId::formIdRefId({ .mIndex = 0x1f, .mContentFile = 0 }), "FormId:0x1f" }, + { RefId::formIdRefId({ .mIndex = 0x1f, .mContentFile = 2 }), "FormId:0x200001f" }, + { 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" }, @@ -350,7 +380,7 @@ namespace ESM template <> struct GenerateRefId { - static RefId call() { return RefId::formIdRefId({ 42, 0 }); } + static RefId call() { return RefId::formIdRefId({ .mIndex = 42, .mContentFile = 0 }); } }; template <> @@ -388,11 +418,27 @@ namespace ESM { const RefId refId = GenerateRefId::call(); const std::string text = refId.serializeText(); - for (std::size_t i = 0; i < text.size(); ++i) - ASSERT_TRUE(std::isprint(text[i])) << "index: " << i << ", int value: " << static_cast(text[i]); EXPECT_EQ(RefId::deserializeText(text), refId); } + TYPED_TEST_P(ESMRefIdTypesTest, serializeTextShouldReturnOnlyPrintableCharacters) + { + const RefId refId = GenerateRefId::call(); + EXPECT_THAT(refId.serializeText(), Each(IsPrint())); + } + + TYPED_TEST_P(ESMRefIdTypesTest, toStringShouldReturnOnlyPrintableCharacters) + { + const RefId refId = GenerateRefId::call(); + EXPECT_THAT(refId.toString(), Each(IsPrint())); + } + + TYPED_TEST_P(ESMRefIdTypesTest, toDebugStringShouldReturnOnlyPrintableCharacters) + { + const RefId refId = GenerateRefId::call(); + EXPECT_THAT(refId.toDebugString(), Each(IsPrint())); + } + TYPED_TEST_P(ESMRefIdTypesTest, shouldBeEqualToItself) { const RefId a = GenerateRefId::call(); @@ -416,7 +462,8 @@ namespace ESM REGISTER_TYPED_TEST_SUITE_P(ESMRefIdTypesTest, serializeThenDeserializeShouldProduceSameValue, serializeTextThenDeserializeTextShouldProduceSameValue, shouldBeEqualToItself, shouldNotBeNotEqualToItself, - shouldBeNotLessThanItself); + shouldBeNotLessThanItself, serializeTextShouldReturnOnlyPrintableCharacters, + toStringShouldReturnOnlyPrintableCharacters, toDebugStringShouldReturnOnlyPrintableCharacters); template struct RefIdTypes; diff --git a/components/esm/esm3exteriorcellrefid.cpp b/components/esm/esm3exteriorcellrefid.cpp index b29e6fdd0c..63373dd055 100644 --- a/components/esm/esm3exteriorcellrefid.cpp +++ b/components/esm/esm3exteriorcellrefid.cpp @@ -9,23 +9,25 @@ namespace ESM { std::string ESM3ExteriorCellRefId::toString() const { + constexpr std::size_t separator = 1; std::string result; - result.resize(getDecIntegralCapacity(mX) + getDecIntegralCapacity(mY) + 3, '\0'); + result.resize(getDecIntegralCapacity(mX) + separator + getDecIntegralCapacity(mY), '\0'); const std::size_t endX = serializeDecIntegral(mX, 0, result); result[endX] = ':'; - const std::size_t endY = serializeDecIntegral(mY, endX + 1, result); + const std::size_t endY = serializeDecIntegral(mY, endX + separator, result); result.resize(endY); return result; } std::string ESM3ExteriorCellRefId::toDebugString() const { + constexpr std::size_t separator = 1; std::string result; serializeRefIdPrefix( - getDecIntegralCapacity(mX) + getDecIntegralCapacity(mY) + 1, esm3ExteriorCellRefIdPrefix, result); + getDecIntegralCapacity(mX) + separator + getDecIntegralCapacity(mY), esm3ExteriorCellRefIdPrefix, result); const std::size_t endX = serializeDecIntegral(mX, esm3ExteriorCellRefIdPrefix.size(), result); result[endX] = ':'; - const std::size_t endY = serializeDecIntegral(mY, endX + 1, result); + const std::size_t endY = serializeDecIntegral(mY, endX + separator, result); result.resize(endY); return result; } diff --git a/components/esm/generatedrefid.cpp b/components/esm/generatedrefid.cpp index b1b55f5143..937e3c6751 100644 --- a/components/esm/generatedrefid.cpp +++ b/components/esm/generatedrefid.cpp @@ -9,7 +9,7 @@ namespace ESM std::string GeneratedRefId::toString() const { std::string result; - result.resize(getHexIntegralSize(mValue) + 2, '\0'); + result.resize(getHexIntegralSizeWith0x(mValue), '\0'); serializeHexIntegral(mValue, 0, result); return result; } diff --git a/components/esm/indexrefid.cpp b/components/esm/indexrefid.cpp index 8c1c691281..0b090f4c97 100644 --- a/components/esm/indexrefid.cpp +++ b/components/esm/indexrefid.cpp @@ -9,20 +9,23 @@ namespace ESM std::string IndexRefId::toString() const { std::string result; - result.resize(sizeof(mRecordType) + getHexIntegralSize(mValue) + 3, '\0'); + constexpr std::size_t separator = 1; + result.resize(sizeof(mRecordType) + separator + getHexIntegralSizeWith0x(mValue), '\0'); std::memcpy(result.data(), &mRecordType, sizeof(mRecordType)); result[sizeof(mRecordType)] = ':'; - serializeHexIntegral(mValue, sizeof(mRecordType) + 1, result); + serializeHexIntegral(mValue, sizeof(mRecordType) + separator, result); return result; } std::string IndexRefId::toDebugString() const { std::string result; - serializeRefIdPrefix(sizeof(mRecordType) + getHexIntegralSize(mValue) + 1, indexRefIdPrefix, result); + constexpr std::size_t separator = 1; + serializeRefIdPrefix( + sizeof(mRecordType) + separator + getHexIntegralSizeWith0x(mValue), indexRefIdPrefix, result); std::memcpy(result.data() + indexRefIdPrefix.size(), &mRecordType, sizeof(mRecordType)); result[indexRefIdPrefix.size() + sizeof(mRecordType)] = ':'; - serializeHexIntegral(mValue, indexRefIdPrefix.size() + sizeof(mRecordType) + 1, result); + serializeHexIntegral(mValue, indexRefIdPrefix.size() + sizeof(mRecordType) + separator, result); return result; } diff --git a/components/esm/serializerefid.hpp b/components/esm/serializerefid.hpp index bc191b3553..6dc2d696ac 100644 --- a/components/esm/serializerefid.hpp +++ b/components/esm/serializerefid.hpp @@ -21,9 +21,11 @@ namespace ESM { if (value == 0) return 1; + constexpr std::size_t lastDigit = 1; if (value > 0) - return static_cast(std::numeric_limits::digits10); - return static_cast(std::numeric_limits::digits10) + 1; + return static_cast(std::numeric_limits::digits10) + lastDigit; + constexpr std::size_t sign = 1; + return static_cast(std::numeric_limits::digits10) + lastDigit + sign; } template @@ -40,9 +42,15 @@ namespace ESM return result; } + template + std::size_t getHexIntegralSizeWith0x(T value) + { + return 2 + getHexIntegralSize(value); + } + inline void serializeRefIdPrefix(std::size_t valueSize, std::string_view prefix, std::string& out) { - out.resize(prefix.size() + valueSize + 2, '\0'); + out.resize(prefix.size() + valueSize, '\0'); std::memcpy(out.data(), prefix.data(), prefix.size()); } @@ -70,7 +78,7 @@ namespace ESM void serializeRefIdValue(T value, std::string_view prefix, std::string& out) { static_assert(!std::is_signed_v); - serializeRefIdPrefix(getHexIntegralSize(value), prefix, out); + serializeRefIdPrefix(getHexIntegralSizeWith0x(value), prefix, out); serializeHexIntegral(value, prefix.size(), out); }