Merge branch 'ref_id_print_tests' into 'master'

Add more tests for printing RefId and clarify some constants

See merge request OpenMW/openmw!2945

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;
     }

components/esm/formidrefid.cpp

@@ -1,18 +1,24 @@
 #include "formidrefid.hpp"
 
-#include <cassert>
 #include <ostream>
 
 #include "serializerefid.hpp"
 
 namespace ESM
 {
+    namespace
+    {
+        std::uint64_t truncate(FormId value)
+        {
+            return (static_cast<std::uint64_t>(value.mContentFile) << 24) | value.mIndex;
+        }
+    }
+
     std::string FormIdRefId::toString() const
     {
         std::string result;
-        assert((mValue.mIndex & 0xff000000) == 0);
-        size_t v = (static_cast<size_t>(mValue.mContentFile) << 24) | mValue.mIndex;
-        result.resize(getHexIntegralSize(v) + 2, '\0');
+        const std::uint64_t v = truncate(mValue);
+        result.resize(getHexIntegralSizeWith0x(v), '\0');
         serializeHexIntegral(v, 0, result);
         return result;
     }
@@ -20,8 +26,7 @@ namespace ESM
     std::string FormIdRefId::toDebugString() const
     {
         std::string result;
-        assert((mValue.mIndex & 0xff000000) == 0);
-        size_t v = (static_cast<size_t>(mValue.mContentFile) << 24) | mValue.mIndex;
+        const std::uint64_t v = truncate(mValue);
         serializeRefIdValue(v, formIdRefIdPrefix, result);
         return result;
     }

components/esm/formidrefid.hpp

@@ -3,6 +3,7 @@
 
 #include <functional>
 #include <iosfwd>
+#include <stdexcept>
 
 #include <components/esm/formid.hpp>
 
@@ -13,9 +14,11 @@ namespace ESM
     public:
         constexpr FormIdRefId() = default;
 
-        constexpr explicit FormIdRefId(ESM::FormId value) noexcept
+        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; }

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;
     }

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;
     }
 

components/esm/refid.hpp

@@ -63,7 +63,7 @@ namespace ESM
         static RefId stringRefId(std::string_view value);
 
         // Constructs RefId from FormId storing the value in-place.
-        static RefId formIdRefId(FormId value) noexcept { return RefId(FormIdRefId(value)); }
+        static RefId formIdRefId(FormId value) { return RefId(FormIdRefId(value)); }
 
         // Constructs RefId from uint64 storing the value in-place. Should be used for generated records where id is a
         // global counter.
@@ -87,7 +87,7 @@ namespace ESM
         {
         }
 
-        constexpr RefId(FormIdRefId value) noexcept
+        constexpr RefId(FormIdRefId value)
             : mValue(value)
         {
         }

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::size_t>(std::numeric_limits<T>::digits10);
-        return static_cast<std::size_t>(std::numeric_limits<T>::digits10) + 1;
+            return static_cast<std::size_t>(std::numeric_limits<T>::digits10) + lastDigit;
+        constexpr std::size_t sign = 1;
+        return static_cast<std::size_t>(std::numeric_limits<T>::digits10) + lastDigit + sign;
     }
 
     template <class T>
@@ -40,9 +42,15 @@ namespace ESM
         return result;
     }
 
+    template <class T>
+    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<T>);
-        serializeRefIdPrefix(getHexIntegralSize(value), prefix, out);
+        serializeRefIdPrefix(getHexIntegralSizeWith0x(value), prefix, out);
         serializeHexIntegral(value, prefix.size(), out);
     }