[Bug - v1.3 & SVN!] sf::Packet

[NeWsOfTzzz]

There is a bug in the packet class in the function CheckSize which affects the bool() function to check if the packet is still readable

for 1.3:

Code: [Select]

00367 bool Packet::CheckSize(std::size_t Size)
00368 {
00369     if (myIsValid && (myReadPos + Size <= myData.size()))
00370     {
00371         return true;
00372     }
00373     else
00374     {
00375         myIsValid = false;
00376         return false;
00377     }
00378 }
change to

Code: [Select]

00367 bool Packet::CheckSize(std::size_t Size)
00368 {
00369     if (myIsValid && (myReadPos + Size < myData.size()))
00370     {
00371         return true;
00372     }
00373     else
00374     {
00375         myIsValid = false;
00376         return false;
00377     }
00378 }






for SVN:

Code: [Select]

00400 bool Packet::CheckSize(std::size_t Size)
00401 {
00402     myIsValid = myIsValid && (myReadPos + Size <= myData.size());
00403
00404     return myIsValid;
00405 }
change to

Code: [Select]

00400 bool Packet::CheckSize(std::size_t Size)
00401 {
00402     myIsValid = myIsValid && (myReadPos + Size < myData.size());
00403
00404     return myIsValid;
00405 }

Because if myReadPos + Size == myData.size() all bytes were already read and no more bytes can be read.








.

[Laurent]

Because if myReadPos + Size == myData.size() all bytes were already read and no more bytes can be read.

So the last extraction was ok and the packet reports a valid state. Only the next attempt to extract data will put the packet in an invalid state.

The boolean operator checks what has been done previously, not what can be done.

[NeWsOfTzzz]

Because if myReadPos + Size == myData.size() all bytes were already read and no more bytes can be read.

So the last extraction was ok and the packet reports a valid state. Only the next attempt to extract data will put the packet in an invalid state.

The boolean operator checks what has been done previously, not what can be done.

Who'd need that??

I changed SVN and compiled it for me so this works, but I tell you what I need it for:

I put several packets in one packet (several commands, like display map or display this, move this) and then I need to check if more is there.

So I do

Code: [Select]

uint8_t protocolID;
while(packet)
{
  packet >> protocolID;
  switch(protocolID)
  {
    ..BLAH..
  }
}

Why would I need a function that tells me if there was an error in the past?

So if this is really intended by you then I request a function either
bool IsReadable()
or
uint32_t GetRemainingSize()

[Laurent]

Who'd need that??

Everyone :

Code: [Select]

float x;
packet >> x;
if (!packet)
{
    // error reading x
}

// Or, another style:

float x
if (packet >> x)
{
    // ok, can proceed with x
}
This is a standard idiom : try to read, and then check if it was successful.
Checking before reading would be a non-sense as it depends on the variable itself : imagine there's one byte left to read in the packet, then you can extract a char but not an int. No class behaves like this.

By the way, this is the exact same behaviour as standard C++ streams.

So if this is really intended by you then I request a function either
bool IsReadable()
or
uint32_t GetRemainingSize()

There's a bool EndOfPacket() function, actually (in SVN version). But again, checking before reading is a mistake. This function should only be used to report an error or warning if there are unread bytes left.

[NeWsOfTzzz]

Hmm maybe you are right :p

You should put that into the tutorial of the network package!

[Laurent]

You should put that into the tutorial of the network package!

I did... Unless you were speaking of the EndOfPacket function ?

[NeWsOfTzzz]

You should put that into the tutorial of the network package!

I did... Unless you were speaking of the EndOfPacket function ?

Oh true, I just read it, sorry! I just didn't notice it as a solution for my problem because it was in another context

Ye well I think it's nearly a philosophical question to check if you can read before or after you read :p