Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.41k stars 377 forks source link

Add a check to InsertPlayerData to prevent data overwrite #2894

Closed sgkoishi closed 1 year ago

sgkoishi commented 1 year ago

In some edge scenarios, players might experience data loss (more likely to happen when the latency is high and ServerSideCharacterSave is set to 0). This might be caused by players having their inventory sent from the client during joining, and data loaded from the database, but a save happens after those two events but before the loaded data were restored to the player. So, the incorrect version was inserted into the database, overwriting existing data (the TSPlayer.PlayerData is still correct for now, but never used, and will be overwritten upon the next join).

Why this guess?

A server owner claims that his server can always reproduce a data loss of hair and style (everyone looks like the guide). We tried to log every call to TShockAPI.DB.CharacterManager.InsertPlayerData and TShockAPI.DB.CharacterManager.GetPlayerData and here's the result:

image

114 is the masked IP address and 234 is the player name. During the first join, things work correctly, the hairstyle 105 was restored to the player correctly. During the second attempt, one InsertPlayerData was called and two GetPlayerData is following. Every InsertPlayerData call uses GetPlayerData, so this means one InsertPlayerData and GetPlayerData pair, and another standalone GetPlayerData. The usage of InsertPlayerData is inside TShock.OnUpdate, and the GetPlayerData happens in GetDataHandlers.HandleConnecting; a save happens right before data were restored to the player.