optimize npc group load & fix some NPE in suite

src/main/java/emu/grasscutter/game/world/Scene.java

@@ -3,6 +3,7 @@ package emu.grasscutter.game.world;
 import emu.grasscutter.Grasscutter;
 import emu.grasscutter.data.GameData;
 import emu.grasscutter.data.GameDepot;
+import emu.grasscutter.data.binout.SceneNpcBornEntry;
 import emu.grasscutter.data.excels.*;
 import emu.grasscutter.game.dungeons.DungeonSettleListener;
 import emu.grasscutter.game.entity.*;
@@ -53,6 +54,7 @@ public class Scene {
 	private DungeonData dungeonData;
 	private int prevScene; // Id of the previous scene
 	private int prevScenePoint;
+	private Set<SceneNpcBornEntry> npcBornEntrySet;
 	public Scene(World world, SceneData sceneData) {
 		this.world = world;
 		this.sceneData = sceneData;
@@ -65,6 +67,7 @@ public class Scene {
 		this.spawnedEntities = ConcurrentHashMap.newKeySet();
 		this.deadSpawnedEntities = ConcurrentHashMap.newKeySet();
 		this.loadedBlocks = ConcurrentHashMap.newKeySet();
+		this.npcBornEntrySet = ConcurrentHashMap.newKeySet();
 		this.scriptManager = new SceneScriptManager(this);
 	}
 
@@ -426,6 +429,8 @@ public class Scene {
 		if(challenge != null){
 			challenge.onCheckTimeOut();
 		}
+
+        checkNpcGroup();
 	}
 
 	public int getEntityLevel(int baseLevel, int worldLevelOverride) {
@@ -435,6 +440,25 @@ public class Scene {
 
 		return level;
 	}
+    public void checkNpcGroup(){
+        Set<SceneNpcBornEntry> npcBornEntries = ConcurrentHashMap.newKeySet();
+        for (Player player : this.getPlayers()) {
+            npcBornEntries.addAll(loadNpcForPlayer(player));
+        }
+
+        // clear the unreachable group for client
+        var toUnload = this.npcBornEntrySet.stream()
+            .filter(i -> !npcBornEntries.contains(i))
+            .map(SceneNpcBornEntry::getGroupId)
+            .toList();
+
+        if(toUnload.size() > 0){
+            broadcastPacket(new PacketGroupUnloadNotify(toUnload));
+            Grasscutter.getLogger().debug("Unload NPC Group {}", toUnload);
+        }
+        // exchange the new npcBornEntry Set
+        this.npcBornEntrySet = npcBornEntries;
+    }
 
 	// TODO - Test
 	public synchronized void checkSpawns() {
@@ -569,9 +593,6 @@ public class Scene {
 						.toList();
 				onLoadGroup(toLoad);
 			}
-			for (Player player : this.getPlayers()) {
-				getScriptManager().meetEntities(loadNpcForPlayer(player, block));
-			}
 		}
 
 	}
@@ -754,47 +775,27 @@ public class Scene {
 			addEntity(entity);
 		}
 	}
-	public List<EntityNPC> loadNpcForPlayer(Player player, SceneBlock block){
-		if(!block.contains(player.getPos())){
-			return List.of();
+    public void loadNpcForPlayerEnter(Player player){
+        this.npcBornEntrySet.addAll(loadNpcForPlayer(player));
     }
-
+	private List<SceneNpcBornEntry> loadNpcForPlayer(Player player){
 		var pos = player.getPos();
 		var data = GameData.getSceneNpcBornData().get(getId());
 		if(data == null){
 			return List.of();
 		}
 
-		var npcs = SceneIndexManager.queryNeighbors(data.getIndex(), pos.toDoubleArray(),
+		var npcList = SceneIndexManager.queryNeighbors(data.getIndex(), pos.toDoubleArray(),
 				Grasscutter.getConfig().server.game.loadEntitiesForPlayerRange);
-		var entityNPCS = npcs.stream().map(item -> {
-					var group = data.getGroups().get(item.getGroupId());
-					if(group == null){
-						group = SceneGroup.of(item.getGroupId());
-						data.getGroups().putIfAbsent(item.getGroupId(), group);
-						group.load(getId());
-					}
 
-					if(group.npc == null){
-						return null;
-					}
-					var npc = group.npc.get(item.getConfigId());
-					if(npc == null){
-						return null;
-					}
-
-					return getScriptManager().createNPC(npc, block.id, item.getSuiteIdList().get(0));
-				})
-				.filter(Objects::nonNull)
-				.filter(item -> getEntities().values().stream()
-						.filter(e -> e instanceof EntityNPC)
-						.noneMatch(e -> e.getConfigId() == item.getConfigId()))
+		var sceneNpcBornEntries = npcList.stream()
+            .filter(i -> !this.npcBornEntrySet.contains(i))
             .toList();
 
-		if(entityNPCS.size() > 0){
-			broadcastPacket(new PacketGroupSuiteNotify(entityNPCS));
+		if(sceneNpcBornEntries.size() > 0){
+			this.broadcastPacket(new PacketGroupSuiteNotify(sceneNpcBornEntries));
+            Grasscutter.getLogger().debug("Loaded Npc Group Suite {}", sceneNpcBornEntries);
 		}
-
-		return entityNPCS;
+        return npcList;
 	}
 }

src/main/java/emu/grasscutter/scripts/data/SceneGroup.java

@@ -5,15 +5,11 @@ import emu.grasscutter.scripts.ScriptLoader;
 import emu.grasscutter.utils.Position;
 import lombok.Setter;
 import lombok.ToString;
+import org.luaj.vm2.LuaValue;
 
 import javax.script.Bindings;
 import javax.script.CompiledScript;
 import javax.script.ScriptException;
-
-import org.luaj.vm2.LuaTable;
-import org.luaj.vm2.LuaValue;
-
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -33,7 +29,6 @@ public class SceneGroup {
 	public Map<Integer,SceneMonster> monsters; // <ConfigId, Monster>
 	public Map<Integer, SceneGadget> gadgets; // <ConfigId, Gadgets>
 	public Map<String, SceneTrigger> triggers;
-	public Map<Integer, SceneNPC> npc; // <NpcId, NPC>
     public Map<Integer, SceneRegion> regions;
 	public List<SceneSuite> suites;
 	public List<SceneVar> variables;
@@ -133,41 +128,9 @@ public class SceneGroup {
 
 			// Add variables to suite
 			this.variables = ScriptLoader.getSerializer().toList(SceneVar.class, this.bindings.get("variables"));
-			// NPC in groups
-            this.npc = ScriptLoader.getSerializer().toList(SceneNPC.class, this.bindings.get("npcs")).stream()
-					.collect(Collectors.toMap(x -> x.npc_id, y -> y));
-            this.npc.values().forEach(n -> n.group = this);
 
 			// Add monsters and gadgets to suite
-			for (SceneSuite suite : this.suites) {
-				suite.sceneMonsters = new ArrayList<>(
-						suite.monsters.stream()
-						.filter(this.monsters::containsKey)
-						.map(this.monsters::get)
-						.toList()
-				);
-
-				suite.sceneGadgets = new ArrayList<>(
-						suite.gadgets.stream()
-								.filter(this.gadgets::containsKey)
-								.map(this.gadgets::get)
-								.toList()
-				);
-
-				suite.sceneTriggers = new ArrayList<>(
-						suite.triggers.stream()
-								.filter(this.triggers::containsKey)
-								.map(this.triggers::get)
-								.toList()
-				);
-
-                suite.sceneRegions = new ArrayList<>(
-                    suite.regions.stream()
-                        .filter(this.regions::containsKey)
-                        .map(this.regions::get)
-                        .toList()
-                );
-			}
+            this.suites.forEach(i -> i.init(this));
 
 		} catch (ScriptException e) {
 			Grasscutter.getLogger().error("An error occurred while loading group " + this.id + " in scene " + sceneId + ".", e);

src/main/java/emu/grasscutter/scripts/data/SceneSuite.java

 package emu.grasscutter.scripts.data;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import lombok.Setter;
@@ -8,15 +9,53 @@ import lombok.ToString;
 @ToString
 @Setter
 public class SceneSuite {
-	public List<Integer> monsters;
-	public List<Integer> gadgets;
-	public List<String> triggers;
-    public List<Integer> regions;
+    // make it refer the default empty list to avoid NPE caused by some group 
+	public List<Integer> monsters = List.of();
+	public List<Integer> gadgets = List.of();
+	public List<String> triggers = List.of();
+    public List<Integer> regions = List.of();
     public int rand_weight;
 
-	public transient List<SceneMonster> sceneMonsters;
-	public transient List<SceneGadget> sceneGadgets;
-	public transient List<SceneTrigger> sceneTriggers;
-    public transient List<SceneRegion> sceneRegions;
+	public transient List<SceneMonster> sceneMonsters = List.of();
+	public transient List<SceneGadget> sceneGadgets = List.of();
+	public transient List<SceneTrigger> sceneTriggers = List.of();
+    public transient List<SceneRegion> sceneRegions = List.of();
 
+    public void init(SceneGroup sceneGroup) {
+        if(sceneGroup.monsters != null){
+            this.sceneMonsters = new ArrayList<>(
+                this.monsters.stream()
+                    .filter(sceneGroup.monsters::containsKey)
+                    .map(sceneGroup.monsters::get)
+                    .toList()
+            );
+        }
+
+        if(sceneGroup.gadgets != null){
+            this.sceneGadgets = new ArrayList<>(
+                this.gadgets.stream()
+                    .filter(sceneGroup.gadgets::containsKey)
+                    .map(sceneGroup.gadgets::get)
+                    .toList()
+            );
+        }
+
+        if(sceneGroup.triggers != null) {
+            this.sceneTriggers = new ArrayList<>(
+                this.triggers.stream()
+                    .filter(sceneGroup.triggers::containsKey)
+                    .map(sceneGroup.triggers::get)
+                    .toList()
+            );
+        }
+        if(sceneGroup.regions != null) {
+            this.sceneRegions = new ArrayList<>(
+                this.regions.stream()
+                    .filter(sceneGroup.regions::containsKey)
+                    .map(sceneGroup.regions::get)
+                    .toList()
+            );
+        }
+
+    }
 }

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

@@ -5,11 +5,7 @@ import emu.grasscutter.net.packet.Opcodes;
 import emu.grasscutter.net.packet.PacketOpcodes;
 import emu.grasscutter.net.packet.PacketHandler;
 import emu.grasscutter.server.game.GameSession;
-import emu.grasscutter.server.packet.send.PacketEnterSceneDoneRsp;
-import emu.grasscutter.server.packet.send.PacketPlayerTimeNotify;
-import emu.grasscutter.server.packet.send.PacketScenePlayerLocationNotify;
-import emu.grasscutter.server.packet.send.PacketWorldPlayerLocationNotify;
-import emu.grasscutter.server.packet.send.PacketWorldPlayerRTTNotify;
+import emu.grasscutter.server.packet.send.*;
 
 @Opcodes(PacketOpcodes.EnterSceneDoneReq)
 public class HandlerEnterSceneDoneReq extends PacketHandler {
@@ -34,6 +30,8 @@ public class HandlerEnterSceneDoneReq extends PacketHandler {
 		session.send(new PacketScenePlayerLocationNotify(session.getPlayer().getScene()));
 		session.send(new PacketWorldPlayerRTTNotify(session.getPlayer().getWorld()));
 
+        // spawn NPC
+        session.getPlayer().getScene().loadNpcForPlayerEnter(session.getPlayer());
 		// Reset timer for sending player locations
 		session.getPlayer().resetSendPlayerLocTime();
 	}

src/main/java/emu/grasscutter/server/packet/send/PacketGroupSuiteNotify.java

 package emu.grasscutter.server.packet.send;
 
-import emu.grasscutter.game.entity.EntityNPC;
+import emu.grasscutter.data.binout.SceneNpcBornEntry;
 import emu.grasscutter.net.packet.BasePacket;
 import emu.grasscutter.net.packet.PacketOpcodes;
 import emu.grasscutter.net.proto.GroupSuiteNotifyOuterClass;
@@ -10,14 +10,18 @@ import java.util.List;
 public class PacketGroupSuiteNotify extends BasePacket {
 
 	/**
-	 * control which npc suite is loaded
+	 * Real control which npc suite is loaded
+     * EntityNPC is useless
 	 */
-	public PacketGroupSuiteNotify(List<EntityNPC> list) {
+	public PacketGroupSuiteNotify(List<SceneNpcBornEntry> npcBornEntries) {
 		super(PacketOpcodes.GroupSuiteNotify);
 
 		var proto = GroupSuiteNotifyOuterClass.GroupSuiteNotify.newBuilder();
 
-		list.forEach(item -> proto.putGroupMap(item.getGroupId(), item.getSuiteId()));
+        npcBornEntries.forEach(x ->
+            x.getSuiteIdList().forEach(y ->
+                proto.putGroupMap(x.getGroupId(), y)
+            ));
 
 		this.setData(proto);
 

src/main/java/emu/grasscutter/server/packet/send/PacketGroupUnloadNotify.java

+package emu.grasscutter.server.packet.send;
+
+import emu.grasscutter.net.packet.BasePacket;
+import emu.grasscutter.net.packet.PacketOpcodes;
+import emu.grasscutter.net.proto.GroupUnloadNotifyOuterClass;
+
+import java.util.List;
+
+public class PacketGroupUnloadNotify extends BasePacket {
+
+	public PacketGroupUnloadNotify(List<Integer> groupList) {
+		super(PacketOpcodes.GroupUnloadNotify);
+
+        var proto = GroupUnloadNotifyOuterClass.GroupUnloadNotify.newBuilder();
+
+        proto.addAllGroupList(groupList);
+
+        this.setData(proto);
+	}
+}