Merge pull request #3031 from PotatoCider/general-devel

Rewrite: Partially fix bed spawning for SSC

TShockAPI/GetDataHandlers.cs

@@ -2249,7 +2249,7 @@ namespace TShockAPI
 
 			var args = new SyncTilePickingEventArgs
 			{
-                                Player = player,
+				Player = player,
 				PlayerIndex = playerIndex,
 				TileX = tileX,
 				TileY = tileY,
@@ -2720,8 +2720,8 @@ namespace TShockAPI
 			}
 
 			byte player = args.Data.ReadInt8();
-			short spawnx = args.Data.ReadInt16();
+			short spawnX = args.Data.ReadInt16();
-			short spawny = args.Data.ReadInt16();
+			short spawnY = args.Data.ReadInt16();
 			int respawnTimer = args.Data.ReadInt32();
 			short numberOfDeathsPVE = args.Data.ReadInt16();
 			short numberOfDeathsPVP = args.Data.ReadInt16();
@@ -2729,43 +2729,62 @@ namespace TShockAPI
 
 			args.Player.FinishedHandshake = true;
 
-			if (OnPlayerSpawn(args.Player, args.Data, player, spawnx, spawny, respawnTimer, numberOfDeathsPVE, numberOfDeathsPVP, context))
+			if (OnPlayerSpawn(args.Player, args.Data, player, spawnX, spawnY, respawnTimer, numberOfDeathsPVE, numberOfDeathsPVP, context))
 				return true;
 			
-			if ((Main.ServerSideCharacter) && (spawnx == -1 && spawny == -1)) //this means they want to spawn to vanilla spawn
+			args.Player.Dead = respawnTimer > 0;
-			{
-				args.Player.sX = Main.spawnTileX;
-				args.Player.sY = Main.spawnTileY;
-				args.Player.Teleport(args.Player.sX * 16, (args.Player.sY * 16) - 48);
-				TShock.Log.ConsoleDebug(GetString("GetDataHandlers / HandleSpawn force teleport 'vanilla spawn' {0}", args.Player.Name));
-			}
 
-			else if ((Main.ServerSideCharacter) && (args.Player.sX > 0) && (args.Player.sY > 0) && (args.TPlayer.SpawnX > 0) && ((args.TPlayer.SpawnX != args.Player.sX) && (args.TPlayer.SpawnY != args.Player.sY)))
+			if (Main.ServerSideCharacter)
 			{
-				args.Player.sX = args.TPlayer.SpawnX;
+				// As long as the player has not changed his spawnpoint since initial connection,
-				args.Player.sY = args.TPlayer.SpawnY;
+				// we should not use the client's spawnpoint value. This is because the spawnpoint 
+				// value is not saved on the client when SSC is enabled. Hence, we have to assert 
+				// the server-saved spawnpoint value until we can detect that the player has changed 
+				// his spawn. Once we detect the spawnpoint changed, the client's spawnpoint value
+				// becomes the correct one to use.
+				//
+				// Note that spawnpoint changes (right-clicking beds) are not broadcasted to the 
+				// server. Hence, the only way to detect spawnpoint changes is from the 
+				// PlayerSpawn packet.
 
-				if (((Main.tile[args.Player.sX, args.Player.sY - 1].active() && Main.tile[args.Player.sX, args.Player.sY - 1].type == TileID.Beds)) && (WorldGen.StartRoomCheck(args.Player.sX, args.Player.sY - 1)))
+				// handle initial connection
+				if (args.Player.State == 3)
 				{
-					args.Player.Teleport(args.Player.sX * 16, (args.Player.sY * 16) - 48);
+					// server saved spawnpoint value
-					TShock.Log.ConsoleDebug(GetString("GetDataHandlers / HandleSpawn force teleport phase 1 {0}", args.Player.Name));
+					args.Player.initialSpawn = true;
-				}
+					args.Player.initialServerSpawnX = args.TPlayer.SpawnX;
-			}
+					args.Player.initialServerSpawnY = args.TPlayer.SpawnY;
 
-			else if ((Main.ServerSideCharacter) && (args.Player.sX > 0) && (args.Player.sY > 0))
+					// initial client spawn point, do not use this to spawn the player
-			{
+					// we only use it to detect if the spawnpoint has changed during this session
-				if (((Main.tile[args.Player.sX, args.Player.sY - 1].active() && Main.tile[args.Player.sX, args.Player.sY - 1].type == TileID.Beds)) && (WorldGen.StartRoomCheck(args.Player.sX, args.Player.sY - 1)))
+					args.Player.initialClientSpawnX = spawnX;
+					args.Player.initialClientSpawnY = spawnY;
+
+					// we first let the game handle completing the connection (state 3 => 10), 
+					// then we will spawn the player at the saved spawnpoint in the next second, 
+					// by reasserting the correct spawnpoint value
+					return false;
+				}
+
+				// once we detect the client has changed his spawnpoint in the current session, 
+				// the client spawnpoint value will be correct for the rest of the session
+				if (args.Player.spawnSynced || args.Player.initialClientSpawnX != spawnX || args.Player.initialClientSpawnY != spawnY)
 				{
-					args.Player.Teleport(args.Player.sX * 16, (args.Player.sY * 16) - 48);
+					// Player has changed his spawnpoint, client and server TPlayer.Spawn{X,Y} is now synced
-					TShock.Log.ConsoleDebug(GetString("GetDataHandlers / HandleSpawn force teleport phase 2 {0}", args.Player.Name));
+					args.Player.spawnSynced = true;
+					return false;
 				}
+
+				// the player has not changed his spawnpoint yet, so we assert the server-saved spawnpoint 
+				// by teleporting the player instead of letting the game use the client's incorrect spawnpoint.
+				TShock.Log.ConsoleDebug(GetString("GetDataHandlers / HandleSpawn force ssc teleport for {0} at ({1},{2})", args.Player.Name, args.TPlayer.SpawnX, args.TPlayer.SpawnY));
+				args.Player.TeleportSpawnpoint();
+				
+				args.TPlayer.respawnTimer = respawnTimer;
+				args.TPlayer.numberOfDeathsPVE = numberOfDeathsPVE;
+				args.TPlayer.numberOfDeathsPVP = numberOfDeathsPVP;
+				return true;
 			}
-
-			if (respawnTimer > 0)
-				args.Player.Dead = true;
-			else
-				args.Player.Dead = false;
-
 			return false;
 		}
 

TShockAPI/PlayerData.cs

@@ -104,16 +104,8 @@ namespace TShockAPI
 			this.maxHealth = player.TPlayer.statLifeMax;
 			this.mana = player.TPlayer.statMana;
 			this.maxMana = player.TPlayer.statManaMax;
-			if (player.sX > 0 && player.sY > 0)
+			this.spawnX = player.TPlayer.SpawnX;
-			{
+			this.spawnY = player.TPlayer.SpawnY;
-				this.spawnX = player.sX;
-				this.spawnY = player.sY;
-			}
-			else
-			{
-				this.spawnX = player.TPlayer.SpawnX;
-				this.spawnY = player.TPlayer.SpawnY;
-			}
 			extraSlot = player.TPlayer.extraAccessory ? 1 : 0;
 			this.skinVariant = player.TPlayer.skinVariant;
 			this.hair = player.TPlayer.hair;
@@ -266,8 +258,6 @@ namespace TShockAPI
 			player.TPlayer.statManaMax = this.maxMana;
 			player.TPlayer.SpawnX = this.spawnX;
 			player.TPlayer.SpawnY = this.spawnY;
-			player.sX = this.spawnX;
-			player.sY = this.spawnY;
 			player.TPlayer.hairDye = this.hairDye;
 			player.TPlayer.anglerQuestsFinished = this.questsCompleted;
 			player.TPlayer.UsingBiomeTorches = this.usingBiomeTorches == 1;

TShockAPI/TSPlayer.cs

@@ -177,8 +177,13 @@ namespace TShockAPI
 		/// </summary>
 		public int RPPending = 0;
 
-		public int sX = -1;
+
-		public int sY = -1;
+		public bool initialSpawn = false;
+		public int initialServerSpawnX = -2;
+		public int initialServerSpawnY = -2;
+		public bool spawnSynced = false;
+		public int initialClientSpawnX = -2;
+		public int initialClientSpawnY = -2;
 
 		/// <summary>
 		/// A queue of tiles destroyed by the player for reverting.
@@ -1383,6 +1388,25 @@ namespace TShockAPI
 			return true;
 		}
 
+		/// <summary>
+		/// Teleports the player to their spawnpoint. 
+		/// Teleports to main spawnpoint if their bed is not active.
+		/// Supports SSC.
+		/// </summary>
+		public bool TeleportSpawnpoint()
+		{
+			// NOTE: it is vanilla behaviour to not permanently override the spawnpoint if the bed spawn is broken/invalid
+			int x = TPlayer.SpawnX;
+			int y = TPlayer.SpawnY;
+			if ((x == -1 && y == -1) || 
+				!Main.tile[x, y - 1].active() || Main.tile[x, y - 1].type != TileID.Beds || !WorldGen.StartRoomCheck(x, y - 1))
+			{
+				x = Main.spawnTileX;
+				y = Main.spawnTileY;
+			}
+			return Teleport(x * 16, y * 16 - 48);
+		}
+
 		/// <summary>
 		/// Heals the player.
 		/// </summary>
@@ -1397,14 +1421,7 @@ namespace TShockAPI
 		/// </summary>
 		public void Spawn(PlayerSpawnContext context, int? respawnTimer = null)
 		{
-			if (this.sX > 0 && this.sY > 0)
+			Spawn(TPlayer.SpawnX, TPlayer.SpawnY, context, respawnTimer);
-			{
-				Spawn(this.sX, this.sY, context, respawnTimer);
-			}
-			else
-			{
-				Spawn(TPlayer.SpawnX, TPlayer.SpawnY, context, respawnTimer);
-			}
 		}
 
 		/// <summary>

TShockAPI/TShock.cs

@@ -1182,16 +1182,16 @@ namespace TShockAPI
 					if (player.RecentFuse > 0)
 						player.RecentFuse--;
 
-					if ((Main.ServerSideCharacter) && (player.TPlayer.SpawnX > 0) && (player.sX != player.TPlayer.SpawnX))
+					if (Main.ServerSideCharacter && player.initialSpawn)
 					{
-						player.sX = player.TPlayer.SpawnX;
+						player.initialSpawn = false;
-						player.sY = player.TPlayer.SpawnY;
-					}
 
-					if ((Main.ServerSideCharacter) && (player.sX > 0) && (player.sY > 0) && (player.TPlayer.SpawnX < 0))
+						// reassert the correct spawnpoint value after the game's Spawn handler changed it
-					{
+						player.TPlayer.SpawnX = player.initialServerSpawnX;
-						player.TPlayer.SpawnX = player.sX;
+						player.TPlayer.SpawnY = player.initialServerSpawnY;
-						player.TPlayer.SpawnY = player.sY;
+
+						player.TeleportSpawnpoint();
+						TShock.Log.ConsoleDebug(GetString("OnSecondUpdate / initial ssc spawn for {0} at ({1}, {2})", player.Name, player.TPlayer.SpawnX, player.TPlayer.SpawnY));
 					}
 
 					if (player.RPPending > 0)

docs/changelog.md

@@ -94,7 +94,9 @@ Use past tense when adding new entries; sign your name off when you add or chang
 * Added a hook `AccountHooks.AccountGroupUpdate`, which is called when you change the user group. (@AgaSpace)
 * * Ensured `TSPlayer.PlayerData` is non-null whilst syncing loadouts. (@drunderscore)
 * * Detected invalid installations, by checking for a file named `TerrariaServer.exe`. (@drunderscore)
-  * This made the two most common installation mistakes (extracting into the Terraria client directory, and extracting TShock 5 or newer into a TShock 4 or older install) prompt the user with a more useful diagnostic, rather than (likely) crashing moments later.
+  * This made the two most common installation mistakes (extracting into the Terraria client directory, and extracting TShock 5 or newer into a TShock 4 or older install) prompt the user with a more useful diagnostic, rather than (likely) crashing moments later. Rewrote bed spawning for SSC. (@PotatoCider)
+  * Removed `TSPlayer.s{X,Y}` in favour of using desyncing client and server spawnpoint values (`Terraria.Player.Spawn{X,Y}`) until the player has changed their spawnpoint per session.
+  * Partially fixed the bed spawning bug when SSC is enabled. Players would need to spawn at their beds at least once to tell TShock that the player's spawnpoint has changed.
 * Changed Bouncer to block updates which set the following fields to infinity or NaN: player position, projectile position, projectile velocity, item position, and item velocity. (@Arthri)
 * Updated `TShockAPI.Handlers.SendTileRectHandler` (@LaoSparrow):
   * Fixed incorrect validating range in `TileRectMatch.MatchRemoval`.