Merge branch 'namefix' into 'master'

Some fixes for bad player info on joining servers

Closes #151

See merge request KartKrew/Kart!470

src/d_clisrv.c

@@ -795,10 +795,14 @@ static boolean CL_SendJoin(void)
 			sizeof netbuffer->u.clientcfg.application);
 
 	for (i = 0; i <= splitscreen; i++)
-		CleanupPlayerName(g_localplayers[i], cv_playername[i].zstring);
-
-	for (i = 0; i < MAXSPLITSCREENPLAYERS; i++)
+	{
+		// the MAXPLAYERS addition is necessary to communicate that g_localplayers is not yet safe to reference
+		CleanupPlayerName(MAXPLAYERS+i, cv_playername[i].zstring);
 		strncpy(netbuffer->u.clientcfg.names[i], cv_playername[i].zstring, MAXPLAYERNAME);
+	}
+	// privacy shield for the local players not joining this session
+	for (; i < MAXSPLITSCREENPLAYERS; i++)
+		strncpy(netbuffer->u.clientcfg.names[i], va("Player %c", 'A' + i), MAXPLAYERNAME);
 
 	return HSendPacket(servernode, false, 0, sizeof (clientconfig_pak));
 }
@@ -1334,7 +1338,7 @@ static void CL_ReloadReceivedSavegame(void)
 	for (i = 0; i < MAXPLAYERS; i++)
 	{
 		LUA_InvalidatePlayer(&players[i]);
-		sprintf(player_names[i], "Player %d", i + 1);
+		sprintf(player_names[i], "Player %c", 'A' + i);
 	}
 
 	CL_LoadReceivedSavegame(true);
@@ -2401,7 +2405,7 @@ void CL_RemovePlayer(INT32 playernum, kickreason_t reason)
 		doomcom->numslots--;
 
 	// Reset the name
-	sprintf(player_names[playernum], "Player %d", playernum+1);
+	sprintf(player_names[playernum], "Player %c", 'A' + playernum);
 
 	player_name_changes[playernum] = 0;
 
@@ -3213,7 +3217,7 @@ void SV_ResetServer(void)
 		playeringame[i] = false;
 		playernode[i] = UINT8_MAX;
 		memset(playeraddress[i], 0, sizeof(*playeraddress));
-		sprintf(player_names[i], "Player %d", i + 1);
+		sprintf(player_names[i], "Player %c", 'A' + i);
 		adminplayers[i] = -1; // Populate the entire adminplayers array with -1.
 		K_ClearClientPowerLevels();
 		splitscreen_invitations[i] = -1;
@@ -3412,7 +3416,7 @@ static void Got_AddPlayer(UINT8 **p, INT32 playernum)
 
 		P_ForceLocalAngle(newplayer, newplayer->angleturn);
 
-		D_SendPlayerConfig();
+		D_SendPlayerConfig(splitscreenplayer);
 		addedtogame = true;
 
 		if (rejoined)
@@ -3558,14 +3562,13 @@ static void Got_AddBot(UINT8 **p, INT32 playernum)
 	LUAh_PlayerJoin(newplayernum);
 }
 
-static boolean SV_AddWaitingPlayers(const char *name, const char *name2, const char *name3, const char *name4)
+static boolean SV_AddWaitingPlayers(SINT8 node, const char *name, const char *name2, const char *name3, const char *name4)
 {
-	INT32 node, n, newplayer = false;
+	INT32 n, newplayernum;
 	UINT8 buf[4 + MAXPLAYERNAME];
 	UINT8 *buf_p = buf;
-	INT32 newplayernum;
+	boolean newplayer = false;
 
-	for (node = 0; node < MAXNETNODES; node++)
 	{
 		// splitscreen can allow 2+ players in one node
 		for (; nodewaiting[node] > 0; nodewaiting[node]--)
@@ -3684,6 +3687,7 @@ boolean SV_SpawnServer(void)
 	I_Error("What do you think you're doing?");
 	return false;
 #else
+	boolean result = false;
 	if (demo.playback)
 		G_StopDemo(); // reset engine parameter
 	if (metalplayback)
@@ -3710,7 +3714,14 @@ boolean SV_SpawnServer(void)
 		else doomcom->numslots = 1;
 	}
 
-	return SV_AddWaitingPlayers(cv_playername[0].zstring, cv_playername[1].zstring, cv_playername[2].zstring, cv_playername[3].zstring);
+	// strictly speaking, i'm not convinced the following is necessary
+	// but I'm not confident enough to remove it entirely in case it breaks something
+	{
+		SINT8 node = 0;
+		for (; node < MAXNETNODES; node++)
+			result |= SV_AddWaitingPlayers(node, cv_playername[0].zstring, cv_playername[1].zstring, cv_playername[2].zstring, cv_playername[3].zstring);
+	}
+	return result;
 #endif
 }
 
@@ -3882,7 +3893,7 @@ static void HandleConnect(SINT8 node)
 				SV_SendSaveGame(node, false); // send a complete game state
 				DEBFILE("send savegame\n");
 			}
-			SV_AddWaitingPlayers(names[0], names[1], names[2], names[3]);
+			SV_AddWaitingPlayers(node, names[0], names[1], names[2], names[3]);
 			joindelay += cv_joindelay.value * TICRATE;
 			player_joining = true;
 		}

src/d_netcmd.c

@@ -222,8 +222,6 @@ static void Command_KartGiveItem_f(void);
 //                           CLIENT VARIABLES
 // =========================================================================
 
-void SendWeaponPref(UINT8 n);
-
 static CV_PossibleValue_t usemouse_cons_t[] = {{0, "Off"}, {1, "On"}, {2, "Force"}, {0, NULL}};
 
 #ifdef LJOYSTICK
@@ -1116,6 +1114,7 @@ boolean EnsurePlayerNameIsGood(char *name, INT32 playernum)
   * is restored to what it was before.
   *
   * We assume that if playernum is in ::g_localplayers
+  * (unless clientjoin is true, a necessary evil)
   * the console variable ::cv_playername[n] is
   * already set to newname. However, the player name table is assumed to
   * contain the old name.
@@ -1134,6 +1133,10 @@ void CleanupPlayerName(INT32 playernum, const char *newname)
 	char *tmpname = NULL;
 	INT32 i;
 	boolean namefailed = true;
+	boolean clientjoin = !!(playernum >= MAXPLAYERS);
+
+	if (clientjoin)
+		playernum -= MAXPLAYERS;
 
 	buf = Z_StrDup(newname);
 
@@ -1191,17 +1194,20 @@ void CleanupPlayerName(INT32 playernum, const char *newname)
 		}
 
 		// no stealing another player's name
-		for (i = 0; i < MAXPLAYERS; i++)
+		if (!clientjoin)
 		{
-			if (i != playernum && playeringame[i]
-				&& strcasecmp(tmpname, player_names[i]) == 0)
+			for (i = 0; i < MAXPLAYERS; i++)
 			{
-				break;
+				if (i != playernum && playeringame[i]
+					&& strcasecmp(tmpname, player_names[i]) == 0)
+				{
+					break;
+				}
 			}
-		}
 
-		if (i < MAXPLAYERS)
-			break;
+			if (i < MAXPLAYERS)
+				break;
+		}
 
 		// name is okay then
 		namefailed = false;
@@ -1212,18 +1218,23 @@ void CleanupPlayerName(INT32 playernum, const char *newname)
 
 	// set consvars whether namefailed or not, because even if it succeeded,
 	// spaces may have been removed
-	for (i = 0; i <= splitscreen; i++)
+	if (clientjoin)
+		CV_StealthSet(&cv_playername[playernum], tmpname);
+	else
 	{
-		if (playernum == g_localplayers[i])
+		for (i = 0; i <= splitscreen; i++)
 		{
-			CV_StealthSet(&cv_playername[i], tmpname);
-			break;
+			if (playernum == g_localplayers[i])
+			{
+				CV_StealthSet(&cv_playername[i], tmpname);
+				break;
+			}
 		}
-	}
 
-	if (i > splitscreen)
-	{
-		I_Assert(((void)"CleanupPlayerName used on non-local player", 0));
+		if (i > splitscreen)
+		{
+			I_Assert(((void)"CleanupPlayerName used on non-local player", 0));
+		}
 	}
 
 	Z_Free(buf);
@@ -1819,33 +1830,28 @@ static void Got_LeaveParty(UINT8 **cp,INT32 playernum)
 	}
 }
 
-void D_SendPlayerConfig(void)
+void D_SendPlayerConfig(UINT8 n)
 {
-	UINT8 i;
+	UINT8 buf[4];
+	UINT8 *p = buf;
 
-	for (i = 0; i <= splitscreen; i++)
+	SendNameAndColor(n);
+	SendWeaponPref(n);
+
+	if (n == 0)
 	{
-		UINT8 buf[4];
-		UINT8 *p = buf;
-
-		SendNameAndColor(i);
-		SendWeaponPref(i);
-
-		if (i == 0)
-		{
-			// Send it over
-			WRITEUINT16(p, vspowerlevel[PWRLV_RACE]);
-			WRITEUINT16(p, vspowerlevel[PWRLV_BATTLE]);
-		}
-		else
-		{
-			// Splitscreen players have invalid powerlevel
-			WRITEUINT16(p, 0);
-			WRITEUINT16(p, 0);
-		}
-
-		SendNetXCmdForPlayer(i, XD_POWERLEVEL, buf, p-buf);
+		// Send it over
+		WRITEUINT16(p, vspowerlevel[PWRLV_RACE]);
+		WRITEUINT16(p, vspowerlevel[PWRLV_BATTLE]);
 	}
+	else
+	{
+		// Splitscreen players have invalid powerlevel
+		WRITEUINT16(p, 0);
+		WRITEUINT16(p, 0);
+	}
+
+	SendNetXCmdForPlayer(n, XD_POWERLEVEL, buf, p-buf);
 }
 
 // Only works for displayplayer, sorry!

src/d_netcmd.h

@@ -207,7 +207,8 @@ void D_RegisterServerCommands(void);
 void D_RegisterClientCommands(void);
 void CleanupPlayerName(INT32 playernum, const char *newname);
 boolean EnsurePlayerNameIsGood(char *name, INT32 playernum);
-void D_SendPlayerConfig(void);
+void SendWeaponPref(UINT8 n);
+void D_SendPlayerConfig(UINT8 n);
 void Command_ExitGame_f(void);
 void Command_Retry_f(void);
 void D_GameTypeChanged(INT32 lastgametype); // not a real _OnChange function anymore

src/g_game.c

@@ -346,7 +346,6 @@ static void kickstartaccel_OnChange(void);
 static void kickstartaccel2_OnChange(void);
 static void kickstartaccel3_OnChange(void);
 static void kickstartaccel4_OnChange(void);
-void SendWeaponPref(UINT8 n);
 
 static CV_PossibleValue_t joyaxis_cons_t[] = {{0, "None"},
 {1, "X-Axis"}, {2, "Y-Axis"}, {-1, "X-Axis-"}, {-2, "Y-Axis-"},

src/r_main.c

@@ -149,10 +149,6 @@ static void ChaseCam_OnChange(void);
 static void ChaseCam2_OnChange(void);
 static void ChaseCam3_OnChange(void);
 static void ChaseCam4_OnChange(void);
-void SendWeaponPref(void);
-void SendWeaponPref2(void);
-void SendWeaponPref3(void);
-void SendWeaponPref4(void);
 
 consvar_t cv_tailspickup = CVAR_INIT ("tailspickup", "On", CV_NETVAR|CV_NOSHOWHELP, CV_OnOff, NULL);
 consvar_t cv_chasecam[MAXSPLITSCREENPLAYERS] = {