Extend Q2PRO protocol to allow clientNum >= 255.

When GMF_CLIENTNUM feature was introduced, it was expected that client
POV number will only be set to valid client slot numbers in range [0,
255). Therefore, clientNum has been transmitted as a byte, with 255
(CLIENTNUM_NONE) reserved for a special case to indicate no current POV
number (this has always been buggy, see below).

However, one might want to set client POV to any entity >= 256, not just
client entitites (to view the game from monster POV, for example). Thus,
make clientNum strictly mean ‘number of POV entity minus 1’ and transmit
it as a short in range [-1, MAX_EDICTS - 1).

To accomodate older Q2PRO clients, reset clientNum to client slot number
and zero out POV entity modelindex if clientNum is >= 255.

clientNum == -1 is legal, and can happen e.g. when playing a MVD without
dummy MVD observer spawned. -1 maps to 255 (CLIENTNUM_NONE) upon reading
on older Q2PRO clients. This is buggy, as it will make entity 256
invisible, but there doesn't seem to be a proper fix. Mapping -1 to
client slot number will make random _player_ entity invisible instead,
which is worse. So don't change anything about -1.

For newer Q2PRO clients clientNum == -1 will be transmitted as literal
-1. This will map to entity 0 (world) and things will work as expected.

This was (and still is) a huge mess. Fixes NVIDIA#260.

inc/common/protocol.h

@@ -47,7 +47,8 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #define PROTOCOL_VERSION_Q2PRO_SERVER_STATE     1019    // r1302
 #define PROTOCOL_VERSION_Q2PRO_EXTENDED_LAYOUT  1020    // r1354
 #define PROTOCOL_VERSION_Q2PRO_ZLIB_DOWNLOADS   1021    // r1358
-#define PROTOCOL_VERSION_Q2PRO_CURRENT          1021    // r1358
+#define PROTOCOL_VERSION_Q2PRO_CLIENTNUM_SHORT  1022    // r2161
+#define PROTOCOL_VERSION_Q2PRO_CURRENT          1022    // r2161
 
 #define PROTOCOL_VERSION_MVD_MINIMUM            2009    // r168
 #define PROTOCOL_VERSION_MVD_CURRENT            2010    // r177
@@ -64,6 +65,9 @@ with this program; if not, write to the Free Software Foundation, Inc.,
     ((x) >= PROTOCOL_VERSION_MVD_MINIMUM && \
      (x) <= PROTOCOL_VERSION_MVD_CURRENT)
 
+#define VALIDATE_CLIENTNUM(x) \
+    ((x) >= -1 && (x) < MAX_EDICTS - 1)
+
 //=========================================
 
 #define UPDATE_BACKUP   16  // copies of entity_state_t to keep buffered

src/client/parse.c

@@ -321,7 +321,14 @@ static void CL_ParseFrame(int extrabits)
         if (cls.serverProtocol == PROTOCOL_VERSION_Q2PRO) {
             // parse clientNum
             if (extraflags & EPS_CLIENTNUM) {
-                frame.clientNum = MSG_ReadByte();
+                if (cls.protocolVersion < PROTOCOL_VERSION_Q2PRO_CLIENTNUM_SHORT) {
+                    frame.clientNum = MSG_ReadByte();
+                } else {
+                    frame.clientNum = MSG_ReadShort();
+                }
+                if (!VALIDATE_CLIENTNUM(frame.clientNum)) {
+                    Com_Error(ERR_DROP, "%s: bad clientNum", __func__);
+                }
             } else if (oldframe) {
                 frame.clientNum = oldframe->clientNum;
             }
@@ -636,7 +643,8 @@ static void CL_ParseServerData(void)
         Com_SetColor(COLOR_NONE);
 
         // make sure clientNum is in range
-        if (cl.clientNum < 0 || cl.clientNum >= MAX_CLIENTS) {
+        if (!VALIDATE_CLIENTNUM(cl.clientNum)) {
+            Com_WPrintf("Serverdata has invalid playernum %d\n", cl.clientNum);
             cl.clientNum = -1;
         }
     }

src/server/entities.c

@@ -298,7 +298,11 @@ void SV_WriteFrameToClient_Enhanced(client_t *client)
             int clientNum = oldframe ? oldframe->clientNum : 0;
             if (clientNum != frame->clientNum) {
                 extraflags |= EPS_CLIENTNUM;
-                MSG_WriteByte(frame->clientNum);
+                if (client->version < PROTOCOL_VERSION_Q2PRO_CLIENTNUM_SHORT) {
+                    MSG_WriteByte(frame->clientNum);
+                } else {
+                    MSG_WriteShort(frame->clientNum);
+                }
             }
         }
     }
@@ -394,6 +398,7 @@ void SV_BuildClientFrame(client_t *client)
     byte        clientpvs[VIS_MAX_BYTES];
     bool    ent_visible;
     int cull_nonvisible_entities = Cvar_Get("sv_cull_nonvisible_entities", "1", CVAR_CHEAT)->integer;
+    bool        need_clientnum_fix;
 
     clent = client->edict;
     if (!clent->client)
@@ -428,10 +433,20 @@ void SV_BuildClientFrame(client_t *client)
     // grab the current clientNum
     if (g_features->integer & GMF_CLIENTNUM) {
         frame->clientNum = clent->client->clientNum;
+        if (!VALIDATE_CLIENTNUM(frame->clientNum)) {
+            Com_WPrintf("%s: bad clientNum %d for client %d\n",
+                        __func__, frame->clientNum, client->number);
+            frame->clientNum = client->number;
+        }
     } else {
         frame->clientNum = client->number;
     }
 
+    // fix clientNum if out of range for older version of Q2PRO protocol
+    need_clientnum_fix = client->protocol == PROTOCOL_VERSION_Q2PRO
+        && client->version < PROTOCOL_VERSION_Q2PRO_CLIENTNUM_SHORT
+        && frame->clientNum >= CLIENTNUM_NONE;
+
 	if (clientcluster >= 0)
 	{
 		CM_FatPVS(client->cm, clientpvs, org, DVIS_PVS2);
@@ -558,7 +573,7 @@ void SV_BuildClientFrame(client_t *client)
 
         // hide POV entity from renderer, unless this is player's own entity
         if (e == frame->clientNum + 1 && ent != clent &&
-            (g_features->integer & GMF_CLIENTNUM) && !Q2PRO_OPTIMIZE(client)) {
+            (!Q2PRO_OPTIMIZE(client) || need_clientnum_fix)) {
             state->modelindex = 0;
         }
 
@@ -582,5 +597,8 @@ void SV_BuildClientFrame(client_t *client)
             break;
         }
     }
+
+    if (need_clientnum_fix)
+        frame->clientNum = client->slot;
 }