Merge remote-tracking branch 'ghsa/handshake-final-update-fix' into general-devel

TShockAPI/Commands.cs

@@ -1607,7 +1607,7 @@ namespace TShockAPI
 					}
 				}
 
-				if (banUuid)
+				if (banUuid && player.UUID.Length > 0)
 				{
 					banResult = DoBan($"{Identifier.UUID}{player.UUID}", reason, expiration);
 				}
@@ -2531,7 +2531,7 @@ namespace TShockAPI
 					foreach (TSPlayer ply in TShock.Players.Where(p => p != null && p.Active && p.TPlayer.name.ToLower().Equals(args.Parameters[0].ToLower())))
 					{
 						//this will always tell the client that they have not done the quest today.
-						ply.SendData((PacketTypes)74, "");
+						ply.SendData(PacketTypes.AnglerQuest, "");
 					}
 					args.Player.SendSuccessMessage(GetString("Removed {0} players from the angler quest completion list for today.", result));
 				}

TShockAPI/DB/BanManager.cs

@@ -189,11 +189,15 @@ namespace TShockAPI.DB
 		{
 			List<string> identifiers = new List<string>
 			{
-				$"{Identifier.UUID}{player.UUID}",
 				$"{Identifier.Name}{player.Name}",
 				$"{Identifier.IP}{player.IP}"
 			};
 
+			if (player.UUID != null && player.UUID.Length > 0)
+			{
+				identifiers.Add($"{Identifier.UUID}{player.UUID}");
+			}
+
 			if (player.Account != null)
 			{
 				identifiers.Add($"{Identifier.Account}{player.Account.Name}");

TShockAPI/DB/CharacterManager.cs

@@ -189,7 +189,7 @@ namespace TShockAPI.DB
 			if (!player.IsLoggedIn)
 				return false;
 
-			if (player.State < 10)
+			if (player.State < (int)ConnectionState.Complete)
 				return false;
 
 			if (player.HasPermission(Permissions.bypassssc) && !fromCommand)

TShockAPI/GetDataHandlers.cs

@@ -2619,7 +2619,7 @@ namespace TShockAPI
 
 		private static bool HandleConnecting(GetDataHandlerArgs args)
 		{
-			var account = TShock.UserAccounts.GetUserAccountByName(args.Player.Name);//
+			var account = TShock.UserAccounts.GetUserAccountByName(args.Player.Name);
 			args.Player.DataWhenJoined = new PlayerData(false);
 			args.Player.DataWhenJoined.CopyCharacter(args.Player);
 			args.Player.PlayerData = new PlayerData(false);
@@ -2629,8 +2629,9 @@ namespace TShockAPI
 			{
 				if (account.UUID == args.Player.UUID)
 				{
-					if (args.Player.State == 1)
+					if (args.Player.State == (int)ConnectionState.AssigningPlayerSlot)
-						args.Player.State = 2;
+						args.Player.State = (int)ConnectionState.AwaitingPlayerInfo;
+
 					NetMessage.SendData((int)PacketTypes.WorldInfo, args.Player.Index);
 
 					var group = TShock.Groups.GetGroupByName(account.Group);
@@ -2688,8 +2689,9 @@ namespace TShockAPI
 				return true;
 			}
 
-			if (args.Player.State == 1)
+			if (args.Player.State == (int)ConnectionState.AssigningPlayerSlot)
-				args.Player.State = 2;
+				args.Player.State = (int)ConnectionState.AwaitingPlayerInfo;
+
 			NetMessage.SendData((int)PacketTypes.WorldInfo, args.Player.Index);
 			return true;
 		}
@@ -2727,7 +2729,8 @@ namespace TShockAPI
 			short numberOfDeathsPVP = args.Data.ReadInt16();
 			PlayerSpawnContext context = (PlayerSpawnContext)args.Data.ReadByte();
 
-			args.Player.FinishedHandshake = true;
+			if (args.Player.State >= (int)ConnectionState.RequestingWorldData && !args.Player.FinishedHandshake)
+				args.Player.FinishedHandshake = true; //If the player has requested world data before sending spawn player, they should be at the obvious ClientRequestedWorldData state. Also only set this once to remove redundant updates.
 
 			if (OnPlayerSpawn(args.Player, args.Data, player, spawnX, spawnY, respawnTimer, numberOfDeathsPVE, numberOfDeathsPVP, context))
 				return true;
@@ -3245,8 +3248,9 @@ namespace TShockAPI
 					args.Player.RequiresPassword = false;
 					args.Player.PlayerData = TShock.CharacterDB.GetPlayerData(args.Player, account.ID);
 
-					if (args.Player.State == 1)
+					if (args.Player.State == (int)ConnectionState.AssigningPlayerSlot)
-						args.Player.State = 2;
+						args.Player.State = (int)ConnectionState.AwaitingPlayerInfo;
+
 					NetMessage.SendData((int)PacketTypes.WorldInfo, args.Player.Index);
 
 					var group = TShock.Groups.GetGroupByName(account.Group);
@@ -3293,8 +3297,10 @@ namespace TShockAPI
 				if (TShock.Config.Settings.ServerPassword == password)
 				{
 					args.Player.RequiresPassword = false;
-					if (args.Player.State == 1)
+
-						args.Player.State = 2;
+					if (args.Player.State == (int)ConnectionState.AssigningPlayerSlot)
+						args.Player.State = (int)ConnectionState.AwaitingPlayerInfo;
+
 					NetMessage.SendData((int)PacketTypes.WorldInfo, args.Player.Index);
 					return true;
 				}
@@ -3462,9 +3468,9 @@ namespace TShockAPI
 				if (buff == 10 && TShock.Config.Settings.DisableInvisPvP && args.TPlayer.hostile)
 					buff = 0;
 
-				if (Netplay.Clients[args.TPlayer.whoAmI].State < 2 && (buff == 156 || buff == 47 || buff == 149))
+				if (Netplay.Clients[args.TPlayer.whoAmI].State < (int)ConnectionState.AwaitingPlayerInfo && (buff == 156 || buff == 47 || buff == 149))
 				{
-					TShock.Log.ConsoleDebug(GetString("GetDataHandlers / HandlePlayerBuffList zeroed player buff due to below state 2 {0} {1}", args.Player.Name, buff));
+					TShock.Log.ConsoleDebug(GetString("GetDataHandlers / HandlePlayerBuffList zeroed player buff due to below state awaiting player information {0} {1}", args.Player.Name, buff));
 					buff = 0;
 				}
 
@@ -3713,10 +3719,15 @@ namespace TShockAPI
 					thing = GetString("{0} summoned the {1}!", args.Player.Name, npc.FullName);
 					break;
 			}
-			if (TShock.Config.Settings.AnonymousBossInvasions)
+
-				TShock.Utils.SendLogs(thing, Color.PaleVioletRed, args.Player);
+			if (NPCID.Sets.MPAllowedEnemies[thingType])
-			else
+			{
-				TShock.Utils.Broadcast(thing, 175, 75, 255);
+				if (TShock.Config.Settings.AnonymousBossInvasions)
+					TShock.Utils.SendLogs(thing, Color.PaleVioletRed, args.Player);
+				else
+					TShock.Utils.Broadcast(thing, 175, 75, 255);
+			}
+
 			return false;
 		}
 

TShockAPI/TSPlayer.cs

@@ -35,7 +35,6 @@ using TShockAPI.Net;
 using Timer = System.Timers.Timer;
 using System.Linq;
 using Terraria.GameContent.Creative;
-
 namespace TShockAPI
 {
 	/// <summary>
@@ -62,6 +61,49 @@ namespace TShockAPI
 		WriteToLogAndConsole
 	}
 
+	/// <summary>
+	/// An enum based on the current client's connection state to the server.
+	/// </summary>
+	public enum ConnectionState : int
+	{
+		/// <summary>
+		/// The server is password protected and the connection is pending until a password is sent by the client.
+		/// </summary>
+		AwaitingPassword = -1,
+		/// <summary>
+		/// The connection has been established, and the client must verify its version.
+		/// </summary>
+		AwaitingVersionCheck = 0,
+		/// <summary>
+		/// The server has accepted the client's password to connect and/or the server has verified the client's version string as being correct. The client is now being assigned a player slot.
+		/// </summary>
+		AssigningPlayerSlot = 1,
+		/// <summary>
+		/// The player slot has been received by the client, and the server is now waiting for the player information.
+		/// </summary>
+		AwaitingPlayerInfo = 2,
+		/// <summary>
+		/// Player information has been received, and the client is requesting world data.
+		/// </summary>
+		RequestingWorldData = 3,
+		/// <summary>
+		/// The world data is being sent to the client.
+		/// </summary>
+		ReceivingWorldData = 4,
+		/// <summary>
+		/// The world data has been received, and the client is now finalizing the load.
+		/// </summary>
+		FinalizingWorldLoad = 5,
+		/// <summary>
+		/// The client is requesting tile data.
+		/// </summary>
+		RequestingTileData = 6,
+		/// <summary>
+		/// The connection process is complete (The player has spawned), and the client has fully joined the game.
+		/// </summary>
+		Complete = 10
+	}
+
 	public class TSPlayer
 	{
 		/// <summary>
@@ -1329,6 +1371,9 @@ namespace TShockAPI
 			FakePlayer = new Player { name = playerName, whoAmI = -1 };
 			Group = Group.DefaultGroup;
 			AwaitingResponse = new Dictionary<string, Action<object>>();
+
+			if (playerName == "All" || playerName == "Server")
+				FinishedHandshake = true; //Hot fix for the all player object not getting packets like TimeSet, etc because they have no state and finished handshake will always be false.
 		}
 
 		/// <summary>
@@ -2119,6 +2164,27 @@ namespace TShockAPI
 			SendData(PacketTypes.PlayerAddBuff, number: Index, number2: type, number3: time);
 		}
 
+		/// <summary>
+		/// The list of necessary packets to make sure gets through to the player upon connection (before they finish the handshake).
+		/// </summary>
+		private static readonly HashSet<PacketTypes> HandshakeNecessaryPackets = new()
+		{
+			PacketTypes.ContinueConnecting,
+			PacketTypes.WorldInfo,
+			PacketTypes.Status,
+			PacketTypes.Disconnect,
+			PacketTypes.TileFrameSection,
+			PacketTypes.TileSendSection,
+			PacketTypes.PlayerSpawnSelf
+		};
+
+		/// <summary>
+		/// Determines if an outgoing packet is necessary to send to a player before they have finished the connection handshake.
+		/// </summary>
+		/// <param name="msgType">The packet type to check against the necessary list.</param>
+		/// <returns>Whether the packet is necessary for connection or not</returns>
+		private bool NecessaryPacket(PacketTypes msgType) => HandshakeNecessaryPackets.Contains(msgType);
+
 		//Todo: Separate this into a few functions. SendTo, SendToAll, etc
 		/// <summary>
 		/// Sends data to the player.
@@ -2136,6 +2202,12 @@ namespace TShockAPI
 			if (RealPlayer && !ConnectionAlive)
 				return;
 
+			if (!NecessaryPacket(msgType) && !FinishedHandshake)
+				return;
+
+			if (FakePlayer != null && FakePlayer.whoAmI != -1 && msgType == PacketTypes.WorldInfo && State < (int)ConnectionState.RequestingWorldData) //So.. the All player doesn't have a state, so we cannot check this, skip over them if their index is -1 (server/all)
+				return;
+
 			NetMessage.SendData((int)msgType, Index, -1, text == null ? null : NetworkText.FromLiteral(text), number, number2, number3, number4, number5);
 		}
 

TShockAPI/TShock.cs

@@ -1440,7 +1440,7 @@ namespace TShockAPI
 
 			if (tsplr.ReceivedInfo)
 			{
-				if (!tsplr.SilentKickInProgress && tsplr.State >= 3 && tsplr.FinishedHandshake) //The player has left, do not broadcast any clients exploiting the behaviour of not spawning their player.
+				if (!tsplr.SilentKickInProgress && tsplr.State >= (int)ConnectionState.RequestingWorldData && tsplr.FinishedHandshake) //The player has left, do not broadcast any clients exploiting the behaviour of not spawning their player.
 					Utils.Broadcast(GetString("{0} has left.", tsplr.Name), Color.Yellow);
 				Log.Info(GetString("{0} disconnected.", tsplr.Name));
 
@@ -1461,7 +1461,6 @@ namespace TShockAPI
 				}
 			}
 
-			
 			tsplr.FinishedHandshake = false;
 
 			// Fire the OnPlayerLogout hook too, if the player was logged in and they have a TSPlayer object.
@@ -1495,6 +1494,7 @@ namespace TShockAPI
 
 			if (!tsplr.FinishedHandshake)
 			{
+				tsplr.Kick(GetString("Your client didn't send the right connection information."), true);
 				args.Handled = true;
 				return;
 			}
@@ -1678,7 +1678,7 @@ namespace TShockAPI
 				return;
 			}
 
-			if ((player.State < 10 || player.Dead) && (int)type > 12 && (int)type != 16 && (int)type != 42 && (int)type != 50 &&
+			if ((player.State < (int)ConnectionState.Complete || player.Dead) && (int)type > 12 && (int)type != 16 && (int)type != 42 && (int)type != 50 &&
 				(int)type != 38 && (int)type != 21 && (int)type != 22 && type != PacketTypes.SyncLoadout)
 			{
 				e.Handled = true;