From e30d6a57823a7c27a11854d0cc6d3652703467cb Mon Sep 17 00:00:00 2001 From: Pyrbu Date: Wed, 18 Dec 2024 07:05:46 +0100 Subject: [PATCH] fix concurrency issues in Viewable by making all view tasks execute one by one through a queue --- .../java/lol/pyr/znpcsplus/api/npc/Npc.java | 7 +- .../pyr/znpcsplus/entity/PacketEntity.java | 26 +++--- .../pyr/znpcsplus/hologram/HologramImpl.java | 9 +- .../pyr/znpcsplus/hologram/HologramLine.java | 5 +- .../pyr/znpcsplus/hologram/HologramText.java | 9 +- .../java/lol/pyr/znpcsplus/npc/NpcImpl.java | 6 +- .../pyr/znpcsplus/packets/PacketFactory.java | 2 +- .../packets/V1_20_2PacketFactory.java | 5 +- .../znpcsplus/packets/V1_8PacketFactory.java | 4 +- .../lol/pyr/znpcsplus/util/FutureUtil.java | 12 +++ .../java/lol/pyr/znpcsplus/util/Viewable.java | 87 +++++++++++++------ 11 files changed, 118 insertions(+), 54 deletions(-) create mode 100644 plugin/src/main/java/lol/pyr/znpcsplus/util/FutureUtil.java diff --git a/api/src/main/java/lol/pyr/znpcsplus/api/npc/Npc.java b/api/src/main/java/lol/pyr/znpcsplus/api/npc/Npc.java index d5e70bb..79872ba 100644 --- a/api/src/main/java/lol/pyr/znpcsplus/api/npc/Npc.java +++ b/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 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 respawn(Player player); /** * Sets the head rotation of this NPC for a player diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/entity/PacketEntity.java b/plugin/src/main/java/lol/pyr/znpcsplus/entity/PacketEntity.java index dc06d00..a46c9d4 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/entity/PacketEntity.java +++ b/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) { - if (type == EntityTypes.PLAYER) packetFactory.spawnPlayer(player, this, properties); - else packetFactory.spawnEntity(player, this, properties); - if (vehicleId != null) { - packetFactory.setPassengers(player, vehicleId, this.getEntityId()); - } - if (passengers != null) { - packetFactory.setPassengers(player, this.getEntityId(), passengers.stream().mapToInt(Integer::intValue).toArray()); - } + public CompletableFuture spawn(Player player) { + return CompletableFuture.runAsync(() -> { + if (type == EntityTypes.PLAYER) packetFactory.spawnPlayer(player, this, properties).join(); + else packetFactory.spawnEntity(player, this, properties); + if (vehicleId != null) { + packetFactory.setPassengers(player, vehicleId, this.getEntityId()); + } + 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); - packetFactory.setPassengers(player, vehicle.getEntityId(), this.getEntityId()); + vehicle.spawn(player).thenRun(() -> { + packetFactory.setPassengers(player, vehicle.getEntityId(), this.getEntityId()); + }); } } diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramImpl.java b/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramImpl.java index c1a4556..6605515 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramImpl.java +++ b/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) { - for (HologramLine line : lines) line.show(player); + protected CompletableFuture UNSAFE_show(Player player) { + return FutureUtil.allOf(lines.stream() + .map(line -> line.show(player)) + .collect(Collectors.toList())); } @Override diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramLine.java b/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramLine.java index 79e45c1..553a5e2 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramLine.java +++ b/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 implements PropertyHolder { private M value; @@ -37,8 +38,8 @@ public class HologramLine implements PropertyHolder { entity.refreshMeta(player); } - protected void show(Player player) { - entity.spawn(player); + protected CompletableFuture show(Player player) { + return entity.spawn(player); } protected void hide(Player player) { diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramText.java b/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramText.java index 40c8995..23f3428 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/hologram/HologramText.java +++ b/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 { private static final Component BLANK = Component.text("%blank%"); @@ -20,10 +22,9 @@ public class HologramText extends HologramLine { } @Override - public void show(Player player) { - if (!getValue().equals(BLANK)) { - super.show(player); - } + public CompletableFuture show(Player player) { + if (getValue().equals(BLANK)) return CompletableFuture.completedFuture(null); + return super.show(player); } @SuppressWarnings("unchecked") diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/npc/NpcImpl.java b/plugin/src/main/java/lol/pyr/znpcsplus/npc/NpcImpl.java index ad0061f..d340f32 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/npc/NpcImpl.java +++ b/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) { - entity.spawn(player); - hologram.show(player); + protected CompletableFuture UNSAFE_show(Player player) { + return CompletableFuture.allOf(entity.spawn(player), hologram.show(player)); } @Override diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/packets/PacketFactory.java b/plugin/src/main/java/lol/pyr/znpcsplus/packets/PacketFactory.java index aaea42a..f2b58c1 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/packets/PacketFactory.java +++ b/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 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); diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_20_2PacketFactory.java b/plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_20_2PacketFactory.java index fd9e126..9d5de17 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_20_2PacketFactory.java +++ b/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) { - addTabPlayer(player, entity, properties).thenAccept(ignored -> { + public CompletableFuture spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties) { + 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(), diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_8PacketFactory.java b/plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_8PacketFactory.java index 2637b83..3fe0504 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/packets/V1_8PacketFactory.java +++ b/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) { - addTabPlayer(player, entity, properties).thenAccept(ignored -> { + public CompletableFuture spawnPlayer(Player player, PacketEntity entity, PropertyHolder properties) { + 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(), diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/util/FutureUtil.java b/plugin/src/main/java/lol/pyr/znpcsplus/util/FutureUtil.java new file mode 100644 index 0000000..3b0c31c --- /dev/null +++ b/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 allOf(Collection> futures) { + return CompletableFuture.runAsync(() -> { + for (CompletableFuture future : futures) future.join(); + }); + } +} diff --git a/plugin/src/main/java/lol/pyr/znpcsplus/util/Viewable.java b/plugin/src/main/java/lol/pyr/znpcsplus/util/Viewable.java index 27e4bad..19637d8 100644 --- a/plugin/src/main/java/lol/pyr/znpcsplus/util/Viewable.java +++ b/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.Collections; -import java.util.List; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; +import java.util.*; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.stream.Collectors; public abstract class Viewable { @@ -21,38 +19,74 @@ public abstract class Viewable { .collect(Collectors.toList()); } - private final Set viewers = ConcurrentHashMap.newKeySet(); + private boolean queueRunning = false; + private final Queue visibilityTaskQueue = new ConcurrentLinkedQueue<>(); + private final Set 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(); - viewers.clear(); + queueVisibilityTask(() -> { + UNSAFE_hideAll(); + viewers.clear(); + }); } - public void respawn() { - UNSAFE_hideAll(); - UNSAFE_showAll(); + public CompletableFuture respawn() { + CompletableFuture future = new CompletableFuture<>(); + queueVisibilityTask(() -> { + UNSAFE_hideAll(); + UNSAFE_showAll().join(); + future.complete(null); + }); + return future; } - public void respawn(Player player) { - if (!viewers.contains(player)) return; - UNSAFE_hide(player); - UNSAFE_show(player); + public CompletableFuture respawn(Player player) { + hide(player); + return show(player); } - public void show(Player player) { - if (viewers.contains(player)) return; - viewers.add(player); - UNSAFE_show(player); + public CompletableFuture show(Player player) { + CompletableFuture future = new CompletableFuture<>(); + queueVisibilityTask(() -> { + 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; - viewers.remove(player); - UNSAFE_hide(player); + queueVisibilityTask(() -> { + if (!viewers.contains(player)) return; + 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() { - for (Player viewer : viewers) UNSAFE_show(viewer); + protected CompletableFuture UNSAFE_showAll() { + return FutureUtil.allOf(viewers.stream() + .map(this::UNSAFE_show) + .collect(Collectors.toList())); + // for (Player viewer : viewers) UNSAFE_show(viewer); } public Set getViewers() { @@ -75,7 +112,7 @@ public abstract class Viewable { return viewers.contains(player); } - protected abstract void UNSAFE_show(Player player); + protected abstract CompletableFuture UNSAFE_show(Player player); protected abstract void UNSAFE_hide(Player player); }