PooledBitReader.ReadBits(16) only reads 8 bits

[figs999]

Describe the bug
When 16 is passed as the quantity of bits to read to PooledBitReader.ReadBits, the stream only advances 8 bits and only the value of the first 8 bits is returned. This does not appear to be a problem with any value other than 16 for some reason. I am able to read other multiples of 8, 15 bits, 17 bits etc.

I do not know if this is specific to my particular use case of using an Unreliable type data channel and CustomMessagingManager.SendUnnamedMessage;

To Reproduce
Steps to reproduce the behavior:

1. Send a stream with a 16 bit value written to it with PooledBitWriter,WriteBits({val},16); via CustomMessagingManager.SendUnnamedMessage over an unreliable type channel
2. On receiving client, call ReadBits(16) on a PooledBitReader constructed with the received stream via PooledBitReader.Get.
3. Notice the stream has only advanced 8 bits, and the value returned is only the value of the leading 8 bits of the intended 16-bit value.

Expected behavior
Reading 16 bits from the PooledBitReader should advance the stream 16 bits and return the value represented in those 16 bits.

Environment (please complete the following information):

• OS: Windows 10
• Unity Version: 2019.1
• MLAPI Version: v12.1.6
• MLAPI Commit: release build

[figs999]

Looking at code for BitReader.ReadBits it appears that ANY multiple of 8 requested to be read will fail to read the last 8 bits. So I am unsure why it appeared that my tests for 8 and 24 worked.

Unless I am missing something elsewhere that makes this needed, it looks like the comparison in the loop which checks (i+8 < bitcount) needs to simply be (i < bitcount).

[0xFA11]

@TwoTenPvP — we have both ReadByteBits and ReadBits. looks like ReadByteBits is meant for 8-bits but ReadBits could be used for from 8-bits up to 64-bits in chunks. So, it sounds like this issue is legitimately a bug. I will create a task for us internally.

[will-mearns]

Added to backlog for triaging as MTT-508

[MishaPh]

I have the same issue, NetworkReader.ReadByte it's not working correctly for these bits 8,16,24,32,...
so, I looked at the code and found there is an equality bug:
in method ReadBits(int bitCount)
for (int i = 0; i + 8 < bitCount; i += 8)
should be
for (int i = 0; i + 8 <= bitCount; i += 8)

[will-mearns]

@ShadauxCat , I assume you can close this as MTT-508 is marked done. Is there a PR we can reference?

[ShadauxCat]

PooledBitReader was removed in #1187 and replaced with FastBufferReader's BitReader introduced in #1082. FastBufferReader's BitReader has unit tests to explicitly exercise this case (in fact it validates everything from 1 bit through 64 bits) and those tests pass... however I realized as I was writing this that those tests only test aligned reads and a similar (but separate) issue still exists for misaligned reads, which I just put up a fix for in #1240