Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 7 additions & 17 deletions Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,13 +459,10 @@ void ConnectionManager::destroyGameMessages() {
* assumption that a command will only be relayed once.
*/
void ConnectionManager::doRelay() {
static Int numPackets = 0;
static Int numCommands = 0;

NetPacket *packet = nullptr;

for (Int i = 0; i < MAX_MESSAGES; ++i) {
if (m_transport->m_inBuffer[i].length != 0) {
for (size_t i = 0; i < ARRAY_SIZE(m_transport->m_inBuffer); ++i) {
if (m_transport->m_inBuffer[i].length > 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status quo: Compares with > 0, != 0

Expectation:

> 0, <= 0

or

== 0, != 0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split this off to a separate commit. I do think this is the right comparator, though, as long as length is still signed.

The original code already does it this way here:

if (m_transport->m_inBuffer[i].length > 0)

if (m_transport->m_inBuffer[i].length > 0) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can change the == 0 cases to <= 0 if you like.

// This transport buffer has yet to be processed.

// make a NetPacket out of this data so it can be broken up into individual commands.
Expand All @@ -476,23 +473,19 @@ void ConnectionManager::doRelay() {

// Get the command list from the packet.
NetCommandList *cmdList = packet->getCommandList();
NetCommandRef *cmd = cmdList->getFirstMessage();

// Iterate through the commands in this packet and send them to the proper connections.
while (cmd != nullptr) {
for (NetCommandRef* cmd = cmdList->getFirstMessage(); cmd; cmd = cmd->getNext()) {
//DEBUG_LOG(("ConnectionManager::doRelay() - Looking at a command of type %s",
//GetNetCommandTypeAsString(cmd->getCommand()->getNetCommandType())));

if (CommandRequiresAck(cmd->getCommand())) {
ackCommand(cmd, m_localSlot);
}
if (!processNetCommand(cmd)) {
sendRemoteCommand(cmd);
}
cmd = cmd->getNext();

++numCommands;
}
++numPackets;

// Delete this packet since we won't be needing it anymore.
deleteInstance(packet);
Expand All @@ -503,23 +496,20 @@ void ConnectionManager::doRelay() {

// signal that this has been processed.
m_transport->m_inBuffer[i].length = 0;
} else {
break;
}
}

NetCommandList *cmdList = m_netCommandWrapperList->getReadyCommands();
NetCommandRef *cmd = cmdList->getFirstMessage();
while (cmd != nullptr) {
for (NetCommandRef* cmd = cmdList->getFirstMessage(); cmd; cmd = cmd->getNext()) {
if (CommandRequiresAck(cmd->getCommand())) {
ackCommand(cmd, m_localSlot);
}
if (!processNetCommand(cmd)) {
sendRemoteCommand(cmd);
}
cmd = cmd->getNext();

++numCommands;
}
++numPackets;

// Delete this packet since we won't be needing it anymore.
deleteInstance(packet);
Expand Down
7 changes: 5 additions & 2 deletions Core/GameEngine/Source/GameNetwork/LANAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,7 @@ void LANAPI::update()
}

// Handle any new messages
int i;
for (i=0; i<MAX_MESSAGES && !LANbuttonPushed; ++i)
for (size_t i = 0; i < ARRAY_SIZE(m_transport->m_inBuffer) && !LANbuttonPushed; ++i)
{
if (m_transport->m_inBuffer[i].length > 0)
{
Expand Down Expand Up @@ -432,6 +431,10 @@ void LANAPI::update()
// Mark it as read
m_transport->m_inBuffer[i].length = 0;
}
else
{
break;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there cannot be non-zero buffer after a zero buffer, yes?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. Transport::doRecv should handle that correctly.

}
}
if(LANbuttonPushed)
return;
Expand Down
4 changes: 3 additions & 1 deletion Core/GameEngine/Source/GameNetwork/NAT.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ NATConnectionState NAT::connectionUpdate() {
m_transport->update();

// check to see if we've been probed.
for (Int i = 0; i < MAX_MESSAGES; ++i) {
for (size_t i = 0; i < ARRAY_SIZE(m_transport->m_inBuffer); ++i) {
if (m_transport->m_inBuffer[i].length > 0) {
#ifdef DEBUG_LOGGING
UnsignedInt ip = m_transport->m_inBuffer[i].addr;
Expand Down Expand Up @@ -368,6 +368,8 @@ NATConnectionState NAT::connectionUpdate() {
PRINTF_IP_AS_4_INTS(ip), m_transport->m_inBuffer[i].port));
m_transport->m_inBuffer[i].length = 0;
}
} else {
break;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively could do early breaks in the loops. But its ok either way.

}
}

Expand Down
77 changes: 41 additions & 36 deletions Core/GameEngine/Source/GameNetwork/Transport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,9 @@ Bool Transport::doSend() {
}

// Send all messages
int i;
for (i=0; i<MAX_MESSAGES; ++i)
for (size_t i = 0; i < ARRAY_SIZE(m_outBuffer); ++i)
{
if (m_outBuffer[i].length != 0)
if (m_outBuffer[i].length > 0)
{
int bytesSent = 0;
// TheSuperHackers @info The handling of data sizing of the payload within a UDP packet is confusing due to the current networking implementation
Expand Down Expand Up @@ -249,17 +248,20 @@ Bool Transport::doSend() {
// Latency simulation - deliver anything we're holding on to that is ready
if (m_useLatency)
{
for (i=0; i<MAX_MESSAGES; ++i)
size_t bufferIndex = 0;

for (size_t i = 0; i < ARRAY_SIZE(m_delayedInBuffer); ++i)
{
if (m_delayedInBuffer[i].message.length != 0 && m_delayedInBuffer[i].deliveryTime <= now)
if (m_delayedInBuffer[i].message.length > 0 && m_delayedInBuffer[i].deliveryTime <= now)
{
for (int j=0; j<MAX_MESSAGES; ++j)
for (; bufferIndex < ARRAY_SIZE(m_inBuffer); ++bufferIndex)
{
if (m_inBuffer[j].length == 0)
if (m_inBuffer[bufferIndex].length <= 0)
{
// Empty slot; use it
memcpy(&m_inBuffer[j], &m_delayedInBuffer[i].message, sizeof(TransportMessage));
memcpy(&m_inBuffer[bufferIndex], &m_delayedInBuffer[i].message, sizeof(TransportMessage));
m_delayedInBuffer[i].message.length = 0;
++bufferIndex;
break;
}
}
Expand Down Expand Up @@ -292,6 +294,7 @@ Bool Transport::doRecv()
TransportMessage incomingMessage;
unsigned char *buf = (unsigned char *)&incomingMessage;
int len = MAX_NETWORK_MESSAGE_LEN;
size_t bufferIndex = 0;
// DEBUG_LOG(("Transport::doRecv - checking"));
while ( (len=m_udpsock->Read(buf, MAX_NETWORK_MESSAGE_LEN, &from)) > 0 )
{
Expand Down Expand Up @@ -330,43 +333,47 @@ Bool Transport::doRecv()
m_incomingPackets[m_statisticsSlot]++;
m_incomingBytes[m_statisticsSlot] += len;

for (int i=0; i<MAX_MESSAGES; ++i)
{
DEBUG_ASSERTCRASH(bufferIndex < MAX_MESSAGES, ("Message lost!"));

#if defined(RTS_DEBUG)
// Latency simulation
if (m_useLatency)
// Latency simulation
if (m_useLatency)
{
for (; bufferIndex < ARRAY_SIZE(m_delayedInBuffer); ++bufferIndex)
{
if (m_delayedInBuffer[i].message.length == 0)
if (m_delayedInBuffer[bufferIndex].message.length <= 0)
{
// Empty slot; use it
m_delayedInBuffer[i].deliveryTime =
m_delayedInBuffer[bufferIndex].deliveryTime =
now + TheGlobalData->m_latencyAverage +
(Int)(TheGlobalData->m_latencyAmplitude * sin(now * TheGlobalData->m_latencyPeriod)) +
GameClientRandomValue(-TheGlobalData->m_latencyNoise, TheGlobalData->m_latencyNoise);
m_delayedInBuffer[i].message.length = incomingMessage.length;
m_delayedInBuffer[i].message.addr = ntohl(from.sin_addr.S_un.S_addr);
m_delayedInBuffer[i].message.port = ntohs(from.sin_port);
memcpy(&m_delayedInBuffer[i].message, buf, len);
m_delayedInBuffer[bufferIndex].message.length = incomingMessage.length;
m_delayedInBuffer[bufferIndex].message.addr = ntohl(from.sin_addr.S_un.S_addr);
m_delayedInBuffer[bufferIndex].message.port = ntohs(from.sin_port);
memcpy(&m_delayedInBuffer[bufferIndex].message, buf, len);
++bufferIndex;
break;
}
}
else
{

continue;
}
#endif
if (m_inBuffer[i].length == 0)
{
// Empty slot; use it
m_inBuffer[i].length = incomingMessage.length;
m_inBuffer[i].addr = ntohl(from.sin_addr.S_un.S_addr);
m_inBuffer[i].port = ntohs(from.sin_port);
memcpy(&m_inBuffer[i], buf, len);
break;
}
#if defined(RTS_DEBUG)

for (; bufferIndex < ARRAY_SIZE(m_inBuffer); ++bufferIndex)
{
if (m_inBuffer[bufferIndex].length <= 0)
{
// Empty slot; use it
m_inBuffer[bufferIndex].length = incomingMessage.length;
m_inBuffer[bufferIndex].addr = ntohl(from.sin_addr.S_un.S_addr);
m_inBuffer[bufferIndex].port = ntohs(from.sin_port);
memcpy(&m_inBuffer[bufferIndex], buf, len);
++bufferIndex;
break;
}
#endif
}
//DEBUG_ASSERTCRASH(i<MAX_MESSAGES, ("Message lost!"));
}

if (len == -1) {
Expand All @@ -381,17 +388,15 @@ Bool Transport::doRecv()
Bool Transport::queueSend(UnsignedInt addr, UnsignedShort port, const UnsignedByte *buf, Int len /*,
NetMessageFlags flags, Int id */)
{
int i;

if (len < 1 || len > MAX_PACKET_SIZE)
{
DEBUG_LOG(("Transport::queueSend - Invalid Packet size"));
return false;
}

for (i=0; i<MAX_MESSAGES; ++i)
for (size_t i = 0; i < ARRAY_SIZE(m_outBuffer); ++i)
{
if (m_outBuffer[i].length == 0)
if (m_outBuffer[i].length <= 0)
{
// Insert data here
m_outBuffer[i].length = len;
Expand Down
Loading