Core/PacketIO: Improved bit functions - no longer reads each bit separately

(cherry picked from commit 38f40f5413ac3cd096fd1db84b25a3c9bc9f50db)

2 files changed, 87 insertions, 50 deletions

diff --git a/src/server/shared/Packets/ByteBuffer.cpp b/src/server/shared/Packets/ByteBuffer.cpp
index 994cd371968..8811a2a6c0c 100644
--- a/src/server/shared/Packets/ByteBuffer.cpp
+++ b/src/server/shared/Packets/ByteBuffer.cpp
@@ -17,18 +17,14 @@
 
 #include "ByteBuffer.h"
 #include "Errors.h"
-#include "MessageBuffer.h"
 #include "Log.h"
+#include "MessageBuffer.h"
 #include "Util.h"
 #include <utf8.h>
 #include <algorithm>
 #include <sstream>
 #include <cmath>
 
-ByteBuffer::ByteBuffer(MessageBuffer&& buffer) : _rpos(0), _wpos(0), _bitpos(InitialBitPos), _curbitval(0), _storage(buffer.Move())
-{
-}
-
 ByteBufferPositionException::ByteBufferPositionException(size_t pos, size_t size, size_t valueSize)
     : ByteBufferException(Trinity::StringFormat("Attempted to get value with size: {} in ByteBuffer (pos: {} size: {})", valueSize, pos, size))
 {
@@ -39,6 +35,10 @@ ByteBufferInvalidValueException::ByteBufferInvalidValueException(char const* typ
 {
 }
 
+ByteBuffer::ByteBuffer(MessageBuffer&& buffer) : _rpos(0), _wpos(0), _bitpos(InitialBitPos), _curbitval(0), _storage(buffer.Move())
+{
+}
+
 ByteBuffer& ByteBuffer::operator>>(float& value)
 {
     read(&value, 1);
@@ -145,63 +145,67 @@ void ByteBuffer::PutBits(std::size_t pos, std::size_t value, uint32 bitCount)
 
 void ByteBuffer::print_storage() const
 {
-    if (!sLog->ShouldLog("network", LOG_LEVEL_TRACE)) // optimize disabled trace output
+    Logger const* networkLogger = sLog->GetEnabledLogger("network", LOG_LEVEL_TRACE);
+    if (!networkLogger) // optimize disabled trace output
         return;
 
     std::ostringstream o;
-    o << "STORAGE_SIZE: " << size();
     for (uint32 i = 0; i < size(); ++i)
-        o << read<uint8>(i) << " - ";
-    o << ' ';
+        o << uint32(_storage[i]) << " - ";
 
-    TC_LOG_TRACE("network", "{}", o.str());
+    TC_LOG_TRACE("network", "STORAGE_SIZE: {} {}", size(), o.view());
 }
 
 void ByteBuffer::textlike() const
 {
-    if (!sLog->ShouldLog("network", LOG_LEVEL_TRACE)) // optimize disabled trace output
+    Logger const* networkLogger = sLog->GetEnabledLogger("network", LOG_LEVEL_TRACE);
+    if (networkLogger) // optimize disabled trace output
         return;
 
     std::ostringstream o;
-    o << "STORAGE_SIZE: " << size();
     for (uint32 i = 0; i < size(); ++i)
-    {
-        char buf[2];
-        snprintf(buf, 2, "%c", read<uint8>(i));
-        o << buf;
-    }
-    o << ' ';
-    TC_LOG_TRACE("network", "{}", o.str());
+        o << char(_storage[i]);
+
+    sLog->OutMessageTo(networkLogger, "network", LOG_LEVEL_TRACE, "STORAGE_SIZE: {} {}", size(), o.view());
 }
 
 void ByteBuffer::hexlike() const
 {
-    if (!sLog->ShouldLog("network", LOG_LEVEL_TRACE)) // optimize disabled trace output
+    Logger const* networkLogger = sLog->GetEnabledLogger("network", LOG_LEVEL_TRACE);
+    if (!networkLogger) // optimize disabled trace output
         return;
 
-    uint32 j = 1, k = 1;
-
     std::ostringstream o;
-    o << "STORAGE_SIZE: " << size();
+    o.setf(std::ios_base::hex, std::ios_base::basefield);
+    o.fill('0');
 
-    for (uint32 i = 0; i < size(); ++i)
+    for (uint32 i = 0; i < size(); )
     {
-        char buf[4];
-        snprintf(buf, 4, "%02X", read<uint8>(i));
-        if ((i == (j * 8)) && ((i != (k * 16))))
+        char const* sep = " | ";
+        for (uint32 j = 0; j < 2; ++j)
         {
-            o << "| ";
-            ++j;
+            for (uint32 k = 0; k < 8; ++k)
+            {
+                o.width(2);
+                o << _storage[i];
+                ++i;
+            }
+
+            o << sep;
+            sep = "\n";
         }
-        else if (i == (k * 16))
-        {
-            o << '\n';
-            ++k;
-            ++j;
-        }
-
-        o << buf;
     }
-    o << ' ';
-    TC_LOG_TRACE("network", "{}", o.str());
+
+    sLog->OutMessageTo(networkLogger, "network", LOG_LEVEL_TRACE, "STORAGE_SIZE: {} {}", size(), o.view());
 }
+
+template TC_SHARED_API uint8 ByteBuffer::read<uint8>();
+template TC_SHARED_API uint16 ByteBuffer::read<uint16>();
+template TC_SHARED_API uint32 ByteBuffer::read<uint32>();
+template TC_SHARED_API uint64 ByteBuffer::read<uint64>();
+template TC_SHARED_API int8 ByteBuffer::read<int8>();
+template TC_SHARED_API int16 ByteBuffer::read<int16>();
+template TC_SHARED_API int32 ByteBuffer::read<int32>();
+template TC_SHARED_API int64 ByteBuffer::read<int64>();
+template TC_SHARED_API float ByteBuffer::read<float>();
+template TC_SHARED_API double ByteBuffer::read<double>();
diff --git a/src/server/shared/Packets/ByteBuffer.h b/src/server/shared/Packets/ByteBuffer.h
index d51fee27d24..9a4761ff3e0 100644
--- a/src/server/shared/Packets/ByteBuffer.h
+++ b/src/server/shared/Packets/ByteBuffer.h
@@ -159,9 +159,6 @@ class TC_SHARED_API ByteBuffer
 
         void ResetBitPos()
         {
-            if (_bitpos > 7)
-                return;
-
             _bitpos = 8;
             _curbitval = 0;
         }
@@ -184,14 +181,13 @@ class TC_SHARED_API ByteBuffer
 
         bool ReadBit()
         {
-            ++_bitpos;
-            if (_bitpos > 7)
+            if (_bitpos >= 8)
             {
-                _curbitval = read<uint8>();
+                read(&_curbitval, 1);
                 _bitpos = 0;
             }
 
-            return ((_curbitval >> (7 - _bitpos)) & 1) != 0;
+            return ((_curbitval >> (8 - ++_bitpos)) & 1) != 0;
         }
 
         void WriteBits(uint64 value, int32 bits)
@@ -236,8 +232,34 @@ class TC_SHARED_API ByteBuffer
         uint32 ReadBits(int32 bits)
         {
             uint32 value = 0;
-            for (int32 i = bits - 1; i >= 0; --i)
-                value |= uint32(ReadBit()) << i;
+            if (bits > 8 - int32(_bitpos))
+            {
+                // first retriever whatever is left in the bit buffer
+                int32 bitsInBuffer = 8 - _bitpos;
+                value = (_curbitval & ((UI64LIT(1) << bitsInBuffer) - 1)) << (bits - bitsInBuffer);
+                bits -= bitsInBuffer;
+
+                // then read as many full bytes as possible
+                while (bits >= 8)
+                {
+                    bits -= 8;
+                    value |= read<uint8>() << bits;
+                }
+
+                // and finally any remaining bits
+                if (bits)
+                {
+                    read(&_curbitval, 1);
+                    value |= (_curbitval >> (8 - bits)) & ((UI64LIT(1) << bits) - 1);
+                    _bitpos = bits;
+                }
+            }
+            else
+            {
+                // entire value is in the bit buffer
+                value = (_curbitval >> (8 - _bitpos - bits)) & ((UI64LIT(1) << bits) - 1);
+                _bitpos += bits;
+            }
 
             return value;
         }
@@ -667,8 +689,8 @@ class TC_SHARED_API ByteBuffer
         std::vector<uint8> _storage;
 };
 
-/// @todo Make a ByteBuffer.cpp and move all this inlining to it.
-template <> inline std::string ByteBuffer::read<std::string>()
+template <>
+inline std::string ByteBuffer::read<std::string>()
 {
     return std::string(ReadCString());
 }
@@ -691,4 +713,15 @@ inline void ByteBuffer::read_skip<std::string>()
     read_skip<char*>();
 }
 
+extern template uint8 ByteBuffer::read<uint8>();
+extern template uint16 ByteBuffer::read<uint16>();
+extern template uint32 ByteBuffer::read<uint32>();
+extern template uint64 ByteBuffer::read<uint64>();
+extern template int8 ByteBuffer::read<int8>();
+extern template int16 ByteBuffer::read<int16>();
+extern template int32 ByteBuffer::read<int32>();
+extern template int64 ByteBuffer::read<int64>();
+extern template float ByteBuffer::read<float>();
+extern template double ByteBuffer::read<double>();
+
 #endif