fixed the udp hello connection message drop on azure environment #172
Conversation
| } | ||
|
|
||
| bool failedUdpReceive = receivedUdp != udpAcknowledgmentSize; | ||
| if (!failedUdpReceive) | ||
| { | ||
| if (protocolVersion != 0) | ||
| { | ||
| for (int i = 0; i < udpAcknowledgmentSize; ++i) | ||
| for (int i = 1; i < buffer.Length; i++) |
There was a problem hiding this comment.
I'm not sure of the point of these changes?
There was a problem hiding this comment.
the lenght of the the buffer is 9 and the udpAcknowledmentSize is now 12 so I had to modify this loop so it does not end out of bounds of the buffer array. ++i and i++ should do the same thing inside forloop afaik and for the sake of consistency with the larger side of the code base I have chosen i++.
There was a problem hiding this comment.
It might be a good idea to keep them both to be of same size? I just wanted to modify as little as possible. I would leave the rest to someone more competent
|
Looks pretty reasonable, since we haven't yet released these changes yet I'm happy with the change in size. I think the Unordinal team are using Azure so would be good if they can test this out to verify! For future reference it looks like your IDE is reformatting code (namely the number of spaces before a comment), that gets annoying for reviewers 🙂 |
|
Yes... I have noticed that... it's imo because it's inconsistent throughout the code... sometimes it's oneway sometimes the other. It did not modify it everywhere. I had also similar problem with the placement of the "{" at the beginning of the method block as it's sometimes at same line with the declaration and sometimes on new line... I reverted those because that was reasonably doable. I am sorry about that. Thank you for your comments. |
|
I'm in favor of merging once tests pass. |
|
Added test fixes to #175 (which contains your commit, so you will be properly credited afaik) |
To avoid the udp "connection acknowledgment/hello" message being dropped. I increased the size of the message buffer from 8 to 12 bytes. The message content is the same as before and remaining 4 bytes are simply ignored.
Linked to the issue #171