Refactoring network messaging

All network messages are now written and read using
the engine's Writer and Reader classes.

Added the build option DENG_WRITER_TYPECHECK
for verifying that the correct type of values are read back
(only for debugging purposes, as it increases the size of
the messages by including the type markers).

doomsday/CMakeLists.txt

@@ -76,6 +76,8 @@ IF (${CMAKE_BINARY_DIR} STREQUAL ${CMAKE_SOURCE_DIR})
     MESSAGE( FATAL_ERROR "In-tree Builds are NOT supported. cd ${CMAKE_SOURCE_DIR}/build and try again")
 ENDIF ()
 
+ADD_DEFINITIONS ( -DDENG_WRITER_TYPECHECK )
+
 ################################################################################
 ######################## Doomsday Paths  #######################################
 ## Default Install Prefix on *NIX is /usr/local

doomsday/engine/api/doomsday.h

@@ -207,8 +207,7 @@ extern          "C" {
                                              const char* propName, valuetype_t type);
 
     // Network.
-    void            Net_SendPacket(int to_player, int type, void* data,
-                                   size_t length);
+    void            Net_SendPacket(int to_player, int type, const void* data, size_t length);
     int             Net_GetTicCmd(void* command, int player);
     const char*     Net_GetPlayerName(int player);
     ident_t         Net_GetPlayerID(int player);

doomsday/engine/api/writer.h

@@ -30,6 +30,21 @@ extern "C" {
 
 #include "dd_types.h"
 
+#ifdef DENG_WRITER_TYPECHECK
+// Writer Type Check Codes.
+enum
+{
+    WTCC_CHAR = 0x13,
+    WTCC_BYTE = 0xf6,
+    WTCC_INT16 = 0x55,
+    WTCC_UINT16 = 0xab,
+    WTCC_INT32 = 0x3f,
+    WTCC_UINT32 = 0xbb,
+    WTCC_FLOAT = 0x71,
+    WTCC_BLOCK = 0x6e
+};
+#endif
+
 struct writer_s; // The writer instance (opaque).
 typedef struct writer_s Writer;
 
@@ -41,7 +56,7 @@ typedef struct writer_s Writer;
 Writer* Writer_New(void);
 
 /**
- * Initializes the writer. The writer will use @a buffer as the writing buffer.
+ * Constructs a new writer. The writer will use @a buffer as the writing buffer.
  * The buffer will use network byte order. The writer has to be destroyed
  * with Writer_Destruct() after it is not needed any more.
  *
@@ -50,6 +65,14 @@ Writer* Writer_New(void);
  */
 Writer* Writer_NewWithBuffer(byte* buffer, size_t maxLen);
 
+/**
+ * Constructs a new writer. The writer will allocate memory for the buffer while
+ * more data gets written.
+ *
+ * @param maxLen  Maximum size for the buffer. Use zero for unlimited size.
+ */
+Writer* Writer_NewWithDynamicBuffer(size_t maxLen);
+
 /**
  * Destroys the writer.
  */
@@ -61,6 +84,12 @@ void Writer_Destruct(Writer* writer);
  */
 size_t Writer_Size(const Writer* writer);
 
+/**
+ * Returns a pointer to the beginning of the written data.
+ * @see Writer_Size()
+ */
+const byte* Writer_Data(const Writer* writer);
+
 /**
  * Returns the maximum size of the writing buffer.
  */

doomsday/engine/portable/include/net_main.h

@@ -274,8 +274,7 @@ void            Net_Init(void);
 void            Net_Shutdown(void);
 void            Net_AllocArrays(void);
 void            Net_DestroyArrays(void);
-void            Net_SendPacket(int to_player, int type, void *data,
-                               size_t length);
+void            Net_SendPacket(int to_player, int type, const void *data, size_t length);
 boolean         Net_GetPacket(void);
 void            Net_SendBuffer(int to_player, int sp_flags);
 void            Net_InitGame(void);

doomsday/engine/portable/src/cl_main.c

@@ -127,7 +127,7 @@ void Cl_CleanUp(void)
  */
 void Cl_SendHello(void)
 {
-    char                buf[256];
+    char buf[256];
 
     Msg_Begin(PCL_HELLO2);
     Writer_WriteUInt32(msgWriter, clientID);

doomsday/engine/portable/src/net_main.c

@@ -224,6 +224,23 @@ void Net_SendBuffer(int toPlayer, int spFlags)
 {
     assert(!Msg_BeingWritten()); // Must finish writing before calling this.
 
+    /*
+#ifdef _DEBUG
+    {
+        char* buf = M_Calloc(netBuffer.length * 3 + 1);
+        int i;
+        for(i = 0; i < netBuffer.length; ++i)
+        {
+            char tmp[10];
+            sprintf(tmp, "%02x ", netBuffer.msg.data[i]);
+            strcat(buf, tmp);
+        }
+        Con_Message("Net_SendBuffer: [%i] %s\n", netBuffer.length, buf);
+        M_Free(buf);
+    }
+#endif
+    */
+
     // Don't send anything during demo playback.
     if(playback)
         return;
@@ -299,13 +316,20 @@ Smoother* Net_PlayerSmoother(int player)
 /**
  * This is the public interface of the message sender.
  */
-void Net_SendPacket(int to_player, int type, void *data, size_t length)
+void Net_SendPacket(int to_player, int type, const void *data, size_t length)
 {
     unsigned int flags = 0;
 
+#ifndef DENG_WRITER_TYPECHECK
     Msg_Begin(type);
     if(data) Writer_Write(msgWriter, data, length);
     Msg_End();
+#else
+    assert(length <= NETBUFFER_MAXDATA);
+    netBuffer.msg.type = type;
+    netBuffer.length = length;
+    if(data) memcpy(netBuffer.msg.data, data, length);
+#endif
 
     if(isClient)
     {   // As a client we can only send messages to the server.
@@ -362,53 +386,6 @@ boolean Net_IsLocalPlayer(int plrNum)
  */
 void Net_SendCommands(void)
 {
-#if 0
-    uint                i;
-    byte               *msg;
-    ticcmd_t           *cmd;
-
-    if(isDedicated)
-        return;
-
-    // Send the commands of all local players.
-    for(i = 0; i < DDMAXPLAYERS; ++i)
-    {
-        if(!Net_IsLocalPlayer(i))
-            continue;
-
-        /**
-         * Clients send their ticcmds to the server at regular intervals,
-         * but significantly less often than new ticcmds are built.
-         * Therefore they need to send a combination of all the cmds built
-         * during the wait period.
-         */
-
-        cmd = clients[i].aggregateCmd;
-
-        /**
-         * The game will pack the commands into a buffer. The returned
-         * pointer points to a buffer that contains its size and the
-         * packed commands.
-         */
-
-        msg = gx.NetWriteCommands(1, cmd);
-
-        Msg_Begin(PCL_COMMANDS);
-        Msg_Write(msg + 2, *(ushort *) msg);
-
-        /**
-         * Send the packet to the server, i.e. player zero.
-         * Player commands are sent over TCP so their integrity and order
-         * are guaranteed.
-         */
-
-        Net_SendBuffer(0, (isClient ? 0 : SPF_REBOUND) | SPF_ORDERED);
-
-        // Clients will begin composing a new aggregate now that this one
-        // has been sent.
-        memset(cmd, 0, TICCMD_SIZE);
-    }
-#endif
 }
 
 static void Net_DoUpdate(void)
@@ -506,35 +483,6 @@ void Net_Update(void)
  */
 void Net_BuildLocalCommands(timespan_t time)
 {
-#if 0
-    uint        i;
-    ticcmd_t   *cmd;
-
-    if(isDedicated)
-        return;
-
-    // Generate ticcmds for local players.
-    for(i = 0; i < DDMAXPLAYERS; ++i)
-    {
-        if(!Net_IsLocalPlayer(i))
-            continue;
-
-        cmd = clients[i].lastCmd;
-
-        // The command will stay 'empty' if no controls are active.
-        memset(cmd, 0, sizeof(*cmd));
-        // No actions can be undertaken during demo playback or when
-        // in UI mode.
-        if(!(playback || UI_IsActive()))
-        {
-            P_BuildCommand(cmd, i);
-        }
-
-        // Be sure to merge each built command into the aggregate that
-        // will be sent periodically to the server.
-        P_MergeCommand(clients[i].aggregateCmd, cmd);
-    }
-#endif
 }
 
 /**
@@ -603,6 +551,7 @@ void Net_StopGame(void)
         // This means we should inform all the connected clients that the
         // server is about to close.
         Msg_Begin(PSV_SERVER_CLOSE);
+        Msg_End();
         Net_SendBuffer(NSP_BROADCAST, 0);
 #if 0
         N_FlushOutgoing();
@@ -611,6 +560,7 @@ void Net_StopGame(void)
     else
     {   // We are a connected client.
         Msg_Begin(PCL_GOODBYE);
+        Msg_End();
         Net_SendBuffer(0, 0);
 
         // Must stop recording, we're disconnecting.

doomsday/engine/portable/src/net_msg.c

@@ -80,7 +80,7 @@ void Msg_End(void)
     }
 }
 
-void Msg_Read(void)
+void Msg_BeginRead(void)
 {
     if(msgWriter)
     {

doomsday/engine/portable/src/reader.c

@@ -26,6 +26,14 @@
 #include "net_buf.h"
 #include "reader.h"
 
+#ifdef DENG_WRITER_TYPECHECK
+#  include "writer.h"
+
+#  define Reader_TypeCheck(r, code)   if(r->data[r->pos++] != code) assert(!code);
+#else
+#  define Reader_TypeCheck(r, code)
+#endif
+
 struct reader_s
 {
     const byte* data;       // The data buffer.
@@ -35,6 +43,11 @@ struct reader_s
 
 static boolean Reader_Check(const Reader* reader, size_t len)
 {
+#ifdef DENG_WRITER_TYPECHECK
+    // One byte for the code.
+    len++;
+#endif
+
     if(!reader || !reader->data) return false;
     if(reader->pos > reader->size - len)
     {
@@ -95,12 +108,14 @@ boolean Reader_AtEnd(const Reader* reader)
 int8_t Reader_ReadChar(Reader* reader)
 {
     if(!Reader_Check(reader, 1)) return 0;
+    Reader_TypeCheck(reader, WTCC_CHAR);
     return ((int8_t*)reader->data)[reader->pos++];
 }
 
 byte Reader_ReadByte(Reader* reader)
 {
     if(!Reader_Check(reader, 1)) return 0;
+    Reader_TypeCheck(reader, WTCC_BYTE);
     return reader->data[reader->pos++];
 }
 
@@ -109,6 +124,7 @@ int16_t Reader_ReadInt16(Reader* reader)
     int16_t result = 0;
     if(Reader_Check(reader, 2))
     {
+        Reader_TypeCheck(reader, WTCC_INT16);
         result = SHORT( *(int16_t*) &reader->data[reader->pos] );
         reader->pos += 2;
     }
@@ -120,6 +136,7 @@ uint16_t Reader_ReadUInt16(Reader* reader)
     uint16_t result = 0;
     if(Reader_Check(reader, 2))
     {
+        Reader_TypeCheck(reader, WTCC_UINT16);
         result = USHORT( *(uint16_t*) &reader->data[reader->pos] );
         reader->pos += 2;
     }
@@ -131,6 +148,7 @@ int32_t Reader_ReadInt32(Reader* reader)
     int32_t result = 0;
     if(Reader_Check(reader, 4))
     {
+        Reader_TypeCheck(reader, WTCC_INT32);
         result = LONG( *(int32_t*) &reader->data[reader->pos] );
         reader->pos += 4;
     }
@@ -142,6 +160,7 @@ uint32_t Reader_ReadUInt32(Reader* reader)
     uint32_t result = 0;
     if(Reader_Check(reader, 4))
     {
+        Reader_TypeCheck(reader, WTCC_UINT32);
         result = ULONG( *(uint32_t*) &reader->data[reader->pos] );
         reader->pos += 4;
     }
@@ -150,14 +169,21 @@ uint32_t Reader_ReadUInt32(Reader* reader)
 
 float Reader_ReadFloat(Reader* reader)
 {
-    uint32_t v = Reader_ReadUInt32(reader);
-    return *(float*) &v;
+    float result = 0;
+    if(Reader_Check(reader, 4))
+    {
+        Reader_TypeCheck(reader, WTCC_FLOAT);
+        result = FLOAT( *(float*) &reader->data[reader->pos] );
+        reader->pos += 4;
+    }
+    return result;
 }
 
 void Reader_Read(Reader* reader, void* buffer, size_t len)
 {
     if(Reader_Check(reader, len))
     {
+        Reader_TypeCheck(reader, WTCC_BLOCK);
         memcpy(buffer, reader->data + reader->pos, len);
         reader->pos += len;
     }
@@ -180,8 +206,8 @@ uint32_t Reader_ReadPackedUInt32(Reader* reader)
     int pos = 0;
     uint value = 0;
 
-    while(Reader_Check(reader, 1))
-    {
+    do {
+        if(!Reader_Check(reader, 1)) return 0;
         pack = Reader_ReadByte(reader);
         value |= ((pack & 0x7f) << pos);
         pos += 7;

doomsday/engine/portable/src/sv_main.c

@@ -780,6 +780,7 @@ void Sv_PlayerLeaves(unsigned int nodeID)
         // Inform other clients about this.
         Msg_Begin(PSV_PLAYER_EXIT);
         Writer_WriteByte(msgWriter, plrNum);
+        Msg_End();
         Net_SendBuffer(NSP_BROADCAST, 0);
     }
 
@@ -1259,6 +1260,7 @@ D_CMD(Logout)
     // Send a logout packet.
     Msg_Begin(PKT_LOGIN);
     Writer_WriteByte(msgWriter, false);       // You're outta here.
+    Msg_End();
     Net_SendBuffer(netRemoteUser, 0);
     netRemoteUser = 0;
     return true;