fix concurrency issues in Viewable by making all view tasks execute one by one through a queue

api/src/main/java/lol/pyr/znpcsplus/api/npc/Npc.java

@@ -11,6 +11,7 @@ import org.jetbrains.annotations.Nullable;
 import java.util.List;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
 
 /**
  * Base class for all NPCs
@@ -136,14 +137,16 @@ public interface Npc extends PropertyHolder {
     /**
      * Shows this NPC to a player
      * @param player The {@link Player} to show to
+     * @return A future that completes when the npc is fully shown to the player
      */
-    void show(Player player);
+    CompletableFuture<Void> show(Player player);
 
     /**
      * Respawns this NPC for a player
      * @param player The {@link Player} to respawn for
+     * @return A future that completes when the npc is fully respawned
      */
-    void respawn(Player player);
+    CompletableFuture<Void> respawn(Player player);
 
     /**
      * Sets the head rotation of this NPC for a player

plugin/src/main/java/lol/pyr/znpcsplus/entity/PacketEntity.java

@@ -14,6 +14,7 @@ import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
 
 import java.util.*;
+import java.util.concurrent.CompletableFuture;
 
 public class PacketEntity implements PropertyHolder {
     private final PacketFactory packetFactory;
@@ -65,15 +66,17 @@ public class PacketEntity implements PropertyHolder {
         for (Player viewer : viewable.getViewers()) packetFactory.teleportEntity(viewer, this);
     }
 
-    public void spawn(Player player) {
+    public CompletableFuture<Void> spawn(Player player) {
-        if (type == EntityTypes.PLAYER) packetFactory.spawnPlayer(player, this, properties);
+        return CompletableFuture.runAsync(() -> {
-        else packetFactory.spawnEntity(player, this, properties);
+            if (type == EntityTypes.PLAYER) packetFactory.spawnPlayer(player, this, properties).join();
-        if (vehicleId != null) {
+            else packetFactory.spawnEntity(player, this, properties);
-            packetFactory.setPassengers(player, vehicleId, this.getEntityId());
+            if (vehicleId != null) {
-        }
+                packetFactory.setPassengers(player, vehicleId, this.getEntityId());
-        if (passengers != null) {
+            }
-            packetFactory.setPassengers(player, this.getEntityId(), passengers.stream().mapToInt(Integer::intValue).toArray());
+            if (passengers != null) {
-        }
+                packetFactory.setPassengers(player, this.getEntityId(), passengers.stream().mapToInt(Integer::intValue).toArray());
+            }
+        });
     }
 
     public void setHeadRotation(Player player, float yaw, float pitch) {
@@ -127,8 +130,9 @@ public class PacketEntity implements PropertyHolder {
 
         vehicle.setLocation(location.withY(location.getY() - 0.9));
         for (Player player : viewable.getViewers()) {
-            vehicle.spawn(player);
+            vehicle.spawn(player).thenRun(() -> {
-            packetFactory.setPassengers(player, vehicle.getEntityId(), this.getEntityId());
+                packetFactory.setPassengers(player, vehicle.getEntityId(), this.getEntityId());
+            });
         }
     }
 

plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramImpl.java

@@ -6,6 +6,7 @@ import lol.pyr.znpcsplus.api.hologram.Hologram;
 import lol.pyr.znpcsplus.config.ConfigManager;
 import lol.pyr.znpcsplus.entity.EntityPropertyRegistryImpl;
 import lol.pyr.znpcsplus.packets.PacketFactory;
+import lol.pyr.znpcsplus.util.FutureUtil;
 import lol.pyr.znpcsplus.util.NpcLocation;
 import lol.pyr.znpcsplus.util.Viewable;
 import net.kyori.adventure.text.Component;
@@ -16,6 +17,8 @@ import org.bukkit.entity.Player;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.Collectors;
 
 public class HologramImpl extends Viewable implements Hologram {
     private final ConfigManager configManager;
@@ -138,8 +141,10 @@ public class HologramImpl extends Viewable implements Hologram {
     }
 
     @Override
-    protected void UNSAFE_show(Player player) {
+    protected CompletableFuture<Void> UNSAFE_show(Player player) {
-        for (HologramLine<?> line : lines) line.show(player);
+        return FutureUtil.allOf(lines.stream()
+                .map(line -> line.show(player))
+                .collect(Collectors.toList()));
     }
 
     @Override

plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramLine.java

@@ -13,6 +13,7 @@ import org.bukkit.inventory.ItemStack;
 
 import java.util.HashSet;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 
 public class HologramLine<M> implements PropertyHolder {
     private M value;
@@ -37,8 +38,8 @@ public class HologramLine<M> implements PropertyHolder {
         entity.refreshMeta(player);
     }
 
-    protected void show(Player player) {
+    protected CompletableFuture<Void> show(Player player) {
-        entity.spawn(player);
+        return entity.spawn(player);
     }
 
     protected void hide(Player player) {

plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramText.java

@@ -9,6 +9,8 @@ import lol.pyr.znpcsplus.util.Viewable;
 import net.kyori.adventure.text.Component;
 import org.bukkit.entity.Player;
 
+import java.util.concurrent.CompletableFuture;
+
 public class HologramText extends HologramLine<Component> {
 
     private static final Component BLANK  = Component.text("%blank%");
@@ -20,10 +22,9 @@ public class HologramText extends HologramLine<Component> {
     }
 
     @Override
-    public void show(Player player) {
+    public CompletableFuture<Void> show(Player player) {
-        if (!getValue().equals(BLANK)) {
+        if (getValue().equals(BLANK)) return CompletableFuture.completedFuture(null);
-            super.show(player);
+        return super.show(player);
-        }
     }
 
     @SuppressWarnings("unchecked")

plugin/src/main/java/lol/pyr/znpcsplus/npc/NpcImpl.java

@@ -24,6 +24,7 @@ import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.*;
+import java.util.concurrent.CompletableFuture;
 import java.util.stream.Collectors;
 
 public class NpcImpl extends Viewable implements Npc {
@@ -126,9 +127,8 @@ public class NpcImpl extends Viewable implements Npc {
     }
 
     @Override
-    protected void UNSAFE_show(Player player) {
+    protected CompletableFuture<Void> UNSAFE_show(Player player) {
-        entity.spawn(player);
+        return CompletableFuture.allOf(entity.spawn(player), hologram.show(player));
-        hologram.show(player);
     }
 
     @Override

plugin/src/main/java/lol/pyr/znpcsplus/packets/PacketFactory.java

@@ -11,7 +11,7 @@ import java.util.List;
 import java.util.concurrent.CompletableFuture;
 
 public interface PacketFactory {
-    void spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties);
+    CompletableFuture<Void> spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties);
     void spawnEntity(Player player, PacketEntity entity, PropertyHolder properties);
     void destroyEntity(Player player, PacketEntity entity, PropertyHolder properties);
     void teleportEntity(Player player, PacketEntity entity);

plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_20_2PacketFactory.java

@@ -16,6 +16,7 @@ import org.bukkit.entity.Player;
 import org.bukkit.plugin.Plugin;
 
 import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
 
 public class V1_20_2PacketFactory extends V1_19_3PacketFactory {
 
@@ -27,8 +28,8 @@ public class V1_20_2PacketFactory extends V1_19_3PacketFactory {
     }
 
     @Override
-    public void spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties) {
+    public CompletableFuture<Void> spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties) {
-        addTabPlayer(player, entity, properties).thenAccept(ignored -> {
+        return addTabPlayer(player, entity, properties).thenAccept(ignored -> {
             createTeam(player, entity, properties.getProperty(propertyRegistry.getByName("glow", NamedColor.class)));
             NpcLocation location = entity.getLocation();
             sendPacket(player, new WrapperPlayServerSpawnEntity(entity.getEntityId(), Optional.of(entity.getUuid()), entity.getType(),

plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_8PacketFactory.java

@@ -47,8 +47,8 @@ public class V1_8PacketFactory implements PacketFactory {
     }
 
     @Override
-    public void spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties) {
+    public CompletableFuture<Void> spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties) {
-        addTabPlayer(player, entity, properties).thenAccept(ignored -> {
+        return addTabPlayer(player, entity, properties).thenAccept(ignored -> {
             createTeam(player, entity, properties.getProperty(propertyRegistry.getByName("glow", NamedColor.class)));
             NpcLocation location = entity.getLocation();
             sendPacket(player, new WrapperPlayServerSpawnPlayer(entity.getEntityId(),

plugin/src/main/java/lol/pyr/znpcsplus/util/FutureUtil.java

@@ -0,0 +1,12 @@
+package lol.pyr.znpcsplus.util;
+
+import java.util.Collection;
+import java.util.concurrent.CompletableFuture;
+
+public class FutureUtil {
+    public static CompletableFuture<Void> allOf(Collection<CompletableFuture<?>> futures) {
+        return CompletableFuture.runAsync(() -> {
+            for (CompletableFuture<?> future : futures) future.join();
+        });
+    }
+}

plugin/src/main/java/lol/pyr/znpcsplus/util/Viewable.java

@@ -4,11 +4,9 @@ import org.bukkit.entity.Player;
 
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collections;
+import java.util.concurrent.CompletableFuture;
-import java.util.List;
+import java.util.concurrent.ConcurrentLinkedQueue;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
 
 public abstract class Viewable {
@@ -21,38 +19,74 @@ public abstract class Viewable {
                 .collect(Collectors.toList());
     }
 
-    private final Set<Player> viewers = ConcurrentHashMap.newKeySet();
+    private boolean queueRunning = false;
+    private final Queue<Runnable> visibilityTaskQueue = new ConcurrentLinkedQueue<>();
+    private final Set<Player> viewers = new HashSet<>();
 
     public Viewable() {
         all.add(new WeakReference<>(this));
     }
 
+    private void tryRunQueue() {
+        if (visibilityTaskQueue.isEmpty() || queueRunning) return;
+        queueRunning = true;
+        CompletableFuture.runAsync(() -> {
+            while (!visibilityTaskQueue.isEmpty()) try {
+                visibilityTaskQueue.remove().run();
+            } catch (Exception e) {
+                e.printStackTrace();
+            }
+            queueRunning = false;
+        });
+    }
+
+    private void queueVisibilityTask(Runnable runnable) {
+        visibilityTaskQueue.add(runnable);
+        tryRunQueue();
+    }
+
     public void delete() {
-        UNSAFE_hideAll();
+        queueVisibilityTask(() -> {
-        viewers.clear();
+            UNSAFE_hideAll();
+            viewers.clear();
+        });
     }
 
-    public void respawn() {
+    public CompletableFuture<Void> respawn() {
-        UNSAFE_hideAll();
+        CompletableFuture<Void> future = new CompletableFuture<>();
-        UNSAFE_showAll();
+        queueVisibilityTask(() -> {
+            UNSAFE_hideAll();
+            UNSAFE_showAll().join();
+            future.complete(null);
+        });
+        return future;
     }
 
-    public void respawn(Player player) {
+    public CompletableFuture<Void> respawn(Player player) {
-        if (!viewers.contains(player)) return;
+        hide(player);
-        UNSAFE_hide(player);
+        return show(player);
-        UNSAFE_show(player);
     }
 
-    public void show(Player player) {
+    public CompletableFuture<Void> show(Player player) {
-        if (viewers.contains(player)) return;
+        CompletableFuture<Void> future = new CompletableFuture<>();
-        viewers.add(player);
+        queueVisibilityTask(() -> {
-        UNSAFE_show(player);
+            if (viewers.contains(player)) {
+                future.complete(null);
+                return;
+            }
+            viewers.add(player);
+            UNSAFE_show(player).join();
+            future.complete(null);
+        });
+        return future;
     }
 
     public void hide(Player player) {
-        if (!viewers.contains(player)) return;
+        queueVisibilityTask(() -> {
-        viewers.remove(player);
+            if (!viewers.contains(player)) return;
-        UNSAFE_hide(player);
+            viewers.remove(player);
+            UNSAFE_hide(player);
+        });
     }
 
     public void UNSAFE_removeViewer(Player player) {
@@ -63,8 +97,11 @@ public abstract class Viewable {
         for (Player viewer : viewers) UNSAFE_hide(viewer);
     }
 
-    protected void UNSAFE_showAll() {
+    protected CompletableFuture<Void> UNSAFE_showAll() {
-        for (Player viewer : viewers) UNSAFE_show(viewer);
+        return FutureUtil.allOf(viewers.stream()
+                .map(this::UNSAFE_show)
+                .collect(Collectors.toList()));
+        // for (Player viewer : viewers) UNSAFE_show(viewer);
     }
 
     public Set<Player> getViewers() {
@@ -75,7 +112,7 @@ public abstract class Viewable {
         return viewers.contains(player);
     }
 
-    protected abstract void UNSAFE_show(Player player);
+    protected abstract CompletableFuture<Void> UNSAFE_show(Player player);
 
     protected abstract void UNSAFE_hide(Player player);
 }