Merge branch 'fixSetNameForBot' into 'master'

Clamp and validate names in K_SetNameForBot correctly for Lua (fixes #238)

Closes #238

See merge request kart-krew-dev/ring-racers!38

src/d_netcmd.c

@@ -618,7 +618,23 @@ static boolean AllowedPlayerNameChar(char ch)
 	return true;
 }
 
-boolean EnsurePlayerNameIsGood(char *name, INT32 playernum)
+boolean IsPlayerNameUnique(const char *name, INT32 playernum)
+{
+	// Check if a player is currently using the name, case-insensitively.
+	for (INT32 ix = 0; ix < MAXPLAYERS; ix++)
+	{
+		if (ix == playernum) // Don't compare with themself.
+			continue;
+		if (playeringame[ix] == false) // This player is not ingame.
+			continue;
+		if (strcasecmp(name, player_names[ix]) == 0) // Are usernames equal?
+			return false;
+	}
+	
+	return true;
+}
+
+boolean IsPlayerNameGood(char *name)
 {
 	size_t ix, len = strlen(name);
 
@@ -650,36 +666,43 @@ boolean EnsurePlayerNameIsGood(char *name, INT32 playernum)
 	for (ix = 0; ix < len; ix++)
 		if (!AllowedPlayerNameChar(name[ix]))
 			return false;
+		
+	return true;
+}
 
-	// Check if a player is currently using the name, case-insensitively.
+boolean EnsurePlayerNameIsGood(char *name, INT32 playernum)
-	for (ix = 0; ix < MAXPLAYERS; ix++)
+{
+	size_t len = strlen(name);
+	
+	// Check if a player is using a valid name.
+	if (!IsPlayerNameGood(name))
+		return false;
+
+	// Check if another player is currently using the name, case-insensitively.
+	if (!IsPlayerNameUnique(name, playernum)) 
 	{
-		if (ix != (size_t)playernum && playeringame[ix]
+		// We shouldn't kick people out just because
-			&& strcasecmp(name, player_names[ix]) == 0)
+		// they joined the game with the same name
-		{
+		// as someone else -- modify the name instead.
-			// We shouldn't kick people out just because
-			// they joined the game with the same name
-			// as someone else -- modify the name instead.
 
-			// Recursion!
+		// Recursion!
-			// Slowly strip characters off the end of the
+		// Slowly strip characters off the end of the
-			// name until we no longer have a duplicate.
+		// name until we no longer have a duplicate.
-			if (len > 1)
+		if (len > 1)
-			{
+		{
-				name[len-1] = '\0';
+			name[len-1] = '\0';
-				if (!EnsurePlayerNameIsGood (name, playernum))
+			if (!EnsurePlayerNameIsGood (name, playernum))
-					return false;
-			}
-			else if (len == 1) // Agh!
-			{
-				// Last ditch effort...
-				sprintf(name, "%d", 'A' + M_RandomKey(26));
-				if (!EnsurePlayerNameIsGood (name, playernum))
-					return false;
-			}
-			else
 				return false;
 		}
+		else if (len == 1) // Agh!
+		{
+			// Last ditch effort...
+			sprintf(name, "%d", 'A' + M_RandomKey(26));
+			if (!EnsurePlayerNameIsGood (name, playernum))
+				return false;
+		}
+		else
+			return false;
 	}
 
 	return true;

src/d_netcmd.h

@@ -200,6 +200,8 @@ extern const char *netxcmdnames[MAXNETXCMD - 1];
 void D_RegisterServerCommands(void);
 void D_RegisterClientCommands(void);
 void CleanupPlayerName(INT32 playernum, const char *newname);
+boolean IsPlayerNameUnique(const char *name, INT32 playernum);
+boolean IsPlayerNameGood(char *name);
 boolean EnsurePlayerNameIsGood(char *name, INT32 playernum);
 void D_FillPlayerSkinAndColor(const UINT8 n, const player_t *player, player_config_t *config);
 void D_PlayerChangeSkinAndColor(player_t *player, UINT16 skin, UINT16 color, INT16 follower, UINT16 followercolor);

src/k_bot.cpp

@@ -55,30 +55,16 @@ extern "C" consvar_t cv_forcebots;
 --------------------------------------------------*/
 void K_SetNameForBot(UINT8 newplayernum, const char *realname)
 {
-	UINT8 ix = MAXPLAYERS;
-
 	// These names are generally sourced from skins.
 	I_Assert(MAXPLAYERNAME >= SKINNAMESIZE+2);
 
+	boolean canApplyNameChange = true;
 	if (netgame == true)
 	{
-		// Check if a player is currently using the name, case-insensitively.
+		canApplyNameChange = IsPlayerNameUnique(realname, newplayernum);
-		// We only do this if online, because it doesn't matter if there are multiple Eggrobo *off*line.
-		// See also EnsurePlayerNameIsGood
-		for (ix = 0; ix < MAXPLAYERS; ix++)
-		{
-			if (ix == newplayernum)
-				continue;
-			if (playeringame[ix] == false)
-				continue;
-			if (strcasecmp(realname, player_names[ix]) != 0)
-				continue;
-
-			break;
-		}
 	}
 
-	if (ix == MAXPLAYERS)
+	if (canApplyNameChange)
 	{
 		// No conflict detected!
 		sprintf(player_names[newplayernum], "%s", realname);

src/lua_baselib.c

@@ -5990,7 +5990,15 @@ static int lib_kSetNameForBot(lua_State *L)
 	if (!player->bot)
 		return luaL_error(L, "You may only change bot names.");
 
-	K_SetNameForBot(player-players, realname);
+	// Doing this to avoid a discarded const warning:
+	char modifiedname[MAXPLAYERNAME+1] = "";
+	strcpy(modifiedname, realname);
+
+	if (!IsPlayerNameGood(modifiedname))
+		return luaL_error(L, "Invalid bot name - it must be between %d and %d characters of length, "
+			"not start with a space, @ or ~ characters, and it must be composed of valid ASCII characters.", 1, MAXPLAYERNAME);	
+
+	K_SetNameForBot(player-players, modifiedname);
 
 	return 0;
 }