improve the code

[pluttotv]

hey iv been using NPCLib for a while and i really like what i see so far.

recently i got to go throw the project source code and i found few places they can be easily enhance the code.
for example:
handleMove(Player player)

why not saving the npc by world, to run on it first OR even better:
there is a getShown set for each npc -> thats how we "know" if player can see the npc or not.

so why not doing the opposite list -> save a list of UUID of npc that a player can see!
this way you can switch this ugly line to work the oposite -> making it run on a list of player.
for (NPCBase npc : NPCManager.getAllNPCs()) {
if (!npc.getShown().contains(player.getUniqueId())) {

we can also create a table of UUID npc per world - and cross it with the table of the npc UUID saved for the player.
making it 2 times more efficiency, as checking UUID on a world HashSet (of each world, has a list of npc UUID) by "containKey"
to check if the NPC happen to be there.

im looking for more method to generate the same result, if i will come by one i will sure to drop it.

[pluttotv]

after consulting the topic on paper discord ->
turn out you can hook into PacketType.Play.Server.MAP_CHUNK
and PacketType.Play.Server.UNLOAD_CHUNK that the server send the client each time he load a chunk , or unload it on his side.

[JitseB]

A few very interesting points I'll be sure to look at for an update of NPCLib. Thanks for the info!

[pluttotv]

here is my code , edited for ur use:

create new class "CollectNPCOfPlayer" i called it

• hook into show player npc a new "CollectNPCOfPlayer" based on it.
  CollectNPCOfPlayer - has:
• reference of NPC (uuid better)
• location to check or ^^^
• true false of isLoaded - like "is it showed",
• last a world reference.

then:

`public void onPacketSending(PacketEvent event) {

            if (event.getPacketType() == PacketType.Play.Server.MAP_CHUNK) {
                //IF Player exist on saved npc
                if (playersDATA.containsKey(event.getPlayer().getUniqueId())) {
                    final HashSet<UUID> npcToShow = new HashSet<>();
                    for (final CollectNPCOfPlayer npcToCheck : playersDATA.get(event.getPlayer().getUniqueId())) {
                        //Continue on loaded if npc is loaded
                        if (npcToShow.getIsLoaded())
                            continue;
                        if (npcToCheck.getWorld() == event.getPlayer().getWorld()) {
                            if (isInRange(event.getPlayer().getChunk().getX(), event.getPlayer().getChunk().getZ(), npcToCheck.getLocationChunkX(), npcToCheck.getLocationChunkZ())) {
                                collectDATA.setIsLoaded(true);
                                npcToShow.add(npcToCheck);
                            }
                        }
                    }
                    if (npcToShow.size() > 0) {
                        Bukkit.getScheduler().runTask(this.plugin, () -> {
                            for (final UUID toShow : npcToShow) {
                                //SHOW NPC
                                }
                        });
                    }
                }
            }
        }

    });`

` this.protocolManager.addPacketListener(new PacketAdapter(this.plugin, ListenerPriority.NORMAL, PacketType.Play.Server.UNLOAD_CHUNK) {

        @Override
        public void onPacketSending(PacketEvent event) {
            if (event.getPacketType() == PacketType.Play.Server.UNLOAD_CHUNK) {

                if (playersDATA.containsKey(event.getPlayer().getUniqueId())) {
                    for (final CollectNPCOfPlayer npcToCheck : playersDATA.get(event.getPlayer().getUniqueId()))) {
                        //Continue on UN LOADED npc
                        if (!npcToCheck.getIsLoaded())
                            continue;

                        if (npcToCheck.getWorld() == event.getPlayer().getWorld()) {
                            if (!isInRange(event.getPlayer().getChunk().getX(), event.getPlayer().getChunk().getZ(), npcToCheck.getLocationChunkX(), npcToCheck.getLocationChunkZ())) {
                                npcToCheck.setIsLoaded(false);
                                // UN-LOAD NPC
                            }
                        }
                    }
                }
            }
        }

    });

`
hold a HashMap called playersDATA<UUID, HashSet< CollectNPCOfPlayer> > and each player login create his new key and new HashSet.

performance side -> i didn't find any better way and fast to do it, and I'm using this same logic code
to handle the show/un-show per player list of fake blocks (easy 100 fake blocks per player) x20-30 ppl and I had zero issues with performance.
not even a tick was lost.

hopes it helps! love ur project can't wait to see moving or items on NPC!

cheers,
Plutto.

[JitseB]

Chunk handling has changed after version 1.8 R3, which makes it indeed possible to use the PacketPlayOutMapChunk and PacketPlayOutUnloadChunk packets for auto-hiding and -showing the NPCs. Looks like I'll have to implement a small statement for using the old hiding system for 1.8, because I don't think I can drop support for it as 45% of the servers using NPCLib still runs 1.8.8.

I'll add this to my todos for next week.