Fix unable to save game data occasionally (#1194)

* Fix unable to save game data occasionally

* No self-kicking

* Game data synchronization

* finally

* prevent duplicated saving

* reverse changing

* keep the previous code

* Update GameServerInitializer.java

* Update GameSession.java

* remove sanity check because of try block

* a session needs can be created without a pipeline.

src/main/java/emu/grasscutter/game/dungeons/DungeonManager.java

@@ -8,6 +8,7 @@ import emu.grasscutter.data.excels.DungeonData;
 import emu.grasscutter.game.player.Player;
 import emu.grasscutter.game.props.SceneType;
 import emu.grasscutter.game.quest.enums.QuestTrigger;
+import emu.grasscutter.game.world.Scene;
 import emu.grasscutter.net.packet.BasePacket;
 import emu.grasscutter.net.packet.PacketOpcodes;
 import emu.grasscutter.server.game.GameServer;
@@ -80,19 +81,21 @@ public class DungeonManager {
 	}
 
 	public void exitDungeon(Player player) {
-		if (player.getScene().getSceneType() != SceneType.SCENE_DUNGEON) {
+		Scene scene = player.getScene();
+		
+		if (scene==null || scene.getSceneType() != SceneType.SCENE_DUNGEON) {
 			return;
 		}
 		
 		// Get previous scene
-		int prevScene = player.getScene().getPrevScene() > 0 ? player.getScene().getPrevScene() : 3;
+		int prevScene = scene.getPrevScene() > 0 ? scene.getPrevScene() : 3;
 		
 		// Get previous position
-		DungeonData dungeonData = player.getScene().getDungeonData();
+		DungeonData dungeonData = scene.getDungeonData();
 		Position prevPos = new Position(GameConstants.START_POSITION);
 		
 		if (dungeonData != null) {
-			ScenePointEntry entry = GameData.getScenePointEntryById(prevScene, player.getScene().getPrevScenePoint());
+			ScenePointEntry entry = GameData.getScenePointEntryById(prevScene, scene.getPrevScenePoint());
 			
 			if (entry != null) {
 				prevPos.set(entry.getPointData().getTranPos());

src/main/java/emu/grasscutter/game/player/Player.java

@@ -2,6 +2,7 @@ package emu.grasscutter.game.player;
 
 import dev.morphia.annotations.*;
 import emu.grasscutter.GameConstants;
+import emu.grasscutter.Grasscutter;
 import emu.grasscutter.data.GameData;
 import emu.grasscutter.data.excels.PlayerLevelData;
 import emu.grasscutter.database.DatabaseHelper;
@@ -1211,13 +1212,6 @@ public class Player {
 			this.getProfile().syncWithCharacter(this);
 		}
 
-		// Check if player object exists in server
-		// TODO - optimize
-		Player exists = this.getServer().getPlayerByUid(getUid());
-		if (exists != null) {
-			exists.getSession().close();
-		}
-
 		// Load from db
 		this.getAvatars().loadFromDatabase();
 		this.getInventory().loadFromDatabase();
@@ -1226,12 +1220,13 @@ public class Player {
 		this.getFriendsList().loadFromDatabase();
 		this.getMailHandler().loadFromDatabase();
 		this.getQuestManager().loadFromDatabase();
-		
+
 		// Add to gameserver (Always handle last)
 		if (getSession().isActive()) {
 			getServer().registerPlayer(this);
 			getProfile().setPlayer(this); // Set online
 		}
+
 	}
 
 	public void onLogin() {
@@ -1291,34 +1286,48 @@ public class Player {
 	}
 
 	public void onLogout() {
-		// stop stamina calculation
+		try{
-		getStaminaManager().stopSustainedStaminaHandler();
+			// stop stamina calculation
+			getStaminaManager().stopSustainedStaminaHandler();
 
-		// force to leave the dungeon
+			// force to leave the dungeon (inside has a "if")
-		if (getScene().getSceneType() == SceneType.SCENE_DUNGEON) {
 			this.getServer().getDungeonManager().exitDungeon(this);
+
+			// Leave world
+			if (this.getWorld() != null) {
+				this.getWorld().removePlayer(this);
+			}
+
+			// Status stuff
+			this.getProfile().syncWithCharacter(this);
+			this.getProfile().setPlayer(null); // Set offline
+
+			this.getCoopRequests().clear();
+
+			// Save to db
+			this.save();
+			this.getTeamManager().saveAvatars();
+			this.getFriendsList().save();
+
+			// Call quit event.
+			PlayerQuitEvent event = new PlayerQuitEvent(this); event.call();
+
+			//reset wood
+			getDeforestationManager().resetWood();
+
+		}catch (Throwable e){
+			e.printStackTrace();
+			Grasscutter.getLogger().warn("Player (UID {}) save failure", getUid());
+		}finally {
+			removeFromServer();
 		}
-		// Leave world
+	}
-		if (this.getWorld() != null) {
-			this.getWorld().removePlayer(this);
-		}
 
-		// Status stuff
+	public void removeFromServer() {
-		this.getProfile().syncWithCharacter(this);
+		// Remove from server.
-		this.getProfile().setPlayer(null); // Set offline
+		//Note: DON'T DELETE BY UID,BECAUSE THERE ARE MULTIPLE SAME UID PLAYERS WHEN DUPLICATED LOGIN!
-
+		//so I decide to delete by object rather than uid
-		this.getCoopRequests().clear();
+		getServer().getPlayers().values().removeIf(player1 -> player1 == this);
-
-		// Save to db
-		this.save();
-		this.getTeamManager().saveAvatars();
-		this.getFriendsList().save();
-		
-		// Call quit event.
-		PlayerQuitEvent event = new PlayerQuitEvent(this); event.call();
-
-		//reset wood
-		getDeforestationManager().resetWood();
 	}
 
 	public enum SceneLoadState {

src/main/java/emu/grasscutter/server/game/GameServerInitializer.java

@@ -13,8 +13,10 @@ public class GameServerInitializer extends KcpServerInitializer {
 	
     @Override
     protected void initChannel(UkcpChannel ch) throws Exception {
-        ChannelPipeline pipeline = ch.pipeline();
+        ChannelPipeline pipeline=null;
-        GameSession session = new GameSession(server);
+        if(ch!=null){
-        pipeline.addLast(session);
+            pipeline = ch.pipeline();
+        }
+        new GameSession(server,pipeline);
     }
 }

src/main/java/emu/grasscutter/server/game/GameSession.java

@@ -3,6 +3,8 @@ package emu.grasscutter.server.game;
 import java.io.File;
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
+import java.util.Iterator;
+import java.util.Map;
 import java.util.Set;
 
 import emu.grasscutter.Grasscutter;
@@ -17,9 +19,11 @@ import emu.grasscutter.server.event.game.SendPacketEvent;
 import emu.grasscutter.utils.Crypto;
 import emu.grasscutter.utils.FileUtils;
 import emu.grasscutter.utils.Utils;
+import io.jpower.kcp.netty.UkcpChannel;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelPipeline;
 
 import static emu.grasscutter.utils.Language.translate;
 import static emu.grasscutter.Configuration.*;
@@ -36,11 +40,30 @@ public class GameSession extends KcpChannel {
 	private int clientTime;
 	private long lastPingTime;
 	private int lastClientSeq = 10;
-	
+
+	private final ChannelPipeline pipeline;
+	@Override
+	public void close() {
+		setState(SessionState.INACTIVE);
+		//send disconnection pack in case of reconnection
+		try {
+			send(new BasePacket(PacketOpcodes.ServerDisconnectClientNotify));
+		}catch (Throwable ignore){
+
+		}
+		super.close();
+	}
 	public GameSession(GameServer server) {
+		this(server,null);
+	}
+	public GameSession(GameServer server, ChannelPipeline pipeline) {
 		this.server = server;
 		this.state = SessionState.WAITING_FOR_TOKEN;
 		this.lastPingTime = System.currentTimeMillis();
+		this.pipeline = pipeline;
+		if(pipeline!=null) {
+			pipeline.addLast(this);
+		}
 	}
 	
 	public GameServer getServer() {
@@ -124,13 +147,17 @@ public class GameSession extends KcpChannel {
 
 		// Set state so no more packets can be handled
 		this.setState(SessionState.INACTIVE);
-		
+
 		// Save after disconnecting
 		if (this.isLoggedIn()) {
+			Player player = getPlayer();
 			// Call logout event.
-			getPlayer().onLogout();
+			player.onLogout();
-			// Remove from server.
+		}
-			getServer().getPlayers().remove(getPlayer().getUid());
+		try {
+			pipeline.remove(this);
+		} catch (Throwable ignore) {
+
 		}
 	}
 	

src/main/java/emu/grasscutter/server/packet/recv/HandlerGetPlayerTokenReq.java

@@ -11,6 +11,7 @@ import emu.grasscutter.net.packet.PacketOpcodes;
 import emu.grasscutter.net.proto.GetPlayerTokenReqOuterClass.GetPlayerTokenReq;
 import emu.grasscutter.net.packet.PacketHandler;
 import emu.grasscutter.server.event.game.PlayerCreationEvent;
+import emu.grasscutter.server.game.GameServer;
 import emu.grasscutter.server.game.GameSession;
 import emu.grasscutter.server.game.GameSession.SessionState;
 import emu.grasscutter.server.packet.send.PacketGetPlayerTokenRsp;
@@ -20,28 +21,45 @@ public class HandlerGetPlayerTokenReq extends PacketHandler {
 	
 	@Override
 	public void handle(GameSession session, byte[] header, byte[] payload) throws Exception {
-		// Max players limit
+
-		if (ACCOUNT.maxPlayer > -1 && Grasscutter.getGameServer().getPlayers().size() >= ACCOUNT.maxPlayer) {
-			session.close();
-			return;
-		}
-		
 		GetPlayerTokenReq req = GetPlayerTokenReq.parseFrom(payload);
 		
 		// Authenticate
 		Account account = DatabaseHelper.getAccountById(req.getAccountUid());
-		if (account == null) {
+		if (account == null || !account.getToken().equals(req.getAccountToken())) {
-			return;
-		}
-		
-		// Check token
-		if (!account.getToken().equals(req.getAccountToken())) {
 			return;
 		}
 		
 		// Set account
 		session.setAccount(account);
-		
+
+
+		// Check if player object exists in server
+		// NOTE: CHECKING MUST SITUATED HERE (BEFORE getPlayerByUid)! because to save firstly ,to load secondly !!!
+		// TODO - optimize
+		boolean kicked = false;
+		Player exists = Grasscutter.getGameServer().getPlayerByAccountId(account.getId());
+		if (exists != null) {
+			GameSession existsSession = exists.getSession();
+			if (existsSession != session) {// No self-kicking
+				exists.onLogout();//must save immediately , or the below will load old data
+				existsSession.close();
+				Grasscutter.getLogger().warn("Player {} was kicked due to duplicated login", account.getUsername());
+				kicked = true;
+			}
+		}
+
+		//NOTE: If there are 5 online players, max count of player is 5,
+		// a new client want to login by kicking one of them ,
+		// I think it should be allowed
+		if(!kicked) {
+			// Max players limit
+			if (ACCOUNT.maxPlayer > -1 && Grasscutter.getGameServer().getPlayers().size() >= ACCOUNT.maxPlayer) {
+				session.close();
+				return;
+			}
+		}
+
 		// Get player
 		Player player = DatabaseHelper.getPlayerByAccount(account);
 
@@ -57,10 +75,10 @@ public class HandlerGetPlayerTokenReq extends PacketHandler {
 			// Save to db
 			DatabaseHelper.generatePlayerUid(player, nextPlayerUid);
 		}
-		
+
 		// Set player object for session
 		session.setPlayer(player);
-		
+
 		// Load player from database
 		player.loadFromDatabase();
 		

src/main/java/emu/grasscutter/server/packet/recv/HandlerQuickUseWidgetReq.java

@@ -1,5 +1,6 @@
 package emu.grasscutter.server.packet.recv;
 
+import emu.grasscutter.Grasscutter;
 import emu.grasscutter.game.inventory.GameItem;
 import emu.grasscutter.game.inventory.Inventory;
 import emu.grasscutter.game.inventory.InventoryTab;
@@ -45,6 +46,7 @@ public class HandlerQuickUseWidgetReq extends PacketHandler {
                 BasePacket rsp = new BasePacket(PacketOpcodes.QuickUseWidgetRsp);
                 rsp.setData(proto);
                 session.send(rsp);
+                Grasscutter.getLogger().warn("class has no effects in the game, feel free to implement it");
                 // but no effects in the game, feel free to implement it!
             }
         }